From ea8afdc5b501c6bb4bea3bd8804d0a2759564afd Mon Sep 17 00:00:00 2001 From: Ayush Srivastava Date: Mon, 17 Mar 2025 14:56:01 +0530 Subject: [PATCH 1/2] Added warning for multiple root pages #1170 Issue "Warn when the documentation contains more than one root page #1170" Added diagnostic for multiple root pages warning, root page count validation --- .../Infrastructure/DocumentationContext.swift | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 5b58b8045..4707e3692 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -3363,3 +3363,47 @@ extension DataAsset { @available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released") extension DocumentationContext: DocumentationContextDataProviderDelegate {} + +//added diagnostic for multiple root pages + +extension Diagnostic { + static func multipleRootPagesWarning(roots: [ResolvedTopicReference]) -> Diagnostic { + let primaryRoot = roots.first! + let additionalRoots = Array(roots.dropFirst()) + + return Diagnostic( + source: nil, + severity: .warning, + identifier: "org.swift.docc.MultipleRootPages", + summary: "Found \(roots.count) root pages in documentation", + explanation: """ + Documentation should have exactly one root page. Found these root pages: + Primary: \(primaryRoot.path) + Additional: + \(additionalRoots.map { $0.path }.joined(separator: "\n")) + """ + ) + } +} + +extension DocumentationContext { + private func validateRootPageCount() { + //get all root pages + let roots = topicGraph.nodes.values.filter { node in + return node.kind == .article && parents(of: node.reference).isEmpty + }.map { $0.reference } + + //warn if multiple roots found + if roots.count > 1 { + let diagnostic = Diagnostic.multipleRootPagesWarning(roots: roots) + let problem = Problem(diagnostic: diagnostic) + diagnosticEngine.emit(problem) + } + } +} + +extension DocumentationContext { + func processRootModules() throws { + validateRootPageCount() + } +} From 179cd1c075aef894248154c4db3f6f164c75c9d6 Mon Sep 17 00:00:00 2001 From: Ayush Srivastava Date: Sun, 23 Mar 2025 11:45:46 +0530 Subject: [PATCH 2/2] Added warning for multiple root pages --- .../Infrastructure/DocumentationContext.swift | 210 ++++++++++++++---- .../DocumentationContext+RootPageTests.swift | 130 +++++++++++ 2 files changed, 296 insertions(+), 44 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 4707e3692..f2b4b57f5 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2809,11 +2809,177 @@ public class DocumentationContext { } } + //Detects and emits warnings for multiple root pages in the documentation. + + private func emitWarningsForMultipleRoots() { + //Check for multiple root modules from symbol graphs + let rootModuleReferences = rootModules ?? [] + let rootModulesFromSymbolGraph = rootModuleReferences.filter { reference in + guard let node = topicGraph.nodeWithReference(reference) else { return false } + //include actual modules from symbol graphs + return node.kind == .module && !node.isVirtual + } + + if rootModulesFromSymbolGraph.count > 1 { + //Multiple modules from symbol graph files + let moduleNames = rootModulesFromSymbolGraph.map { $0.lastPathComponent } + + //create diagnostic notes for each module + var notes = [DiagnosticNote]() + for moduleRef in rootModulesFromSymbolGraph { + let moduleName = moduleRef.lastPathComponent + notes.append(DiagnosticNote( + source: nil, + message: "Module '\(moduleName)' is one of multiple root modules" + )) + } + + let diagnostic = Diagnostic( + source: nil, + severity: .warning, + range: nil, + identifier: "org.swift.docc.MultipleModuleRoots", + summary: "Documentation contains symbol graphs for more than one main module: \(moduleNames.joined(separator: ", ").singleQuoted)", + explanation: "This may lead to unexpected behavior as it's not clear which module should be the primary root.", + notes: notes + ) + diagnosticEngine.emit(Problem(diagnostic: diagnostic, possibleSolutions: [])) + } + + //check for manual technology roots + let manualRootPages = topicGraph.nodes.values.filter { node in + guard let documentationNode = documentationCache[node.reference] else { return false } + // Find nodes with @TechnologyRoot directive that aren't from symbol graphs + return documentationNode.semantic is Article && + (documentationNode.semantic as? Article)?.metadata?.technologyRoot != nil + } + + //multiple manual @TechnologyRoot pages + if manualRootPages.count > 1 { + //a mapping of root pages to their URLs for diagnostic notes + var rootPageURLs = [ResolvedTopicReference: URL]() + for rootPage in manualRootPages { + if let documentURL = try? documentURL(for: rootPage.reference) { + rootPageURLs[rootPage.reference] = documentURL + } + } + + for rootPage in manualRootPages { + if let documentURL = try? documentURL(for: rootPage.reference), + let documentationNode = documentationCache[rootPage.reference], + let article = documentationNode.semantic as? Article, + let technologyRoot = article.metadata?.technologyRoot { + + //diagnostic notes for the other root pages + var notes = [DiagnosticNote]() + for otherRootPage in manualRootPages where otherRootPage.reference != rootPage.reference { + if let otherURL = rootPageURLs[otherRootPage.reference] { + let technologyRootRange = (documentationCache[otherRootPage.reference]?.semantic as? Article)?.metadata?.technologyRoot?.originalMarkup.range + notes.append(DiagnosticNote( + source: otherURL, + range: technologyRootRange, + message: "'\(otherRootPage.title)' is also marked as a technology root" + )) + } + } + + let diagnostic = Diagnostic( + source: documentURL, + severity: .warning, + range: technologyRoot.originalMarkup.range, + identifier: "org.swift.docc.MultipleManualRoots", + summary: "Multiple @TechnologyRoot pages found in documentation", + explanation: "The page \(rootPage.title.singleQuoted) is marked as a technology root, but there are other technology root pages in the documentation. This may lead to unexpected behavior.", + notes: notes + ) + + let solutions: [Solution] + if let range = technologyRoot.originalMarkup.range { + solutions = [ + Solution(summary: "Remove the TechnologyRoot directive", replacements: [Replacement(range: range, replacement: "")]) + ] + } else { + solutions = [] + } + + diagnosticEngine.emit(Problem(diagnostic: diagnostic, possibleSolutions: solutions)) + } + } + } + + //@TechnologyRoot page + module from symbol graph + if manualRootPages.count == 1 && rootModulesFromSymbolGraph.count == 1 { + let rootPage = manualRootPages.first! + + //skip warning if either is a descendant of the other + let moduleReference = rootModulesFromSymbolGraph.first! + if !isDescendant(of: rootPage.reference, moduleReference) && !isDescendant(of: moduleReference, rootPage.reference) { + if let documentURL = try? documentURL(for: rootPage.reference), + let documentationNode = documentationCache[rootPage.reference], + let article = documentationNode.semantic as? Article, + let technologyRoot = article.metadata?.technologyRoot { + + let moduleName = moduleReference.lastPathComponent + + //create a diagnostic note for the module + let notes = [DiagnosticNote( + source: nil, + message: "Module '\(moduleName)' is already serving as a root" + )] + + let diagnostic = Diagnostic( + source: documentURL, + severity: .warning, + range: technologyRoot.originalMarkup.range, + identifier: "org.swift.docc.ManualRootWithModuleRoot", + summary: "Manual @TechnologyRoot found with a module root from symbol graph file", + explanation: "The page \(rootPage.title.singleQuoted) is marked as a technology root, but the documentation also contains a module \(moduleName.singleQuoted) from a symbol graph file. This may lead to unexpected behavior.", + notes: notes + ) + + let solutions: [Solution] + if let range = technologyRoot.originalMarkup.range { + solutions = [ + Solution(summary: "Remove the TechnologyRoot directive", replacements: [Replacement(range: range, replacement: "")]) + ] + } else { + solutions = [] + } + + diagnosticEngine.emit(Problem(diagnostic: diagnostic, possibleSolutions: solutions)) + } + } + } + } + + /** + Checks if a reference is a descendant of another reference in the topic graph. + + - Parameters: + - possibleDescendant: The reference to check if it's a descendant + - possibleAncestor: The reference to check if it's an ancestor + - Returns: `true` if possibleDescendant is a descendant of possibleAncestor, `false` otherwise + */ + private func isDescendant(of possibleDescendant: ResolvedTopicReference, _ possibleAncestor: ResolvedTopicReference) -> Bool { + var current = possibleDescendant + while let parentReferences = try? parents(of: current), !parentReferences.isEmpty { + if parentReferences.contains(possibleAncestor) { + return true + } + // Check the first parent (we just need to find any path to the ancestor) + current = parentReferences[0] + } + return false + } + /** Analysis that runs after all nodes are successfully registered in the context. Useful for checks that need the complete node graph. */ func topicGraphGlobalAnalysis() { + //check for multiple root pages and emit warnings + emitWarningsForMultipleRoots() + // Run any checks added to the context. let problems = knownIdentifiers.flatMap { reference in return configuration.topicAnalysisConfiguration.additionalChecks.flatMap { check in @@ -3363,47 +3529,3 @@ extension DataAsset { @available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released") extension DocumentationContext: DocumentationContextDataProviderDelegate {} - -//added diagnostic for multiple root pages - -extension Diagnostic { - static func multipleRootPagesWarning(roots: [ResolvedTopicReference]) -> Diagnostic { - let primaryRoot = roots.first! - let additionalRoots = Array(roots.dropFirst()) - - return Diagnostic( - source: nil, - severity: .warning, - identifier: "org.swift.docc.MultipleRootPages", - summary: "Found \(roots.count) root pages in documentation", - explanation: """ - Documentation should have exactly one root page. Found these root pages: - Primary: \(primaryRoot.path) - Additional: - \(additionalRoots.map { $0.path }.joined(separator: "\n")) - """ - ) - } -} - -extension DocumentationContext { - private func validateRootPageCount() { - //get all root pages - let roots = topicGraph.nodes.values.filter { node in - return node.kind == .article && parents(of: node.reference).isEmpty - }.map { $0.reference } - - //warn if multiple roots found - if roots.count > 1 { - let diagnostic = Diagnostic.multipleRootPagesWarning(roots: roots) - let problem = Problem(diagnostic: diagnostic) - diagnosticEngine.emit(problem) - } - } -} - -extension DocumentationContext { - func processRootModules() throws { - validateRootPageCount() - } -} diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContext+RootPageTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContext+RootPageTests.swift index aab9b5b85..4e452ae36 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContext+RootPageTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContext+RootPageTests.swift @@ -169,4 +169,134 @@ class DocumentationContext_RootPageTests: XCTestCase { XCTAssertEqual(context.problems.count, 0) } + + func testWarnsAboutMultipleModuleRoots() throws { + let moduleOneSymbolGraph = SymbolGraph( + metadata: SymbolGraph.Metadata( + formatVersion: SymbolGraph.FormatVersion(major: 1, minor: 0, patch: 0), + generator: "unit-test" + ), + module: SymbolGraph.Module( + name: "ModuleOne", + platform: .init(architecture: nil, vendor: nil, operatingSystem: nil, environment: nil), + version: nil + ), + symbols: [], + relationships: [] + ) + + let moduleTwoSymbolGraph = SymbolGraph( + metadata: SymbolGraph.Metadata( + formatVersion: SymbolGraph.FormatVersion(major: 1, minor: 0, patch: 0), + generator: "unit-test" + ), + module: SymbolGraph.Module( + name: "ModuleTwo", + platform: .init(architecture: nil, vendor: nil, operatingSystem: nil, environment: nil), + version: nil + ), + symbols: [], + relationships: [] + ) + + let moduleOneSymbolGraphURL = try createTempFile(name: "module-one.symbols.json", content: try JSONEncoder().encode(moduleOneSymbolGraph)) + let moduleTwoSymbolGraphURL = try createTempFile(name: "module-two.symbols.json", content: try JSONEncoder().encode(moduleTwoSymbolGraph)) + + let (_, context) = try loadBundle(copying: "TestBundle", excludingPaths: ["TestBundle.symbols.json"], additionalSymbolGraphs: [moduleOneSymbolGraphURL, moduleTwoSymbolGraphURL]) + + // Verify warning about multiple roots + let multipleRootsProblem = try XCTUnwrap(context.problems.first(where: { $0.diagnostic.identifier == "org.swift.docc.MultipleModuleRoots" })) + XCTAssertEqual(multipleRootsProblem.diagnostic.severity, .warning) + XCTAssertTrue(multipleRootsProblem.diagnostic.summary.contains("ModuleOne") && multipleRootsProblem.diagnostic.summary.contains("ModuleTwo"), "The warning should mention both module names") + + // Verify diagnostic notes + XCTAssertEqual(multipleRootsProblem.diagnostic.notes.count, 2, "There should be a note for each module") + XCTAssertTrue(multipleRootsProblem.diagnostic.notes.contains { $0.message.contains("ModuleOne") }) + XCTAssertTrue(multipleRootsProblem.diagnostic.notes.contains { $0.message.contains("ModuleTwo") }) + } + + func testWarnsAboutMultipleManualRoots() throws { + let (_, context) = try loadBundle(catalog: + Folder(name: "MultipleRoots.docc", content: [ + TextFile(name: "RootOne.md", utf8Content: """ + # Root One + @Metadata { + @TechnologyRoot + } + First root article + """), + + TextFile(name: "RootTwo.md", utf8Content: """ + # Root Two + @Metadata { + @TechnologyRoot + } + Second root article + """), + + InfoPlist(displayName: "MultipleRoots", identifier: "com.test.multipleroots"), + ]) + ) + + // Verify warning about multiple manual roots + let multipleManualRootProblems = context.problems.filter { $0.diagnostic.identifier == "org.swift.docc.MultipleManualRoots" } + XCTAssertEqual(multipleManualRootProblems.count, 2, "There should be a warning for each manual root page") + + for problem in multipleManualRootProblems { + XCTAssertEqual(problem.diagnostic.severity, .warning) + XCTAssertTrue(problem.diagnostic.source?.lastPathComponent == "RootOne.md" || problem.diagnostic.source?.lastPathComponent == "RootTwo.md") + XCTAssertEqual(problem.possibleSolutions.count, 1, "There should be a solution to remove the TechnologyRoot directive") + + // Verify diagnostic notes + XCTAssertEqual(problem.diagnostic.notes.count, 1, "There should be a note for the other root page") + let otherRootName = problem.diagnostic.source?.lastPathComponent == "RootOne.md" ? "Root Two" : "Root One" + XCTAssertTrue(problem.diagnostic.notes.first?.message.contains(otherRootName) ?? false, "The note should mention the other root page") + } + } + + func testWarnsAboutManualRootWithModuleRoot() throws { + let symbolGraph = SymbolGraph( + metadata: SymbolGraph.Metadata( + formatVersion: SymbolGraph.FormatVersion(major: 1, minor: 0, patch: 0), + generator: "unit-test" + ), + module: SymbolGraph.Module( + name: "TestModule", + platform: .init(architecture: nil, vendor: nil, operatingSystem: nil, environment: nil), + version: nil + ), + symbols: [], + relationships: [] + ) + + let symbolGraphURL = try createTempFile(name: "test-module.symbols.json", content: try JSONEncoder().encode(symbolGraph)) + + let tempFolder = try createTempFolder(content: [ + Folder(name: "MixedRoots.docc", content: [ + TextFile(name: "ManualRoot.md", utf8Content: """ + # Manual Root + @Metadata { + @TechnologyRoot + } + A manual root page + """), + + InfoPlist(displayName: "MixedRoots", identifier: "com.test.mixedroots"), + ]) + ]) + + let (_, context) = try loadBundle(from: URL(fileURLWithPath: tempFolder).appendingPathComponent("MixedRoots.docc"), additionalSymbolGraphs: [symbolGraphURL]) + + //verify warning about manual root with module root + let manualWithModuleProblem = try XCTUnwrap(context.problems.first(where: { $0.diagnostic.identifier == "org.swift.docc.ManualRootWithModuleRoot" })) + XCTAssertEqual(manualWithModuleProblem.diagnostic.severity, .warning) + XCTAssertEqual(manualWithModuleProblem.diagnostic.source?.lastPathComponent, "ManualRoot.md") + XCTAssertTrue(manualWithModuleProblem.diagnostic.summary.contains("Manual @TechnologyRoot found with a module root")) + XCTAssertTrue(manualWithModuleProblem.diagnostic.explanation.contains("TestModule")) + XCTAssertEqual(manualWithModuleProblem.possibleSolutions.count, 1, "There should be a solution to remove the TechnologyRoot directive") + + //verify diagnostic notes + XCTAssertEqual(manualWithModuleProblem.diagnostic.notes.count, 1, "There should be a note about the module") + XCTAssertTrue(manualWithModuleProblem.diagnostic.notes.first?.message.contains("TestModule") ?? false, "The note should mention the module name") + } }