Skip to content

Commit

Permalink
Fix arithmetic stability issue (#2817)
Browse files Browse the repository at this point in the history
It turns out "simple_stmt" isn't that simple: it can contain multiple
statements separated by semicolons. Invisible parenthesis logic for
arithmetic expressions only looked at the first child of simple_stmt.
This causes instability in the presence of semicolons, since the next
run through the statement following the semicolon will be the first
child of another simple_stmt.

I believe this along with #2572 fix the known stability issues.
  • Loading branch information
hauntsaninja committed Jan 29, 2022
1 parent df0aeee commit 95e77cb
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Expand Up @@ -24,6 +24,7 @@ and the first release covered by our new stability policy.
- Don't add whitespace for attribute access on hexadecimal, binary, octal, and complex
literals (#2799)
- Treat blank lines in stubs the same inside top-level `if` statements (#2820)
- Fix unstable formatting with semicolons and arithmetic expressions (#2817)

### Parser

Expand Down
10 changes: 7 additions & 3 deletions src/black/linegen.py
Expand Up @@ -7,7 +7,7 @@

from black.nodes import WHITESPACE, RARROW, STATEMENT, STANDALONE_COMMENT
from black.nodes import ASSIGNMENTS, OPENING_BRACKETS, CLOSING_BRACKETS
from black.nodes import Visitor, syms, first_child_is_arith, ensure_visible
from black.nodes import Visitor, syms, is_arith_like, ensure_visible
from black.nodes import is_docstring, is_empty_tuple, is_one_tuple, is_one_tuple_between
from black.nodes import is_name_token, is_lpar_token, is_rpar_token
from black.nodes import is_walrus_assignment, is_yield, is_vararg, is_multiline_string
Expand Down Expand Up @@ -156,8 +156,12 @@ def visit_suite(self, node: Node) -> Iterator[Line]:

def visit_simple_stmt(self, node: Node) -> Iterator[Line]:
"""Visit a statement without nested statements."""
if first_child_is_arith(node):
wrap_in_parentheses(node, node.children[0], visible=False)
prev_type: Optional[int] = None
for child in node.children:
if (prev_type is None or prev_type == token.SEMI) and is_arith_like(child):
wrap_in_parentheses(node, child, visible=False)
prev_type = child.type

is_suite_like = node.parent and node.parent.type in STATEMENT
if is_suite_like:
if self.mode.is_pyi and is_stub_body(node):
Expand Down
7 changes: 3 additions & 4 deletions src/black/nodes.py
Expand Up @@ -531,15 +531,14 @@ def first_leaf_column(node: Node) -> Optional[int]:
return None


def first_child_is_arith(node: Node) -> bool:
"""Whether first child is an arithmetic or a binary arithmetic expression"""
expr_types = {
def is_arith_like(node: LN) -> bool:
"""Whether node is an arithmetic or a binary arithmetic expression"""
return node.type in {
syms.arith_expr,
syms.shift_expr,
syms.xor_expr,
syms.and_expr,
}
return bool(node.children and node.children[0].type in expr_types)


def is_docstring(leaf: Leaf) -> bool:
Expand Down

0 comments on commit 95e77cb

Please sign in to comment.