Use extensionTo and declaredIn relationships to add extensions to path hierarchy (#720)

Author: d-ronnqvist

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift

@@ -96,8 +96,27 @@ extension PathHierarchy {
             if modules.count == 1 {
                 do {
                     return try searchForNode(descendingFrom: modules.first!.value, pathComponents: remaining, parsedPathForError: parsedPathForError, onlyFindSymbols: onlyFindSymbols)
-                } catch {
-                    // Ignore this error and raise an error about not finding the module instead.
+                } catch let error as PathHierarchy.Error {
+                    switch error {
+                    case .notFound:
+                        // Ignore this error and raise an error about not finding the module instead.
+                        break
+                    case .unknownName(let partialResult, remaining: _, availableChildren: _):
+                        if partialResult.node.symbol?.kind.identifier == .module {
+                            // Failed to find the first path component. Ignore this error and raise an error about not finding the module instead.
+                            break
+                        } else {
+                            // Partially resolved the link. Raise the more specific error instead of a module-not-found error.
+                            throw error
+                        }
+                        
+                    // These errors are all more specific than a module-not-found error would be.
+                    case .unfindableMatch,
+                         .nonSymbolMatchForSymbolLink,
+                         .unknownDisambiguation,
+                         .lookupCollision:
+                        throw error
+                    }
                 }
             }
             let topLevelNames = Set(modules.keys + [articlesContainer.name, tutorialContainer.name])
@@ -193,7 +212,8 @@ extension PathHierarchy {
                     try handleCollision(node: node, parsedPath: parsedPathForError, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols)
                 }
 
-                // See if the collision can be resolved by looking ahead on level deeper.
+                // 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.
                     //
@@ -219,18 +239,38 @@ extension PathHierarchy {
                     // A wrapped error would have been raised while iterating over the collection.
                     return uniqueCollisions.first!.value
                 }
-                // Try resolving the rest of the path for each collision ...
-                let possibleMatches = collisions.compactMap {
+                
+                // 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[nextPathComponent.name]?.find(nextPathComponent.kind, nextPathComponent.hash)
                 }
-                // If only one collision matches, return that match.
-                if possibleMatches.count == 1 {
-                    return possibleMatches.first!
+                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 all matches are the same symbol, return the Swift version of that symbol
-                if !possibleMatches.isEmpty, possibleMatches.dropFirst().allSatisfy({ $0.symbol?.identifier.precise == possibleMatches.first!.symbol?.identifier.precise }) {
-                    return possibleMatches.first(where: { $0.symbol?.identifier.interfaceLanguage == "swift" }) ?? possibleMatches.first!
+                
+                if let onlyPossibleMatch = 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 handleCollision(node: node, parsedPath: parsedPathForError, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols)
             }

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+PathComponent.swift

@@ -13,7 +13,19 @@ import SymbolKit
 /// All known symbol kind identifiers.
 ///
 /// This is used to identify parsed path components as kind information.
-private let knownSymbolKinds = Set(SymbolGraph.Symbol.KindIdentifier.allCases.map { $0.identifier })
+private let knownSymbolKinds: Set<String> = {
+    // There's nowhere else that registers these extended symbol kinds and we need to know them in this list.
+    SymbolGraph.Symbol.KindIdentifier.register(
+        .extendedProtocol,
+        .extendedStructure,
+        .extendedClass,
+        .extendedEnumeration,
+        .unknownExtendedType,
+        .extendedModule
+    )
+    return Set(SymbolGraph.Symbol.KindIdentifier.allCases.map(\.identifier))
+}()
+
 /// All known source language identifiers.
 ///
 /// This is used to skip language prefixes from kind disambiguation information.

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift

@@ -115,7 +115,7 @@ struct PathHierarchy {
             }
 
             var topLevelCandidates = nodes
-            for relationship in graph.relationships where [.memberOf, .requirementOf, .optionalRequirementOf].contains(relationship.kind) {
+            for relationship in graph.relationships where relationship.kind.formsHierarchy {
                 guard let sourceNode = nodes[relationship.source] else {
                     continue
                 }
@@ -145,6 +145,11 @@ struct PathHierarchy {
                 // Disfavor the default implementation to favor the protocol requirement (or other symbol with the same path).
                 sourceNode.isDisfavoredInCollision = true
 
+                guard sourceNode.parent == nil else {
+                    // This node already has a direct member-of parent. No need to go via the default-implementation-of relationship to find its location in the hierarchy.
+                    continue
+                }
+                
                 let targetNodes = nodes[relationship.target].map { [$0] } ?? allNodes[relationship.target] ?? []
                 guard !targetNodes.isEmpty else {
                     continue
@@ -204,7 +209,10 @@ struct PathHierarchy {
 
         var lookup = [ResolvedIdentifier: Node]()
         func descend(_ node: Node) {
-            assert(node.identifier == nil)
+            assert(
+                node.identifier == nil,
+                "Already encountered \(node.name). This is an indication that a symbol is the source of more than one memberOf relationship."
+            )
             if node.symbol != nil {
                 node.identifier = ResolvedIdentifier()
                 lookup[node.identifier] = node
@@ -460,3 +468,15 @@ extension PathHierarchy.DisambiguationContainer {
         }))
     }
 }
+
+private extension SymbolGraph.Relationship.Kind {
+    /// Whether or not this relationship kind forms a hierarchical relationship between the source and the target.
+    var formsHierarchy: Bool {
+        switch self {
+        case .memberOf, .requirementOf, .optionalRequirementOf, .extensionTo, .declaredIn:
+            return true
+        default:
+            return false
+        }
+    }
+}

Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

@@ -1225,6 +1225,49 @@ class PathHierarchyTests: XCTestCase {
         XCTAssertEqual(try tree.findSymbol(path: "Something/second", parent: topLevelSymbolID).identifier.precise, "s:9SomethingAAV6secondSivp")
     }
 
+    func testSymbolsWithSameNameAsExtendedModule() throws {
+        // ---- Inner
+        // public struct InnerStruct {}
+        // public class InnerClass {}
+        //
+        // ---- Outer
+        // // Shadow the Inner module with a local type
+        // public struct Inner {}
+        //
+        // public extension InnerStruct {
+        //     func something() {}
+        // }
+        // public extension InnerClass {
+        //     func something() {}
+        // }
+        let (_, context) = try testBundleAndContext(named: "ShadowExtendedModuleWithLocalSymbol")
+        let tree = try XCTUnwrap(context.hierarchyBasedLinkResolver?.pathHierarchy)
+
+        try assertPathCollision("Outer/Inner", in: tree, collisions: [
+            ("s:m:s:e:s:5Inner0A5ClassC5OuterE9somethingyyF", "module.extension"),
+            ("s:5Outer5InnerV", "struct"),
+        ])
+        // If the first path component is ambiguous, it should have the same error as if that was a later path component.
+        try assertPathCollision("Inner", in: tree, collisions: [
+            ("s:m:s:e:s:5Inner0A5ClassC5OuterE9somethingyyF", "module.extension"),
+            ("s:5Outer5InnerV", "struct"),
+        ])
+        
+        try assertFindsPath("Inner-struct", in: tree, asSymbolID: "s:5Outer5InnerV")
+        try assertFindsPath("Inner-module.extension", in: tree, asSymbolID: "s:m:s:e:s:5Inner0A5ClassC5OuterE9somethingyyF")
+        
+        try assertFindsPath("Inner-module.extension/InnerStruct", in: tree, asSymbolID: "s:e:s:5Inner0A6StructV5OuterE9somethingyyF")
+        try assertFindsPath("Inner-module.extension/InnerClass", in: tree, asSymbolID: "s:e:s:5Inner0A5ClassC5OuterE9somethingyyF")
+        try assertFindsPath("Inner-module.extension/InnerStruct/something()", in: tree, asSymbolID: "s:5Inner0A6StructV5OuterE9somethingyyF")
+        try assertFindsPath("Inner-module.extension/InnerClass/something()", in: tree, asSymbolID: "s:5Inner0A5ClassC5OuterE9somethingyyF")
+        
+        // The "Inner" struct doesn't have "InnerStruct" or "InnerClass" descendants so the path is not ambiguous.
+        try assertFindsPath("Inner/InnerStruct", in: tree, asSymbolID: "s:e:s:5Inner0A6StructV5OuterE9somethingyyF")
+        try assertFindsPath("Inner/InnerClass", in: tree, asSymbolID: "s:e:s:5Inner0A5ClassC5OuterE9somethingyyF")
+        try assertFindsPath("Inner/InnerStruct/something()", in: tree, asSymbolID: "s:5Inner0A6StructV5OuterE9somethingyyF")
+        try assertFindsPath("Inner/InnerClass/something()", in: tree, asSymbolID: "s:5Inner0A5ClassC5OuterE9somethingyyF")
+    }
+    
     func testSnippets() throws {
         let (_, context) = try testBundleAndContext(named: "Snippets")
         let tree = try XCTUnwrap(context.hierarchyBasedLinkResolver?.pathHierarchy)

Tests/SwiftDocCTests/Test Bundles/ShadowExtendedModuleWithLocalSymbol.docc/Outer.symbols.json

@@ -0,0 +1,85 @@
+{
+    "metadata": {
+        "formatVersion": {
+            "major": 0,
+            "minor": 6,
+            "patch": 0
+        },
+        "generator": "Apple Swift version 5.9 (swiftlang-5.9.0.128.108 clang-1500.0.40.1)"
+    },
+    "module": {
+        "name": "Outer",
+        "platform": {
+            "architecture": "arm64",
+            "operatingSystem": {
+                "minimumVersion": {
+                    "major": 14,
+                    "minor": 0
+                },
+                "name": "macosx"
+            },
+            "vendor": "apple"
+        }
+    },
+    "relationships": [],
+    "symbols": [
+        {
+            "accessLevel": "public",
+            "declarationFragments": [
+                {
+                    "kind": "keyword",
+                    "spelling": "struct"
+                },
+                {
+                    "kind": "text",
+                    "spelling": " "
+                },
+                {
+                    "kind": "identifier",
+                    "spelling": "Inner"
+                }
+            ],
+            "identifier": {
+                "interfaceLanguage": "swift",
+                "precise": "s:5Outer5InnerV"
+            },
+            "kind": {
+                "displayName": "Structure",
+                "identifier": "swift.struct"
+            },
+            "location": {
+                "position": {
+                    "character": 14,
+                    "line": 11
+                },
+                "uri": "file:///Users/username/path/to/ShadowExtendedModuleWithLocalSymbol/Outer.swift"
+            },
+            "names": {
+                "navigator": [
+                    {
+                        "kind": "identifier",
+                        "spelling": "Inner"
+                    }
+                ],
+                "subHeading": [
+                    {
+                        "kind": "keyword",
+                        "spelling": "struct"
+                    },
+                    {
+                        "kind": "text",
+                        "spelling": " "
+                    },
+                    {
+                        "kind": "identifier",
+                        "spelling": "Inner"
+                    }
+                ],
+                "title": "Inner"
+            },
+            "pathComponents": [
+                "Inner"
+            ]
+        }
+    ]
+}