Skip to content

Commit cfbf9ce

Browse files
authored
Fix crash when a nested symbol is extended and the a local symbol shadow extended module name (#1202)
* Use `inContextOf` relationships to form hierarchy * Fixed inversed filters in debug assertion messages * Avoid computing disambiguation during path hierarchy initialization rdar://149838723
1 parent 777fda8 commit cfbf9ce

File tree

4 files changed

+94
-13
lines changed

4 files changed

+94
-13
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,8 @@ extension PathHierarchy.DisambiguationContainer {
193193
includeLanguage: Bool = false,
194194
allowAdvancedDisambiguation: Bool = true
195195
) -> [(value: PathHierarchy.Node, disambiguation: Disambiguation)] {
196+
assert(elements.allSatisfy({ $0.node.identifier != nil }), "All nodes should have been assigned an identifier before their disambiguation can be computed.")
197+
196198
var collisions = _disambiguatedValues(for: elements, includeLanguage: includeLanguage, allowAdvancedDisambiguation: allowAdvancedDisambiguation)
197199

198200
// If all but one of the collisions are disfavored, remove the disambiguation for the only favored element.

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -447,19 +447,26 @@ extension PathHierarchy.DisambiguationContainer {
447447
case lookupCollision([(node: PathHierarchy.Node, disambiguation: String)])
448448
}
449449

450-
/// Attempts to find a value in the disambiguation tree based on partial disambiguation information.
450+
/// Attempts to find the only element in the disambiguation container without using any disambiguation information.
451+
///
452+
/// - Returns: The only element in the container or `nil` if the container has more than one element.
453+
func singleMatch() -> PathHierarchy.Node? {
454+
if storage.count <= 1 {
455+
return storage.first?.node
456+
} else {
457+
return storage.singleMatch({ !$0.node.isDisfavoredInLinkCollisions })?.node
458+
}
459+
}
460+
461+
/// Attempts to find a value in the disambiguation container based on partial disambiguation information.
451462
///
452463
/// There are 3 possible results:
453464
/// - No match is found; indicated by a `nil` return value.
454465
/// - Exactly one match is found; indicated by a non-nil return value.
455466
/// - More than one match is found; indicated by a raised error listing the matches and their missing disambiguation.
456467
func find(_ disambiguation: PathHierarchy.PathComponent.Disambiguation?) throws -> PathHierarchy.Node? {
457-
if disambiguation == nil {
458-
if storage.count <= 1 {
459-
return storage.first?.node
460-
} else if let favoredMatch = storage.singleMatch({ !$0.node.isDisfavoredInLinkCollisions }) {
461-
return favoredMatch.node
462-
}
468+
if disambiguation == nil, let match = singleMatch() {
469+
return match
463470
}
464471

465472
switch disambiguation {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ struct PathHierarchy {
232232
return original
233233
}
234234
}(node.symbol!)[...].dropLast()
235-
while !components.isEmpty, let child = try? parent.children[components.first!]?.find(nil) {
235+
while !components.isEmpty, let child = parent.children[components.first!]?.singleMatch() {
236236
parent = child
237237
components = components.dropFirst()
238238
}
@@ -311,7 +311,7 @@ struct PathHierarchy {
311311
assert(
312312
allNodes.allSatisfy({ $0.value[0].parent != nil || roots[$0.key] != nil }), """
313313
Every node should either have a parent node or be a root node. \
314-
This wasn't true for \(allNodes.filter({ $0.value[0].parent != nil || roots[$0.key] != nil }).map(\.key).sorted())
314+
This wasn't true for \(allNodes.filter({ $0.value[0].parent == nil && roots[$0.key] == nil }).map(\.key).sorted())
315315
"""
316316
)
317317

@@ -320,7 +320,7 @@ struct PathHierarchy {
320320
Array(sequence(first: node, next: \.parent)).last!.symbol!.kind.identifier == .module })
321321
}), """
322322
Every node should reach a root node by following its parents up. \
323-
This wasn't true for \(allNodes.filter({ $0.value.allSatisfy({ Array(sequence(first: $0, next: \.parent)).last!.symbol!.kind.identifier == .module }) }).map(\.key).sorted())
323+
This wasn't true for \(allNodes.filter({ $0.value.allSatisfy({ Array(sequence(first: $0, next: \.parent)).last!.symbol!.kind.identifier != .module }) }).map(\.key).sorted())
324324
"""
325325
)
326326

@@ -366,7 +366,7 @@ struct PathHierarchy {
366366
assert(
367367
lookup.allSatisfy({ $0.value.parent != nil || roots[$0.value.name] != nil }), """
368368
Every node should either have a parent node or be a root node. \
369-
This wasn't true for \(allNodes.filter({ $0.value[0].parent != nil || roots[$0.key] != nil }).map(\.key).sorted())
369+
This wasn't true for \(allNodes.filter({ $0.value[0].parent == nil && roots[$0.key] == nil }).map(\.key).sorted())
370370
"""
371371
)
372372

@@ -389,7 +389,7 @@ struct PathHierarchy {
389389
assert(
390390
lookup.values.allSatisfy({ $0.parent?.identifier == nil || lookup[$0.parent!.identifier] != nil }), """
391391
Every node's findable parent should exist in the lookup. \
392-
This wasn't true for \(lookup.values.filter({ $0.parent?.identifier == nil || lookup[$0.parent!.identifier] != nil }).map(\.symbol!.identifier.precise).sorted())
392+
This wasn't true for \(lookup.values.filter({ $0.parent?.identifier != nil && lookup[$0.parent!.identifier] == nil }).map(\.symbol!.identifier.precise).sorted())
393393
"""
394394
)
395395

@@ -799,7 +799,7 @@ private extension SymbolGraph.Relationship.Kind {
799799
/// Whether or not this relationship kind forms a hierarchical relationship between the source and the target.
800800
var formsHierarchy: Bool {
801801
switch self {
802-
case .memberOf, .optionalMemberOf, .requirementOf, .optionalRequirementOf, .extensionTo, .declaredIn:
802+
case .memberOf, .optionalMemberOf, .requirementOf, .optionalRequirementOf, .extensionTo, .inContextOf, .declaredIn:
803803
return true
804804
default:
805805
return false

Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,6 +2191,78 @@ class PathHierarchyTests: XCTestCase {
21912191
try assertFindsPath("Inner/InnerClass/something()", in: tree, asSymbolID: "s:5Inner0A5ClassC5OuterE9somethingyyF")
21922192
}
21932193

2194+
func testExtensionSymbolsWithSameNameAsExtendedModule() throws {
2195+
// ---- ExtendedModule
2196+
// public struct SomeStruct {
2197+
// public struct SomeNestedStruct {}
2198+
// }
2199+
//
2200+
// ---- ModuleName
2201+
// public import ExtendedModule
2202+
//
2203+
// // Shadow the ExtendedModule module with a local type
2204+
// public enum ExtendedModule {}
2205+
//
2206+
// // Extend the nested type from the extended module
2207+
// public extension SomeStruct.SomeNestedStruct {
2208+
// func doSomething() {}
2209+
// }
2210+
2211+
let extensionMixin = SymbolGraph.Symbol.Swift.Extension(extendedModule: "ExtendedModule", typeKind: .struct, constraints: [])
2212+
2213+
let extensionSymbolID = "s:e:s:14ExtendedModule10SomeStructV0c6NestedD0V0B4NameE11doSomethingyyF"
2214+
let extendedMethodSymbolID = "s:14ExtendedModule10SomeStructV0c6NestedD0V0B4NameE11doSomethingyyF"
2215+
2216+
let catalog = Folder(name: "CatalogName.docc", content: [
2217+
JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
2218+
moduleName: "ModuleName",
2219+
symbols: [
2220+
makeSymbol(id: "s:10ModuleName08ExtendedA0O", kind: .enum, pathComponents: ["ExtendedModule"])
2221+
])
2222+
),
2223+
2224+
JSONFile(name: "[email protected]", content: makeSymbolGraph(
2225+
moduleName: "ModuleName",
2226+
symbols: [
2227+
// The 'SomeNestedStruct' extension
2228+
makeSymbol(id: extensionSymbolID, kind: .extension, pathComponents: ["SomeStruct", "SomeNestedStruct"], otherMixins: [extensionMixin]),
2229+
// The 'doSomething()' method added in the extension
2230+
makeSymbol(id: extendedMethodSymbolID, kind: .method, pathComponents: ["SomeStruct", "SomeNestedStruct", "doSomething()"], otherMixins: [extensionMixin]),
2231+
],
2232+
relationships: [
2233+
// 'doSomething()' is a member of the extension
2234+
.init(source: extendedMethodSymbolID, target: extensionSymbolID, kind: .memberOf, targetFallback: "ExtendedModule.SomeStruct.SomeNestedStruct"),
2235+
// The extension extends the external 'SomeNestedStruct' symbol
2236+
.init(source: extensionSymbolID, target: "s:14ExtendedModule10SomeStructV0c6NestedD0V", kind: .extensionTo, targetFallback: "ExtendedModule.SomeStruct.SomeNestedStruct"),
2237+
])
2238+
),
2239+
])
2240+
2241+
let (_, context) = try loadBundle(catalog: catalog)
2242+
let tree = context.linkResolver.localResolver.pathHierarchy
2243+
2244+
let paths = tree.caseInsensitiveDisambiguatedPaths()
2245+
XCTAssertEqual(paths[extendedMethodSymbolID], "/ModuleName/ExtendedModule/SomeStruct/SomeNestedStruct/doSomething()")
2246+
2247+
try assertPathCollision("ModuleName/ExtendedModule", in: tree, collisions: [
2248+
("s:m:s:e:\(extensionSymbolID)", "-module.extension"),
2249+
("s:10ModuleName08ExtendedA0O", "-enum"),
2250+
])
2251+
// If the first path component is ambiguous, it should have the same error as if that was a later path component.
2252+
try assertPathCollision("ExtendedModule", in: tree, collisions: [
2253+
("s:m:s:e:\(extensionSymbolID)", "-module.extension"),
2254+
("s:10ModuleName08ExtendedA0O", "-enum"),
2255+
])
2256+
2257+
try assertFindsPath("ExtendedModule-enum", in: tree, asSymbolID: "s:10ModuleName08ExtendedA0O")
2258+
try assertFindsPath("ExtendedModule-module.extension", in: tree, asSymbolID: "s:m:s:e:\(extensionSymbolID)")
2259+
2260+
// The "Inner" struct doesn't have "InnerStruct" or "InnerClass" descendants so the path is not ambiguous.
2261+
try assertFindsPath("ExtendedModule/SomeStruct", in: tree, asSymbolID: "s:e:\(extensionSymbolID)")
2262+
try assertFindsPath("ExtendedModule/SomeStruct/SomeNestedStruct", in: tree, asSymbolID: extensionSymbolID)
2263+
try assertFindsPath("ExtendedModule/SomeStruct/SomeNestedStruct/doSomething()", in: tree, asSymbolID: extendedMethodSymbolID)
2264+
}
2265+
21942266
func testContinuesSearchingIfNonSymbolMatchesSymbolLink() throws {
21952267
let exampleDocumentation = Folder(name: "CatalogName.docc", content: [
21962268
JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: [

0 commit comments

Comments
 (0)