use SymbolKit's unified graph overload grouping (#986)

rdar://128624412

Package.resolved

@@ -60,7 +60,7 @@
       "location" : "https://github.com/apple/swift-docc-symbolkit",
       "state" : {
         "branch" : "main",
-        "revision" : "19e41b0f84770c41375f5f58844038dda4c5c935"
+        "revision" : "4c245d4b7264fbabb0fa1f7b3411c2c5bce4e2d9"
       }
     },
     {

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

@@ -105,7 +105,7 @@ struct PathHierarchy {
             if let language {
                 moduleNode.languages.insert(language)
             }
-            
+
             var nodes: [String: Node] = [:]
             nodes.reserveCapacity(graph.symbols.count)
             for (id, symbol) in graph.symbols {
@@ -120,16 +120,6 @@ struct PathHierarchy {
                 } else {
                     assert(!symbol.pathComponents.isEmpty, "A symbol should have at least its own name in its path components.")
 
-                    if symbol.identifier.precise.hasSuffix(SymbolGraph.Symbol.overloadGroupIdentifierSuffix),
-                       loader.unifiedGraphs[moduleNode.name]?.symbols.keys.contains(symbol.identifier.precise) != true {
-                        // Overload groups can be discarded in the unified symbol graph collector if
-                        // they don't reflect the default overload across all platforms. In this
-                        // case, we don't want to add these nodes to the path hierarchy since
-                        // they've been discarded from the unified graph that's used to generate
-                        // documentation nodes.
-                        continue
-                    }
-
                     let node = Node(symbol: symbol, name: symbol.pathComponents.last!)
                     // Disfavor synthesized symbols when they collide with other symbol with the same path.
                     // FIXME: Get information about synthesized symbols from SymbolKit https://github.com/swiftlang/swift-docc-symbolkit/issues/58
@@ -148,13 +138,6 @@ struct PathHierarchy {
                 }
             }
 
-            for relationship in graph.relationships where relationship.kind == .overloadOf {
-                // An 'overloadOf' relationship points from symbol -> group. We want to disfavor the
-                // individual overload symbols in favor of resolving links to their overload group
-                // symbol.
-                nodes[relationship.source]?.specialBehaviors.formUnion([.disfavorInLinkCollision, .excludeFromAutomaticCuration])
-            }
-
             // If there are multiple symbol graphs (for example for different source languages or platforms) then the nodes may have already been added to the hierarchy.
             var topLevelCandidates = nodes.filter { _, node in node.parent == nil }
             for relationship in graph.relationships where relationship.kind.formsHierarchy {
@@ -283,7 +266,43 @@ struct PathHierarchy {
                 parent.add(symbolChild: node)
             }
         }
-
+
+        // Overload group don't exist in the individual symbol graphs.
+        // Since overload groups don't change the _structure_ of the path hierarchy, we can add them after after all symbols for all platforms have already been added.
+        for unifiedGraph in loader.unifiedGraphs.values {
+            // Create nodes for all the overload groups
+            let overloadGroupNodes: [String: Node] = unifiedGraph.overloadGroupSymbols.reduce(into: [:]) { acc, uniqueID in
+                assert(allNodes[uniqueID] == nil,
+                       "Overload group ID \(uniqueID) already has a symbol node in the hierarchy: \(allNodes[uniqueID]!.map(\.name).sorted().joined(separator: ","))")
+                guard let unifiedSymbol = unifiedGraph.symbols[uniqueID] else { return }
+                guard let symbol = unifiedSymbol.defaultSymbol else {
+                    fatalError("Overload group \(uniqueID) doesn't have a default symbol.")
+                }
+                acc[uniqueID] = Node(symbol: symbol, name: symbol.pathComponents.last!)
+            }
+
+            for relationship in unifiedGraph.relationshipsByLanguage.flatMap(\.value) where relationship.kind == .overloadOf {
+                guard let groupNode = overloadGroupNodes[relationship.target], let overloadedSymbolNodes = allNodes[relationship.source] else {
+                    continue
+                }
+
+                for overloadedSymbolNode in overloadedSymbolNodes {
+                    // We want to disfavor the individual overload symbols in favor of resolving links to their overload group symbol.
+                    overloadedSymbolNode.specialBehaviors.formUnion([.disfavorInLinkCollision, .excludeFromAutomaticCuration])
+
+                    guard let parent = overloadedSymbolNode.parent else { continue }
+
+                    assert(groupNode.parent == nil || groupNode.parent === parent, """
+                    Unexpectedly grouped symbols with different locations in the symbol hierarchy:
+                    Group ID: \(groupNode.symbol!.identifier.precise)
+                    Locations: \(Set(overloadedSymbolNodes.map { $0.symbol!.pathComponents.joined(separator: "/") }.sorted()))
+                    """)
+                    parent.add(symbolChild: groupNode)
+                }
+                assert(groupNode.parent != nil, "Unexpectedly found no location in the hierarchy for overload group \(relationship.source)")
+            }
+        }
+
         assert(
             allNodes.allSatisfy({ $0.value[0].parent != nil || roots[$0.key] != nil }), """
             Every node should either have a parent node or be a root node. \

Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift

@@ -78,10 +78,6 @@ struct SymbolGraphLoader {
 
                 configureSymbolGraph?(&symbolGraph)
 
-                if FeatureFlags.current.isExperimentalOverloadedSymbolPresentationEnabled {
-                    symbolGraph.createOverloadGroupSymbols()
-                }
-
                 let (moduleName, isMainSymbolGraph) = Self.moduleNameFor(symbolGraph, at: symbolGraphURL)
                 // If the bundle provides availability defaults add symbol availability data.
                 self.addDefaultAvailability(to: &symbolGraph, moduleName: moduleName)
@@ -149,8 +145,10 @@ struct SymbolGraphLoader {
         }
 
         self.symbolGraphs = loadedGraphs.mapValues(\.graph)
-        (self.unifiedGraphs, self.graphLocations) = graphLoader.finishLoading()
-
+        (self.unifiedGraphs, self.graphLocations) = graphLoader.finishLoading(
+            createOverloadGroups: FeatureFlags.current.isExperimentalOverloadedSymbolPresentationEnabled
+        )
+
         for var unifiedGraph in unifiedGraphs.values {
             var defaultUnavailablePlatforms = [PlatformName]()
             var defaultAvailableInformation = [DefaultAvailability.ModuleAvailability]()

Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift

@@ -5276,6 +5276,75 @@ let expected = """
         }
     }
 
+    func testContextGeneratesOverloadGroupsForDisjointOverloads() throws {
+        enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled)
+
+        let symbolKind = try XCTUnwrap(SymbolGraph.Symbol.KindIdentifier.allCases.filter({ $0.isOverloadableKind }).first)
+
+        let tempURL = try createTempFolder(content: [
+            Folder(name: "unit-test.docc", content: [
+                JSONFile(name: "ModuleName-macos.symbols.json", content: makeSymbolGraph(
+                    moduleName: "ModuleName",
+                    platform: .init(operatingSystem: .init(name: "macosx")),
+                    symbols: [
+                        makeSymbol(identifier: "symbol-1", kind: symbolKind),
+                    ])),
+                JSONFile(name: "ModuleName-ios.symbols.json", content: makeSymbolGraph(
+                    moduleName: "ModuleName",
+                    platform: .init(operatingSystem: .init(name: "ios")),
+                    symbols: [
+                        makeSymbol(identifier: "symbol-2", kind: symbolKind),
+                    ])),
+            ])
+        ])
+        let (_, bundle, context) = try loadBundle(from: tempURL)
+        let moduleReference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/ModuleName", sourceLanguage: .swift)
+
+        let overloadGroupNode: DocumentationNode
+        let overloadGroupSymbol: Symbol
+        let overloadGroupReferences: Symbol.Overloads
+
+        switch context.resolve(.unresolved(.init(topicURL: .init(symbolPath: "SymbolName"))), in: moduleReference, fromSymbolLink: true) {
+        case let .failure(_, errorMessage):
+            XCTFail("Could not resolve overload group page. Error message: \(errorMessage)")
+            return
+        case let .success(overloadGroupReference):
+            overloadGroupNode = try context.entity(with: overloadGroupReference)
+            overloadGroupSymbol = try XCTUnwrap(overloadGroupNode.semantic as? Symbol)
+            overloadGroupReferences = try XCTUnwrap(overloadGroupSymbol.overloadsVariants.firstValue)
+
+            XCTAssertEqual(overloadGroupReferences.displayIndex, 0)
+
+            let unifiedSymbol = try XCTUnwrap(overloadGroupNode.unifiedSymbol)
+            XCTAssertEqual(unifiedSymbol.uniqueIdentifier, "symbol-1" + SymbolGraph.Symbol.overloadGroupIdentifierSuffix)
+        }
+
+        let overloadedReferences = try ["symbol-1", "symbol-2"]
+            .map { try XCTUnwrap(context.documentationCache.reference(symbolID: $0)) }
+
+        for (index, reference) in overloadedReferences.indexed() {
+            let overloadedDocumentationNode = try XCTUnwrap(context.documentationCache[reference])
+            let overloadedSymbol = try XCTUnwrap(overloadedDocumentationNode.semantic as? Symbol)
+
+            let overloads = try XCTUnwrap(overloadedSymbol.overloadsVariants.firstValue)
+
+            // Make sure that each symbol contains all of its sibling overloads.
+            XCTAssertEqual(overloads.references.count, overloadedReferences.count - 1)
+            for (otherIndex, otherReference) in overloadedReferences.indexed() where otherIndex != index {
+                XCTAssert(overloads.references.contains(otherReference))
+            }
+
+            if overloads.displayIndex == 0 {
+                // The first declaration in the display list should be the same declaration as
+                // the overload group page
+                XCTAssertEqual(overloadedSymbol.declaration.first?.value.declarationFragments, overloadGroupSymbol.declaration.first?.value.declarationFragments)
+            } else {
+                // Otherwise, this reference should also be referenced by the overload group
+                XCTAssert(overloadGroupReferences.references.contains(reference))
+            }
+        }
+    }
+
     // A test helper that creates a symbol with a given identifier and kind.
     private func makeSymbol(
         name: String = "SymbolName",