Skip to content

Commit

Permalink
Support function signature disambiguation for external links
Browse files Browse the repository at this point in the history
  • Loading branch information
d-ronnqvist committed Dec 1, 2023
1 parent 067d943 commit 4b2581b
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
}),
Expand Down Expand Up @@ -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
Expand All @@ -109,6 +115,8 @@ extension PathHierarchy {
struct Disambiguation: Codable {
var kind: String?
var hash: String?
var parameterTypes: [String]?
var returnTypes: [String]?
var nodeID: Int
}
}
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,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
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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)),
]
)

Expand Down

0 comments on commit 4b2581b

Please sign in to comment.