From 3370b68ffcd482755ee6be415889f59a51062da6 Mon Sep 17 00:00:00 2001 From: Richard Si <63936253+ichard26@users.noreply.github.com> Date: Sun, 28 Aug 2022 15:53:06 -0400 Subject: [PATCH 1/3] Remove even more unnecessary for target parentheses What remains is removing unnecessary brackets because yes, you can do this: for [x, y] in points: print("this is valid code!!!") I'm leaving this as a future task because it turns out simply converting these square brackets into parentheses is not an AST-safe transformation. And`maybe_make_parens_invisible_in_atom` is currently not designed to handle square brackets and initial attemps to add a boolean `treat_square_brackets_as_parens` parameter were unsuccessful. --- CHANGES.md | 2 + src/black/linegen.py | 32 +++++++-- tests/data/preview/remove_for_brackets.py | 83 +++++++++++++++++++++++ 3 files changed, 110 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 34c54710775..755d89b3806 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -28,6 +28,8 @@ subscript expressions with more than 1 element (#3209) - Fix a string merging/split issue when a comment is present in the middle of implicitly concatenated strings on its own line (#3227) +- All unnecessary parentheses in `for` assignments are now removed, previously certain + redundant parentheses were still kept (#3243) ### _Blackd_ diff --git a/src/black/linegen.py b/src/black/linegen.py index a2e41bf5912..28303a2bf11 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -951,18 +951,14 @@ def normalize_invisible_parens( if check_lpar: if ( preview - and child.type == syms.atom + and isinstance(child, Node) + and child.type in (syms.atom, syms.exprlist) and node.type == syms.for_stmt and isinstance(child.prev_sibling, Leaf) and child.prev_sibling.type == token.NAME and child.prev_sibling.value == "for" ): - if maybe_make_parens_invisible_in_atom( - child, - parent=node, - remove_brackets_around_comma=True, - ): - wrap_in_parentheses(node, child, visible=False) + remove_for_target_parens(child, node) elif preview and isinstance(child, Node) and node.type == syms.with_stmt: remove_with_parens(child, node) elif child.type == syms.atom: @@ -1042,6 +1038,28 @@ def remove_await_parens(node: Node) -> None: remove_await_parens(bracket_contents) +def remove_for_target_parens(node: Node, parent: Node) -> None: + """Recursively hide optional parens in `for` statements.""" + # The main goal is to run `maybe_make_parens_invisible_in_atom` on every atom Node + # between "for" and "in". + if node.type == syms.atom: + # Parenthesized group of nodes/leaves, eg. `(x, y)` + # First try removing the group's surrounding parentheses. + if maybe_make_parens_invisible_in_atom( + node, parent, remove_brackets_around_comma=(parent.type == syms.for_stmt) + ): + wrap_in_parentheses(parent, node, visible=False) + # Then check if this atom could contain more atoms. + middle = node.children[1] + if isinstance(middle, Node): + remove_for_target_parens(middle, node) + elif node.type in (syms.exprlist, syms.testlist_gexp): + # A series of nodes/leaves separated by commas, eg. `(x), (y)` + for c in node.children: + if isinstance(c, Node) and c.type == syms.atom: + remove_for_target_parens(c, node) + + def remove_with_parens(node: Node, parent: Node) -> None: """Recursively hide optional parens in `with` statements.""" # Removing all unnecessary parentheses in with statements in one pass is a tad diff --git a/tests/data/preview/remove_for_brackets.py b/tests/data/preview/remove_for_brackets.py index cd5340462da..0b2655852de 100644 --- a/tests/data/preview/remove_for_brackets.py +++ b/tests/data/preview/remove_for_brackets.py @@ -1,3 +1,6 @@ +for (((x))) in points: + pass + # Only remove tuple brackets after `for` for (k, v) in d.items(): print(k, v) @@ -18,7 +21,47 @@ for (((((k, v))))) in d.items(): print(k, v) +# One-tuple +for (x,) in points: + pass + +# A series of atoms that each need their brackets removed +for (x), (y) in points: + pass + +# A mix of "simple" atoms and atoms that contain more atoms +for ((x), (y)) in points: + pass + +for ((((x)), (((y))))) in points: + pass + +# Mixed again; some of these brackets matter. +for ((x), (y)), z in points: + pass + +for ((x, y), z) in points: + pass + +for ((x, (((y)))), (((z)))) in points: + pass + +for (((x,), (y)), ((z)),) in points: + pass + +for ((a, b)), ((c, d)) in points: + pass + +for ((((a), (b))), (((c), (d)))) in points: + pass + +for (a, (b, (c, (d)))) in points: + pass + # output +for x in points: + pass + # Only remove tuple brackets after `for` for k, v in d.items(): print(k, v) @@ -46,3 +89,43 @@ # Test deeply nested brackets for k, v in d.items(): print(k, v) + +# One-tuple +for (x,) in points: + pass + +# A series of atoms that each need their brackets removed +for x, y in points: + pass + +# A mix of "simple" atoms and atoms that contain more atoms +for x, y in points: + pass + +for x, y in points: + pass + +# Mixed again; some of these brackets matter. +for (x, y), z in points: + pass + +for (x, y), z in points: + pass + +for (x, y), z in points: + pass + +for ( + ((x,), y), + z, +) in points: + pass + +for (a, b), (c, d) in points: + pass + +for (a, b), (c, d) in points: + pass + +for a, (b, (c, d)) in points: + pass From 31d0d26fc37fdf3e4234e50701f30b96b871514b Mon Sep 17 00:00:00 2001 From: Richard Si <63936253+ichard26@users.noreply.github.com> Date: Sun, 28 Aug 2022 16:17:44 -0400 Subject: [PATCH 2/3] Minor rename for clarity --- src/black/linegen.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 28303a2bf11..40090397693 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -190,7 +190,7 @@ def visit_funcdef(self, node: Node) -> Iterator[Line]: if maybe_make_parens_invisible_in_atom( child, parent=node, - remove_brackets_around_comma=False, + remove_parens_around_comma=False, ): wrap_in_parentheses(node, child, visible=False) else: @@ -1011,7 +1011,7 @@ def remove_await_parens(node: Node) -> None: if maybe_make_parens_invisible_in_atom( node.children[1], parent=node, - remove_brackets_around_comma=True, + remove_parens_around_comma=True, ): wrap_in_parentheses(node, node.children[1], visible=False) @@ -1046,7 +1046,7 @@ def remove_for_target_parens(node: Node, parent: Node) -> None: # Parenthesized group of nodes/leaves, eg. `(x, y)` # First try removing the group's surrounding parentheses. if maybe_make_parens_invisible_in_atom( - node, parent, remove_brackets_around_comma=(parent.type == syms.for_stmt) + node, parent, remove_parens_around_comma=(parent.type == syms.for_stmt) ): wrap_in_parentheses(parent, node, visible=False) # Then check if this atom could contain more atoms. @@ -1082,7 +1082,7 @@ def remove_with_parens(node: Node, parent: Node) -> None: if maybe_make_parens_invisible_in_atom( node, parent=parent, - remove_brackets_around_comma=True, + remove_parens_around_comma=True, ): wrap_in_parentheses(parent, node, visible=False) if isinstance(node.children[1], Node): @@ -1097,7 +1097,7 @@ def remove_with_parens(node: Node, parent: Node) -> None: if maybe_make_parens_invisible_in_atom( node.children[0], parent=node, - remove_brackets_around_comma=True, + remove_parens_around_comma=True, ): wrap_in_parentheses(node, node.children[0], visible=False) @@ -1105,7 +1105,8 @@ def remove_with_parens(node: Node, parent: Node) -> None: def maybe_make_parens_invisible_in_atom( node: LN, parent: LN, - remove_brackets_around_comma: bool = False, + *, + remove_parens_around_comma: bool = False, ) -> bool: """If it's safe, make the parens in the atom `node` invisible, recursively. Additionally, remove repeated, adjacent invisible parens from the atom `node` @@ -1119,10 +1120,10 @@ def maybe_make_parens_invisible_in_atom( or is_one_tuple(node) or (is_yield(node) and parent.type != syms.expr_stmt) or ( - # This condition tries to prevent removing non-optional brackets + # This condition tries to prevent removing non-optional parentheses # around a tuple, however, can be a bit overzealous so we provide # and option to skip this check for `for` and `with` statements. - not remove_brackets_around_comma + not remove_parens_around_comma and max_delimiter_priority_in_atom(node) >= COMMA_PRIORITY ) ): @@ -1150,7 +1151,7 @@ def maybe_make_parens_invisible_in_atom( maybe_make_parens_invisible_in_atom( middle, parent=parent, - remove_brackets_around_comma=remove_brackets_around_comma, + remove_parens_around_comma=remove_parens_around_comma, ) if is_atom_with_invisible_parens(middle): From 01dbe47e682c5575cb208ea2ffeb6c6f9244e266 Mon Sep 17 00:00:00 2001 From: Richard Si <63936253+ichard26@users.noreply.github.com> Date: Mon, 29 Aug 2022 18:28:56 -0400 Subject: [PATCH 3/3] Add line too long test cases --- tests/data/preview/remove_for_brackets.py | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/data/preview/remove_for_brackets.py b/tests/data/preview/remove_for_brackets.py index 0b2655852de..64dba3296e2 100644 --- a/tests/data/preview/remove_for_brackets.py +++ b/tests/data/preview/remove_for_brackets.py @@ -11,6 +11,15 @@ module._verify_python3_env = lambda: None # Brackets remain for long for loop lines +for (one_super_long_name_as_the_for_target_list_because_why_not_you_dont_control_me) in points: + pass + +for (one_super_long_name_as_the_for_target_list_because_why_not_you_dont_control_me_i_have_total_freedom) in points: + pass + +for ((many_long_name_tuples, many_long_name_tuples), (many_long_name_tuples, many_long_name_tuples), (many_long_name_tuples, many_long_name_tuples)) in points: + pass + for (why_would_anyone_choose_to_name_a_loop_variable_with_a_name_this_long, i_dont_know_but_we_should_still_check_the_behaviour_if_they_do) in d.items(): print(k, v) @@ -72,6 +81,21 @@ module._verify_python3_env = lambda: None # Brackets remain for long for loop lines +for ( + one_super_long_name_as_the_for_target_list_because_why_not_you_dont_control_me +) in points: + pass + +for one_super_long_name_as_the_for_target_list_because_why_not_you_dont_control_me_i_have_total_freedom in (points): + pass + +for ( + (many_long_name_tuples, many_long_name_tuples), + (many_long_name_tuples, many_long_name_tuples), + (many_long_name_tuples, many_long_name_tuples), +) in points: + pass + for ( why_would_anyone_choose_to_name_a_loop_variable_with_a_name_this_long, i_dont_know_but_we_should_still_check_the_behaviour_if_they_do,