From 1f367f91e50a6a09acef2ee0e1baf3d34753854e Mon Sep 17 00:00:00 2001 From: Andrea Fernandez Buitrago <15234535+anferbui@users.noreply.github.com> Date: Wed, 11 Dec 2024 16:43:15 +0000 Subject: [PATCH] fixup! Diagnose duplicate representations of `@AlternateRepresentation` --- .../Infrastructure/DocumentationContext.swift | 79 ++++++++----------- .../DocumentationContextTests.swift | 30 +++---- 2 files changed, 50 insertions(+), 59 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index ccbd86bff..6b4545056 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -3213,64 +3213,55 @@ extension DocumentationContext { var sourceLanguageToReference: [SourceLanguage: AlternateRepresentation] = [:] for alternateRepresentation in entity.metadata?.alternateRepresentations ?? [] { - guard case .resolved(.success(let counterpartReference)) = alternateRepresentation.reference, - let counterpartEntity = try? self.entity(with: counterpartReference) else { + guard case .resolved(.success(let alternateRepresentationReference)) = alternateRepresentation.reference, + let alternateRepresentationEntity = try? self.entity(with: alternateRepresentationReference) else { continue } - - // Case where the original symbol already was defined in the languages of the counterpart symbol. - let duplicateSourceLanguages = counterpartEntity.availableSourceLanguages.intersection(entity.availableSourceLanguages) + + // Check if the documented symbol already has alternate representations from in-source annotations. + let duplicateSourceLanguages = alternateRepresentationEntity.availableSourceLanguages.intersection(entity.availableSourceLanguages) if !duplicateSourceLanguages.isEmpty { - problems - .append( - Problem( - diagnostic: Diagnostic( - source: alternateRepresentation.originalMarkup.range?.source, - severity: .warning, - range: alternateRepresentation.originalMarkup.range, - identifier: "org.swift.docc.AlternateRepresentation.DuplicateLanguageDefinition", - summary: "This node already has a representation in \(listSourceLanguages(duplicateSourceLanguages))", - explanation: "This node is already available in \(listSourceLanguages(entity.availableSourceLanguages))." - ), - possibleSolutions: [Solution(summary: "Replace the counterpart link with a node which isn't available in \(listSourceLanguages(entity.availableSourceLanguages))", replacements: [])] - ) - ) - + problems.append(Problem( + diagnostic: Diagnostic( + source: alternateRepresentation.originalMarkup.range?.source, + severity: .warning, + range: alternateRepresentation.originalMarkup.range, + identifier: "org.swift.docc.AlternateRepresentation.DuplicateLanguageDefinition", + summary: "\(entity.name.plainText.singleQuoted) already has a representation in \(listSourceLanguages(duplicateSourceLanguages))", + explanation: "Symbols can only specify custom alternate language representations for languages that the documented symbol doesn't already have a representation for." + ), + possibleSolutions: [Solution(summary: "Replace this alternate language representation with a symbol which isn't available in \(listSourceLanguages(entity.availableSourceLanguages))", replacements: [])] + )) } - let duplicateCounterpartLanguages = Set(sourceLanguageToReference.keys).intersection(counterpartEntity.availableSourceLanguages) - if !duplicateCounterpartLanguages.isEmpty { + let duplicateAlternateLanguages = Set(sourceLanguageToReference.keys).intersection(alternateRepresentationEntity.availableSourceLanguages) + if !duplicateAlternateLanguages.isEmpty { let replacements = alternateRepresentation.originalMarkup.range.flatMap { [Replacement(range: $0, replacement: "")] } ?? [] - let notes: [DiagnosticNote] = duplicateCounterpartLanguages.compactMap { duplicateCounterpartLanguage in - guard let alreadyExistingCounterpart = sourceLanguageToReference[duplicateCounterpartLanguage], - let range = alreadyExistingCounterpart.originalMarkup.range, + let notes: [DiagnosticNote] = duplicateAlternateLanguages.compactMap { duplicateAlternateLanguage in + guard let alreadyExistingRepresentation = sourceLanguageToReference[duplicateAlternateLanguage], + let range = alreadyExistingRepresentation.originalMarkup.range, let source = range.source else { return nil } - return DiagnosticNote(source: source, range: range, message: """ - An alternate representation for \(duplicateCounterpartLanguage.name) has already been defined by an @\(AlternateRepresentation.self) directive. - """) + return DiagnosticNote(source: source, range: range, message: "This directive already specifies an alternate \(duplicateAlternateLanguage.name) representation.") } - problems - .append( - Problem( - diagnostic: Diagnostic( - source: alternateRepresentation.originalMarkup.range?.source, - severity: .warning, - range: alternateRepresentation.originalMarkup.range, - identifier: "org.swift.docc.AlternateRepresentation.DuplicateLanguageDefinition", - summary: "An alternate representation for \(listSourceLanguages(duplicateCounterpartLanguages)) already exists", - explanation: "This node is already available in \(listSourceLanguages(entity.availableSourceLanguages.union(sourceLanguageToReference.keys))).", - notes: notes - ), - possibleSolutions: [Solution(summary: "Remove this alternate representation", replacements: replacements)] - ) - ) + problems.append(Problem( + diagnostic: Diagnostic( + source: alternateRepresentation.originalMarkup.range?.source, + severity: .warning, + range: alternateRepresentation.originalMarkup.range, + identifier: "org.swift.docc.AlternateRepresentation.DuplicateLanguageDefinition", + summary: "A custom alternate language representation for \(listSourceLanguages(duplicateAlternateLanguages)) has already been specified", + explanation: "Only one custom alternate language representation can be specified per language.", + notes: notes + ), + possibleSolutions: [Solution(summary: "Remove this alternate representation", replacements: replacements)] + )) } // Update mapping from source language to alternate declaration, for diagnostic purposes - counterpartEntity.availableSourceLanguages.forEach { sourceLanguageToReference[$0] = alternateRepresentation } + alternateRepresentationEntity.availableSourceLanguages.forEach { sourceLanguageToReference[$0] = alternateRepresentation } } } diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index 631a5600a..5cf247048 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -5463,30 +5463,30 @@ let expected = """ let alternateRepresentationProblems = context.problems.sorted(by: \.diagnostic.summary) XCTAssertEqual(alternateRepresentationProblems.count, 2) - // Verify a problem is reported for having alternate representations with duplicate source languages + // Verify a problem is reported for trying to define an alternate representation for a language the symbol already supports var problem = try XCTUnwrap(alternateRepresentationProblems.first) XCTAssertEqual(problem.diagnostic.severity, .warning) - XCTAssertEqual(problem.diagnostic.summary, "An alternate representation for Objective-C already exists") - XCTAssertEqual(problem.diagnostic.explanation, "This node is already available in Swift and Objective-C.") + XCTAssertEqual(problem.diagnostic.summary, "'SwiftSymbol' already has a representation in Swift") + XCTAssertEqual(problem.diagnostic.explanation, "Symbols can only specify custom alternate language representations for languages that the documented symbol doesn't already have a representation for.") XCTAssertEqual(problem.possibleSolutions.count, 1) - // Verify solutions provide context and suggest to remove the duplicate directive + // Verify solutions provide context, but no replacements var solution = try XCTUnwrap(problem.possibleSolutions.first) - XCTAssertEqual(solution.summary, "Remove this alternate representation") - XCTAssertEqual(solution.replacements.count, 1) - XCTAssertEqual(solution.replacements.first?.replacement, "") - - // Verify a problem is reported for trying to define an alternate representation for a language the symbol already supports + XCTAssertEqual(solution.summary, "Replace this alternate language representation with a symbol which isn't available in Swift") + XCTAssertEqual(solution.replacements.count, 0) + + // Verify a problem is reported for having alternate representations with duplicate source languages problem = try XCTUnwrap(alternateRepresentationProblems[1]) XCTAssertEqual(problem.diagnostic.severity, .warning) - XCTAssertEqual(problem.diagnostic.summary, "This node already has a representation in Swift") - XCTAssertEqual(problem.diagnostic.explanation, "This node is already available in Swift.") + XCTAssertEqual(problem.diagnostic.summary, "A custom alternate language representation for Objective-C has already been specified") + XCTAssertEqual(problem.diagnostic.explanation, "Only one custom alternate language representation can be specified per language.") XCTAssertEqual(problem.possibleSolutions.count, 1) - - // Verify solutions provide context, but no replacements + + // Verify solutions provide context and suggest to remove the duplicate directive solution = try XCTUnwrap(problem.possibleSolutions.first) - XCTAssertEqual(solution.summary, "Replace the counterpart link with a node which isn\'t available in Swift") - XCTAssertEqual(solution.replacements.count, 0) + XCTAssertEqual(solution.summary, "Remove this alternate representation") + XCTAssertEqual(solution.replacements.count, 1) + XCTAssertEqual(solution.replacements.first?.replacement, "") } }