Skip to content

Commit f8d16c5

Browse files
authored
Support disambiguating links with type signature information (#643)
* Experimental support for disambiguating links with type signatures rdar://112224233 * Add a more robust parser for type signature disambiguation in the link * Support function signature disambiguation for external links * Fix parsing of subtract operators with parameter type disambiguation * Only use hash and kind disambiguation in topic references and URLs * Display function signature in PathHierarchy debug dump * Update tests for subscripts with type signature disambiguation * Improve presentation of solutions for ambiguous links on command line * Update tests to expect the added space in the warning summary * Extract the full type from the function signature for disambiguation * Update new code to account for C++ operator parsing logic * Handle disambiguation in error messages more consistently. Also, format the error message better in single-line presentation * Fix new bug where overload groups prevent type disambiguation for one of the overloaded symbols Also, improve paths for overload groups and other preferred symbols * Add convenience accessors for inspecting node's special behaviors * Extract private `onlyIndex(where:)` utility * Remove accidental print statement * Reimplement private `typesMatch` helper using `allSatisfy` * Fix typo and missing info in implementation code comment. * Create an empty substring using the standard initializer * Extract common code for disambiguating by type signature * Change `typeSpellings(for:)` to specify _included_ token kinds * Add examples of disambiguation string in code comment * Avoid creating disambiguation string to find `Disambiguation.none`
1 parent 02346ee commit f8d16c5

23 files changed

+1708
-327
lines changed

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

Lines changed: 153 additions & 29 deletions
Large diffs are not rendered by default.

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
// This API isn't exposed anywhere and is only used from a debugger.
1212
#if DEBUG
1313

14+
import SymbolKit
15+
1416
/// A node in a tree structure that can be printed into a visual representation for debugging.
1517
private struct DumpableNode {
1618
var name: String
@@ -21,20 +23,25 @@ private extension PathHierarchy.Node {
2123
/// Maps the path hierarchy subtree into a representation that can be printed into a visual form for debugging.
2224
func dumpableNode() -> DumpableNode {
2325
// Each node is printed as 3-layer hierarchy with the child names, their kind disambiguation, and their hash disambiguation.
26+
27+
// One layer for the node itself that displays information about the symbol
2428
return DumpableNode(
25-
name: symbol.map { "{ \($0.identifier.precise) : \($0.kind.identifier.identifier) [\(languages.map(\.name).joined(separator: ", "))] }" } ?? "[ \(name) ]",
26-
children: children.sorted(by: \.key).map { (key, disambiguationTree) -> DumpableNode in
29+
name: symbol.map(describe(_:)) ?? "[ \(name) ]",
30+
children: children.sorted(by: \.key).map { (childName, disambiguationTree) -> DumpableNode in
31+
32+
// A second layer that displays the kind disambiguation
2733
let grouped = [String: [PathHierarchy.DisambiguationContainer.Element]](grouping: disambiguationTree.storage, by: { $0.kind ?? "_" })
2834
return DumpableNode(
29-
name: key,
35+
name: childName,
3036
children: grouped.sorted(by: \.key).map { (kind, kindTree) -> DumpableNode in
37+
38+
// A third layer that displays the hash disambiguation
3139
DumpableNode(
3240
name: kind,
3341
children: kindTree.sorted(by: { lhs, rhs in (lhs.hash ?? "_") < (rhs.hash ?? "_") }).map { (element) -> DumpableNode in
34-
DumpableNode(
35-
name: element.hash ?? "_",
36-
children: [element.node.dumpableNode()]
37-
)
42+
43+
// Recursively dump the subtree
44+
DumpableNode(name: element.hash ?? "_", children: [element.node.dumpableNode()])
3845
}
3946
)
4047
}
@@ -44,6 +51,14 @@ private extension PathHierarchy.Node {
4451
}
4552
}
4653

54+
private func describe(_ symbol: SymbolGraph.Symbol) -> String {
55+
guard let (parameterTypes, returnValueTypes) = PathHierarchy.functionSignatureTypeNames(for: symbol) else {
56+
return "{ \(symbol.identifier.precise) }"
57+
}
58+
59+
return "{ \(symbol.identifier.precise) : (\(parameterTypes.joined(separator: ", "))) -> (\(returnValueTypes.joined(separator: ", "))) }"
60+
}
61+
4762
extension PathHierarchy {
4863
/// Creates a visual representation or the path hierarchy for debugging.
4964
func dump() -> String {

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

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
See https://swift.org/CONTRIBUTORS.txt for Swift project authors
99
*/
1010

11+
import Foundation
1112
import struct Markdown.SourceRange
1213
import struct Markdown.SourceLocation
1314
import SymbolKit
@@ -80,10 +81,38 @@ extension PathHierarchy.Error {
8081
/// - fullNameOfNode: A closure that determines the full name of a node, to be displayed in collision diagnostics to precisely identify symbols and other pages.
8182
/// - Note: `Replacement`s produced by this function use `SourceLocation`s relative to the link text excluding its surrounding syntax.
8283
func makeTopicReferenceResolutionErrorInfo(fullNameOfNode: (PathHierarchy.Node) -> String) -> TopicReferenceResolutionErrorInfo {
84+
// Both `.unknownDisambiguation(...)` and `.lookupCollisions(...)` create solutions on the same format from the same information.
8385
// This is defined inline because it captures `fullNameOfNode`.
84-
func collisionIsBefore(_ lhs: (node: PathHierarchy.Node, disambiguation: String), _ rhs: (node: PathHierarchy.Node, disambiguation: String)) -> Bool {
85-
return fullNameOfNode(lhs.node) + lhs.disambiguation
86-
< fullNameOfNode(rhs.node) + rhs.disambiguation
86+
func makeCollisionSolutions(
87+
from candidates: [(node: PathHierarchy.Node, disambiguation: String)],
88+
nextPathComponent: PathHierarchy.PathComponent,
89+
partialResultPrefix: Substring
90+
) -> (
91+
pathPrefix: Substring,
92+
foundDisambiguation: Substring,
93+
solutions: [Solution]
94+
) {
95+
let pathPrefix = partialResultPrefix + nextPathComponent.name
96+
let foundDisambiguation = nextPathComponent.full.dropFirst(nextPathComponent.name.count)
97+
let replacementRange = SourceRange.makeRelativeRange(startColumn: pathPrefix.count, length: foundDisambiguation.count)
98+
99+
let solutions: [Solution] = candidates
100+
.map { (fullName: fullNameOfNode($0.node), disambiguation: $0.disambiguation) }
101+
.sorted { lhs, rhs in
102+
// Sort by name first and disambiguation second
103+
if lhs.fullName == rhs.fullName {
104+
return lhs.disambiguation < rhs.disambiguation
105+
}
106+
return lhs.fullName < rhs.fullName
107+
}
108+
.map { (fullName: String, suggestedDisambiguation: String) -> Solution in
109+
// In contexts that display the solution message on a single line by removing newlines, this extra whitespace makes it look correct ─────────────╮
110+
// ▼
111+
return Solution(summary: "\(Self.replacementOperationDescription(from: foundDisambiguation, to: suggestedDisambiguation, forCollision: true)) for \n\(fullName.singleQuoted)", replacements: [
112+
Replacement(range: replacementRange, replacement: suggestedDisambiguation)
113+
])
114+
}
115+
return (pathPrefix, foundDisambiguation, solutions)
87116
}
88117

89118
switch self {
@@ -127,7 +156,7 @@ extension PathHierarchy.Error {
127156

128157
case .unfindableMatch(let node):
129158
return TopicReferenceResolutionErrorInfo("""
130-
\(node.name.singleQuoted) can't be linked to in a partial documentation build
159+
\(node.name.singleQuoted) can't be linked to in a partial documentation build
131160
""")
132161

133162
case .nonSymbolMatchForSymbolLink(path: let path):
@@ -142,24 +171,13 @@ extension PathHierarchy.Error {
142171

143172
case .unknownDisambiguation(partialResult: let partialResult, remaining: let remaining, candidates: let candidates):
144173
let nextPathComponent = remaining.first!
145-
let validPrefix = partialResult.pathPrefix + nextPathComponent.name
146-
147-
let disambiguations = nextPathComponent.full.dropFirst(nextPathComponent.name.count)
148-
let replacementRange = SourceRange.makeRelativeRange(startColumn: validPrefix.count, length: disambiguations.count)
149-
150-
let solutions: [Solution] = candidates
151-
.sorted(by: collisionIsBefore)
152-
.map { (node: PathHierarchy.Node, disambiguation: String) -> Solution in
153-
return Solution(summary: "\(Self.replacementOperationDescription(from: disambiguations, to: disambiguation)) for\n\(fullNameOfNode(node).singleQuoted)", replacements: [
154-
Replacement(range: replacementRange, replacement: disambiguation)
155-
])
156-
}
174+
let (pathPrefix, foundDisambiguation, solutions) = makeCollisionSolutions(from: candidates, nextPathComponent: nextPathComponent, partialResultPrefix: partialResult.pathPrefix)
157175

158176
return TopicReferenceResolutionErrorInfo("""
159-
\(disambiguations.dropFirst().singleQuoted) isn't a disambiguation for \(nextPathComponent.name.singleQuoted) at \(partialResult.node.pathWithoutDisambiguation().singleQuoted)
177+
\(foundDisambiguation.dropFirst().singleQuoted) isn't a disambiguation for \(nextPathComponent.name.singleQuoted) at \(partialResult.node.pathWithoutDisambiguation().singleQuoted)
160178
""",
161179
solutions: solutions,
162-
rangeAdjustment: .makeRelativeRange(startColumn: validPrefix.count, length: disambiguations.count)
180+
rangeAdjustment: .makeRelativeRange(startColumn: pathPrefix.count, length: foundDisambiguation.count)
163181
)
164182

165183
case .unknownName(partialResult: let partialResult, remaining: let remaining, availableChildren: let availableChildren):
@@ -201,17 +219,7 @@ extension PathHierarchy.Error {
201219

202220
case .lookupCollision(partialResult: let partialResult, remaining: let remaining, collisions: let collisions):
203221
let nextPathComponent = remaining.first!
204-
205-
let pathPrefix = partialResult.pathPrefix + nextPathComponent.name
206-
207-
let disambiguations = nextPathComponent.full.dropFirst(nextPathComponent.name.count)
208-
let replacementRange = SourceRange.makeRelativeRange(startColumn: pathPrefix.count, length: disambiguations.count)
209-
210-
let solutions: [Solution] = collisions.sorted(by: collisionIsBefore).map { (node: PathHierarchy.Node, disambiguation: String) -> Solution in
211-
return Solution(summary: "\(Self.replacementOperationDescription(from: disambiguations.dropFirst(), to: disambiguation)) for\n\(fullNameOfNode(node).singleQuoted)", replacements: [
212-
Replacement(range: replacementRange, replacement: "-" + disambiguation)
213-
])
214-
}
222+
let (pathPrefix, _, solutions) = makeCollisionSolutions(from: collisions, nextPathComponent: nextPathComponent, partialResultPrefix: partialResult.pathPrefix)
215223

216224
return TopicReferenceResolutionErrorInfo("""
217225
\(nextPathComponent.full.singleQuoted) is ambiguous at \(partialResult.node.pathWithoutDisambiguation().singleQuoted)
@@ -222,14 +230,25 @@ extension PathHierarchy.Error {
222230
}
223231
}
224232

225-
private static func replacementOperationDescription(from: some StringProtocol, to: some StringProtocol) -> String {
233+
private static func replacementOperationDescription(from: some StringProtocol, to: some StringProtocol, forCollision: Bool = false) -> String {
226234
if from.isEmpty {
227235
return "Insert \(to.singleQuoted)"
228236
}
229237
if to.isEmpty {
230238
return "Remove \(from.singleQuoted)"
231239
}
232-
return "Replace \(from.singleQuoted) with \(to.singleQuoted)"
240+
241+
guard forCollision else {
242+
return "Replace \(from.singleQuoted) with \(to.singleQuoted)"
243+
}
244+
245+
if to.hasPrefix("->") || from.hasPrefix("->") {
246+
// If either the "to" or "from" descriptions are a return type disambiguation, include the full arrow for both.
247+
// Only a ">" prefix doesn't read as an "arrow", and it looks incorrect when only of the descriptions have a "-" prefix.
248+
return "Replace \(from.singleQuoted) with \(to.singleQuoted)"
249+
}
250+
// For other replacement descriptions, drop the leading "-" to focus on the text that's different.
251+
return "Replace \(from.dropFirst().singleQuoted) with \(to.dropFirst().singleQuoted)"
233252
}
234253
}
235254

0 commit comments

Comments
 (0)