Skip to content

Only omit optional parentheses for starting or ending with parentheses#8238

Merged
MichaReiser merged 2 commits intomainfrom
10-26-Fix_parenthesizing_of_expression_starting_with_unary_expressions
Oct 26, 2023
Merged

Only omit optional parentheses for starting or ending with parentheses#8238
MichaReiser merged 2 commits intomainfrom
10-26-Fix_parenthesizing_of_expression_starting_with_unary_expressions

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Oct 26, 2023 •
edited
Loading

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

MichaReiser commented Oct 26, 2023

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
charliermarsh
charliermarsh reviewed Oct 26, 2023
View reviewed changes
crates/ruff_python_formatter/src/expression/mod.rs
.first
.expression()
.is_some_and(|first| is_parenthesized(first, context))
}
Copy link
Member

@charliermarsh charliermarsh Oct 26, 2023

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 😂

charliermarsh
charliermarsh approved these changes Oct 26, 2023
View reviewed changes
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.

charliermarsh
charliermarsh reviewed Oct 26, 2023
View reviewed changes
crates/ruff_python_formatter/src/expression/mod.rs
if op.is_invert() {
self.update_max_precedence(OperatorPrecedence::BitwiseInversion);
}
self.first.set_if_none(First::Token);
Copy link
Member

@charliermarsh charliermarsh Oct 26, 2023 •
edited
Loading

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

@MichaReiser MichaReiser Oct 26, 2023 •
edited
Loading

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
@MichaReiser MichaReiser deleted the 10-26-Fix_parenthesizing_of_expression_starting_with_unary_expressions branch October 26, 2023 06:28
@p-linnane p-linnane mentioned this pull request Oct 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees

No one assigned

Labels

formatter Related to the formatter

Projects

None yet

Milestone

No milestone

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

@MichaReiser @charliermarsh