Skip to content

Introduce FixIt.Change API that replaces the child of a syntax node #2758

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 1, 2024
Merged

Introduce FixIt.Change API that replaces the child of a syntax node #2758

merged 1 commit into from
Aug 1, 2024

Conversation

AppAppWorks
Copy link
Contributor

fix #2205

@AppAppWorks AppAppWorks marked this pull request as draft July 27, 2024 03:25
@AppAppWorks
Copy link
Contributor Author

I don't know if I've got the direction right, anyways I just gave a try.

@AppAppWorks AppAppWorks force-pushed the add-FixIt.Change-replace-child-of-syntax-node branch from da96362 to d65c034 Compare July 29, 2024 18:45
ahoppen
ahoppen reviewed Jul 29, 2024
View reviewed changes
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Very nice stuff.

Could you also add an entry to the 601 release notes? Adding a new case to FixIt.Change is technically source breaking because clients might exhaustively switch over it.

@AppAppWorks AppAppWorks force-pushed the add-FixIt.Change-replace-child-of-syntax-node branch from d65c034 to 3796e79 Compare July 30, 2024 00:46
@AppAppWorks AppAppWorks marked this pull request as ready for review July 30, 2024 00:56
@AppAppWorks AppAppWorks force-pushed the add-FixIt.Change-replace-child-of-syntax-node branch from 3796e79 to 6ff3c32 Compare July 30, 2024 06:05
@ahoppen
Copy link
Member

ahoppen commented Jul 30, 2024

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge July 30, 2024 21:05
auto-merge was automatically disabled July 30, 2024 21:33

Head branch was pushed to by a user without write access

@AppAppWorks AppAppWorks force-pushed the add-FixIt.Change-replace-child-of-syntax-node branch from 6ff3c32 to 8676b36 Compare July 30, 2024 21:33
@AppAppWorks
Copy link
Contributor Author

Fixed the type checker issue, please test again.

@bnbarham
Copy link
Contributor

@swift-ci please test

@ahoppen
Copy link
Member

ahoppen commented Jul 30, 2024

@swift-ci Please test Windows

@AppAppWorks
introduced .replaceChild to FixIt.Change to allow replacing an op…
1c4c1cd 
…tional child node with a proper node

- introduced `ReplacingChildData` as the type-erased payload for `.replaceChild`
- added an entry to 601.md describing this API-breaking change
@AppAppWorks AppAppWorks force-pushed the add-FixIt.Change-replace-child-of-syntax-node branch from 8676b36 to 1c4c1cd Compare July 30, 2024 22:51
@AppAppWorks
Copy link
Contributor Author

I figured that older Swift versions wouldn't compile for [keyPath: (WritableKeyPath<Parent, Child?> & Sendable)] but [keyPath: WritableKeyPath<Parent, Child?>].

@ahoppen
Copy link
Member

ahoppen commented Jul 30, 2024

@swift-ci Please test

@AppAppWorks
Copy link
Contributor Author

The CI prompt seems to have failed again ;(

@ahoppen
Copy link
Member

ahoppen commented Jul 31, 2024

@swift-ci Please test

@ahoppen
Copy link
Member

ahoppen commented Jul 31, 2024

Adding a new case to FixIt.Change means that the compiler needs to be handle it here. Would you like to try implementing that change and opening a PR to swiftlang/swift? We can then test the two PRs together.

@AppAppWorks
Copy link
Contributor Author

It'd be a great idea, but I wonder how could I orchestrate the version bump across two projects?

@ahoppen
Copy link
Member

ahoppen commented Jul 31, 2024

We can test the PRs together using Cross Repository Testing and then we’ll merge them at the same time.

@AppAppWorks AppAppWorks mentioned this pull request Jul 31, 2024
Merged
@ahoppen
Copy link
Member

ahoppen commented Jul 31, 2024

swiftlang/swift#75582

@swift-ci Please test

@ahoppen
Copy link
Member

ahoppen commented Aug 1, 2024

swiftlang/swift#75582

@swift-ci Please test Linux

@ahoppen ahoppen merged commit 829e487 into swiftlang:main Aug 1, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahoppen ahoppen ahoppen approved these changes

@bnbarham bnbarham Awaiting requested review from bnbarham bnbarham is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Introduce FixIt.Change API that replaces the child of a syntax node
3 participants
@AppAppWorks @ahoppen @bnbarham