Continues searching if a non-symbol matches a symbol link

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.
         ///

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<PathComponent>,
+            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(

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