From 10fa24f1b669b003df74d645c95403568ea3cc1e Mon Sep 17 00:00:00 2001 From: Andrea Fernandez Buitrago <15234535+anferbui@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:12:20 +0000 Subject: [PATCH] Diagnose duplicate representations of `@AlternateRepresentation` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an `@AlternateRepresentation` clashes with already available source languages, this will now be reported as diagnostics. These diagnostics are performed in the final stage of registering a bundle, during the global analysis of the topic graph, where all nodes are available and all links will have been resolved. This is so that we have all the information we need for detecting duplicates. The following cases are detected: - if the symbol the alternate representation is being defined for (the "original" symbol) was already available in one of the languages the counterpart symbol is available in - if the alternate representations have duplicate source languages in common, i.e. if counterpart1 is available in Objective-C and counterpart2 is **also** available in Objective-C. Suggestions will be provided depending on context: - which languages are duplicate - all the languages the symbol is already available in will be available as part of the diagnostic explanation - if the `@AlternateRepresentation` directive is a duplicate, a suggestion will be made to remove it, with a suitable replacement - if the `@AlternateRepresentation` directive is a duplicate, a note pointing to the original directive will be added Example diagnostics: ``` warning: An alternate representation for Swift already exists This node is already available in Swift and Objective-C. SynonymSample.docc/SymbolExtension2.md:4:5: An alternate representation for Swift has already been defined by an @AlternateRepresentation directive. --> SynonymSample.docc/SymbolExtension2.md:5:5-5:57 3 | @Metadata { 4 | @AlternateRepresentation(``Synonyms/Synonym-5zxmc``) 5 + @AlternateRepresentation(``Synonyms/Synonym-5zxmc``) | ╰─suggestion: Remove this alternate representation 6 | } 7 | ``` ``` warning: This node already has a representation in Swift This node is already available in Swift. --> SynonymSample.docc/SynonymExtension.md:5:5-5:56 3 | @Metadata { 4 | @AlternateRepresentation(``Synonyms/Synonym-1wqxt``) 5 + @AlternateRepresentation(``Synonyms/OtherSynonym``) | ╰─suggestion: Replace the counterpart link with a node which isn't available in Swift 6 | } 7 | ``` --- .../Infrastructure/DocumentationContext.swift | 96 +++++++++++++++++++ .../DocumentationContextTests.swift | 72 +++++++++++++- 2 files changed, 165 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 08e8f8cca..84c56e3ba 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2779,6 +2779,9 @@ public class DocumentationContext { } } + // Run analysis to determine whether manually configured alternate representations are valid. + analyzeAlternateRepresentations() + // Run global ``TopicGraph`` global analysis. analyzeTopicGraph() } @@ -3143,6 +3146,77 @@ extension DocumentationContext { } diagnosticEngine.emit(problems) } + + func analyzeAlternateRepresentations() { + var problems = [Problem]() + for node in topicGraph.nodes.values { + guard let entity = try? self.entity(with: node.reference) else { continue } + + 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 { + continue + } + + // Case where the original symbol already was defined in the languages of the counterpart symbol. + let duplicateSourceLanguages = counterpartEntity.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 \(duplicateSourceLanguages.diagnosticString)", + explanation: "This node is already available in \(entity.availableSourceLanguages.diagnosticString).", + ), + possibleSolutions: [Solution(summary: "Replace the counterpart link with a node which isn't available in \(entity.availableSourceLanguages.diagnosticString)", replacements: [])] + ) + ) + + } + + let duplicateCounterpartLanguages = Set(sourceLanguageToReference.keys).intersection(counterpartEntity.availableSourceLanguages) + if !duplicateCounterpartLanguages.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 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. + """) + } + 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 \(duplicateCounterpartLanguages.diagnosticString) already exists", + explanation: "This node is already available in \(entity.availableSourceLanguages.union(sourceLanguageToReference.keys).diagnosticString).", + 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 } + } + } + + diagnosticEngine.emit(problems) + } } extension GraphCollector.GraphKind { @@ -3192,5 +3266,27 @@ extension DataAsset { } } +extension Set where Element == SourceLanguage { + fileprivate var diagnosticString: String { + var languageNames = self.sorted(by: { language1, language2 in + // Emit Swift first, then alphabetically. + switch (language1, language2) { + case (.swift, _): return true + case (_, .swift): return false + default: return language1.id < language2.id + } + }).map(\.name) + + guard languageNames.count > 1 else { + return languageNames.first ?? "" + } + + // Returns "Language1, Language2 and Language3" + let finalElement = languageNames.removeLast() + let commaSeparatedElements = languageNames.joined(separator: ", ") + return "\(commaSeparatedElements) and \(finalElement)" + } +} + @available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released") extension DocumentationContext: DocumentationContextDataProviderDelegate {} diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index bf0afb284..43d7151d6 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -5376,7 +5376,7 @@ let expected = """ ) let tempURL = try createTempFolder(content: [exampleDocumentation]) let (_, bundle, context) = try loadBundle(from: tempURL) - + let reference = ResolvedTopicReference(bundleID: bundle.id, path: "/documentation/unit-test/Symbol", sourceLanguage: .swift) let entity = try context.entity(with: reference) @@ -5396,14 +5396,80 @@ let expected = """ return } XCTAssertEqual(unresolvedPath, .init(topicURL: .init(parsingAuthoredLink: "MissingSymbol")!)) - + // And an error should have been reportes XCTAssertEqual(context.problems.count, 1) let problem = try XCTUnwrap(context.problems.first) XCTAssertEqual(problem.diagnostic.severity, .warning) XCTAssertEqual(problem.diagnostic.summary, "Can't resolve 'MissingSymbol'") - } + } + + func testDiagnosesAlternateDeclarations() throws { + let exampleDocumentation = Folder( + name: "unit-test.docc", + content: [ + TextFile(name: "Symbol.md", utf8Content: """ + # ``Symbol`` + @Metadata { + @AlternateRepresentation(``CounterpartSymbol``) + @AlternateRepresentation(``OtherCounterpartSymbol``) + } + A symbol extension file defining an alternate representation which overlaps source languages with another one. + """), + TextFile(name: "SwiftSymbol.md", utf8Content: """ + # ``SwiftSymbol`` + @Metadata { + @AlternateRepresentation(``Symbol``) + } + A symbol extension file defining an alternate representation which overlaps source languages with the current node. + """), + JSONFile( + name: "unit-test.symbols.json", + content: makeSymbolGraph( + moduleName: "unit-test", + symbols: [ + makeSymbol(id: "symbol-id", kind: .class, pathComponents: ["Symbol"]), + makeSymbol(id: "other-symbol-id", kind: .class, pathComponents: ["SwiftSymbol"]), + makeSymbol(id: "counterpart-symbol-id", language: .objectiveC, kind: .class, pathComponents: ["CounterpartSymbol"]), + makeSymbol(id: "other-counterpart-symbol-id", language: .objectiveC, kind: .class, pathComponents: ["OtherCounterpartSymbol"]), + ] + ) + ), + ] + ) + let tempURL = try createTempFolder(content: [exampleDocumentation]) + + let (_, _, context) = try loadBundle(from: tempURL) + + 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 + 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.possibleSolutions.count, 1) + + // Verify solutions provide context and suggest to remove the duplicate directive + 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 + 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.possibleSolutions.count, 1) + + // Verify solutions provide context, but no replacements + 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) + } } func assertEqualDumps(_ lhs: String, _ rhs: String, file: StaticString = #file, line: UInt = #line) {