Skip to content

Commit 3a3abff

Browse files
daniel-grumbergdaniel-grumberg
committed
Ensure that identical nodes don't get created when building up a path hierarchy
rdar://101650517
1 parent af7c45f commit 3a3abff

File tree

6 files changed

+287
-22
lines changed

6 files changed

+287
-22
lines changed

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

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,13 @@ struct PathHierarchy {
102102
var nodes: [String: Node] = [:]
103103
nodes.reserveCapacity(graph.symbols.count)
104104
for (id, symbol) in graph.symbols {
105-
let node = Node(symbol: symbol)
106-
nodes[id] = node
107-
allNodes[id, default: []].append(node)
105+
if let existingNode = allNodes[id]?.first(where: { $0.symbol!.identifier == symbol.identifier }) {
106+
nodes[id] = existingNode
107+
} else {
108+
let node = Node(symbol: symbol)
109+
nodes[id] = node
110+
allNodes[id, default: []].append(node)
111+
}
108112
}
109113

110114
var topLevelCandidates = nodes
@@ -113,15 +117,11 @@ struct PathHierarchy {
113117
continue
114118
}
115119
if let targetNode = nodes[relationship.target] {
116-
if targetNode.add(symbolChild: sourceNode) {
117-
nodes[relationship.source] = nil
118-
}
120+
targetNode.add(symbolChild: sourceNode)
119121
topLevelCandidates.removeValue(forKey: relationship.source)
120122
} else if let targetNodes = allNodes[relationship.target] {
121123
for targetNode in targetNodes {
122-
if targetNode.add(symbolChild: sourceNode) {
123-
nodes[relationship.source] = nil
124-
}
124+
targetNode.add(symbolChild: sourceNode)
125125
}
126126
topLevelCandidates.removeValue(forKey: relationship.source)
127127
} else {
@@ -136,7 +136,7 @@ struct PathHierarchy {
136136

137137
// The hierarchy doesn't contain any non-symbol nodes yet. It's OK to unwrap the `symbol` property.
138138
for topLevelNode in topLevelCandidates.values where topLevelNode.symbol!.pathComponents.count == 1 {
139-
_ = moduleNode.add(symbolChild: topLevelNode)
139+
moduleNode.add(symbolChild: topLevelNode)
140140
}
141141

142142
for node in topLevelCandidates.values where node.symbol!.pathComponents.count > 1 {
@@ -156,10 +156,10 @@ struct PathHierarchy {
156156
for component in components {
157157
let component = Self.parse(pathComponent: component[...])
158158
let nodeWithoutSymbol = Node(name: component.name)
159-
_ = parent.add(child: nodeWithoutSymbol, kind: component.kind, hash: component.hash)
159+
parent.add(child: nodeWithoutSymbol, kind: component.kind, hash: component.hash)
160160
parent = nodeWithoutSymbol
161161
}
162-
_ = parent.add(symbolChild: node)
162+
parent.add(symbolChild: node)
163163
}
164164
}
165165

@@ -240,7 +240,7 @@ struct PathHierarchy {
240240
let newNode = Node(name: name)
241241
newNode.identifier = newReference
242242
self.lookup[newReference] = newNode
243-
_ = parent.add(child: newNode, kind: kind, hash: nil)
243+
parent.add(child: newNode, kind: kind, hash: nil)
244244

245245
return newReference
246246
}
@@ -554,19 +554,19 @@ extension PathHierarchy {
554554
}
555555

556556
/// Adds a descendant to this node, providing disambiguation information from the node's symbol.
557-
fileprivate func add(symbolChild: Node) -> Bool {
557+
fileprivate func add(symbolChild: Node) {
558558
precondition(symbolChild.symbol != nil)
559-
return add(
559+
add(
560560
child: symbolChild,
561561
kind: symbolChild.symbol!.kind.identifier.identifier,
562562
hash: symbolChild.symbol!.identifier.precise.stableHashString
563563
)
564564
}
565565

566566
/// Adds a descendant of this node.
567-
fileprivate func add(child: Node, kind: String?, hash: String?) -> Bool {
567+
fileprivate func add(child: Node, kind: String?, hash: String?) {
568568
child.parent = self
569-
return children[child.name, default: .init()].add(kind ?? "_", hash ?? "_", child)
569+
children[child.name, default: .init()].add(kind ?? "_", hash ?? "_", child)
570570
}
571571

572572
/// Combines this node with another node.
@@ -980,20 +980,16 @@ private struct DisambiguationTree {
980980
/// - hash: The hash disambiguation for this value.
981981
/// - value: The new value
982982
/// - Returns: If a value already exist with the same pair of kind and hash disambiguations.
983-
@discardableResult
984-
mutating func add(_ kind: String, _ hash: String, _ value: PathHierarchy.Node) -> Bool {
983+
mutating func add(_ kind: String, _ hash: String, _ value: PathHierarchy.Node) {
985984
if let existing = storage[kind]?[hash] {
986985
existing.merge(with: value)
987-
return true
988986
} else if storage.count == 1, let existing = storage["_"]?["_"] {
989987
// It is possible for articles and other non-symbols to collide with unfindable symbol placeholder nodes.
990988
// When this happens, remove the placeholder node and move its children to the real (non-symbol) node.
991989
value.merge(with: existing)
992990
storage = [kind: [hash: value]]
993-
return true
994991
} else {
995992
storage[kind, default: [:]][hash] = value
996-
return false
997993
}
998994
}
999995

‎Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,6 +1194,15 @@ class PathHierarchyTests: XCTestCase {
11941194
XCTAssertEqual(paths["X.Y2.Z.W"], "/Module/X/Y2/Z/W")
11951195
}
11961196

1197+
func testMultiPlatformModuleWithExtension() throws {
1198+
try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver)
1199+
let (_, context) = try testBundleAndContext(named: "MultiPlatformModuleWithExtension")
1200+
let tree = try XCTUnwrap(context.hierarchyBasedLinkResolver?.pathHierarchy)
1201+
1202+
try assertFindsPath("/MainModule/TopLevelProtocol/extensionMember(_:)", in: tree, asSymbolID: "extensionMember1")
1203+
try assertFindsPath("/MainModule/TopLevelProtocol/InnerStruct/extensionMember(_:)", in: tree, asSymbolID: "extensionMember2")
1204+
}
1205+
11971206
func testParsingPaths() {
11981207
// Check path components without disambiguation
11991208
assertParsedPathComponents("", [])

‎Tests/SwiftDocCTests/Test Bundles/MultiPlatformModuleWithExtension.docc/Info.plist

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
3+
<plist version="1.0">
4+
<dict>
5+
<key>CFBundleName</key>
6+
<string>MainModule</string>
7+
<key>CFBundleDisplayName</key>
8+
<string>MainModule</string>
9+
<key>CFBundleIdentifier</key>
10+
<string>org.swift.MainModule</string>
11+
<key>CFBundleDevelopmentRegion</key>
12+
<string>en</string>
13+
<key>CFBundleIconFile</key>
14+
<string>DocumentationIcon</string>
15+
<key>CFBundleIconName</key>
16+
<string>DocumentationIcon</string>
17+
<key>CFBundlePackageType</key>
18+
<string>DOCS</string>
19+
<key>CFBundleSignature</key>
20+
<string>????</string>
21+
<key>CFBundleShortVersionString</key>
22+
<string>0.1.0</string>
23+
<key>CFBundleVersion</key>
24+
<string>0.1.0</string>
25+
</dict>
26+
</plist>

‎Tests/SwiftDocCTests/Test Bundles/MultiPlatformModuleWithExtension.docc/Symbols/platform1-MainModule.symbols.json

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
{
2+
"metadata" : {
3+
"formatVersion" : {
4+
"major" : 0,
5+
"minor" : 5,
6+
"patch" : 3
7+
},
8+
"generator" : "tests"
9+
},
10+
"module" : {
11+
"name" : "MainModule",
12+
"platform" : {
13+
"architecture" : "arm64",
14+
"operatingSystem" : {
15+
"minimumVersion" : {
16+
"major" : 16,
17+
"minor" : 0,
18+
"patch" : 0
19+
},
20+
"name" : "ios"
21+
},
22+
"vendor" : "apple"
23+
}
24+
},
25+
"relationships" : [
26+
{
27+
"kind" : "memberOf",
28+
"source" : "InnerStruct",
29+
"target" : "TopLevelProtocolP"
30+
}
31+
],
32+
"symbols" : [
33+
{
34+
"accessLevel" : "public",
35+
"identifier" : {
36+
"interfaceLanguage" : "swift",
37+
"precise" : "TopLevelProtocolP"
38+
},
39+
"kind" : {
40+
"displayName" : "Protocol",
41+
"identifier" : "swift.protocol"
42+
},
43+
"names" : {
44+
"title" : "TopLevelProtocol"
45+
},
46+
"pathComponents" : [
47+
"TopLevelProtocol"
48+
]
49+
},
50+
{
51+
"accessLevel" : "public",
52+
"identifier" : {
53+
"interfaceLanguage" : "swift",
54+
"precise" : "InnerStruct"
55+
},
56+
"kind" : {
57+
"displayName" : "Struct",
58+
"identifier" : "swift.struct"
59+
},
60+
"names" : {
61+
"title" : "InnerStruct"
62+
},
63+
"pathComponents" : [
64+
"TopLevelProtocol",
65+
"InnerStruct"
66+
]
67+
}
68+
]
69+
}

‎Tests/SwiftDocCTests/Test Bundles/MultiPlatformModuleWithExtension.docc/Symbols/[email protected]

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
{
2+
"metadata" : {
3+
"formatVersion" : {
4+
"major" : 0,
5+
"minor" : 5,
6+
"patch" : 3
7+
},
8+
"generator" : "tests"
9+
},
10+
"module" : {
11+
"name" : "ExtensionModule",
12+
"platform" : {
13+
"architecture" : "armv7k",
14+
"operatingSystem" : {
15+
"minimumVersion" : {
16+
"major" : 9,
17+
"minor" : 0,
18+
"patch" : 0
19+
},
20+
"name" : "watchos"
21+
},
22+
"vendor" : "apple"
23+
}
24+
},
25+
"relationships" : [
26+
{
27+
"kind" : "memberOf",
28+
"source" : "extensionMember1",
29+
"target" : "TopLevelProtocolP"
30+
},
31+
{
32+
"kind" : "memberOf",
33+
"source" : "extensionMember2",
34+
"target" : "InnerStruct"
35+
}
36+
],
37+
"symbols" : [
38+
{
39+
"accessLevel" : "public",
40+
"identifier" : {
41+
"interfaceLanguage" : "swift",
42+
"precise" : "extensionMember1"
43+
},
44+
"docComment" : {
45+
"lines" : [
46+
{
47+
"text" : "``extensionMember(:_)``"
48+
}
49+
]
50+
},
51+
"kind" : {
52+
"displayName" : "Instance Method",
53+
"identifier" : "swift.method"
54+
},
55+
"names" : {
56+
"title" : "extensionMember(_:)"
57+
},
58+
"pathComponents" : [
59+
"TopLevelProtocol",
60+
"extensionMember(_:)"
61+
],
62+
"swiftExtension" : {
63+
"extendedModule" : "MainModule"
64+
}
65+
},
66+
{
67+
"accessLevel" : "public",
68+
"identifier" : {
69+
"interfaceLanguage" : "swift",
70+
"precise" : "extensionMember2"
71+
},
72+
"docComment" : {
73+
"lines" : [
74+
{
75+
"text" : "``extensionMember(:_)``"
76+
}
77+
]
78+
},
79+
"kind" : {
80+
"displayName" : "Instance Method",
81+
"identifier" : "swift.method"
82+
},
83+
"names" : {
84+
"title" : "extensionMember(_:)"
85+
},
86+
"pathComponents" : [
87+
"TopLevelProtocol",
88+
"InnerStruct",
89+
"extensionMember(_:)"
90+
],
91+
"swiftExtension" : {
92+
"extendedModule" : "MainModule"
93+
}
94+
}
95+
]
96+
}

‎Tests/SwiftDocCTests/Test Bundles/MultiPlatformModuleWithExtension.docc/Symbols/platform2-MainModule.symbols.json

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
{
2+
"metadata" : {
3+
"formatVersion" : {
4+
"major" : 0,
5+
"minor" : 5,
6+
"patch" : 3
7+
},
8+
"generator" : "tests"
9+
},
10+
"module" : {
11+
"name" : "MainModule",
12+
"platform" : {
13+
"architecture" : "armv7k",
14+
"operatingSystem" : {
15+
"minimumVersion" : {
16+
"major" : 9,
17+
"minor" : 0,
18+
"patch" : 0
19+
},
20+
"name" : "watchos"
21+
},
22+
"vendor" : "apple"
23+
}
24+
},
25+
"relationships" : [
26+
{
27+
"kind" : "memberOf",
28+
"source" : "InnerStruct",
29+
"target" : "TopLevelProtocolP"
30+
}
31+
],
32+
"symbols" : [
33+
{
34+
"accessLevel" : "public",
35+
"identifier" : {
36+
"interfaceLanguage" : "swift",
37+
"precise" : "TopLevelProtocolP"
38+
},
39+
"kind" : {
40+
"displayName" : "Protocol",
41+
"identifier" : "swift.protocol"
42+
},
43+
"names" : {
44+
"title" : "TopLevelProtocol"
45+
},
46+
"pathComponents" : [
47+
"TopLevelProtocol"
48+
]
49+
},
50+
{
51+
"accessLevel" : "public",
52+
"identifier" : {
53+
"interfaceLanguage" : "swift",
54+
"precise" : "InnerStruct"
55+
},
56+
"kind" : {
57+
"displayName" : "Struct",
58+
"identifier" : "swift.struct"
59+
},
60+
"names" : {
61+
"title" : "InnerStruct"
62+
},
63+
"pathComponents" : [
64+
"TopLevelProtocol",
65+
"InnerStruct"
66+
]
67+
}
68+
]
69+
}

0 commit comments

Comments
 (0)