Skip to content
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

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Oct 26, 2023

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:

def foo():
    while (
        not (aaaaaaaaaaaaaaaaaaaaa(bbbbbbbb, ccccccc)) and dddddddddd < eeeeeeeeeeeeeee
    ):
        pass

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

project similarity index total files changed files
cpython 0.75803 1799 1648
django 0.99984 2772 33
home-assistant 0.99955 10596 178
poetry 0.99891 317 17
transformers 0.99967 2657 328
twine 1.00000 33 0
typeshed 0.99978 3669 20
warehouse 0.99977 654 13
zulip 0.99970 1459 22

main

project similarity index total files changed files
cpython 0.75803 1799 1648
django 0.99983 2772 34
home-assistant 0.99953 10596 186
poetry 0.99891 317 17
transformers 0.99966 2657 330
twine 1.00000 33 0
typeshed 0.99978 3669 20
warehouse 0.99977 654 13
zulip 0.99970 1459 22

@MichaReiser
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser added the formatter Related to the formatter label Oct 26, 2023
@MichaReiser MichaReiser marked this pull request as ready for review October 26, 2023 03:37
@MichaReiser MichaReiser force-pushed the 10-26-Fix_parenthesizing_of_expression_starting_with_unary_expressions branch from 84532b9 to 2b676e8 Compare October 26, 2023 03:38
.first
.expression()
.is_some_and(|first| is_parenthesized(first, context))
}
Copy link
Member

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 😂

Copy link
Member

@charliermarsh charliermarsh left a 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);
Copy link
Member

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?

Copy link
Member Author

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 +

@MichaReiser MichaReiser changed the title Fix parenthesizing of expression starting with unary expressions Only omit optional parentheses for starting or ending with parentheses Oct 26, 2023
@MichaReiser MichaReiser merged commit f5e8507 into main Oct 26, 2023
17 checks passed
@MichaReiser MichaReiser deleted the 10-26-Fix_parenthesizing_of_expression_starting_with_unary_expressions branch October 26, 2023 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatter undocumented deviation: Redundant parentheses in boolean expression can lead to extra line breaks
2 participants