Skip to content
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

Add local refactoring code actions based on swift-syntax #1169

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//
//===----------------------------------------------------------------------===//

public typealias CodeActionProvider = (CodeActionRequest) async throws -> [CodeAction]
public typealias CodeActionRequestProvider = (CodeActionRequest) async throws -> [CodeAction]
DougGregor marked this conversation as resolved.
Show resolved Hide resolved

/// Request for returning all possible code actions for a given text document and range.
///
Expand Down
7 changes: 7 additions & 0 deletions Sources/SourceKitLSP/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ target_sources(SourceKitLSP PRIVATE
Clang/ClangLanguageService.swift)
target_sources(SourceKitLSP PRIVATE
Swift/AdjustPositionToStartOfIdentifier.swift
Swift/CodeActions/CodeActionProvider.swift
Swift/CodeActions/CodeActions.swift
Swift/CodeActions/ConvertIntegerLiteral.swift
Swift/CodeActions/ConvertJSONToCodableStruct.swift
Swift/CodeActions/Demorgan.swift
Swift/CodeActions/EditBuilder.swift
Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift
Swift/CodeCompletion.swift
Swift/CodeCompletionSession.swift
Swift/CommentXML.swift
Expand Down
73 changes: 73 additions & 0 deletions Sources/SourceKitLSP/Swift/CodeActions/CodeActionProvider.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import LanguageServerProtocol
import SwiftSyntax
import LSPLogging
import SwiftRefactor

public protocol CodeActionProvider {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a doc comment describing what this type does?

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay!

static var kind: CodeActionKind { get }
static func provideAssistance(in scope: CodeActionScope) -> [ProvidedAction]
DougGregor marked this conversation as resolved.
Show resolved Hide resolved
}

public struct CodeActionScope {
DougGregor marked this conversation as resolved.
Show resolved Hide resolved
public var snapshot: DocumentSnapshot
public var parameters: CodeActionRequest
DougGregor marked this conversation as resolved.
Show resolved Hide resolved
public var file: SourceFileSyntax
public var range: ByteSourceRange
DougGregor marked this conversation as resolved.
Show resolved Hide resolved

init(snapshot: DocumentSnapshot, syntaxTree tree: SourceFileSyntax, parameters: CodeActionRequest) throws {
self.snapshot = snapshot
self.parameters = parameters
self.file = tree

let start = self.snapshot.utf8Offset(of: self.parameters.range.lowerBound) ?? 0
let end = self.snapshot.utf8Offset(of: self.parameters.range.upperBound) ?? start
DougGregor marked this conversation as resolved.
Show resolved Hide resolved
let left = self.file.token(at: start)
let right = self.file.token(at: end)

let leftOff = left?.positionAfterSkippingLeadingTrivia.utf8Offset ?? 0
let rightOff = right?.endPositionBeforeTrailingTrivia.utf8Offset ?? leftOff
assert(leftOff <= rightOff)
self.range = ByteSourceRange(offset: leftOff, length: rightOff - leftOff)
}

public func starts(with tokenKind: TokenKind) -> TokenSyntax? {
guard
let token = self.file.token(at: self.range.offset),
token.tokenKind == tokenKind
else {
return nil
}
return token
}
Comment on lines +45 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this function used for? I couldn’t find any calls of it.

}

extension SyntaxProtocol {
func token(at utf8Offset: Int) -> TokenSyntax? {
return token(at: AbsolutePosition(utf8Offset: utf8Offset))
}
}

extension ByteSourceRange {
fileprivate func contains(_ other: Int) -> Bool {
return self.offset <= other && other <= self.endOffset
}
}

extension SyntaxProtocol {
var textRange: ByteSourceRange {
return ByteSourceRange(offset: self.positionAfterSkippingLeadingTrivia.utf8Offset,
length: self.trimmedLength.utf8Length)
}
}
25 changes: 25 additions & 0 deletions Sources/SourceKitLSP/Swift/CodeActions/CodeActions.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import SwiftRefactor

/// List of all of the syntax-local code actions.
public let allLocalCodeActions: [CodeActionProvider.Type] = [
AddSeparatorsToIntegerLiteral.self,
ConvertIntegerLiteral.self,
ConvertJSONToCodableStruct.self,
Demorgan.self,
FormatRawStringLiteral.self,
MigrateToNewIfLetSyntax.self,
OpaqueParameterToGeneric.self,
RemoveSeparatorsFromIntegerLiteral.self,
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import SwiftSyntax
import LanguageServerProtocol
import SwiftRefactor

// TODO: Make the type IntegerLiteralExprSyntax.Radix conform to CaseEnumerable
// in swift-syntax.
Comment on lines +17 to +18
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we just do this instead of having a TODO?

Copy link
Member Author

Choose a reason for hiding this comment

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

grumble grumble dependent PRs grumble grumble


extension IntegerLiteralExprSyntax.Radix {
static var allCases: [Self] = [.binary, .octal, .decimal, .hex]
}

public struct ConvertIntegerLiteral: CodeActionProvider {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these CodeActionProvider types need to be public or could we make them internal?


Can’t we implement this as a SyntaxRefactoringProvider in swift-syntax so it’s consistent with eg. AddSeparatorsToIntegerLiteral? We could pass the desired target radix in via the context parameter.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could do this, yes.

public static var kind: CodeActionKind { .refactorInline }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think .refactorRewrite would make more sense here than .refactorInline because this is rewriting something instead of inlining.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have a good sense of how these manifest. Does this go for all of the refactoring we're creating?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don’t actually know either (I guess also depends on the IDE) but it’s good practice to choose the closest possible case.


public static func provideAssistance(in scope: CodeActionScope) -> [ProvidedAction] {
guard
let token = scope.file.token(at: scope.range.offset),
let lit = token.parent?.as(IntegerLiteralExprSyntax.self),
DougGregor marked this conversation as resolved.
Show resolved Hide resolved
let integerValue = Int(lit.split().value, radix: lit.radix.size)
else {
return []
}

var actions = [ProvidedAction]()
let currentRadix = lit.radix
for radix in IntegerLiteralExprSyntax.Radix.allCases {
guard radix != currentRadix else {
continue
}

//TODO: Add this to swift-syntax?
let prefix: String
switch radix {
case .binary:
prefix = "0b"
case .octal:
prefix = "0o"
case .hex:
prefix = "0x"
case .decimal:
prefix = ""
}
Comment on lines +43 to +54
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think adding this to swift-syntax would make sense.


let convertedValue: ExprSyntax =
"\(raw: prefix)\(raw: String(integerValue, radix: radix.size))"
actions.append(ProvidedAction(title: "Convert \(lit) to \(convertedValue)") {
Replace(lit, with: convertedValue)
})
}

return actions
}
}