diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift index 7f5fcf58b7..4ee41eb93d 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift +++ b/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) } diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+PathComponent.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+PathComponent.swift index 3eddd954b4..fbae8423d5 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+PathComponent.swift +++ b/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 = { + // 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. diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift index 1f740673ae..f0a7438c22 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift +++ b/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 + } + } +} diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index f1f406700b..240ab3d56e 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/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) diff --git a/Tests/SwiftDocCTests/Test Bundles/ShadowExtendedModuleWithLocalSymbol.docc/Outer.symbols.json b/Tests/SwiftDocCTests/Test Bundles/ShadowExtendedModuleWithLocalSymbol.docc/Outer.symbols.json new file mode 100644 index 0000000000..4120a6f7e3 --- /dev/null +++ b/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" + ] + } + ] +} diff --git a/Tests/SwiftDocCTests/Test Bundles/ShadowExtendedModuleWithLocalSymbol.docc/Outer@Inner.symbols.json b/Tests/SwiftDocCTests/Test Bundles/ShadowExtendedModuleWithLocalSymbol.docc/Outer@Inner.symbols.json new file mode 100644 index 0000000000..897241f319 --- /dev/null +++ b/Tests/SwiftDocCTests/Test Bundles/ShadowExtendedModuleWithLocalSymbol.docc/Outer@Inner.symbols.json @@ -0,0 +1,326 @@ +{ + "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": [ + { + "kind": "extensionTo", + "source": "s:e:s:5Inner0A6StructV5OuterE9somethingyyF", + "target": "s:5Inner0A6StructV", + "targetFallback": "Inner.InnerStruct" + }, + { + "kind": "extensionTo", + "source": "s:e:s:5Inner0A5ClassC5OuterE9somethingyyF", + "target": "s:5Inner0A5ClassC", + "targetFallback": "Inner.InnerClass" + }, + { + "kind": "memberOf", + "source": "s:5Inner0A5ClassC5OuterE9somethingyyF", + "target": "s:e:s:5Inner0A5ClassC5OuterE9somethingyyF", + "targetFallback": "Inner.InnerClass" + }, + { + "kind": "memberOf", + "source": "s:5Inner0A6StructV5OuterE9somethingyyF", + "target": "s:e:s:5Inner0A6StructV5OuterE9somethingyyF", + "targetFallback": "Inner.InnerStruct" + } + ], + "symbols": [ + { + "accessLevel": "public", + "declarationFragments": [ + { + "kind": "keyword", + "spelling": "func" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "something" + }, + { + "kind": "text", + "spelling": "()" + } + ], + "functionSignature": { + "returns": [ + { + "kind": "text", + "spelling": "()" + } + ] + }, + "identifier": { + "interfaceLanguage": "swift", + "precise": "s:5Inner0A5ClassC5OuterE9somethingyyF" + }, + "kind": { + "displayName": "Instance Method", + "identifier": "swift.method" + }, + "location": { + "position": { + "character": 9, + "line": 17 + }, + "uri": "file:///Users/username/path/to/ShadowExtendedModuleWithLocalSymbol/Outer.swift" + }, + "names": { + "subHeading": [ + { + "kind": "keyword", + "spelling": "func" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "something" + }, + { + "kind": "text", + "spelling": "()" + } + ], + "title": "something()" + }, + "pathComponents": [ + "InnerClass", + "something()" + ], + "swiftExtension": { + "extendedModule": "Inner", + "typeKind": "swift.class" + } + }, + { + "accessLevel": "public", + "declarationFragments": [ + { + "kind": "keyword", + "spelling": "func" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "something" + }, + { + "kind": "text", + "spelling": "()" + } + ], + "functionSignature": { + "returns": [ + { + "kind": "text", + "spelling": "()" + } + ] + }, + "identifier": { + "interfaceLanguage": "swift", + "precise": "s:5Inner0A6StructV5OuterE9somethingyyF" + }, + "kind": { + "displayName": "Instance Method", + "identifier": "swift.method" + }, + "location": { + "position": { + "character": 9, + "line": 14 + }, + "uri": "file:///Users/username/path/to/ShadowExtendedModuleWithLocalSymbol/Outer.swift" + }, + "names": { + "subHeading": [ + { + "kind": "keyword", + "spelling": "func" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "something" + }, + { + "kind": "text", + "spelling": "()" + } + ], + "title": "something()" + }, + "pathComponents": [ + "InnerStruct", + "something()" + ], + "swiftExtension": { + "extendedModule": "Inner", + "typeKind": "swift.struct" + } + }, + { + "accessLevel": "public", + "declarationFragments": [ + { + "kind": "keyword", + "spelling": "extension" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "typeIdentifier", + "preciseIdentifier": "s:5Inner0A5ClassC", + "spelling": "InnerClass" + } + ], + "identifier": { + "interfaceLanguage": "swift", + "precise": "s:e:s:5Inner0A5ClassC5OuterE9somethingyyF" + }, + "kind": { + "displayName": "Extension", + "identifier": "swift.extension" + }, + "location": { + "position": { + "character": 7, + "line": 16 + }, + "uri": "file:///Users/username/path/to/ShadowExtendedModuleWithLocalSymbol/Outer.swift" + }, + "names": { + "navigator": [ + { + "kind": "identifier", + "spelling": "InnerClass" + } + ], + "subHeading": [ + { + "kind": "keyword", + "spelling": "extension" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "typeIdentifier", + "preciseIdentifier": "s:5Inner0A5ClassC", + "spelling": "InnerClass" + } + ], + "title": "InnerClass" + }, + "pathComponents": [ + "InnerClass" + ], + "swiftExtension": { + "extendedModule": "Inner", + "typeKind": "swift.class" + } + }, + { + "accessLevel": "public", + "declarationFragments": [ + { + "kind": "keyword", + "spelling": "extension" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "typeIdentifier", + "preciseIdentifier": "s:5Inner0A6StructV", + "spelling": "InnerStruct" + } + ], + "identifier": { + "interfaceLanguage": "swift", + "precise": "s:e:s:5Inner0A6StructV5OuterE9somethingyyF" + }, + "kind": { + "displayName": "Extension", + "identifier": "swift.extension" + }, + "location": { + "position": { + "character": 7, + "line": 13 + }, + "uri": "file:///Users/username/path/to/ShadowExtendedModuleWithLocalSymbol/Outer.swift" + }, + "names": { + "navigator": [ + { + "kind": "identifier", + "spelling": "InnerStruct" + } + ], + "subHeading": [ + { + "kind": "keyword", + "spelling": "extension" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "typeIdentifier", + "preciseIdentifier": "s:5Inner0A6StructV", + "spelling": "InnerStruct" + } + ], + "title": "InnerStruct" + }, + "pathComponents": [ + "InnerStruct" + ], + "swiftExtension": { + "extendedModule": "Inner", + "typeKind": "swift.struct" + } + } + ] +}