From f1c960c19fcec0828bef00f97731211b35abcbf2 Mon Sep 17 00:00:00 2001 From: jpy-git Date: Tue, 22 Mar 2022 00:39:42 +0000 Subject: [PATCH 1/3] Remove redundant parens from tuples in for loop --- src/black/linegen.py | 20 ++++++++++++++++---- src/black/trans.py | 12 ++++++------ tests/data/long_strings__regression.py | 2 +- tests/data/remove_for_brackets.py | 14 ++++++++++++++ tests/test_format.py | 1 + 5 files changed, 38 insertions(+), 11 deletions(-) create mode 100644 tests/data/remove_for_brackets.py diff --git a/src/black/linegen.py b/src/black/linegen.py index 79475a83f0e..9124e00f50c 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -836,7 +836,9 @@ 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) @@ -860,7 +862,7 @@ 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) -> 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. @@ -868,13 +870,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 @@ -897,7 +909,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/trans.py b/src/black/trans.py index 74d052fe2dc..01aa80eaaf8 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -365,7 +365,7 @@ def do_match(self, line: Line) -> TMatchResult: 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 @@ def make_naked(string: str, string_prefix: str) -> str: # 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 @@ def do_match(self, line: Line) -> TMatchResult: 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 @@ def _assert_match(LL: List[Leaf]) -> Optional[int]: 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 @@ def _assign_match(LL: List[Leaf]) -> Optional[int]: ): 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 @@ def _dict_match(LL: List[Leaf]) -> Optional[int]: 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 36f323e04d6..58ccc4ac0b1 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -599,7 +599,7 @@ def foo(): 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 00000000000..c52897929c0 --- /dev/null +++ b/tests/data/remove_for_brackets.py @@ -0,0 +1,14 @@ +for (k, v) in d.items(): + print(k, v) + +for module in (core, _unicodefun): + if hasattr(module, "_verify_python3_env"): + module._verify_python3_env = lambda: None + +# output +for k, v in d.items(): + print(k, v) + +for module in (core, _unicodefun): + if hasattr(module, "_verify_python3_env"): + module._verify_python3_env = lambda: None diff --git a/tests/test_format.py b/tests/test_format.py index 667d5c110fa..b93df9a1033 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -80,6 +80,7 @@ "long_strings__edge_case", "long_strings__regression", "percent_precedence", + "remove_for_brackets", ] SOURCES: List[str] = [ From 3fe040c23f08a30c3cf162ed38b13ecce62caa02 Mon Sep 17 00:00:00 2001 From: jpy-git Date: Tue, 22 Mar 2022 10:23:49 +0000 Subject: [PATCH 2/3] Tidy up --- CHANGES.md | 1 + src/black/linegen.py | 10 ++++++++-- src/black/mode.py | 5 ++--- tests/data/remove_for_brackets.py | 4 ++++ 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d0faf7cecb9..3a9c0dd17ed 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) ### _Blackd_ diff --git a/src/black/linegen.py b/src/black/linegen.py index 9124e00f50c..af5577c814c 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -837,7 +837,9 @@ def normalize_invisible_parens( if check_lpar: if child.type == syms.atom: if maybe_make_parens_invisible_in_atom( - child, parent=node, preview=preview + child, + parent=node, + preview=preview, ): wrap_in_parentheses(node, child, visible=False) elif is_one_tuple(child): @@ -862,7 +864,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, preview: bool) -> 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. diff --git a/src/black/mode.py b/src/black/mode.py index 35a072c23e0..3c3d94de186 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() class Deprecated(UserWarning): @@ -162,9 +163,7 @@ def __contains__(self, feature: Preview) -> bool: """ if feature is Preview.string_processing: return self.preview or self.experimental_string_processing - # TODO: Remove type ignore comment once preview contains more features - # than just ESP - return self.preview # type: ignore + return self.preview def get_cache_key(self) -> str: if self.target_versions: diff --git a/tests/data/remove_for_brackets.py b/tests/data/remove_for_brackets.py index c52897929c0..a1d9b10f2ef 100644 --- a/tests/data/remove_for_brackets.py +++ b/tests/data/remove_for_brackets.py @@ -1,14 +1,18 @@ +# 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 # 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 From 62c55b732e16468bb77620a97925e980ad1d542a Mon Sep 17 00:00:00 2001 From: jpy-git Date: Thu, 24 Mar 2022 09:32:45 +0000 Subject: [PATCH 3/3] Add long line tests --- tests/data/remove_for_brackets.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/data/remove_for_brackets.py b/tests/data/remove_for_brackets.py index a1d9b10f2ef..c8d88abcc50 100644 --- a/tests/data/remove_for_brackets.py +++ b/tests/data/remove_for_brackets.py @@ -7,6 +7,13 @@ 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(): @@ -16,3 +23,18 @@ 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)