From 14e5ce5412efa53438df0180e735b3834df3b579 Mon Sep 17 00:00:00 2001 From: Joe Young <80432516+jpy-git@users.noreply.github.com> Date: Thu, 24 Mar 2022 14:59:54 +0000 Subject: [PATCH] Remove unnecessary parentheses from tuple unpacking in `for` loops (#2945) --- CHANGES.md | 1 + src/black/linegen.py | 26 ++++++++++++++--- src/black/mode.py | 1 + src/black/trans.py | 12 ++++---- tests/data/long_strings__regression.py | 2 +- tests/data/remove_for_brackets.py | 40 ++++++++++++++++++++++++++ tests/test_format.py | 1 + 7 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 tests/data/remove_for_brackets.py diff --git a/CHANGES.md b/CHANGES.md index d753a24..b2325b6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,6 +15,7 @@ - Code cell separators `#%%` are now standardised to `# %%` (#2919) +- Remove unnecessary parentheses from tuple unpacking in `for` loops (#2945) - Avoid magic-trailing-comma in single-element subscripts (#2942) ### _Blackd_ diff --git a/src/black/linegen.py b/src/black/linegen.py index 5d92011..cb605ee 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -841,7 +841,11 @@ def normalize_invisible_parens( if check_lpar: if child.type == syms.atom: - if maybe_make_parens_invisible_in_atom(child, parent=node): + if maybe_make_parens_invisible_in_atom( + child, + parent=node, + preview=preview, + ): wrap_in_parentheses(node, child, visible=False) elif is_one_tuple(child): wrap_in_parentheses(node, child, visible=True) @@ -865,7 +869,11 @@ def normalize_invisible_parens( check_lpar = isinstance(child, Leaf) and child.value in parens_after -def maybe_make_parens_invisible_in_atom(node: LN, parent: LN) -> bool: +def maybe_make_parens_invisible_in_atom( + node: LN, + parent: LN, + preview: 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` as they are redundant. @@ -873,13 +881,23 @@ def maybe_make_parens_invisible_in_atom(node: LN, parent: LN) -> bool: Returns whether the node should itself be wrapped in invisible parentheses. """ + if ( + preview + and parent.type == syms.for_stmt + and isinstance(node.prev_sibling, Leaf) + and node.prev_sibling.type == token.NAME + and node.prev_sibling.value == "for" + ): + for_stmt_check = False + else: + for_stmt_check = True if ( node.type != syms.atom or is_empty_tuple(node) or is_one_tuple(node) or (is_yield(node) and parent.type != syms.expr_stmt) - or max_delimiter_priority_in_atom(node) >= COMMA_PRIORITY + or (max_delimiter_priority_in_atom(node) >= COMMA_PRIORITY and for_stmt_check) ): return False @@ -902,7 +920,7 @@ def maybe_make_parens_invisible_in_atom(node: LN, parent: LN) -> bool: # make parentheses invisible first.value = "" last.value = "" - maybe_make_parens_invisible_in_atom(middle, parent=parent) + maybe_make_parens_invisible_in_atom(middle, parent=parent, preview=preview) if is_atom_with_invisible_parens(middle): # Strip the invisible parens from `middle` by replacing diff --git a/src/black/mode.py b/src/black/mode.py index 77b1cab..6b74c14 100644 --- a/src/black/mode.py +++ b/src/black/mode.py @@ -127,6 +127,7 @@ class Preview(Enum): """Individual preview style features.""" string_processing = auto() + remove_redundant_parens = auto() one_element_subscript = auto() diff --git a/src/black/trans.py b/src/black/trans.py index 74d052f..01aa80e 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -365,7 +365,7 @@ class StringMerger(StringTransformer, CustomSplitMapMixin): is_valid_index = is_valid_index_factory(LL) - for (i, leaf) in enumerate(LL): + for i, leaf in enumerate(LL): if ( leaf.type == token.STRING and is_valid_index(i + 1) @@ -570,7 +570,7 @@ class StringMerger(StringTransformer, CustomSplitMapMixin): # Build the final line ('new_line') that this method will later return. new_line = line.clone() - for (i, leaf) in enumerate(LL): + for i, leaf in enumerate(LL): if i == string_idx: new_line.append(string_leaf) @@ -691,7 +691,7 @@ class StringParenStripper(StringTransformer): is_valid_index = is_valid_index_factory(LL) - for (idx, leaf) in enumerate(LL): + for idx, leaf in enumerate(LL): # Should be a string... if leaf.type != token.STRING: continue @@ -1713,7 +1713,7 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin): if parent_type(LL[0]) == syms.assert_stmt and LL[0].value == "assert": is_valid_index = is_valid_index_factory(LL) - for (i, leaf) in enumerate(LL): + for i, leaf in enumerate(LL): # We MUST find a comma... if leaf.type == token.COMMA: idx = i + 2 if is_empty_par(LL[i + 1]) else i + 1 @@ -1751,7 +1751,7 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin): ): is_valid_index = is_valid_index_factory(LL) - for (i, leaf) in enumerate(LL): + for i, leaf in enumerate(LL): # We MUST find either an '=' or '+=' symbol... if leaf.type in [token.EQUAL, token.PLUSEQUAL]: idx = i + 2 if is_empty_par(LL[i + 1]) else i + 1 @@ -1794,7 +1794,7 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin): if syms.dictsetmaker in [parent_type(LL[0]), parent_type(LL[0].parent)]: is_valid_index = is_valid_index_factory(LL) - for (i, leaf) in enumerate(LL): + for i, leaf in enumerate(LL): # We MUST find a colon... if leaf.type == token.COLON: idx = i + 2 if is_empty_par(LL[i + 1]) else i + 1 diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py index 36f323e..58ccc4a 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -599,7 +599,7 @@ class A: def foo(xxxx): - for (xxx_xxxx, _xxx_xxx, _xxx_xxxxx, xxx_xxxx) in xxxx: + for xxx_xxxx, _xxx_xxx, _xxx_xxxxx, xxx_xxxx in xxxx: for xxx in xxx_xxxx: assert ("x" in xxx) or (xxx in xxx_xxx_xxxxx), ( "{0} xxxxxxx xx {1}, xxx {1} xx xxx xx xxxx xx xxx xxxx: xxx xxxx {2}" diff --git a/tests/data/remove_for_brackets.py b/tests/data/remove_for_brackets.py new file mode 100644 index 0000000..c8d88ab --- /dev/null +++ b/tests/data/remove_for_brackets.py @@ -0,0 +1,40 @@ +# Only remove tuple brackets after `for` +for (k, v) in d.items(): + print(k, v) + +# Don't touch tuple brackets after `in` +for module in (core, _unicodefun): + if hasattr(module, "_verify_python3_env"): + module._verify_python3_env = lambda: None + +# Brackets remain for long for loop lines +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) + +for (k, v) in dfkasdjfldsjflkdsjflkdsjfdslkfjldsjfgkjdshgkljjdsfldgkhsdofudsfudsofajdslkfjdslkfjldisfjdffjsdlkfjdlkjjkdflskadjldkfjsalkfjdasj.items(): + print(k, v) + +# output +# Only remove tuple brackets after `for` +for k, v in d.items(): + print(k, v) + +# Don't touch tuple brackets after `in` +for module in (core, _unicodefun): + if hasattr(module, "_verify_python3_env"): + module._verify_python3_env = lambda: None + +# Brackets remain for long for loop lines +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) + +for ( + k, + v, +) in ( + dfkasdjfldsjflkdsjflkdsjfdslkfjldsjfgkjdshgkljjdsfldgkhsdofudsfudsofajdslkfjdslkfjldisfjdffjsdlkfjdlkjjkdflskadjldkfjsalkfjdasj.items() +): + print(k, v) diff --git a/tests/test_format.py b/tests/test_format.py index 4de3170..b744685 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -80,6 +80,7 @@ PREVIEW_CASES: List[str] = [ "long_strings__edge_case", "long_strings__regression", "percent_precedence", + "remove_for_brackets", "one_element_subscript", ] -- 2.39.5