-
Notifications
You must be signed in to change notification settings - Fork 146
Support disambiguating links with type signature information #643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6fb3ae4
8dc4fb0
6e69607
9e6d3a6
db3e63f
f39847f
3fb6cec
7341760
d4371a8
3827762
73e46d5
bd4a4a3
37b6b43
ec9307b
393716b
39a2636
945a74f
0dc50d5
53429bd
94bf56e
744363e
9da0dff
3b46080
f57ac6c
b476a7d
216a038
d2045a8
30e71ad
316351c
eca9ce5
7f7f5f5
528a45d
7c99db6
2e18f6b
34d4035
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
See https://swift.org/CONTRIBUTORS.txt for Swift project authors | ||
*/ | ||
|
||
import Foundation | ||
import struct Markdown.SourceRange | ||
import struct Markdown.SourceLocation | ||
import SymbolKit | ||
|
@@ -80,10 +81,38 @@ extension PathHierarchy.Error { | |
/// - fullNameOfNode: A closure that determines the full name of a node, to be displayed in collision diagnostics to precisely identify symbols and other pages. | ||
/// - Note: `Replacement`s produced by this function use `SourceLocation`s relative to the link text excluding its surrounding syntax. | ||
func makeTopicReferenceResolutionErrorInfo(fullNameOfNode: (PathHierarchy.Node) -> String) -> TopicReferenceResolutionErrorInfo { | ||
// Both `.unknownDisambiguation(...)` and `.lookupCollisions(...)` create solutions on the same format from the same information. | ||
// This is defined inline because it captures `fullNameOfNode`. | ||
func collisionIsBefore(_ lhs: (node: PathHierarchy.Node, disambiguation: String), _ rhs: (node: PathHierarchy.Node, disambiguation: String)) -> Bool { | ||
return fullNameOfNode(lhs.node) + lhs.disambiguation | ||
< fullNameOfNode(rhs.node) + rhs.disambiguation | ||
func makeCollisionSolutions( | ||
from candidates: [(node: PathHierarchy.Node, disambiguation: String)], | ||
nextPathComponent: PathHierarchy.PathComponent, | ||
partialResultPrefix: Substring | ||
) -> ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this tuple with three named attributes warrant an actual structure with some name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly. It's internal to this function so only this function can call it. |
||
pathPrefix: Substring, | ||
foundDisambiguation: Substring, | ||
solutions: [Solution] | ||
) { | ||
let pathPrefix = partialResultPrefix + nextPathComponent.name | ||
let foundDisambiguation = nextPathComponent.full.dropFirst(nextPathComponent.name.count) | ||
let replacementRange = SourceRange.makeRelativeRange(startColumn: pathPrefix.count, length: foundDisambiguation.count) | ||
|
||
let solutions: [Solution] = candidates | ||
.map { (fullName: fullNameOfNode($0.node), disambiguation: $0.disambiguation) } | ||
.sorted { lhs, rhs in | ||
// Sort by name first and disambiguation second | ||
if lhs.fullName == rhs.fullName { | ||
return lhs.disambiguation < rhs.disambiguation | ||
} | ||
return lhs.fullName < rhs.fullName | ||
} | ||
.map { (fullName: String, suggestedDisambiguation: String) -> Solution in | ||
// In contexts that display the solution message on a single line by removing newlines, this extra whitespace makes it look correct ─────────────╮ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure I understand what this is referring to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default diagnostic formatter outputs diagnostic solutions (phrased as "suggestions") on a single line. For example:
If you look closely at the suggestion text you can see that there's a missing space between "for" and the symbol name. With these changes this suggestion text isn't missing that space:
|
||
// ▼ | ||
return Solution(summary: "\(Self.replacementOperationDescription(from: foundDisambiguation, to: suggestedDisambiguation, forCollision: true)) for \n\(fullName.singleQuoted)", replacements: [ | ||
Replacement(range: replacementRange, replacement: suggestedDisambiguation) | ||
]) | ||
} | ||
return (pathPrefix, foundDisambiguation, solutions) | ||
} | ||
|
||
switch self { | ||
|
@@ -127,7 +156,7 @@ extension PathHierarchy.Error { | |
|
||
case .unfindableMatch(let node): | ||
return TopicReferenceResolutionErrorInfo(""" | ||
\(node.name.singleQuoted) can't be linked to in a partial documentation build | ||
\(node.name.singleQuoted) can't be linked to in a partial documentation build | ||
""") | ||
|
||
case .nonSymbolMatchForSymbolLink(path: let path): | ||
|
@@ -142,24 +171,13 @@ extension PathHierarchy.Error { | |
|
||
case .unknownDisambiguation(partialResult: let partialResult, remaining: let remaining, candidates: let candidates): | ||
let nextPathComponent = remaining.first! | ||
let validPrefix = partialResult.pathPrefix + nextPathComponent.name | ||
d-ronnqvist marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
let disambiguations = nextPathComponent.full.dropFirst(nextPathComponent.name.count) | ||
let replacementRange = SourceRange.makeRelativeRange(startColumn: validPrefix.count, length: disambiguations.count) | ||
|
||
let solutions: [Solution] = candidates | ||
.sorted(by: collisionIsBefore) | ||
.map { (node: PathHierarchy.Node, disambiguation: String) -> Solution in | ||
return Solution(summary: "\(Self.replacementOperationDescription(from: disambiguations, to: disambiguation)) for\n\(fullNameOfNode(node).singleQuoted)", replacements: [ | ||
Replacement(range: replacementRange, replacement: disambiguation) | ||
]) | ||
} | ||
let (pathPrefix, foundDisambiguation, solutions) = makeCollisionSolutions(from: candidates, nextPathComponent: nextPathComponent, partialResultPrefix: partialResult.pathPrefix) | ||
|
||
return TopicReferenceResolutionErrorInfo(""" | ||
\(disambiguations.dropFirst().singleQuoted) isn't a disambiguation for \(nextPathComponent.name.singleQuoted) at \(partialResult.node.pathWithoutDisambiguation().singleQuoted) | ||
\(foundDisambiguation.dropFirst().singleQuoted) isn't a disambiguation for \(nextPathComponent.name.singleQuoted) at \(partialResult.node.pathWithoutDisambiguation().singleQuoted) | ||
""", | ||
solutions: solutions, | ||
rangeAdjustment: .makeRelativeRange(startColumn: validPrefix.count, length: disambiguations.count) | ||
rangeAdjustment: .makeRelativeRange(startColumn: pathPrefix.count, length: foundDisambiguation.count) | ||
) | ||
|
||
case .unknownName(partialResult: let partialResult, remaining: let remaining, availableChildren: let availableChildren): | ||
|
@@ -201,17 +219,7 @@ extension PathHierarchy.Error { | |
|
||
case .lookupCollision(partialResult: let partialResult, remaining: let remaining, collisions: let collisions): | ||
let nextPathComponent = remaining.first! | ||
|
||
let pathPrefix = partialResult.pathPrefix + nextPathComponent.name | ||
|
||
let disambiguations = nextPathComponent.full.dropFirst(nextPathComponent.name.count) | ||
let replacementRange = SourceRange.makeRelativeRange(startColumn: pathPrefix.count, length: disambiguations.count) | ||
|
||
let solutions: [Solution] = collisions.sorted(by: collisionIsBefore).map { (node: PathHierarchy.Node, disambiguation: String) -> Solution in | ||
return Solution(summary: "\(Self.replacementOperationDescription(from: disambiguations.dropFirst(), to: disambiguation)) for\n\(fullNameOfNode(node).singleQuoted)", replacements: [ | ||
Replacement(range: replacementRange, replacement: "-" + disambiguation) | ||
]) | ||
} | ||
let (pathPrefix, _, solutions) = makeCollisionSolutions(from: collisions, nextPathComponent: nextPathComponent, partialResultPrefix: partialResult.pathPrefix) | ||
|
||
return TopicReferenceResolutionErrorInfo(""" | ||
\(nextPathComponent.full.singleQuoted) is ambiguous at \(partialResult.node.pathWithoutDisambiguation().singleQuoted) | ||
|
@@ -222,14 +230,25 @@ extension PathHierarchy.Error { | |
} | ||
} | ||
|
||
private static func replacementOperationDescription(from: some StringProtocol, to: some StringProtocol) -> String { | ||
private static func replacementOperationDescription(from: some StringProtocol, to: some StringProtocol, forCollision: Bool = false) -> String { | ||
if from.isEmpty { | ||
return "Insert \(to.singleQuoted)" | ||
} | ||
if to.isEmpty { | ||
return "Remove \(from.singleQuoted)" | ||
} | ||
return "Replace \(from.singleQuoted) with \(to.singleQuoted)" | ||
|
||
guard forCollision else { | ||
return "Replace \(from.singleQuoted) with \(to.singleQuoted)" | ||
} | ||
|
||
if to.hasPrefix("->") || from.hasPrefix("->") { | ||
// If either the "to" or "from" descriptions are a return type disambiguation, include the full arrow for both. | ||
// Only a ">" prefix doesn't read as an "arrow", and it looks incorrect when only of the descriptions have a "-" prefix. | ||
return "Replace \(from.singleQuoted) with \(to.singleQuoted)" | ||
} | ||
// For other replacement descriptions, drop the leading "-" to focus on the text that's different. | ||
return "Replace \(from.dropFirst().singleQuoted) with \(to.dropFirst().singleQuoted)" | ||
} | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.