Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion Sources/SwiftDocC/Infrastructure/DocumentationContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
13 changes: 10 additions & 3 deletions Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down