Skip to content

[5.9] Slightly clean up FixIt.Change and FixIt.Changes #1496

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

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 5, 2023

Cherry-pick #1460 to release/5.9


The existing API with FixIt.Change and FixIt.Changes and the fact that we passed [FixIt.Changes] around what fairly weird.

Ideally, we would only have FixIt.Change and make it powerful enough to represent all the trivia transfer that FixIt.Changes is doing. But given that that will be a larger effort, let’s make some smaller changes now that reduce the badness of the public API impact:

  • Move FixIt.Changes to SwiftParserDiagnostics, make it internal and rename it to MultiNodeChange. That way this type is no longer exposed in the public API, so we can iterate on it or remove it without breaking clients. The only thing that remains exposed is FixIt.Change, which we want to continue to support. We will probably add more cases to it in the future.
  • Make FixIt.Changes no longer conform to ExpressibleByArrayLiteral. That just makes everything a lot more explicit and easier to follow.
  • Remove some unecessary initializations of FixIt.Changes where FixIt.Change was sufficient.
  • Make presence on TokenSyntax settable so you can do token.with(\.presence, .missing)
  • Slightly unrelated: Rename SyntaxOtherNodes.swift to TokenSyntax.swift because that’s the only node it contains.

The existing API with `FixIt.Change` and `FixIt.Changes` and the fact that we passed `[FixIt.Changes]` around what fairly weird.

Ideally, we would only have `FixIt.Change` and make it powerful enough to represent all the trivia transfer that `FixIt.Changes` is doing. But given that that will be a larger effort, let’s make some smaller changes now that reduce the badness of the public API impact:

- Move `FixIt.Changes` to `SwiftParserDiagnostics`, make it internal and rename it to `MultiNodeChange`. That way this type is no longer exposed in the public API, so we can iterate on it or remove it without breaking clients. The only thing that remains exposed is `FixIt.Change`, which we want to continue to support. We will probably add more cases to it in the future.
- Make `FixIt.Changes` no longer conform to `ExpressibleByArrayLiteral`. That just makes everything a lot more explicit and easier to follow.
- Remove some unecessary initializations of `FixIt.Changes` where `FixIt.Change` was sufficient.
- Make `presence` on `TokenSyntax` settable so you can do `token.with(\.presence, .missing)`
- Slightly unrelated: Rename SyntaxOtherNodes.swift to TokenSyntax.swift because that’s the only node it contains.
@ahoppen
Copy link
Member Author

ahoppen commented Apr 5, 2023

swiftlang/swift#64960

@swift-ci Please test

@ahoppen ahoppen merged commit ae4fe2b into swiftlang:release/5.9 Apr 6, 2023
@ahoppen ahoppen deleted the ahoppen/5.9/fixit branch April 6, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants