Support function signature disambiguation for external links

swiftlang · Dec 1, 2023 · 7341760 · 7341760
1 parent 3fb6cec
commit 7341760
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 16 deletions.
diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift
@@ -46,7 +46,13 @@ extension PathHierarchy.FileRepresentation {
                         children: node.children.values.flatMap({ tree in
                             var disambiguations = [Node.Disambiguation]()
                             for element in tree.storage where element.node.identifier != nil { // nodes without identifiers can't be found in the tree
-                                disambiguations.append(.init(kind: element.kind, hash: element.hash, nodeID: identifierMap[element.node.identifier]!))
+                                disambiguations.append(.init(
+                                    kind: element.kind,
+                                    hash: element.hash,
+                                    parameterTypes: element.parameterTypes,
+                                    returnTypes: element.returnTypes,
+                                    nodeID: identifierMap[element.node.identifier]!
+                                ))
                             }
                             return disambiguations
                         }),
@@ -99,7 +105,7 @@ extension PathHierarchy {
         /// The container of tutorial overview pages.
         var tutorialOverviewContainer: Int
 
-        /// A node in the
+        /// A node in the hierarchy.
         struct Node: Codable {
             var name: String
             var isDisfavoredInCollision: Bool = false
@@ -109,6 +115,8 @@ extension PathHierarchy {
             struct Disambiguation: Codable {
                 var kind: String?
                 var hash: String?
+                var parameterTypes: [String]?
+                var returnTypes: [String]?
                 var nodeID: Int
             }
         }
@@ -172,7 +180,7 @@ extension PathHierarchyBasedLinkResolver {
         }
 
         return SerializableLinkResolutionInformation(
-            version: .init(major: 0, minor: 0, patch: 1), // This is still in development
+            version: .init(major: 0, minor: 0, patch: 2), // This is still in development
             bundleID: bundleID,
             pathHierarchy: hierarchyFileRepresentation,
             nonSymbolPaths: nonSymbolPaths

diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift
@@ -604,7 +604,13 @@ extension PathHierarchy {
             for child in fileNode.children {
                 let childNode = lookup[identifiers[child.nodeID]]!
                 // Even if this is a symbol node, explicitly pass the kind and hash disambiguation.
-                node.add(child: childNode, kind: child.kind, hash: child.hash)
+                node.add(
+                    child: childNode,
+                    kind: child.kind,
+                    hash: child.hash,
+                    parameterTypes: child.parameterTypes,
+                    returnTypes: child.returnTypes
+                )
             }
         }
 

diff --git a/Tests/SwiftDocCTests/Infrastructure/ExternalPathHierarchyResolverTests.swift b/Tests/SwiftDocCTests/Infrastructure/ExternalPathHierarchyResolverTests.swift
@@ -204,13 +204,40 @@ class ExternalPathHierarchyResolverTests: XCTestCase {
         //     public func something(argument: String) -> Int { 0 }
         // }
         try linkResolvers.assertSuccessfullyResolves(authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments")
-        try linkResolvers.assertSuccessfullyResolves(authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-1cyvp")
-        try linkResolvers.assertSuccessfullyResolves(authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-2vke2")
+        try linkResolvers.assertSuccessfullyResolves(
+            authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-1cyvp",
+            to: "doc://org.swift.MixedFramework/documentation/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(Int)"
+        )
+        try linkResolvers.assertSuccessfullyResolves(
+            authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-2vke2",
+            to: "doc://org.swift.MixedFramework/documentation/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(String)"
+        )
 
+        try linkResolvers.assertSuccessfullyResolves(authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(Int)")
+        try linkResolvers.assertSuccessfullyResolves(authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(String)")
+
+        try linkResolvers.assertSuccessfullyResolves(
+            authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(Int)->Int",
+            to: "doc://org.swift.MixedFramework/documentation/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(Int)"
+        )
+        try linkResolvers.assertSuccessfullyResolves(
+            authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(String)->Int",
+            to: "doc://org.swift.MixedFramework/documentation/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(String)"
+        )
+        try linkResolvers.assertSuccessfullyResolves(
+            authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(Int)->_",
+            to: "doc://org.swift.MixedFramework/documentation/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(Int)"
+        )
+        try linkResolvers.assertSuccessfullyResolves(
+            authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(String)->_",
+            to: "doc://org.swift.MixedFramework/documentation/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-(String)"
+        )
+
         // public enum CollisionsWithDifferentSubscriptArguments {
         //     public subscript(something: Int) -> Int { 0 }
         //     public subscript(somethingElse: String) -> Int { 0 }
         // }
+        // Subscripts don't have function signature information in the symbol graph file (rdar://111072228)
         try linkResolvers.assertSuccessfullyResolves(authoredLink: "/MixedFramework/CollisionsWithDifferentSubscriptArguments")
         try linkResolvers.assertSuccessfullyResolves(authoredLink: "/MixedFramework/CollisionsWithDifferentSubscriptArguments/subscript(_:)-4fd0l")
         try linkResolvers.assertSuccessfullyResolves(authoredLink: "/MixedFramework/CollisionsWithDifferentSubscriptArguments/subscript(_:)-757cj")
@@ -458,24 +485,24 @@ class ExternalPathHierarchyResolverTests: XCTestCase {
             authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)",
             errorMessage: "'something(argument:)' is ambiguous at '/MixedFramework/CollisionsWithDifferentFunctionArguments'",
             solutions: [
-                .init(summary: "Insert '1cyvp' for\n'func something(argument: Int) -> Int'", replacement: ("-1cyvp", 77, 77)),
-                .init(summary: "Insert '2vke2' for\n'func something(argument: String) -> Int'", replacement: ("-2vke2", 77, 77)),
+                .init(summary: "Insert '(Int)' for\n'func something(argument: Int) -> Int'", replacement: ("-(Int)", 77, 77)),
+                .init(summary: "Insert '(String)' for\n'func something(argument: String) -> Int'", replacement: ("-(String)", 77, 77)),
             ]
         )
         try linkResolvers.assertFailsToResolve(
             authoredLink: "/documentation/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)",
             errorMessage: "'something(argument:)' is ambiguous at '/MixedFramework/CollisionsWithDifferentFunctionArguments'",
             solutions: [
-                .init(summary: "Insert '1cyvp' for\n'func something(argument: Int) -> Int'", replacement: ("-1cyvp", 91, 91)),
-                .init(summary: "Insert '2vke2' for\n'func something(argument: String) -> Int'", replacement: ("-2vke2", 91, 91)),
+                .init(summary: "Insert '(Int)' for\n'func something(argument: Int) -> Int'", replacement: ("-(Int)", 91, 91)),
+                .init(summary: "Insert '(String)' for\n'func something(argument: String) -> Int'", replacement: ("-(String)", 91, 91)),
             ]
         )
         try linkResolvers.assertFailsToResolve(
             authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-abc123",
             errorMessage: "'abc123' isn't a disambiguation for 'something(argument:)' at '/MixedFramework/CollisionsWithDifferentFunctionArguments'",
             solutions: [
-                .init(summary: "Replace 'abc123' with '1cyvp' for\n'func something(argument: Int) -> Int'", replacement: ("-1cyvp", 77, 84)),
-                .init(summary: "Replace 'abc123' with '2vke2' for\n'func something(argument: String) -> Int'", replacement: ("-2vke2", 77, 84)),
+                .init(summary: "Replace 'abc123' with '(Int)' for\n'func something(argument: Int) -> Int'", replacement: ("-(Int)", 77, 84)),
+                .init(summary: "Replace 'abc123' with '(String)' for\n'func something(argument: String) -> Int'", replacement: ("-(String)", 77, 84)),
             ]
         )
         // Providing disambiguation will narrow down the suggestions. Note that `argument` label is missing in the last path component
@@ -497,16 +524,16 @@ class ExternalPathHierarchyResolverTests: XCTestCase {
             authoredLink: "/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-method",
             errorMessage: "'something(argument:)-method' is ambiguous at '/MixedFramework/CollisionsWithDifferentFunctionArguments'",
             solutions: [
-                .init(summary: "Replace 'method' with '1cyvp' for\n'func something(argument: Int) -> Int'", replacement: ("-1cyvp", 77, 84)),
-                .init(summary: "Replace 'method' with '2vke2' for\n'func something(argument: String) -> Int'", replacement: ("-2vke2", 77, 84)),
+                .init(summary: "Replace 'method' with '(Int)' for\n'func something(argument: Int) -> Int'", replacement: ("-(Int)", 77, 84)),
+                .init(summary: "Replace 'method' with '(String)' for\n'func something(argument: String) -> Int'", replacement: ("-(String)", 77, 84)),
             ]
         )
         try linkResolvers.assertFailsToResolve(
             authoredLink: "/documentation/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)-method",
             errorMessage: "'something(argument:)-method' is ambiguous at '/MixedFramework/CollisionsWithDifferentFunctionArguments'",
             solutions: [
-                .init(summary: "Replace 'method' with '1cyvp' for\n'func something(argument: Int) -> Int'", replacement: ("-1cyvp", 91, 98)),
-                .init(summary: "Replace 'method' with '2vke2' for\n'func something(argument: String) -> Int'", replacement: ("-2vke2", 91, 98)),
+                .init(summary: "Replace 'method' with '(Int)' for\n'func something(argument: Int) -> Int'", replacement: ("-(Int)", 91, 98)),
+                .init(summary: "Replace 'method' with '(String)' for\n'func something(argument: String) -> Int'", replacement: ("-(String)", 91, 98)),
             ]
         )