diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift index 65b6e0659..7ec787f3d 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift @@ -46,7 +46,7 @@ extension PathHierarchy { /// /// Includes information about: /// - The path to the non-symbol match. - case nonSymbolMatchForSymbolLink(path: Substring) + case nonSymbolMatchForSymbolLink(path: String) /// Encountered an unknown disambiguation for a found node. /// diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift index 82c85731f..bf040427b 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift @@ -18,14 +18,7 @@ extension PathHierarchy { /// - Returns: Returns the unique identifier for the found match or raises an error if no match can be found. /// - Throws: Raises a ``PathHierarchy/Error`` if no match can be found. func find(path rawPath: String, parent: ResolvedIdentifier? = nil, onlyFindSymbols: Bool) throws -> ResolvedIdentifier { - let node = try findNode(path: rawPath, parentID: parent, onlyFindSymbols: onlyFindSymbols) - if node.identifier == nil { - throw Error.unfindableMatch(node) - } - if onlyFindSymbols, node.symbol == nil { - throw Error.nonSymbolMatchForSymbolLink(path: rawPath[...]) - } - return node.identifier + return try findNode(path: rawPath, parentID: parent, onlyFindSymbols: onlyFindSymbols).identifier } private func findNode(path rawPath: String, parentID: ResolvedIdentifier?, onlyFindSymbols: Bool) throws -> Node { @@ -216,98 +209,121 @@ extension PathHierarchy { onlyFindSymbols: Bool, rawPathForError: String ) throws -> Node { - var node = startingPoint - var remaining = pathComponents[...] - - // Third, search for the match relative to the start node. - if remaining.isEmpty { - // If all path components were consumed, then the start of the search is the match. - return node - } + // All code paths through this function wants to perform extra verification on the return value before returning it to the caller. + // To accomplish that, the core implementation happens in `_innerImplementation`, which is called once, right below its definition. - // Search for the remaining components from the node - while true { - let (children, pathComponent) = try findChildContainer(node: &node, remaining: remaining, rawPathForError: rawPathForError) + func _innerImplementation( + descendingFrom startingPoint: Node, + pathComponents: ArraySlice, + onlyFindSymbols: Bool, + rawPathForError: String + ) throws -> Node { + var node = startingPoint + var remaining = pathComponents[...] - do { - guard let child = try children.find(pathComponent.disambiguation) else { - // The search has ended with a node that doesn't have a child matching the next path component. - throw makePartialResultError(node: node, remaining: remaining, rawPathForError: rawPathForError) - } - node = child - remaining = remaining.dropFirst() - if remaining.isEmpty { - // If all path components are consumed, then the match is found. - return child - } - } catch DisambiguationContainer.Error.lookupCollision(let collisions) { - func handleWrappedCollision() throws -> Node { - try handleCollision(node: node, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPathForError) - } - - // When there's a collision, use the remaining path components to try and narrow down the possible collisions. + // Search for the match relative to the start node. + if remaining.isEmpty { + // If all path components were consumed, then the start of the search is the match. + return node + } + + // Search for the remaining components from the node + while true { + let (children, pathComponent) = try findChildContainer(node: &node, remaining: remaining, rawPathForError: rawPathForError) - guard let nextPathComponent = remaining.dropFirst().first else { - // This was the last path component so there's nothing to look ahead. - // - // It's possible for a symbol that exist on multiple languages to collide with itself. - // Check if the collision can be resolved by finding a unique symbol or an otherwise preferred match. - var uniqueCollisions: [String: Node] = [:] - for (node, _) in collisions { - guard let symbol = node.symbol else { - // Non-symbol collisions should have already been resolved - return try handleWrappedCollision() - } - - let id = symbol.identifier.precise - if symbol.identifier.interfaceLanguage == "swift" || !uniqueCollisions.keys.contains(id) { - uniqueCollisions[id] = node - } - - guard uniqueCollisions.count < 2 else { - // Encountered more than one unique symbol - return try handleWrappedCollision() - } + do { + guard let child = try children.find(pathComponent.disambiguation) else { + // The search has ended with a node that doesn't have a child matching the next path component. + throw makePartialResultError(node: node, remaining: remaining, rawPathForError: rawPathForError) } - // A wrapped error would have been raised while iterating over the collection. - return uniqueCollisions.first!.value - } - - // Look ahead one path component to narrow down the list of collisions. - // For each collision where the next path component can be found unambiguously, return that matching node one level down. - let possibleMatchesOneLevelDown = collisions.compactMap { - return try? $0.node.children[String(nextPathComponent.name)]?.find(nextPathComponent.disambiguation) - } - let onlyPossibleMatch: Node? - - if possibleMatchesOneLevelDown.count == 1 { - // Only one of the collisions found a match for the next path component - onlyPossibleMatch = possibleMatchesOneLevelDown.first! - } else if !possibleMatchesOneLevelDown.isEmpty, possibleMatchesOneLevelDown.dropFirst().allSatisfy({ $0.symbol?.identifier.precise == possibleMatchesOneLevelDown.first!.symbol?.identifier.precise }) { - // It's also possible that different language representations of the same symbols appear as different collisions. - // If _all_ collisions that can find the next path component are the same symbol, then we prefer the Swift version of that symbol. - onlyPossibleMatch = possibleMatchesOneLevelDown.first(where: { $0.symbol?.identifier.interfaceLanguage == "swift" }) ?? possibleMatchesOneLevelDown.first! - } else { - onlyPossibleMatch = nil - } - - if let onlyPossibleMatch { - // If we found only a single match one level down then we've processed both this path component and the next. - remaining = remaining.dropFirst(2) + node = child + remaining = remaining.dropFirst() if remaining.isEmpty { - // If that was the end of the path we can simply return the result. - return onlyPossibleMatch + // If all path components are consumed, then the match is found. + return child + } + } catch DisambiguationContainer.Error.lookupCollision(let collisions) { + func handleWrappedCollision() throws -> Node { + let match = try handleCollision(node: node, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPathForError) + return match + } + + // When there's a collision, use the remaining path components to try and narrow down the possible collisions. + + guard let nextPathComponent = remaining.dropFirst().first else { + // This was the last path component so there's nothing to look ahead. + // + // It's possible for a symbol that exist on multiple languages to collide with itself. + // Check if the collision can be resolved by finding a unique symbol or an otherwise preferred match. + var uniqueCollisions: [String: Node] = [:] + for (node, _) in collisions { + guard let symbol = node.symbol else { + // Non-symbol collisions should have already been resolved + return try handleWrappedCollision() + } + + let id = symbol.identifier.precise + if symbol.identifier.interfaceLanguage == "swift" || !uniqueCollisions.keys.contains(id) { + uniqueCollisions[id] = node + } + + guard uniqueCollisions.count < 2 else { + // Encountered more than one unique symbol + return try handleWrappedCollision() + } + } + // A wrapped error would have been raised while iterating over the collection. + return uniqueCollisions.first!.value + } + + // Look ahead one path component to narrow down the list of collisions. + // For each collision where the next path component can be found unambiguously, return that matching node one level down. + let possibleMatchesOneLevelDown = collisions.compactMap { + try? $0.node.children[String(nextPathComponent.name)]?.find(nextPathComponent.disambiguation) + } + let onlyPossibleMatch: Node? + + if possibleMatchesOneLevelDown.count == 1 { + // Only one of the collisions found a match for the next path component + onlyPossibleMatch = possibleMatchesOneLevelDown.first! + } else if !possibleMatchesOneLevelDown.isEmpty, possibleMatchesOneLevelDown.dropFirst().allSatisfy({ $0.symbol?.identifier.precise == possibleMatchesOneLevelDown.first!.symbol?.identifier.precise }) { + // It's also possible that different language representations of the same symbols appear as different collisions. + // If _all_ collisions that can find the next path component are the same symbol, then we prefer the Swift version of that symbol. + onlyPossibleMatch = possibleMatchesOneLevelDown.first(where: { $0.symbol?.identifier.interfaceLanguage == "swift" }) ?? possibleMatchesOneLevelDown.first! } else { - // Otherwise we continue looping over the remaining path components. - node = onlyPossibleMatch - continue + onlyPossibleMatch = nil + } + + if let onlyPossibleMatch { + // If we found only a single match one level down then we've processed both this path component and the next. + remaining = remaining.dropFirst(2) + if remaining.isEmpty { + // If that was the end of the path we can simply return the result. + return onlyPossibleMatch + } else { + // Otherwise we continue looping over the remaining path components. + node = onlyPossibleMatch + continue + } } + + // Couldn't resolve the collision by look ahead. + return try handleWrappedCollision() } - - // Couldn't resolve the collision by look ahead. - return try handleCollision(node: node, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPathForError) } } + + // Run the core implementation, defined above. + let node = try _innerImplementation(descendingFrom: startingPoint, pathComponents: pathComponents, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPathForError) + + // Perform extra validation on the return value before returning it to the caller. + if node.identifier == nil { + throw Error.unfindableMatch(node) + } + if onlyFindSymbols, node.symbol == nil { + throw Error.nonSymbolMatchForSymbolLink(path: rawPathForError) + } + return node } private func handleCollision( diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index a86a7e311..c1af593e5 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -1454,6 +1454,49 @@ class PathHierarchyTests: XCTestCase { try assertFindsPath("Inner/InnerClass/something()", in: tree, asSymbolID: "s:5Inner0A5ClassC5OuterE9somethingyyF") } + func testContinuesSearchingIfNonSymbolMatchesSymbolLink() throws { + let exampleDocumentation = Folder(name: "CatalogName.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: [ + ("some-class-id", .swift, ["SomeClass"], .class), + ])), + + TextFile(name: "Some-Article.md", utf8Content: """ + # Some article + + An article with a heading with the same name as a symbol and another heading. + + ### SomeClass + + - ``SomeClass`` + + ### OtherHeading + """), + ]) + let catalogURL = try exampleDocumentation.write(inside: createTemporaryDirectory()) + let (_, _, context) = try loadBundle(from: catalogURL) + let tree = context.linkResolver.localResolver.pathHierarchy + + XCTAssert(context.problems.isEmpty, "Unexpected problems \(context.problems.map(\.diagnostic.summary))") + + let articleID = try tree.find(path: "/CatalogName/Some-Article", onlyFindSymbols: false) + + XCTAssertEqual(try tree.findNode(path: "SomeClass", onlyFindSymbols: false, parent: articleID).symbol?.identifier.precise, nil, + "A general documentation link will find the heading because its closer than the symbol.") + XCTAssertEqual(try tree.findNode(path: "SomeClass", onlyFindSymbols: true, parent: articleID).symbol?.identifier.precise, "some-class-id", + "A symbol link will skip the heading and continue searching until it finds the symbol.") + + XCTAssertEqual(try tree.findNode(path: "OtherHeading", onlyFindSymbols: false, parent: articleID).symbol?.identifier.precise, nil, + "A general documentation link will find the other heading.") + XCTAssertThrowsError( + try tree.findNode(path: "OtherHeading", onlyFindSymbols: true, parent: articleID), + "A symbol link that find the header but doesn't find a symbol will raise the error about the heading not being a symbol" + ) { untypedError in + let error = untypedError as! PathHierarchy.Error + let referenceError = error.makeTopicReferenceResolutionErrorInfo() { context.linkResolver.localResolver.fullName(of: $0, in: context) } + XCTAssertEqual(referenceError.message, "Symbol links can only resolve symbols") + } + } + func testSnippets() throws { let (_, context) = try testBundleAndContext(named: "Snippets") let tree = context.linkResolver.localResolver.pathHierarchy