diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 9d4d83216b..b40d8dd7f6 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -1726,7 +1726,26 @@ public class DocumentationContext { } let reference = documentation.reference - documentationCache[reference] = documentation + if let existing = documentationCache[reference], existing.kind.isSymbol { + // By the time we get here it's already to late to fix the collision. All we can do is make the author aware of it and handle the collision deterministically. + // rdar://79745455 and https://github.com/swiftlang/swift-docc/issues/593 tracks fixing the root cause of this issue, avoiding the collision and allowing the article and symbol to both exist. + diagnosticEngine.emit( + Problem( + diagnostic: Diagnostic(source: article.source, severity: .warning, identifier: "org.swift.docc.articleCollisionProblem", summary: """ + Article '\(article.source.lastPathComponent)' (\(title)) would override \(existing.kind.name.lowercased()) '\(existing.name.description)'. + """, explanation: """ + DocC computes unique URLs for symbols, even if they have the same name, but doesn't account for article filenames that collide with symbols because of a bug. + Until rdar://79745455 (issue #593) is fixed, DocC favors the symbol in this collision and drops the article to have deterministic behavior. + """), + possibleSolutions: [ + Solution(summary: "Rename '\(article.source.lastPathComponent)'", replacements: [ /* Renaming a file isn't something that we can represent with a replacement */ ]) + ] + ) + ) + return article // Don't continue processing this article + } else { + documentationCache[reference] = documentation + } documentLocationMap[article.source] = reference let graphNode = TopicGraph.Node(reference: reference, kind: .article, source: .file(url: article.source), title: title) diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index e788f1b192..7b48aa7794 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -2458,7 +2458,10 @@ let expected = """ let curatedTopic = try XCTUnwrap(renderNode.topicSections.first?.identifiers.first) let topicReference = try XCTUnwrap(renderNode.references[curatedTopic] as? TopicRenderReference) - XCTAssertEqual(topicReference.title, "An article") + + // FIXME: Verify that article matches are preferred for general (non-symbol) links once rdar://79745455 https://github.com/swiftlang/swift-docc/issues/593 is fixed + XCTAssertEqual(topicReference.title, "Wrapper") +// XCTAssertEqual(topicReference.title, "An article") // This test also reproduce https://github.com/swiftlang/swift-docc/issues/593 // When that's fixed this test should also use a symbol link to curate the top-level symbol and verify that @@ -4806,6 +4809,41 @@ let expected = """ XCTAssertEqual(problem.diagnostic.summary, "\'NonExistingDoc\' doesn\'t exist at \'/BestBook/MyArticle\'") } + func testArticleCollidingWithSymbol() async throws { + let catalog = Folder(name: "ModuleName.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: [ + makeSymbol(id: "some-symbol-id", kind: .class, pathComponents: ["SomeClass"]), // Collision + ])), + + TextFile(name: "SomeClass.md", utf8Content: """ + # Some article + + This article has the same reference as the symbol. One will override the other. + We should at least warn about that. + """), + ]) + + let (_, context) = try await loadBundle(catalog: catalog) + let moduleReference = try XCTUnwrap(context.soleRootModuleReference) + let collidingPageReference = moduleReference.appendingPath("SomeClass") + + XCTAssertEqual( + Set(context.knownPages), [moduleReference, collidingPageReference], + "Ideally there should be 3 pages here but because of rdar://79745455 (issue #593) there isn't" // https://github.com/swiftlang/swift-docc/issues/593 + ) + + let node = try context.entity(with: collidingPageReference) + XCTAssert(node.kind.isSymbol, "Given #593 / rdar://79745455 we should deterministically prioritize the symbol over the article") + + XCTAssertEqual(context.problems.map(\.diagnostic.summary), [ + "Article 'SomeClass.md' (Some article) would override class 'SomeClass'." + ]) + + let problem = try XCTUnwrap(context.problems.first) + let solution = try XCTUnwrap(problem.possibleSolutions.first) + XCTAssertEqual(solution.summary, "Rename 'SomeClass.md'") + } + func testContextRecognizesOverloads() async throws { enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled) diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index c7c54f5495..2814377607 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -1239,8 +1239,11 @@ class PathHierarchyTests: XCTestCase { // The added article above has the same path as an existing symbol in the this module. let symbolNode = try tree.findNode(path: "/MixedLanguageFramework/Bar", onlyFindSymbols: true) XCTAssertNotNil(symbolNode.symbol, "Symbol link finds the symbol") + let articleNode = try tree.findNode(path: "/MixedLanguageFramework/Bar", onlyFindSymbols: false) - XCTAssertNil(articleNode.symbol, "General documentation link find the article") + XCTAssertNotNil(articleNode.symbol, "This should be an article but can't be because of rdar://79745455") + // FIXME: Verify that article matches are preferred for general (non-symbol) links once https://github.com/swiftlang/swift-docc/issues/593 is fixed +// XCTAssertNil(articleNode.symbol, "General documentation link find the article") } func testArticleSelfAnchorLinks() async throws { @@ -2491,13 +2494,17 @@ class PathHierarchyTests: XCTestCase { // Links to non-symbols can use only the file name, without specifying the module or catalog name. let articleID = try tree.find(path: "Wrapper", onlyFindSymbols: false) let articleMatch = try XCTUnwrap(tree.lookup[articleID]) - XCTAssertNil(articleMatch.symbol, "Should have found the article") + XCTAssertNotNil(articleMatch.symbol, "This should be an article but can't be because of rdar://79745455") + // FIXME: Verify that article matches are preferred for general (non-symbol) links once rdar://79745455 https://github.com/swiftlang/swift-docc/issues/593 is fixed +// XCTAssertNil(articleMatch.symbol, "Should have found the article") } do { // Links to non-symbols can also use module-relative links. let articleID = try tree.find(path: "/Something/Wrapper", onlyFindSymbols: false) let articleMatch = try XCTUnwrap(tree.lookup[articleID]) - XCTAssertNil(articleMatch.symbol, "Should have found the article") + XCTAssertNotNil(articleMatch.symbol, "This should be an article but can't be because of rdar://79745455") + // FIXME: Verify that article matches are preferred for general (non-symbol) links once rdar://79745455 https://github.com/swiftlang/swift-docc/issues/593 is fixed +// XCTAssertNil(articleMatch.symbol, "Should have found the article") } // Symbols can only use absolute links or be found relative to another page. let symbolID = try tree.find(path: "/Something/Wrapper", onlyFindSymbols: true)