Skip to content

Commit 0b356e8

Browse files
authored
Warn when an article would override a symbol page (#1339)
rdar://109177620
1 parent a9569b4 commit 0b356e8

File tree

3 files changed

+69
-5
lines changed

3 files changed

+69
-5
lines changed

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1726,7 +1726,26 @@ public class DocumentationContext {
17261726
}
17271727
let reference = documentation.reference
17281728

1729-
documentationCache[reference] = documentation
1729+
if let existing = documentationCache[reference], existing.kind.isSymbol {
1730+
// 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.
1731+
// 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.
1732+
diagnosticEngine.emit(
1733+
Problem(
1734+
diagnostic: Diagnostic(source: article.source, severity: .warning, identifier: "org.swift.docc.articleCollisionProblem", summary: """
1735+
Article '\(article.source.lastPathComponent)' (\(title)) would override \(existing.kind.name.lowercased()) '\(existing.name.description)'.
1736+
""", explanation: """
1737+
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.
1738+
Until rdar://79745455 (issue #593) is fixed, DocC favors the symbol in this collision and drops the article to have deterministic behavior.
1739+
"""),
1740+
possibleSolutions: [
1741+
Solution(summary: "Rename '\(article.source.lastPathComponent)'", replacements: [ /* Renaming a file isn't something that we can represent with a replacement */ ])
1742+
]
1743+
)
1744+
)
1745+
return article // Don't continue processing this article
1746+
} else {
1747+
documentationCache[reference] = documentation
1748+
}
17301749

17311750
documentLocationMap[article.source] = reference
17321751
let graphNode = TopicGraph.Node(reference: reference, kind: .article, source: .file(url: article.source), title: title)

Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2458,7 +2458,10 @@ let expected = """
24582458
let curatedTopic = try XCTUnwrap(renderNode.topicSections.first?.identifiers.first)
24592459

24602460
let topicReference = try XCTUnwrap(renderNode.references[curatedTopic] as? TopicRenderReference)
2461-
XCTAssertEqual(topicReference.title, "An article")
2461+
2462+
// 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
2463+
XCTAssertEqual(topicReference.title, "Wrapper")
2464+
// XCTAssertEqual(topicReference.title, "An article")
24622465

24632466
// This test also reproduce https://github.com/swiftlang/swift-docc/issues/593
24642467
// 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 = """
48064809
XCTAssertEqual(problem.diagnostic.summary, "\'NonExistingDoc\' doesn\'t exist at \'/BestBook/MyArticle\'")
48074810
}
48084811

4812+
func testArticleCollidingWithSymbol() async throws {
4813+
let catalog = Folder(name: "ModuleName.docc", content: [
4814+
JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: [
4815+
makeSymbol(id: "some-symbol-id", kind: .class, pathComponents: ["SomeClass"]), // Collision
4816+
])),
4817+
4818+
TextFile(name: "SomeClass.md", utf8Content: """
4819+
# Some article
4820+
4821+
This article has the same reference as the symbol. One will override the other.
4822+
We should at least warn about that.
4823+
"""),
4824+
])
4825+
4826+
let (_, context) = try await loadBundle(catalog: catalog)
4827+
let moduleReference = try XCTUnwrap(context.soleRootModuleReference)
4828+
let collidingPageReference = moduleReference.appendingPath("SomeClass")
4829+
4830+
XCTAssertEqual(
4831+
Set(context.knownPages), [moduleReference, collidingPageReference],
4832+
"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
4833+
)
4834+
4835+
let node = try context.entity(with: collidingPageReference)
4836+
XCTAssert(node.kind.isSymbol, "Given #593 / rdar://79745455 we should deterministically prioritize the symbol over the article")
4837+
4838+
XCTAssertEqual(context.problems.map(\.diagnostic.summary), [
4839+
"Article 'SomeClass.md' (Some article) would override class 'SomeClass'."
4840+
])
4841+
4842+
let problem = try XCTUnwrap(context.problems.first)
4843+
let solution = try XCTUnwrap(problem.possibleSolutions.first)
4844+
XCTAssertEqual(solution.summary, "Rename 'SomeClass.md'")
4845+
}
4846+
48094847
func testContextRecognizesOverloads() async throws {
48104848
enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled)
48114849

Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,8 +1239,11 @@ class PathHierarchyTests: XCTestCase {
12391239
// The added article above has the same path as an existing symbol in the this module.
12401240
let symbolNode = try tree.findNode(path: "/MixedLanguageFramework/Bar", onlyFindSymbols: true)
12411241
XCTAssertNotNil(symbolNode.symbol, "Symbol link finds the symbol")
1242+
12421243
let articleNode = try tree.findNode(path: "/MixedLanguageFramework/Bar", onlyFindSymbols: false)
1243-
XCTAssertNil(articleNode.symbol, "General documentation link find the article")
1244+
XCTAssertNotNil(articleNode.symbol, "This should be an article but can't be because of rdar://79745455")
1245+
// FIXME: Verify that article matches are preferred for general (non-symbol) links once https://github.com/swiftlang/swift-docc/issues/593 is fixed
1246+
// XCTAssertNil(articleNode.symbol, "General documentation link find the article")
12441247
}
12451248

12461249
func testArticleSelfAnchorLinks() async throws {
@@ -2556,13 +2559,17 @@ class PathHierarchyTests: XCTestCase {
25562559
// Links to non-symbols can use only the file name, without specifying the module or catalog name.
25572560
let articleID = try tree.find(path: "Wrapper", onlyFindSymbols: false)
25582561
let articleMatch = try XCTUnwrap(tree.lookup[articleID])
2559-
XCTAssertNil(articleMatch.symbol, "Should have found the article")
2562+
XCTAssertNotNil(articleMatch.symbol, "This should be an article but can't be because of rdar://79745455")
2563+
// 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
2564+
// XCTAssertNil(articleMatch.symbol, "Should have found the article")
25602565
}
25612566
do {
25622567
// Links to non-symbols can also use module-relative links.
25632568
let articleID = try tree.find(path: "/Something/Wrapper", onlyFindSymbols: false)
25642569
let articleMatch = try XCTUnwrap(tree.lookup[articleID])
2565-
XCTAssertNil(articleMatch.symbol, "Should have found the article")
2570+
XCTAssertNotNil(articleMatch.symbol, "This should be an article but can't be because of rdar://79745455")
2571+
// 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
2572+
// XCTAssertNil(articleMatch.symbol, "Should have found the article")
25662573
}
25672574
// Symbols can only use absolute links or be found relative to another page.
25682575
let symbolID = try tree.find(path: "/Something/Wrapper", onlyFindSymbols: true)

0 commit comments

Comments
 (0)