From 7341760831ec616f902d2678673f21fa333acf4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Fri, 1 Dec 2023 16:45:22 +0100 Subject: [PATCH] Support function signature disambiguation for external links --- .../PathHierarchy+Serialization.swift | 14 +++-- .../Link Resolution/PathHierarchy.swift | 8 ++- .../ExternalPathHierarchyResolverTests.swift | 51 ++++++++++++++----- 3 files changed, 57 insertions(+), 16 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift index 30790756e8..59a9e5be4e 100644 --- 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 index de42c53bd5..61332d383d 100644 --- 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 index 24eb76ab92..55bb8f77e2 100644 --- 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)), ] )