-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Only omit optional parentheses for starting or ending with parentheses #8238
Only omit optional parentheses for starting or ending with parentheses #8238
Conversation
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
84532b9
to
2b676e8
Compare
.first | ||
.expression() | ||
.is_some_and(|first| is_parenthesized(first, context)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to write this as:
if first_condition {
return true;
}
if second_condition {
return true;
}
false
But it's extremely personal 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
@@ -670,6 +674,7 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { | |||
if op.is_invert() { | |||
self.update_max_precedence(OperatorPrecedence::BitwiseInversion); | |||
} | |||
self.first.set_if_none(First::Token); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct for all unary operators?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All unary operators come before the operand, so I think yes. Like +(a, b, c)
doesn't start with a (
but with +
Summary
This PR fixes a black deviation where Ruff omitted parentheses for expressions that don't end with a parenthesized expression
but the "first" node was parenthesized:
Ruff ommitted parentheses for this expression because it assumed that the expression starts with
(aaaaa...(bbbb, ccc)
which is parenthesized.However, this is incorrect. The expression starts with the
not
keyword which isn't a parentheses and Ruff should thus add parentheses.Closes #8183
Test Plan
Added tests.
PR
main