Fix parenthesization of sequence expressions in callee and function params #1566
+94
−72
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
devongovett
commented
Apr 11, 2021
| @@ -389,6 +396,13 @@ impl VisitMut for Fixer<'_> { | |||
| } | |||
| } | |||
|
|
|||
| fn visit_mut_stmt(&mut self, s: &mut Stmt) { | |||
There was a problem hiding this comment.
This fixes a broken test once I added Context::Callee to the list of contexts to wrap with parens around a sequence expression. I think a previous statement was setting the context and it wasn't being reset when moving on to the next statement.
There was a problem hiding this comment.
Seems like a reasonable fix to me.
kdy1
approved these changes
Apr 12, 2021
| @@ -389,6 +396,13 @@ impl VisitMut for Fixer<'_> { | |||
| } | |||
| } | |||
|
|
|||
| fn visit_mut_stmt(&mut self, s: &mut Stmt) { | |||
There was a problem hiding this comment.
Seems like a reasonable fix to me.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an issue where parentheses would be removed around sequence expressions when they shouldn't, specifically in the case of a call expression callee, and a default value for a function param.
For example:
was compiling to:
which is a syntax error.
A similar issue was seen with default assignment patterns in parameters. The fix was to ensure the correct context is set when visiting a sequence expression so that parentheses are added when needed. I checked a bunch of other places where commas are allowed and these were the ones I found, but there could be more.