Move `SwiftRefactor` code actions from swift-syntax into sourcekit-lsp

[rintaro]

Per the Code Action contributor documentation, code actions should generally be implemented in sourcekit-lsp since they are primarily a language-server feature, and should only be implemented in swift-syntax when the functionality needs to be reused outside of the language server.

Several code actions are currently implemented as SyntaxRefactoringProvider conformances in swift-syntax/Sources/SwiftRefactor/, but their only consumer is sourcekit-lsp, which wraps them via the SyntaxRefactoringCodeActionProvider adapter. These should be moved to sourcekit-lsp directly as SyntaxCodeActionProvider types.

Actions to move:

• AddSeparatorsToIntegerLiteral — "Add digit separators"
• ConvertComputedPropertyToStored — "Convert to stored property"
• ConvertComputedPropertyToZeroParameterFunction — "Convert to zero parameter function"
• ConvertStoredPropertyToComputed — "Convert to computed property" (currently not wired up)
• ConvertZeroParameterFunctionToComputedProperty — "Convert to computed property"
• FormatRawStringLiteral — "Convert string literal to minimal number of '#'s"
• MigrateToNewIfLetSyntax — "Migrate to shorthand 'if let' syntax"
• OpaqueParameterToGeneric — "Expand 'some' parameters to generic parameters"
• RemoveRedundantParentheses — "Remove Redundant Parentheses"
• RemoveSeparatorsFromIntegerLiteral — "Remove digit separators"

Out of scope:

• ExpandEditorPlaceholdersToLiteralClosures and its supporting types (ExpandEditorPlaceholder, ExpandSingleEditorPlaceholder) — used directly in sourcekit-lsp's code completion, not as a code action
• CallToTrailingClosures — used as an internal implementation detail of ExpandEditorPlaceholder within swift-syntax
• PackageManifest/ refactorings (AddPackageDependency, AddPackageTarget, AddProduct, etc.) — used as building blocks for structured manifest editing

[25harsh]

Hi @rintaro, I'd like to pick this up

[25harsh]

Hi @rintaro, apologies for the delay, got busy. Planning to start with AddSeparatorsToIntegerLiteral and RemoveSeparatorsFromIntegerLiteral.

Quick questions:

1. Would you prefer separate PRs per refactor? (maybe i can get some feedback and improve subsequent ones this way)
2. sourcekit-lsp addition PR first then swift-syntax removal makes sense
3. lastly regarding Change code actions to provide functionality through SyntaxRefactoringCodeActionProvider #2628, should the new sourcekit-lsp types conform to SyntaxRefactoringCodeActionProvider or directly to SyntaxCodeActionProvider?

[25harsh]

hey @ahoppen can you help here

[ahoppen]

Thanks for your help in doing this, @25harsh.

• Would you prefer separate PRs per refactor? (maybe i can get some feedback and improve subsequent ones this way)

I would prefer one PR that moves all the refactoring actions with no to minimal changes to the source code itself. Reviewing source changes for code removed in one repository and added it another is just really hard to review.

If there are any refactoring actions that require non-trivial (aka. anything beyond change access control etc.) source changes, I’d prefer those to have their own PR so we can give them their deserved attention.

• sourcekit-lsp addition PR first then swift-syntax removal makes sense

You can open both PRs and we can merge them at the same time.

• lastly regarding Change code actions to provide functionality through SyntaxRefactoringCodeActionProvider #2628, should the new sourcekit-lsp types conform to SyntaxRefactoringCodeActionProvider or directly to SyntaxCodeActionProvider?

I would prefer them to continue conforming to SyntaxRefactoringCodeActionProvider. I thought we had some documentation for that as well but I can’t find it.

[rintaro]

Ah sorry for the late reply. Yeah we agreed we'd go with SyntaxRefactoringCodeActionProvider (link), so this task is basically just moving the code. "one PR that moves all" sounds fine to me.