Add local refactoring code actions based on swift-syntax #1169
Conversation
…oringProviders For swift-syntax SwiftRefactoringProviders that have no context information, we only need to supply a title.
|
@swift-ci please test |
| /// ## After | ||
| /// ```swift | ||
| /// | ||
| /// ``` |
There was a problem hiding this comment.
Heh the fact I didn't finish documenting this likely means it may not be ready for prime time. I dunno, tho, the implementation looks mostly fine.
There was a problem hiding this comment.
Yeah.... we should sink it down into SwiftRefactor and put some proper testing in there.
| switch unexpected { | ||
| case let .closure(closure): | ||
| closure.write(to: &text) | ||
| case let .tail(closure, unexpected): | ||
| closure.write(to: &text) | ||
| unexpected.write(to: &text) | ||
| } |
There was a problem hiding this comment.
| switch unexpected { | |
| case let .closure(closure): | |
| closure.write(to: &text) | |
| case let .tail(closure, unexpected): | |
| closure.write(to: &text) | |
| unexpected.write(to: &text) | |
| } | |
| switch unexpected { | |
| case let .closure(closure): | |
| closure.trimmed.write(to: &text) | |
| case let .tail(closure, unexpected): | |
| closure.trimmed.write(to: &text) | |
| unexpected.write(to: &text) | |
| } |
There was a problem hiding this comment.
Otherwise we'll pick up comments!
// Foo Bar Baz
{
"abc": "xyz"
}
|
@swift-ci please test |
|
@swift-ci please test windows |
|
@swift-ci please test Windows |
|
Love the JSON-to-Codable conversion, that looks super useful! |
|
@swift-ci please test |
|
@swift-ci please test Windows |
1 similar comment
|
@swift-ci please test Windows |
| import SwiftRefactor | ||
| import SwiftSyntax | ||
|
|
||
| public protocol CodeActionProvider { |
There was a problem hiding this comment.
Could you add a doc comment describing what this type does?
| guard let firstValue = (value as! [Any]).first else { | ||
| return .array(.null) | ||
| } | ||
| let innerType = self.jsonType(of: firstValue, suggestion: name) | ||
| return .array(innerType) |
There was a problem hiding this comment.
Hmm, I think it’s unfortunate that we don’t allow merging the keys from multiple objects in the array. I don’t think it’s uncommon for JSON to omit keys for null values. We should be able to take the union of all the keys in the array. And I think it would also be good to infer optionality of a value eg. from
[
{
"value": 1
},
{
"value": null
}
]| var values: [Any] = self | ||
| for i in 0..<depth { | ||
| if i + 1 == depth { | ||
| return values[0] as? Dictionary<String, Any> |
There was a problem hiding this comment.
Do we know that self is not empty here?
| } | ||
|
|
||
| extension Array where Element == Any { | ||
| fileprivate func unwrap(_ depth: Int) -> Dictionary<String, Any>? { |
There was a problem hiding this comment.
Could you add a comment of what unwrap does?
| } | ||
|
|
||
| if !nestedTypes.isEmpty { | ||
| members.append("") |
There was a problem hiding this comment.
Having to append an empty string to members to (presumably) bet an empty line between the members seems wrong. I think we should add an empty line between all the members instead.
| case null | ||
| case object(String) | ||
|
|
||
| var outermostObject: (Int, String)? { |
There was a problem hiding this comment.
Instead of returning unwraps here and then calling unwrap with it, wouldn’t it be easier to just return the name and JSONType.object here?
|
I'm tackling the first part of this, exposing swift-syntax-based actions, via the PR at #1179. It drops Demorgans and the JSON-to-structs refactoring actions and addresses most of the feedback here. (Not all of it, yet) |
|
Everything but DeMorgan's has merged via other pull requests, so I'm going to close this out. DeMorgan's needs a some work that I'm not up for just now. |
Introduce new local refactoring code actions based on the Swift syntax tree, without going through SourceKit.
This change includes a number of new refactoring code actions. Most of them adapt the syntax refactorings from the
SwiftRefactormodule of swift-syntax:100000->1_000_000.1_000_000->1000000. (from SwiftRefactor)"Hello \#(world)"->##"Hello \#(world)"##if let x = x { ... }->if let x { ... }func f(p: some P)-->func f<T1: P>(p: T1).This is generally easy to do, requiring one conformance to provide a name for the refactoring:
There are also a few new refactoring code actions implemented here. These could be sunk down into SwiftRefactor to be made more widely available, or stay here if they're too IDE-centric to live in swift-refactor:
172387->0x2a163).!(a || b)-->!a && !b.There is a small amount of overlap between the refactoring code actions added here and those SourceKit provides. For example, SourceKit provides a "Simplify long number literal" operation that is similar to "Add digit separators". We have several directions we could go with those:
Most of the code here is actually from @CodaFi, and I've given it a light dusting to bring it up to modern times.