diff --git a/Sources/SourceKitLSP/CMakeLists.txt b/Sources/SourceKitLSP/CMakeLists.txt index e56f172fa..a86c621f9 100644 --- a/Sources/SourceKitLSP/CMakeLists.txt +++ b/Sources/SourceKitLSP/CMakeLists.txt @@ -14,6 +14,7 @@ add_library(SourceKitLSP STATIC SourceKitLSPServer+Options.swift SymbolLocation+DocumentURI.swift TestDiscovery.swift + TextEdit+IsNoop.swift WorkDoneProgressManager.swift Workspace.swift ) diff --git a/Sources/SourceKitLSP/Rename.swift b/Sources/SourceKitLSP/Rename.swift index a8bb0b15d..3ac6984b6 100644 --- a/Sources/SourceKitLSP/Rename.swift +++ b/Sources/SourceKitLSP/Rename.swift @@ -1490,13 +1490,3 @@ fileprivate extension RelatedIdentifiersResponse { } } } - -fileprivate extension TextEdit { - /// Returns `true` the replaced text is the same as the new text - func isNoOp(in snapshot: DocumentSnapshot) -> Bool { - if snapshot.text[snapshot.indexRange(of: range)] == newText { - return true - } - return false - } -} diff --git a/Sources/SourceKitLSP/Swift/CodeActions/AddDocumentation.swift b/Sources/SourceKitLSP/Swift/CodeActions/AddDocumentation.swift index f37a6b661..1d3543cd7 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/AddDocumentation.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/AddDocumentation.swift @@ -37,22 +37,25 @@ import SwiftSyntax public struct AddDocumentation: EditRefactoringProvider { @_spi(Testing) public static func textRefactor(syntax: DeclSyntax, in context: Void) -> [SourceEdit] { - let hasDocumentation = syntax.leadingTrivia.contains(where: { trivia in + let hasDocumentation = syntax.leadingTrivia.contains { trivia in switch trivia { - case .blockComment(_), .docBlockComment(_), .lineComment(_), .docLineComment(_): + case .blockComment, .docBlockComment, .lineComment, .docLineComment: return true default: return false } - }) + } + + // We consider nodes at the start of the source file at being on a new line + let isOnNewLine = + syntax.leadingTrivia.contains(where: \.isNewline) || syntax.previousToken(viewMode: .sourceAccurate) == nil - guard !hasDocumentation else { + guard !hasDocumentation && isOnNewLine else { return [] } let newlineAndIndentation = [.newlines(1)] + (syntax.firstToken(viewMode: .sourceAccurate)?.indentationOfLine ?? []) var content: [TriviaPiece] = [] - content += newlineAndIndentation content.append(.docLineComment("/// A description")) if let parameters = syntax.parameters?.parameters { @@ -82,8 +85,9 @@ public struct AddDocumentation: EditRefactoringProvider { content += newlineAndIndentation content.append(.docLineComment("/// - Returns:")) } + content += newlineAndIndentation - let insertPos = syntax.position + let insertPos = syntax.positionAfterSkippingLeadingTrivia return [ SourceEdit( range: insertPos.. Input? { + return scope.innermostNodeContainingRange?.findParentOfSelf( + ofType: DeclSyntax.self, + stoppingIf: { $0.is(CodeBlockItemSyntax.self) || $0.is(MemberBlockItemSyntax.self) || $0.is(ExprSyntax.self) } + ) + } + static var title: String { "Add documentation" } } diff --git a/Sources/SourceKitLSP/Swift/CodeActions/ConvertIntegerLiteral.swift b/Sources/SourceKitLSP/Swift/CodeActions/ConvertIntegerLiteral.swift index b541f1c91..f00d0a80c 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/ConvertIntegerLiteral.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/ConvertIntegerLiteral.swift @@ -14,9 +14,6 @@ import LanguageServerProtocol import SwiftRefactor import SwiftSyntax -// TODO: Make the type IntegerLiteralExprSyntax.Radix conform to CaseEnumerable -// in swift-syntax. - extension IntegerLiteralExprSyntax.Radix { static var allCases: [Self] = [.binary, .octal, .decimal, .hex] } @@ -26,7 +23,7 @@ extension IntegerLiteralExprSyntax.Radix { struct ConvertIntegerLiteral: SyntaxCodeActionProvider { static func codeActions(in scope: SyntaxCodeActionScope) -> [CodeAction] { guard - let token = scope.firstToken, + let token = scope.innermostNodeContainingRange, let integerExpr = token.parent?.as(IntegerLiteralExprSyntax.self), let integerValue = Int( integerExpr.split().value.filter { $0 != "_" }, diff --git a/Sources/SourceKitLSP/Swift/CodeActions/ConvertJSONToCodableStruct.swift b/Sources/SourceKitLSP/Swift/CodeActions/ConvertJSONToCodableStruct.swift index 73f583627..dc1fab7a0 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/ConvertJSONToCodableStruct.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/ConvertJSONToCodableStruct.swift @@ -192,6 +192,17 @@ public struct ConvertJSONToCodableStruct: EditRefactoringProvider { } extension ConvertJSONToCodableStruct: SyntaxRefactoringCodeActionProvider { + static func nodeToRefactor(in scope: SyntaxCodeActionScope) -> Syntax? { + var node: Syntax? = scope.innermostNodeContainingRange + while let unwrappedNode = node, ![.codeBlockItem, .memberBlockItem].contains(unwrappedNode.kind) { + if preflightRefactoring(unwrappedNode) != nil { + return unwrappedNode + } + node = unwrappedNode.parent + } + return nil + } + static var title = "Create Codable structs from JSON" } diff --git a/Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift b/Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift index 306d4698f..439513e2b 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/PackageManifestEdits.swift @@ -20,9 +20,7 @@ import SwiftSyntax /// edit a package manifest. struct PackageManifestEdits: SyntaxCodeActionProvider { static func codeActions(in scope: SyntaxCodeActionScope) -> [CodeAction] { - guard let token = scope.firstToken, - let call = token.findEnclosingCall() - else { + guard let call = scope.innermostNodeContainingRange?.findEnclosingCall() else { return [] } diff --git a/Sources/SourceKitLSP/Swift/CodeActions/SyntaxCodeActionProvider.swift b/Sources/SourceKitLSP/Swift/CodeActions/SyntaxCodeActionProvider.swift index 45bebc378..0674c5859 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/SyntaxCodeActionProvider.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/SyntaxCodeActionProvider.swift @@ -40,26 +40,50 @@ struct SyntaxCodeActionScope { /// considered, i.e., where the cursor or selection is. var range: Range - init( + /// The innermost node that contains the entire selected source range + var innermostNodeContainingRange: Syntax? + + init?( snapshot: DocumentSnapshot, - syntaxTree tree: SourceFileSyntax, + syntaxTree file: SourceFileSyntax, request: CodeActionRequest - ) throws { + ) { self.snapshot = snapshot self.request = request - self.file = tree + self.file = file - let start = snapshot.absolutePosition(of: request.range.lowerBound) - let end = snapshot.absolutePosition(of: request.range.upperBound) - let left = file.token(at: start) - let right = file.token(at: end) - let leftOff = left?.position ?? AbsolutePosition(utf8Offset: 0) - let rightOff = right?.endPosition ?? leftOff - self.range = leftOff.. TokenSyntax? { + let absolutePosition = snapshot.absolutePosition(of: position) + if absolutePosition == syntaxTree.endPosition { + // token(at:) will not find the end of file token if the end of file token has length 0. Special case this and + // return the last proper token in this case. + return syntaxTree.endOfFileToken.previousToken(viewMode: .sourceAccurate) + } + guard let token = syntaxTree.token(at: absolutePosition) else { + return nil + } + // See `adjustPositionToStartOfIdentifier`. We need to be a little more aggressive for the refactorings and also + // adjust to the start of punctuation eg. if the end of the selected range is after a `}`, we want the end token for + // the refactoring to be the `}`, not the token after `}`. + if absolutePosition == token.position, + let previousToken = token.previousToken(viewMode: .sourceAccurate), + previousToken.endPositionBeforeTrailingTrivia == absolutePosition + { + return previousToken } + return token } diff --git a/Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift b/Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift index 1fb72e0ed..4a42f52f4 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift @@ -18,29 +18,35 @@ import SwiftSyntax /// swift-syntax) into a SyntaxCodeActionProvider. protocol SyntaxRefactoringCodeActionProvider: SyntaxCodeActionProvider, EditRefactoringProvider { static var title: String { get } + + /// Returns the node that the syntax refactoring should be performed on, if code actions are requested for the given + /// scope. + static func nodeToRefactor(in scope: SyntaxCodeActionScope) -> Input? } /// SyntaxCodeActionProviders with a \c Void context can automatically be /// adapted provide a code action based on their refactoring operation. extension SyntaxRefactoringCodeActionProvider where Self.Context == Void { static func codeActions(in scope: SyntaxCodeActionScope) -> [CodeAction] { - guard - let token = scope.firstToken, - let node = token.parent?.as(Input.self) - else { + guard let node = nodeToRefactor(in: scope) else { return [] } let sourceEdits = Self.textRefactor(syntax: node) - if sourceEdits.isEmpty { - return [] - } - let textEdits = sourceEdits.map { edit in - TextEdit( + let textEdits = sourceEdits.compactMap { (edit) -> TextEdit? in + let edit = TextEdit( range: scope.snapshot.range(of: edit.range), newText: edit.replacement ) + if edit.isNoOp(in: scope.snapshot) { + return nil + } + return edit + } + + if textEdits.isEmpty { + return [] } return [ @@ -57,22 +63,77 @@ extension SyntaxRefactoringCodeActionProvider where Self.Context == Void { extension AddSeparatorsToIntegerLiteral: SyntaxRefactoringCodeActionProvider { public static var title: String { "Add digit separators" } + + static func nodeToRefactor(in scope: SyntaxCodeActionScope) -> Input? { + return scope.innermostNodeContainingRange?.findParentOfSelf( + ofType: IntegerLiteralExprSyntax.self, + stoppingIf: { $0.is(CodeBlockSyntax.self) || $0.is(MemberBlockSyntax.self) } + ) + } } extension FormatRawStringLiteral: SyntaxRefactoringCodeActionProvider { public static var title: String { "Convert string literal to minimal number of '#'s" } + + static func nodeToRefactor(in scope: SyntaxCodeActionScope) -> Input? { + return scope.innermostNodeContainingRange?.findParentOfSelf( + ofType: StringLiteralExprSyntax.self, + stoppingIf: { + $0.is(CodeBlockSyntax.self) || $0.is(MemberBlockSyntax.self) + || $0.keyPathInParent == \ExpressionSegmentSyntax.expressions + } + ) + } } extension MigrateToNewIfLetSyntax: SyntaxRefactoringCodeActionProvider { public static var title: String { "Migrate to shorthand 'if let' syntax" } + + static func nodeToRefactor(in scope: SyntaxCodeActionScope) -> Input? { + return scope.innermostNodeContainingRange?.findParentOfSelf( + ofType: IfExprSyntax.self, + stoppingIf: { $0.is(CodeBlockSyntax.self) || $0.is(MemberBlockSyntax.self) } + ) + } } extension OpaqueParameterToGeneric: SyntaxRefactoringCodeActionProvider { public static var title: String { "Expand 'some' parameters to generic parameters" } + + static func nodeToRefactor(in scope: SyntaxCodeActionScope) -> Input? { + return scope.innermostNodeContainingRange?.findParentOfSelf( + ofType: DeclSyntax.self, + stoppingIf: { $0.is(CodeBlockSyntax.self) || $0.is(MemberBlockSyntax.self) } + ) + } } extension RemoveSeparatorsFromIntegerLiteral: SyntaxRefactoringCodeActionProvider { public static var title: String { "Remove digit separators" } + + static func nodeToRefactor(in scope: SyntaxCodeActionScope) -> Input? { + return scope.innermostNodeContainingRange?.findParentOfSelf( + ofType: IntegerLiteralExprSyntax.self, + stoppingIf: { $0.is(CodeBlockSyntax.self) || $0.is(MemberBlockSyntax.self) } + ) + } +} + +extension Syntax { + /// Finds the innermost parent of the given type while not walking outside of nodes that satisfy `stoppingIf`. + func findParentOfSelf( + ofType: ParentType.Type, + stoppingIf: (Syntax) -> Bool + ) -> ParentType? { + var node: Syntax? = self + while let unwrappedNode = node, !stoppingIf(unwrappedNode) { + if let expectedType = unwrappedNode.as(ParentType.self) { + return expectedType + } + node = unwrappedNode.parent + } + return nil + } } diff --git a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift index 6f21b250f..49095bea3 100644 --- a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift +++ b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift @@ -755,9 +755,10 @@ extension SwiftLanguageService { return response } - func retrieveCodeActions(_ req: CodeActionRequest, providers: [CodeActionProvider]) async throws - -> [CodeAction] - { + func retrieveCodeActions( + _ req: CodeActionRequest, + providers: [CodeActionProvider] + ) async throws -> [CodeAction] { guard providers.isEmpty == false else { return [] } @@ -776,7 +777,9 @@ extension SwiftLanguageService { let snapshot = try documentManager.latestSnapshot(uri) let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot) - let scope = try SyntaxCodeActionScope(snapshot: snapshot, syntaxTree: syntaxTree, request: request) + guard let scope = SyntaxCodeActionScope(snapshot: snapshot, syntaxTree: syntaxTree, request: request) else { + return [] + } return await allSyntaxCodeActions.concurrentMap { provider in return provider.codeActions(in: scope) }.flatMap { $0 } @@ -1152,8 +1155,8 @@ extension DocumentSnapshot { callerFile: StaticString = #fileID, callerLine: UInt = #line ) -> Range { - let lowerBound = self.position(of: node.position) - let upperBound = self.position(of: node.endPosition) + let lowerBound = self.position(of: node.position, callerFile: callerFile, callerLine: callerLine) + let upperBound = self.position(of: node.endPosition, callerFile: callerFile, callerLine: callerLine) return lowerBound.. Bool { + if snapshot.text[snapshot.indexRange(of: range)] == newText { + return true + } + return false + } +} diff --git a/Tests/SourceKitLSPTests/CodeActionTests.swift b/Tests/SourceKitLSPTests/CodeActionTests.swift index 7939c3d75..8a340a21e 100644 --- a/Tests/SourceKitLSPTests/CodeActionTests.swift +++ b/Tests/SourceKitLSPTests/CodeActionTests.swift @@ -16,23 +16,22 @@ import SKTestSupport import SourceKitLSP import XCTest -final class CodeActionTests: XCTestCase { - - typealias CodeActionCapabilities = TextDocumentClientCapabilities.CodeAction - typealias CodeActionLiteralSupport = CodeActionCapabilities.CodeActionLiteralSupport - typealias CodeActionKindCapabilities = CodeActionLiteralSupport.CodeActionKind - - private func clientCapabilitiesWithCodeActionSupport() -> ClientCapabilities { - var documentCapabilities = TextDocumentClientCapabilities() - var codeActionCapabilities = CodeActionCapabilities() - let codeActionKinds = CodeActionKindCapabilities(valueSet: [.refactor, .quickFix]) - let codeActionLiteralSupport = CodeActionLiteralSupport(codeActionKind: codeActionKinds) - codeActionCapabilities.codeActionLiteralSupport = codeActionLiteralSupport - documentCapabilities.codeAction = codeActionCapabilities - documentCapabilities.completion = .init(completionItem: .init(snippetSupport: true)) - return ClientCapabilities(workspace: nil, textDocument: documentCapabilities) - } +private typealias CodeActionCapabilities = TextDocumentClientCapabilities.CodeAction +private typealias CodeActionLiteralSupport = CodeActionCapabilities.CodeActionLiteralSupport +private typealias CodeActionKindCapabilities = CodeActionLiteralSupport.CodeActionKind + +private var clientCapabilitiesWithCodeActionSupport: ClientCapabilities = { + var documentCapabilities = TextDocumentClientCapabilities() + var codeActionCapabilities = CodeActionCapabilities() + let codeActionKinds = CodeActionKindCapabilities(valueSet: [.refactor, .quickFix]) + let codeActionLiteralSupport = CodeActionLiteralSupport(codeActionKind: codeActionKinds) + codeActionCapabilities.codeActionLiteralSupport = codeActionLiteralSupport + documentCapabilities.codeAction = codeActionCapabilities + documentCapabilities.completion = .init(completionItem: .init(snippetSupport: true)) + return ClientCapabilities(workspace: nil, textDocument: documentCapabilities) +}() +final class CodeActionTests: XCTestCase { func testCodeActionResponseLegacySupport() throws { let command = Command(title: "Title", command: "Command", arguments: [1, "text", 2.2, nil]) let codeAction = CodeAction(title: "1") @@ -191,7 +190,7 @@ final class CodeActionTests: XCTestCase { } func testEmptyCodeActionResult() async throws { - let testClient = try await TestSourceKitLSPClient(capabilities: clientCapabilitiesWithCodeActionSupport()) + let testClient = try await TestSourceKitLSPClient(capabilities: clientCapabilitiesWithCodeActionSupport) let uri = DocumentURI.for(.swift) let positions = testClient.openDocument( """ @@ -214,7 +213,7 @@ final class CodeActionTests: XCTestCase { } func testSemanticRefactorLocalRenameResult() async throws { - let testClient = try await TestSourceKitLSPClient(capabilities: clientCapabilitiesWithCodeActionSupport()) + let testClient = try await TestSourceKitLSPClient(capabilities: clientCapabilitiesWithCodeActionSupport) let uri = DocumentURI.for(.swift) let positions = testClient.openDocument( """ @@ -227,16 +226,20 @@ final class CodeActionTests: XCTestCase { ) let request = CodeActionRequest( - range: positions["1️⃣"]..(_ input: T1) {}" + ) + ] + ] + ), + command: nil + ) + ] + } + } + + func testOpaqueParameterToGenericIsNotShownFromTheBody() async throws { + try await assertCodeActions( + ##""" + func someFunction(_ input: some Value) 1️⃣{ + 2️⃣print("x") + }3️⃣ + """##, + exhaustive: false + ) { uri, positions in + [] + } + } + + func testConvertJSONToCodable() async throws { + try await assertCodeActions( + ##""" + 1️⃣{ + 2️⃣"id": 3️⃣1, + "values": 4️⃣["foo", "bar"] + }5️⃣ + + """##, + ranges: [("1️⃣", "5️⃣")], + exhaustive: false + ) { uri, positions in + [ + CodeAction( + title: "Create Codable structs from JSON", + kind: .refactorInline, + diagnostics: nil, + edit: WorkspaceEdit( + changes: [ + uri: [ + TextEdit( + range: positions["1️⃣"].. [CodeAction], + file: StaticString = #filePath, + line: UInt = #line + ) async throws { + let testClient = try await TestSourceKitLSPClient(capabilities: clientCapabilitiesWithCodeActionSupport) + let uri = DocumentURI.for(.swift) + let positions = testClient.openDocument(markedText, uri: uri) + + var ranges = ranges + if let markers { + ranges += markers.map { ($0, $0) } + } else { + ranges += extractMarkers(markedText).markers.map(\.key).map { ($0, $0) } + } + + for (startMarker, endMarker) in ranges { + let result = try await testClient.send( + CodeActionRequest( + range: positions[startMarker]..