Skip to content

Use new API's from SwiftSyntax SourceEdit #1250

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 13, 2024
Merged

Use new API's from SwiftSyntax SourceEdit #1250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a71b428
Remove `?? position` as it never was used
kimdv May 9, 2024
c5699fb
Fix deprecated `ByteSourceRange`
kimdv May 9, 2024
ed61630
Replace `IncrementalEdit` with `SourceEdit`
kimdv May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/Rename.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,7 @@ extension SwiftLanguageService {
}

if let startToken, let endToken {
return snapshot.range(
return snapshot.absolutePositionRange(
of: startToken.positionAfterSkippingLeadingTrivia..<endToken.endPositionBeforeTrailingTrivia
)
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/Swift/AdjustPositionToStartOfIdentifier.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ extension SwiftLanguageService {
let visitor = StartOfIdentifierFinder(position: snapshot.absolutePosition(of: position))
visitor.walk(tree)
if let resolvedPosition = visitor.resolvedPosition {
return snapshot.position(of: resolvedPosition) ?? position
return snapshot.position(of: resolvedPosition)
}
return position
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ fileprivate extension PackageEditResult {
// The edits to perform on the manifest itself.
let manifestTextEdits = manifestEdits.map { edit in
TextEdit(
range: snapshot.range(of: edit.range),
range: snapshot.absolutePositionRange(of: edit.range),
newText: edit.replacement
)
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ extension SyntaxRefactoringCodeActionProvider where Self.Context == Void {

let textEdits = sourceEdits.compactMap { (edit) -> TextEdit? in
let edit = TextEdit(
range: scope.snapshot.range(of: edit.range),
range: scope.snapshot.absolutePositionRange(of: edit.range),
newText: edit.replacement
)
if edit.isNoOp(in: scope.snapshot) {
Expand Down
6 changes: 3 additions & 3 deletions Sources/SourceKitLSP/Swift/Diagnostic.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ extension CodeAction {
init?(_ fixIt: FixIt, in snapshot: DocumentSnapshot) {
var textEdits = [TextEdit]()
for edit in fixIt.edits {
textEdits.append(TextEdit(range: snapshot.range(of: edit.range), newText: edit.replacement))
textEdits.append(TextEdit(range: snapshot.absolutePositionRange(of: edit.range), newText: edit.replacement))
}

self.init(
Expand Down Expand Up @@ -281,7 +281,7 @@ extension Diagnostic {
var range = Range(snapshot.position(of: diag.position))
for highlight in diag.highlights {
let swiftSyntaxRange = highlight.positionAfterSkippingLeadingTrivia..<highlight.endPositionBeforeTrailingTrivia
let highlightRange = snapshot.range(of: swiftSyntaxRange)
let highlightRange = snapshot.absolutePositionRange(of: swiftSyntaxRange)
if range.upperBound == highlightRange.lowerBound {
range = range.lowerBound..<highlightRange.upperBound
} else {
Expand Down Expand Up @@ -345,7 +345,7 @@ extension DiagnosticRelatedInformation {
init(_ note: Note, in snapshot: DocumentSnapshot) {
let nodeRange = note.node.positionAfterSkippingLeadingTrivia..<note.node.endPositionBeforeTrailingTrivia
self.init(
location: Location(uri: snapshot.uri, range: snapshot.range(of: nodeRange)),
location: Location(uri: snapshot.uri, range: snapshot.absolutePositionRange(of: nodeRange)),
message: note.message
)
}
Expand Down
18 changes: 4 additions & 14 deletions Sources/SourceKitLSP/Swift/DocumentFormatting.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,29 +106,19 @@ private func edits(from original: DocumentSnapshot, to edited: String) -> [TextE
switch change {
case .insert(offset: let offset, element: let element, associatedWith: _):
let absolutePosition = AbsolutePosition(utf8Offset: offset)
return IncrementalEdit(range: absolutePosition..<absolutePosition, replacement: [element])
return SourceEdit(range: absolutePosition..<absolutePosition, replacement: [element])
case .remove(offset: let offset, element: _, associatedWith: _):
let absolutePosition = AbsolutePosition(utf8Offset: offset)
return IncrementalEdit(range: absolutePosition..<absolutePosition.advanced(by: 1), replacement: [])
return SourceEdit(range: absolutePosition..<absolutePosition.advanced(by: 1), replacement: [])
}
}

let concurrentEdits = ConcurrentEdits(fromSequential: sequentialEdits)

// Map the offset-based edits to line-column based edits to be consumed by LSP

return concurrentEdits.edits.compactMap { (edit) -> TextEdit? in
let (startLine, startColumn) = original.lineTable.lineAndUTF16ColumnOf(utf8Offset: edit.offset)
let (endLine, endColumn) = original.lineTable.lineAndUTF16ColumnOf(utf8Offset: edit.endOffset)
guard let newText = String(bytes: edit.replacement, encoding: .utf8) else {
logger.fault("Failed to get String from UTF-8 bytes \(edit.replacement)")
return nil
}

return TextEdit(
range: Position(line: startLine, utf16index: startColumn)..<Position(line: endLine, utf16index: endColumn),
newText: newText
)
return concurrentEdits.edits.compactMap {
TextEdit(range: original.absolutePositionRange(of: $0.range), newText: $0.replacement)
}
}

Expand Down
8 changes: 5 additions & 3 deletions Sources/SourceKitLSP/Swift/DocumentSymbols.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ fileprivate final class DocumentSymbolsFinder: SyntaxAnyVisitor {
if !self.range.overlaps(range) {
return .skipChildren
}
let positionRange = snapshot.range(of: range)
let selectionPositionRange = snapshot.range(of: selection)
let positionRange = snapshot.absolutePositionRange(of: range)
let selectionPositionRange = snapshot.absolutePositionRange(of: selection)

// Record MARK comments on the node's leading and trailing trivia in `result` not as a child of `node`.
visit(node.leadingTrivia, position: node.position)
Expand Down Expand Up @@ -141,7 +141,9 @@ fileprivate final class DocumentSymbolsFinder: SyntaxAnyVisitor {
let trimmedComment = commentText.trimmingCharacters(in: CharacterSet(["/", "*"]).union(.whitespaces))
if trimmedComment.starts(with: markPrefix) {
let markText = trimmedComment.dropFirst(markPrefix.count)
let range = snapshot.range(of: position..<position.advanced(by: piece.sourceLength.utf8Length))
let range = snapshot.absolutePositionRange(
of: position..<position.advanced(by: piece.sourceLength.utf8Length)
)
result.append(
DocumentSymbol(
name: String(markText),
Expand Down
4 changes: 2 additions & 2 deletions Sources/SourceKitLSP/Swift/SemanticTokens.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ extension SwiftLanguageService {
if let range = range.flatMap({ snapshot.byteSourceRange(of: $0) }) {
range
} else {
ByteSourceRange(offset: 0, length: await tree.totalLength.utf8Length)
await tree.range
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahoppen we await here but also on line 65.
Would it not be best to change async let tree = syntaxTreeManager.syntaxTree(for: snapshot) -> let tree = await syntaxTreeManager.syntaxTree(for: snapshot)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benefit of async let is allow us to compute the syntax tree and semantic highlighting tokens in parallel. If we await syntaxTreeManager.syntaxTree(for:), we would wait for the syntax tree to be computed before starting the computation of semantic tokens.

}

let tokens =
Expand Down Expand Up @@ -107,7 +107,7 @@ extension SyntaxClassifiedRange {
return SyntaxHighlightingTokens(tokens: [])
}

let multiLineRange = snapshot.positionOf(utf8Offset: self.offset)..<snapshot.positionOf(utf8Offset: self.endOffset)
let multiLineRange = snapshot.absolutePositionRange(of: self.range)
let ranges = multiLineRange.splitToSingleLineRanges(in: snapshot)

let tokens = ranges.map {
Expand Down
21 changes: 9 additions & 12 deletions Sources/SourceKitLSP/Swift/SwiftLanguageService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ extension SwiftLanguageService {
keys.enableDiagnostics: 0,
keys.syntacticOnly: 1,
keys.offset: edit.range.lowerBound.utf8Offset,
keys.length: edit.length.utf8Length,
keys.length: edit.range.length.utf8Length,
keys.sourceText: edit.replacement,
])
do {
Expand All @@ -553,13 +553,7 @@ extension SwiftLanguageService {
}

let concurrentEdits = ConcurrentEdits(
fromSequential: edits.map {
IncrementalEdit(
offset: $0.range.lowerBound.utf8Offset,
length: $0.length.utf8Length,
replacementLength: $0.replacement.utf8.count
)
}
fromSequential: edits
)
await syntaxTreeManager.registerEdit(
preEditSnapshot: preEditSnapshot,
Expand Down Expand Up @@ -690,7 +684,7 @@ extension SwiftLanguageService {

result.append(
ColorInformation(
range: snapshot.range(of: node.position..<node.endPosition),
range: snapshot.absolutePositionRange(of: node.position..<node.endPosition),
color: Color(red: red, green: green, blue: blue, alpha: alpha)
)
)
Expand Down Expand Up @@ -1138,7 +1132,7 @@ extension DocumentSnapshot {
/// If the bounds of the range do not refer to a valid positions with in the snapshot, this function adjusts them to
/// the closest valid positions and logs a fault containing the file and line of the caller (from `callerFile` and
/// `callerLine`).
func range(
func absolutePositionRange(
of range: Range<AbsolutePosition>,
callerFile: StaticString = #fileID,
callerLine: UInt = #line
Expand Down Expand Up @@ -1169,9 +1163,12 @@ extension DocumentSnapshot {
of range: Range<Position>,
callerFile: StaticString = #fileID,
callerLine: UInt = #line
) -> ByteSourceRange {
) -> Range<AbsolutePosition> {
let utf8OffsetRange = utf8OffsetRange(of: range, callerFile: callerFile, callerLine: callerLine)
return ByteSourceRange(offset: utf8OffsetRange.startIndex, length: utf8OffsetRange.count)
return Range<AbsolutePosition>(
position: AbsolutePosition(utf8Offset: utf8OffsetRange.startIndex),
length: SourceLength(utf8Length: utf8OffsetRange.count)
)
}

// MARK: Position <-> RenameLocation
Expand Down
8 changes: 6 additions & 2 deletions Sources/SourceKitLSP/Swift/SwiftTestingScanner.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ final class SyntacticSwiftTestingTestScanner: SyntaxVisitor {
return .skipChildren
}

let range = snapshot.range(of: node.positionAfterSkippingLeadingTrivia..<node.endPositionBeforeTrailingTrivia)
let range = snapshot.absolutePositionRange(
of: node.positionAfterSkippingLeadingTrivia..<node.endPositionBeforeTrailingTrivia
)
// Members won't be extensions since extensions will only be at the top level.
let testItem = AnnotatedTestItem(
testItem: TestItem(
Expand Down Expand Up @@ -306,7 +308,9 @@ final class SyntacticSwiftTestingTestScanner: SyntaxVisitor {
let name =
node.name.text + "(" + node.signature.parameterClause.parameters.map { "\($0.firstName.text):" }.joined() + ")"

let range = snapshot.range(of: node.positionAfterSkippingLeadingTrivia..<node.endPositionBeforeTrailingTrivia)
let range = snapshot.absolutePositionRange(
of: node.positionAfterSkippingLeadingTrivia..<node.endPositionBeforeTrailingTrivia
)
let testItem = AnnotatedTestItem(
testItem: TestItem(
id: (parentTypeNames + [name]).joined(separator: "/"),
Expand Down
6 changes: 4 additions & 2 deletions Sources/SourceKitLSP/TestDiscovery.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ final class SyntacticSwiftXCTestScanner: SyntaxVisitor {
// declarations are probably less common than helper functions that start with `test` and have a return type.
return nil
}
let range = snapshot.range(
let range = snapshot.absolutePositionRange(
of: function.positionAfterSkippingLeadingTrivia..<function.endPositionBeforeTrailingTrivia
)

Expand Down Expand Up @@ -436,7 +436,9 @@ final class SyntacticSwiftXCTestScanner: SyntaxVisitor {
// Don't report a test class if it doesn't contain any test methods.
return .visitChildren
}
let range = snapshot.range(of: node.positionAfterSkippingLeadingTrivia..<node.endPositionBeforeTrailingTrivia)
let range = snapshot.absolutePositionRange(
of: node.positionAfterSkippingLeadingTrivia..<node.endPositionBeforeTrailingTrivia
)
let testItem = AnnotatedTestItem(
testItem: TestItem(
id: node.name.text,
Expand Down