Skip to content

[5.9] BasicFormat should not format unexpected nodes #1829

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 3 commits into from
Jun 22, 2023

Conversation

bnbarham
Copy link
Contributor

  • Explanation: Updates BasicFormat to not format unexpected nodes, which has been a constant source of new issues for swift-syntax. Also included here is Kim's patch to use HEAD For CodeGeneration + re-running it (there were a number of conflicts in the generated code and re-running codegen is far simpler than trying to fix them).
  • Scope: Macro expansion formatting
  • Risk: Low
  • Testing: Added a test case for the unexpected case
  • Original PR: BasicFormat should not format unexpected nodes #1759

kimdv and others added 3 commits June 21, 2023 17:48
`BasicFormat` depends on the parent layout nodes to determine formatting
in some cases, so it doesn't make sense to visit unexpected nodes.

Also fix up `SyntaxRewriter` to take a `viewMode`, since as it was
`BasicFormat` was actually visiting `missing` nodes, which also doesn't
make much sense.

Resolves rdar://110463876.

(cherry picked from commit 9227d5b)
release/5.9 was missing the CodeGeneration update to use HEAD and thus
many of the generated files are out of date. Update them.
@bnbarham
Copy link
Contributor Author

@swift-ci please test

@bnbarham
Copy link
Contributor Author

@ahoppen I've cherry-picked the HEAD change for CodeGeneration here. I didn't notice it in the refactoring change for some reason, even though I did re-generate there as well 🤔

@bnbarham bnbarham merged commit 312c303 into swiftlang:release/5.9 Jun 22, 2023
@bnbarham bnbarham deleted the cherry-macro-formatting branch June 22, 2023 18:15
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.

3 participants