Fix inconsistent assertions with some language variant symbols (#894)

* Add more information in debug assert message

* Fix a bug where symbol with only capitalization differences in their language counterparts (same ID and kind) sometimes wouldn't get assigned a path.

rdar://125948615

* Fix a bug where extensions to symbols with different kinds in each language representation hit a debug assertion about inconsistent path hierarchies

rdar://125948615

* Minor implementation comment refinements

Author: d-ronnqvist

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

@@ -157,9 +157,23 @@ struct PathHierarchy {
                 if let targetNode = nodes[relationship.target], targetNode.name == expectedContainerName {
                     targetNode.add(symbolChild: sourceNode)
                     topLevelCandidates.removeValue(forKey: relationship.source)
-                } else if let targetNodes = allNodes[relationship.target] {
-                    for targetNode in targetNodes where targetNode.name == expectedContainerName {
+                } else if var targetNodes = allNodes[relationship.target] {
+                    // If the source was added in an extension symbol graph file, then its target won't be found in the same symbol graph file (in `nodes`).
+                    
+                    // We may have encountered multiple language representations of the target symbol. Try to find the best matching representation of the target to add the source to.
+                    // Remove any targets that don't match the source symbol's path components (see comment above for more details).
+                    targetNodes.removeAll(where: { $0.name != expectedContainerName })
+                    
+                    // Prefer the symbol that matches the relationship's language.
+                    if let targetNode = targetNodes.first(where: { $0.symbol!.identifier.interfaceLanguage == language?.id }) {
                         targetNode.add(symbolChild: sourceNode)
+                    } else {
+                        // It's not clear which target to add the source to, so we add it to all of them.
+                        // This will likely hit a _debug_ assertion (later in this initializer) about inconsistent traversal through the hierarchy,
+                        // but in release builds DocC will "repair" the inconsistent hierarchy.
+                        for targetNode in targetNodes {
+                            targetNode.add(symbolChild: sourceNode)
+                        }
                     }
                     topLevelCandidates.removeValue(forKey: relationship.source)
                 } else {
@@ -294,8 +308,9 @@ struct PathHierarchy {
                     assert(element.node.parent === node, {
                         func describe(_ node: Node?) -> String {
                             guard let node else { return "<nil>" }
-                            guard let identifier = node.symbol?.identifier else { return node.name }
-                            return "\(identifier.precise) (\(identifier.interfaceLanguage))"
+                            guard let symbol = node.symbol else { return node.name }
+                            let id = symbol.identifier
+                            return "\(id.precise) (\(id.interfaceLanguage).\(symbol.kind.identifier.identifier)) [\(symbol.pathComponents.joined(separator: "/"))]"
                         }
                         return """
                             Every child node should point back to its parent so that the tree can be traversed both up and down without any dead-ends. \
@@ -613,8 +628,19 @@ extension PathHierarchy.DisambiguationContainer {
     /// Combines the data from this tree with another tree to form a new, merged disambiguation tree.
     func merge(with other: Self) -> Self {
         var newStorage = storage
-        for element in other.storage where !storage.contains(where: { $0.matches(kind: element.kind, hash: element.hash )}) {
-            newStorage.append(element)
+        for element in other.storage {
+            if let existingIndex = storage.firstIndex(where: { $0.matches(kind: element.kind, hash: element.hash )}) {
+                let existing = storage[existingIndex]
+                // If the same element exist in both containers, keep it unless the "other" element is the Swift counterpart of this symbol.
+                if existing.node.counterpart === element.node,
+                   element.node.symbol?.identifier.interfaceLanguage == "swift"
+                {
+                    // The "other" element is the Swift counterpart. Replace the existing element with it.
+                    newStorage[existingIndex] = element
+                }
+            } else {
+                newStorage.append(element)
+            }
         }
         return .init(storage: newStorage)
     }

Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

@@ -1716,7 +1716,7 @@ class PathHierarchyTests: XCTestCase {
         let exampleDocumentation = Folder(name: "unit-test.docc", content: [
             JSONFile(name: "Module.symbols.json", content: makeSymbolGraph(
                 moduleName: "Module",
-                symbols: symbolPaths.map { ($0.joined(separator: "."), .swift, $0) }
+                symbols: symbolPaths.map { ($0.joined(separator: "."), .swift, $0, .class) }
             )),
         ])
         let tempURL = try createTemporaryDirectory()
@@ -1744,7 +1744,7 @@ class PathHierarchyTests: XCTestCase {
         XCTAssertEqual(paths["X.Y2.Z.W"], "/Module/X/Y2/Z/W")
     }
 
-    func testMixedLanguageSymbolAndItsExtendingModule() throws {
+    func testMixedLanguageSymbolWithSameKindAndAddedMemberFromExtendingModule() throws {
         let containerID = "some-container-symbol-id"
         let memberID = "some-member-symbol-id"
 
@@ -1753,7 +1753,7 @@ class PathHierarchyTests: XCTestCase {
                 JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
                     moduleName: "ModuleName", 
                     symbols: [
-                        (containerID, .objectiveC, ["ContainerName"])
+                        (containerID, .objectiveC, ["ContainerName"], .class)
                     ]
                 )),
             ]),
@@ -1762,14 +1762,14 @@ class PathHierarchyTests: XCTestCase {
                 JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
                     moduleName: "ModuleName",
                     symbols: [
-                        (containerID, .swift, ["ContainerName"])
+                        (containerID, .swift, ["ContainerName"], .class)
                     ]
                 )),
 
                 JSONFile(name: "[email protected]", content: makeSymbolGraph(
                     moduleName: "ExtendingModule",
                     symbols: [
-                        (memberID, .swift, ["ContainerName", "MemberName"])
+                        (memberID, .swift, ["ContainerName", "MemberName"], .property)
                     ],
                     relationships: [
                         .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil)
@@ -1787,6 +1787,90 @@ class PathHierarchyTests: XCTestCase {
         XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName/MemberName")
     }
 
+    func testMixedLanguageSymbolWithDifferentKindsAndAddedMemberFromExtendingModule() throws {
+        let containerID = "some-container-symbol-id"
+        let memberID = "some-member-symbol-id"
+        
+        let exampleDocumentation = Folder(name: "unit-test.docc", content: [
+            Folder(name: "clang", content: [
+                JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
+                    moduleName: "ModuleName",
+                    symbols: [
+                        (containerID, .objectiveC, ["ContainerName"], .typealias)
+                    ]
+                )),
+            ]),
+            
+            Folder(name: "swift", content: [
+                JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
+                    moduleName: "ModuleName",
+                    symbols: [
+                        (containerID, .swift, ["ContainerName"], .struct)
+                    ]
+                )),
+                
+                JSONFile(name: "[email protected]", content: makeSymbolGraph(
+                    moduleName: "ExtendingModule",
+                    symbols: [
+                        (memberID, .swift, ["ContainerName", "MemberName"], .property)
+                    ],
+                    relationships: [
+                        .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil)
+                    ]
+                )),
+            ])
+        ])
+        
+        let tempURL = try createTempFolder(content: [exampleDocumentation])
+        let (_, _, context) = try loadBundle(from: tempURL)
+        let tree = context.linkResolver.localResolver.pathHierarchy
+        
+        let paths = tree.caseInsensitiveDisambiguatedPaths()
+        XCTAssertEqual(paths[containerID], "/ModuleName/ContainerName")
+        XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName/MemberName")
+    }
+    
+    func testLanguageRepresentationsWithDifferentCapitalization() throws {
+        let containerID = "some-container-symbol-id"
+        let memberID = "some-member-symbol-id"
+        
+        let exampleDocumentation = Folder(name: "unit-test.docc", content: [
+            Folder(name: "clang", content: [
+                JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
+                    moduleName: "ModuleName", 
+                    symbols: [
+                        (containerID, .objectiveC, ["ContainerName"], .class),
+                        (memberID, .objectiveC, ["ContainerName", "MemberName"], .property), // member starts with uppercase "M"
+                    ],
+                    relationships: [
+                        .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil)
+                    ]
+                )),
+            ]),
+            
+            Folder(name: "swift", content: [
+                JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
+                    moduleName: "ModuleName",
+                    symbols: [
+                        (containerID, .swift, ["ContainerName"], .class),
+                        (memberID, .swift, ["ContainerName", "memberName"], .property), // member starts with lowercase "m"
+                    ],
+                    relationships: [
+                        .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil)
+                    ]
+                )),
+            ])
+        ])
+        
+        let tempURL = try createTempFolder(content: [exampleDocumentation])
+        let (_, _, context) = try loadBundle(from: tempURL)
+        let tree = context.linkResolver.localResolver.pathHierarchy
+        
+        let paths = tree.caseInsensitiveDisambiguatedPaths()
+        XCTAssertEqual(paths[containerID], "/ModuleName/ContainerName")
+        XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName/memberName") // The Swift spelling is preferred
+    }
+    
     func testMixedLanguageSymbolAndItsExtendingModuleWithDifferentContainerNames() throws {
         let containerID = "some-container-symbol-id"
         let memberID = "some-member-symbol-id"
@@ -1796,7 +1880,7 @@ class PathHierarchyTests: XCTestCase {
                 JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
                     moduleName: "ModuleName",
                     symbols: [
-                        (containerID, .objectiveC, ["ObjectiveCContainerName"])
+                        (containerID, .objectiveC, ["ObjectiveCContainerName"], .class)
                     ]
                 )),
             ]),
@@ -1805,14 +1889,14 @@ class PathHierarchyTests: XCTestCase {
                 JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
                     moduleName: "ModuleName",
                     symbols: [
-                        (containerID, .swift, ["SwiftContainerName"])
+                        (containerID, .swift, ["SwiftContainerName"], .class)
                     ]
                 )),
 
                 JSONFile(name: "[email protected]", content: makeSymbolGraph(
                     moduleName: "ExtendingModule",
                     symbols: [
-                        (memberID, .swift, ["SwiftContainerName", "MemberName"])
+                        (memberID, .swift, ["SwiftContainerName", "MemberName"], .property)
                     ],
                     relationships: [
                         .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil)
@@ -1839,9 +1923,9 @@ class PathHierarchyTests: XCTestCase {
             JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
                 moduleName: "ModuleName",
                 symbols: [
-                    (containerID, .swift, ["ContainerName"]),
-                    (otherID, .swift, ["ContainerName"]),
-                    (memberID, .swift, ["ContainerName", "MemberName1"]),
+                    (containerID, .swift, ["ContainerName"], .class),
+                    (otherID, .swift, ["ContainerName"], .class),
+                    (memberID, .swift, ["ContainerName", "MemberName1"], .property),
                 ],
                 relationships: [
                     .init(source: memberID, target: containerID, kind: .optionalMemberOf, targetFallback: nil),
@@ -1864,7 +1948,7 @@ class PathHierarchyTests: XCTestCase {
             JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
                 moduleName: "ModuleName",
                 symbols: [
-                    ("some-symbol-id", .swift, ["SymbolName"]),
+                    ("some-symbol-id", .swift, ["SymbolName"], .class),
                 ],
                 relationships: []
             )),
@@ -1963,7 +2047,7 @@ class PathHierarchyTests: XCTestCase {
             JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
                 moduleName: "ModuleName",
                 symbols: [
-                    (symbolID, .swift, ["SymbolName"]),
+                    (symbolID, .swift, ["SymbolName"], .class),
                 ],
                 relationships: []
             )),
@@ -1997,9 +2081,9 @@ class PathHierarchyTests: XCTestCase {
                 moduleName: "ModuleName",
                 platformName: platformName,
                 symbols: [
-                    (protocolID, .swift, ["SomeProtocolName"]),
-                    (protocolRequirementID, .swift, ["SomeProtocolName", "someProtocolRequirement()"]),
-                    (defaultImplementationID, .swift, ["SomeConformingType", "someProtocolRequirement()"]),
+                    (protocolID, .swift, ["SomeProtocolName"], .class),
+                    (protocolRequirementID, .swift, ["SomeProtocolName", "someProtocolRequirement()"], .class),
+                    (defaultImplementationID, .swift, ["SomeConformingType", "someProtocolRequirement()"], .class),
                 ],
                 relationships: [
                     .init(source: protocolRequirementID, target: protocolID, kind: .requirementOf, targetFallback: nil),
@@ -2089,20 +2173,20 @@ class PathHierarchyTests: XCTestCase {
     private func makeSymbolGraph(
         moduleName: String,
         platformName: String? = nil,
-        symbols: [(identifier: String, language: SourceLanguage, pathComponents: [String])],
+        symbols: [(identifier: String, language: SourceLanguage, pathComponents: [String], kindID: SymbolGraph.Symbol.KindIdentifier)],
         relationships: [SymbolGraph.Relationship] = []
     ) -> SymbolGraph {
         return SymbolGraph(
             metadata: SymbolGraph.Metadata(formatVersion: .init(major: 0, minor: 5, patch: 3), generator: "unit-test"),
             module: SymbolGraph.Module(name: moduleName, platform: .init(operatingSystem: platformName.map { .init(name: $0) })),
-            symbols: symbols.map { identifier, language, pathComponents in
+            symbols: symbols.map { identifier, language, pathComponents, kindID in
                 SymbolGraph.Symbol(
                     identifier: .init(precise: identifier, interfaceLanguage: language.id),
                     names: .init(title: "SymbolName", navigator: nil, subHeading: nil, prose: nil), // names doesn't matter for path disambiguation
                     pathComponents: pathComponents,
                     docComment: nil,
                     accessLevel: .public,
-                    kind: .init(parsedIdentifier: .class, displayName: "Kind Display Name"), // kind display names doesn't matter for path disambiguation
+                    kind: .init(parsedIdentifier: kindID, displayName: "Kind Display Name"), // kind display names doesn't matter for path disambiguation
                     mixins: [:]
                 )
             },