Try to repair symbol graphs with deep hierarchies but no actual memberOf relationships

Author: d-ronnqvist

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

@@ -299,12 +299,13 @@ struct PathHierarchy {
                     // FIXME:
                     // This code path is both expected (when `knownDisambiguatedPathComponents` is non-nil) and unexpected (when the symbol graph is missing data or contains extra relationships).
                     // It would be good to restructure this code to better distinguish what's supported behavior and what's a best-effort attempt at gracefully handle invalid symbol graphs.
-                    if let existing = parent.children[components.first!] {
-                        //
+                    if let existing = parent.children[component] {
+                        // This code tries to repair incomplete symbol graph files by guessing that the symbol with the most overlapping languages is the intended container.
+                        // Valid symbol graph files we should never end up here.
                         var bestLanguageMatch: (node: Node, count: Int)?
                         for element in existing.storage {
                             let numberOfMatchingLanguages = node.languages.intersection(element.node.languages).count
-                            if numberOfMatchingLanguages < (bestLanguageMatch?.count ?? .max) {
+                            if (bestLanguageMatch?.count ?? .min) < numberOfMatchingLanguages {
                                 bestLanguageMatch = (node: element.node, count: numberOfMatchingLanguages)
                             }
                         }
@@ -316,7 +317,7 @@ struct PathHierarchy {
                     }
 
                     assert(
-                        parent.children[components.first!] == nil,
+                        parent.children[component] == nil,
                         "Shouldn't create a new sparse node when symbol node already exist. This is an indication that a symbol is missing a relationship."
                     )
 

Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

@@ -3200,6 +3200,89 @@ class PathHierarchyTests: XCTestCase {
         try assertFindsPath("/ModuleName/ContainerName", in: tree, asSymbolID: containerID)
     }
 
+    func testInvalidSymbolGraphWithNoMemberOfRelationshipsDesptiteDeepHierarchyAcrossManyPlatforms() throws {
+        // We make a best-effort attempt to create a valid path hierarchy, even if the symbol graph inputs are not valid.
+        
+        // If the symbol graph files define a deep hierarchy, with the same symbol names but different symbol kinds across different, we try to match them up by language.
+        
+        // Repeat the same symbols in both languages for many platforms.
+        let platforms = (1...10).map {
+            let name = "Platform\($0)"
+            return (name: name, availability: [makeAvailabilityItem(domainName: name)])
+        }
+        
+        let catalog = Folder(name: "unit-test.docc", content: [
+            Folder(name: "clang", content: platforms.map { platform in
+                return JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph(
+                    moduleName: "ModuleName",
+                    symbols: [
+                        makeSymbol(id: "some-outer-container-id",  language: .objectiveC, kind: .class, pathComponents: ["OuterContainerName"]),
+                        makeSymbol(id: "some-middle-container-id", language: .objectiveC, kind: .class, pathComponents: ["OuterContainerName", "MiddleContainerName"]),
+                        makeSymbol(id: "some-inner-container-id",  language: .objectiveC, kind: .class, pathComponents: ["OuterContainerName", "MiddleContainerName", "InnerContainerName"]),
+                        makeSymbol(id: "some-objc-specific-member-id", language: .objectiveC, kind: .property, pathComponents: ["OuterContainerName", "MiddleContainerName", "InnerContainerName", "objcSpecificMember"]),
+                    ],
+                    relationships: [/* all required memberOf relationships all missing */]
+                ))
+            }),
+            
+            Folder(name: "swift", content: platforms.map { platform in
+                return JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph(
+                    moduleName: "ModuleName",
+                    symbols: [
+                        makeSymbol(id: "some-outer-container-id",  kind: .struct, pathComponents: ["OuterContainerName"]),
+                        makeSymbol(id: "some-middle-container-id", kind: .struct, pathComponents: ["OuterContainerName", "MiddleContainerName"]),
+                        makeSymbol(id: "some-inner-container-id",  kind: .struct, pathComponents: ["OuterContainerName", "MiddleContainerName", "InnerContainerName"]),
+                        makeSymbol(id: "some-swift-specific-member-id", kind: .method, pathComponents: ["OuterContainerName", "MiddleContainerName", "InnerContainerName", "swiftSpecificMember()"]),
+                    ],
+                    relationships: [/* all required memberOf relationships all missing */]
+                ))
+            })
+        ])
+        
+        let (_, context) = try loadBundle(catalog: catalog)
+        let tree = context.linkResolver.localResolver.pathHierarchy
+        
+        let swiftSpecificNode = try tree.findNode(path: "/ModuleName/OuterContainerName-struct/MiddleContainerName-struct/InnerContainerName-struct/swiftSpecificMember()", onlyFindSymbols: true, parent: nil)
+        XCTAssertEqual(swiftSpecificNode.symbol?.identifier.precise, "some-swift-specific-member-id")
+        // Trace up and check that each node is represented by a symbol
+        XCTAssertEqual(swiftSpecificNode.parent?.symbol?.identifier.precise, "some-inner-container-id")
+        XCTAssertEqual(swiftSpecificNode.parent?.parent?.symbol?.identifier.precise, "some-middle-container-id")
+        XCTAssertEqual(swiftSpecificNode.parent?.parent?.parent?.symbol?.identifier.precise, "some-outer-container-id")
+        
+        let objcSpecificNode = try tree.findNode(path: "/ModuleName/OuterContainerName-class/MiddleContainerName-class/InnerContainerName-class/objcSpecificMember", onlyFindSymbols: true, parent: nil)
+        XCTAssertEqual(objcSpecificNode.symbol?.identifier.precise, "some-objc-specific-member-id")
+        // Trace up and check that each node is represented by a symbol
+        XCTAssertEqual(objcSpecificNode.parent?.symbol?.identifier.precise, "some-inner-container-id")
+        XCTAssertEqual(objcSpecificNode.parent?.parent?.symbol?.identifier.precise, "some-middle-container-id")
+        XCTAssertEqual(objcSpecificNode.parent?.parent?.parent?.symbol?.identifier.precise, "some-outer-container-id")
+        
+        // Check that each language has different nodes
+        XCTAssertNotEqual(swiftSpecificNode.parent?.identifier, objcSpecificNode.parent?.identifier)
+        XCTAssertNotEqual(swiftSpecificNode.parent?.parent?.identifier, objcSpecificNode.parent?.parent?.identifier)
+        XCTAssertNotEqual(swiftSpecificNode.parent?.parent?.parent?.identifier, objcSpecificNode.parent?.parent?.parent?.identifier)
+        
+        // Check that neither path require disambiguation
+        let paths = tree.caseInsensitiveDisambiguatedPaths()
+        
+        XCTAssertEqual(paths["some-outer-container-id"], "/ModuleName/OuterContainerName")
+        XCTAssertEqual(paths["some-middle-container-id"], "/ModuleName/OuterContainerName/MiddleContainerName")
+        XCTAssertEqual(paths["some-inner-container-id"], "/ModuleName/OuterContainerName/MiddleContainerName/InnerContainerName")
+        XCTAssertEqual(paths["some-swift-specific-member-id"], "/ModuleName/OuterContainerName/MiddleContainerName/InnerContainerName/swiftSpecificMember()")
+        XCTAssertEqual(paths["some-objc-specific-member-id"], "/ModuleName/OuterContainerName/MiddleContainerName/InnerContainerName/objcSpecificMember")
+        
+        // Check that the hierarchy doesn't contain any sparse nodes
+        var remaining = tree.modules[...]
+        XCTAssertFalse(remaining.isEmpty)
+        
+        while let node = remaining.popFirst() {
+            XCTAssertNotNil(node.symbol, "Unexpected sparse node named '\(node.name)' in hierarchy")
+            
+            for container in node.children.values {
+                remaining.append(contentsOf: container.storage.map(\.node))
+            }
+        }
+    }
+    
     func testMissingReferencedContainerSymbolOnSomePlatforms() throws {
         // We make a best-effort attempt to create a valid path hierarchy, even if the symbol graph inputs are not valid.