From fa0b64d2c1882b3c2a24dacb369600a1ac48e163 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 01:15:46 +0000 Subject: [PATCH 01/96] Add illustrative test data --- tests/data/invisible_parens_instability.py | 1 + tests/test_black.py | 6 ++++++ 2 files changed, 7 insertions(+) create mode 100644 tests/data/invisible_parens_instability.py diff --git a/tests/data/invisible_parens_instability.py b/tests/data/invisible_parens_instability.py new file mode 100644 index 00000000000..d73379996ea --- /dev/null +++ b/tests/data/invisible_parens_instability.py @@ -0,0 +1 @@ +assert function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15) != [None] diff --git a/tests/test_black.py b/tests/test_black.py index 28b75787bd6..b422d53e284 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -233,6 +233,12 @@ def _test_wip(self) -> None: black.assert_equivalent(source, actual) black.assert_stable(source, actual, black.FileMode()) + @patch("black.dump_to_file", dump_to_stderr) + def test_invisible_parens_instability(self) -> None: + source, _expected = read_data("invisible_parens_instability") + actual = fs(source) + black.assert_stable(source, actual, DEFAULT_MODE) + @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability1(self) -> None: From 21842f22573d8da956faae01835fbf6e6ea26ae0 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 01:02:23 +0000 Subject: [PATCH 02/96] Fixup: handling of invisible parentheses for assert and return statements --- src/black/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 9034bf6cf77..bc0d3d1b481 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5381,6 +5381,9 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break + elif node.type in [syms.assert_stmt, syms.return_stmt]: + if maybe_make_parens_invisible_in_atom(child, parent=node): + wrap_in_parentheses(node, child, visible=False) elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) From fdff0840f9fa67aebedda6b42bd1bd99488bdb2d Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 01:12:28 +0000 Subject: [PATCH 03/96] Re-enable test_trailing_comma_optional_parens_stability3 since it has stabilized --- tests/test_black.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_black.py b/tests/test_black.py index b422d53e284..cc116939972 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -253,7 +253,6 @@ def test_trailing_comma_optional_parens_stability2(self) -> None: actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) - @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability3(self) -> None: source, _expected = read_data("trailing_comma_optional_parens3") From 0bb718c0ecbecc0143851d1ff19722f8427b9957 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 10:04:33 +0000 Subject: [PATCH 04/96] Use a 'pass' statement since 'maybe_make_parens_invisible_in_atom' currently always returns False for non-atom nodes, making the 'wrap_in_parentheses' call unreachable --- src/black/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index bc0d3d1b481..7fb49fa01dc 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5382,8 +5382,7 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.append_child(Leaf(token.RPAR, "")) break elif node.type in [syms.assert_stmt, syms.return_stmt]: - if maybe_make_parens_invisible_in_atom(child, parent=node): - wrap_in_parentheses(node, child, visible=False) + pass elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) From 82ab09d2c18a00bc38cdcbc7439055524fe703df Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 17:07:45 +0000 Subject: [PATCH 05/96] Do not omit any tokens when selecting the last two leaves --- src/black/__init__.py | 9 +++------ tests/test_black.py | 2 -- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 7fb49fa01dc..e7b3c9b0ed5 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6608,7 +6608,7 @@ def can_omit_invisible_parens( last = line.leaves[-1] if line.should_explode: try: - penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) + penultimate, last = last_two_except(line.leaves) except LookupError: # Turns out we'd omit everything. We cannot skip the optional parentheses. return False @@ -6685,7 +6685,7 @@ def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool return False -def last_two_except(leaves: List[Leaf], omit: Collection[LeafID]) -> Tuple[Leaf, Leaf]: +def last_two_except(leaves: List[Leaf]) -> Tuple[Leaf, Leaf]: """Return (penultimate, last) leaves skipping brackets in `omit` and contents.""" stop_after = None last = None @@ -6698,10 +6698,7 @@ def last_two_except(leaves: List[Leaf], omit: Collection[LeafID]) -> Tuple[Leaf, if last: return leaf, last - if id(leaf) in omit: - stop_after = leaf.opening_bracket - else: - last = leaf + last = leaf else: raise LookupError("Last two leaves were also skipped") diff --git a/tests/test_black.py b/tests/test_black.py index cc116939972..15a4251b415 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -239,14 +239,12 @@ def test_invisible_parens_instability(self) -> None: actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) - @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability1(self) -> None: source, _expected = read_data("trailing_comma_optional_parens1") actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) - @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability2(self) -> None: source, _expected = read_data("trailing_comma_optional_parens2") From f154042291e893685724115a808c01f9bfe9fef4 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 17:08:22 +0000 Subject: [PATCH 06/96] Cleanup: remove slightly arbitrary 'pass' handling for assert and return statements --- src/black/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index e7b3c9b0ed5..07f629675a7 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5381,8 +5381,6 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break - elif node.type in [syms.assert_stmt, syms.return_stmt]: - pass elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) From d262d295baf7ec91e2a7cb6a9ce1bf7621b51b1f Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 17:17:11 +0000 Subject: [PATCH 07/96] Update output expectation for trailing comma reformatting test case --- tests/data/function_trailing_comma.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index d15459cbeb5..a9212b46019 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -23,6 +23,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ # output + def f( a, ): @@ -61,16 +62,19 @@ def f( "a": 1, "b": 2, }["a"] - if a == { - "a": 1, - "b": 2, - "c": 3, - "d": 4, - "e": 5, - "f": 6, - "g": 7, - "h": 8, - }["a"]: + if ( + a + == { + "a": 1, + "b": 2, + "c": 3, + "d": 4, + "e": 5, + "f": 6, + "g": 7, + "h": 8, + }["a"] + ): pass From 23facce645f4c0c9c5b277aa2adc1e5fc6ceb62b Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 21:31:38 +0000 Subject: [PATCH 08/96] Cleanup: remove redundant code path --- src/black/__init__.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 07f629675a7..f58f7d1009e 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6604,12 +6604,6 @@ def can_omit_invisible_parens( penultimate = line.leaves[-2] last = line.leaves[-1] - if line.should_explode: - try: - penultimate, last = last_two_except(line.leaves) - except LookupError: - # Turns out we'd omit everything. We cannot skip the optional parentheses. - return False if ( last.type == token.RPAR @@ -6683,24 +6677,6 @@ def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool return False -def last_two_except(leaves: List[Leaf]) -> Tuple[Leaf, Leaf]: - """Return (penultimate, last) leaves skipping brackets in `omit` and contents.""" - stop_after = None - last = None - for leaf in reversed(leaves): - if stop_after: - if leaf is stop_after: - stop_after = None - continue - - if last: - return leaf, last - - last = leaf - else: - raise LookupError("Last two leaves were also skipped") - - def run_transformer( line: Line, transform: Transformer, From 543fc7eb561213fedf9dd389852f9ec8b10cf5b0 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 22:17:08 +0000 Subject: [PATCH 09/96] Update primer.json to permit formatting changes to be passed by continuous integration --- src/black_primer/primer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index 32df01571a7..ff158ae31e6 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -98,7 +98,7 @@ }, "sqlalchemy": { "cli_arguments": [], - "expect_formatting_changes": false, + "expect_formatting_changes": true, "git_clone_url": "https://github.com/sqlalchemy/sqlalchemy.git", "long_checkout": false, "py_versions": ["all"] From 6c6d26262ee3c791118589a77d30c2f39800d709 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 13:28:26 +0000 Subject: [PATCH 10/96] Temporarily revert src directory changes --- src/black/__init__.py | 27 +++++++++++++++++++++++++++ src/black_primer/primer.json | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index f58f7d1009e..9034bf6cf77 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6604,6 +6604,12 @@ def can_omit_invisible_parens( penultimate = line.leaves[-2] last = line.leaves[-1] + if line.should_explode: + try: + penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) + except LookupError: + # Turns out we'd omit everything. We cannot skip the optional parentheses. + return False if ( last.type == token.RPAR @@ -6677,6 +6683,27 @@ def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool return False +def last_two_except(leaves: List[Leaf], omit: Collection[LeafID]) -> Tuple[Leaf, Leaf]: + """Return (penultimate, last) leaves skipping brackets in `omit` and contents.""" + stop_after = None + last = None + for leaf in reversed(leaves): + if stop_after: + if leaf is stop_after: + stop_after = None + continue + + if last: + return leaf, last + + if id(leaf) in omit: + stop_after = leaf.opening_bracket + else: + last = leaf + else: + raise LookupError("Last two leaves were also skipped") + + def run_transformer( line: Line, transform: Transformer, diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index ff158ae31e6..32df01571a7 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -98,7 +98,7 @@ }, "sqlalchemy": { "cli_arguments": [], - "expect_formatting_changes": true, + "expect_formatting_changes": false, "git_clone_url": "https://github.com/sqlalchemy/sqlalchemy.git", "long_checkout": false, "py_versions": ["all"] From 21528480a85f26c7b8192cb18892be309fe9d31c Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 13:55:41 +0000 Subject: [PATCH 11/96] Add additional test cases from psf/black#1629 These are generally cases where example code was provided by a user and it was straightforward to copy and confirm the stability test failure for that code --- tests/data/invisible_parens_instability.py | 56 ++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/data/invisible_parens_instability.py b/tests/data/invisible_parens_instability.py index d73379996ea..4fbff0e4d28 100644 --- a/tests/data/invisible_parens_instability.py +++ b/tests/data/invisible_parens_instability.py @@ -1 +1,57 @@ +# https://github.com/psf/black/issues/1629#issuecomment-681953135 +assert ( + xxxxxx( + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, + xxxxxxxxxxxxxxxxxxxxxxxxx + ) + == xxxxxx(xxxxxxxxxxx, xxxxxxxxxxxxxxxxxxxxxxxxx) +) + + +# https://github.com/psf/black/issues/1629#issuecomment-688804670 +if any( + k in t + for k in ["AAAAAAAAAA", "AAAAA", "AAAAAA", "AAAAAAAAA", "AAA", "AAAAAA", "AAAAAAAA", "AAA", "AAAAA", "AAAAA", "AAAA"] +) and not any(k in t for k in ["AAA"]): + pass + + +# https://github.com/psf/black/issues/1629#issuecomment-693816208 +aaaaaaaaaaaaaaaaaaaaaaaaaa = bbbbbbbbbbbbbbbbbbbb( # ccccccccccccccccccccccccccccccccccc + d=0 +) + + +# https://github.com/psf/black/issues/1629#issuecomment-699006156 +def f(): + return a( + b( + c(n) + # + ), + [] + ) + d(x, y) + + +# https://github.com/psf/black/issues/1629#issuecomment-704782983 +def test_foo(): + assert foo( + "foo", + ) == [{"raw": {"person": "1"}, "error": "Invalid field unknown", "status": "error"}] + + +# https://github.com/psf/black/issues/1629#issuecomment-707066734 +my_string = '______________ %s _________________ %s _____________________________________________' % ( + '____________', '________________', '___________________________________________') + + +# https://github.com/psf/black/issues/1629#issuecomment-764562642 +class Foo(object): + def bar(self): + x = 'foo ' + \ + 'subscription {} resource_group {} Site {}, foobar_name {}. Error {}'.format( + self._subscription, self._resource_group, self._site, self._foobar_name, error) + + +# https://github.com/psf/black/issues/1629#issuecomment-772691120 assert function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15) != [None] From a23e87dc3e04d0369f5bed7a22893a3995b20d8e Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 13:55:47 +0000 Subject: [PATCH 12/96] Revert "Temporarily revert src directory changes" This reverts commit 6c6d26262ee3c791118589a77d30c2f39800d709. --- src/black/__init__.py | 27 --------------------------- src/black_primer/primer.json | 2 +- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 9034bf6cf77..f58f7d1009e 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6604,12 +6604,6 @@ def can_omit_invisible_parens( penultimate = line.leaves[-2] last = line.leaves[-1] - if line.should_explode: - try: - penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) - except LookupError: - # Turns out we'd omit everything. We cannot skip the optional parentheses. - return False if ( last.type == token.RPAR @@ -6683,27 +6677,6 @@ def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool return False -def last_two_except(leaves: List[Leaf], omit: Collection[LeafID]) -> Tuple[Leaf, Leaf]: - """Return (penultimate, last) leaves skipping brackets in `omit` and contents.""" - stop_after = None - last = None - for leaf in reversed(leaves): - if stop_after: - if leaf is stop_after: - stop_after = None - continue - - if last: - return leaf, last - - if id(leaf) in omit: - stop_after = leaf.opening_bracket - else: - last = leaf - else: - raise LookupError("Last two leaves were also skipped") - - def run_transformer( line: Line, transform: Transformer, diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index 32df01571a7..ff158ae31e6 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -98,7 +98,7 @@ }, "sqlalchemy": { "cli_arguments": [], - "expect_formatting_changes": false, + "expect_formatting_changes": true, "git_clone_url": "https://github.com/sqlalchemy/sqlalchemy.git", "long_checkout": false, "py_versions": ["all"] From 065abbfa96f54229eb9d11604931e3296f0cffc7 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:06:17 +0000 Subject: [PATCH 13/96] Revert "Update primer.json to permit formatting changes to be passed by continuous integration" This reverts commit 543fc7eb561213fedf9dd389852f9ec8b10cf5b0. --- src/black_primer/primer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index ff158ae31e6..32df01571a7 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -98,7 +98,7 @@ }, "sqlalchemy": { "cli_arguments": [], - "expect_formatting_changes": true, + "expect_formatting_changes": false, "git_clone_url": "https://github.com/sqlalchemy/sqlalchemy.git", "long_checkout": false, "py_versions": ["all"] From 10015f6e37a11abd70c7061952677ef4926e32c2 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:06:23 +0000 Subject: [PATCH 14/96] Revert "Cleanup: remove redundant code path" This reverts commit 23facce645f4c0c9c5b277aa2adc1e5fc6ceb62b. --- src/black/__init__.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index f58f7d1009e..07f629675a7 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6604,6 +6604,12 @@ def can_omit_invisible_parens( penultimate = line.leaves[-2] last = line.leaves[-1] + if line.should_explode: + try: + penultimate, last = last_two_except(line.leaves) + except LookupError: + # Turns out we'd omit everything. We cannot skip the optional parentheses. + return False if ( last.type == token.RPAR @@ -6677,6 +6683,24 @@ def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool return False +def last_two_except(leaves: List[Leaf]) -> Tuple[Leaf, Leaf]: + """Return (penultimate, last) leaves skipping brackets in `omit` and contents.""" + stop_after = None + last = None + for leaf in reversed(leaves): + if stop_after: + if leaf is stop_after: + stop_after = None + continue + + if last: + return leaf, last + + last = leaf + else: + raise LookupError("Last two leaves were also skipped") + + def run_transformer( line: Line, transform: Transformer, From 16bf728b6749c3b73709303415c7c857c059ab78 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:06:28 +0000 Subject: [PATCH 15/96] Revert "Update output expectation for trailing comma reformatting test case" This reverts commit d262d295baf7ec91e2a7cb6a9ce1bf7621b51b1f. --- tests/data/function_trailing_comma.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index a9212b46019..d15459cbeb5 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -23,7 +23,6 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ # output - def f( a, ): @@ -62,19 +61,16 @@ def f( "a": 1, "b": 2, }["a"] - if ( - a - == { - "a": 1, - "b": 2, - "c": 3, - "d": 4, - "e": 5, - "f": 6, - "g": 7, - "h": 8, - }["a"] - ): + if a == { + "a": 1, + "b": 2, + "c": 3, + "d": 4, + "e": 5, + "f": 6, + "g": 7, + "h": 8, + }["a"]: pass From f4ee9dcff70c64e41cded70c21dbd782b9e8cd30 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:06:34 +0000 Subject: [PATCH 16/96] Revert "Cleanup: remove slightly arbitrary 'pass' handling for assert and return statements" This reverts commit f154042291e893685724115a808c01f9bfe9fef4. --- src/black/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 07f629675a7..e7b3c9b0ed5 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5381,6 +5381,8 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break + elif node.type in [syms.assert_stmt, syms.return_stmt]: + pass elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) From 4c2947f5932f83afb44bd679171c1479e1ef0c9f Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:06:41 +0000 Subject: [PATCH 17/96] Revert "Do not omit any tokens when selecting the last two leaves" This reverts commit 82ab09d2c18a00bc38cdcbc7439055524fe703df. --- src/black/__init__.py | 9 ++++++--- tests/test_black.py | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index e7b3c9b0ed5..7fb49fa01dc 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6608,7 +6608,7 @@ def can_omit_invisible_parens( last = line.leaves[-1] if line.should_explode: try: - penultimate, last = last_two_except(line.leaves) + penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) except LookupError: # Turns out we'd omit everything. We cannot skip the optional parentheses. return False @@ -6685,7 +6685,7 @@ def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool return False -def last_two_except(leaves: List[Leaf]) -> Tuple[Leaf, Leaf]: +def last_two_except(leaves: List[Leaf], omit: Collection[LeafID]) -> Tuple[Leaf, Leaf]: """Return (penultimate, last) leaves skipping brackets in `omit` and contents.""" stop_after = None last = None @@ -6698,7 +6698,10 @@ def last_two_except(leaves: List[Leaf]) -> Tuple[Leaf, Leaf]: if last: return leaf, last - last = leaf + if id(leaf) in omit: + stop_after = leaf.opening_bracket + else: + last = leaf else: raise LookupError("Last two leaves were also skipped") diff --git a/tests/test_black.py b/tests/test_black.py index 15a4251b415..cc116939972 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -239,12 +239,14 @@ def test_invisible_parens_instability(self) -> None: actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) + @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability1(self) -> None: source, _expected = read_data("trailing_comma_optional_parens1") actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) + @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability2(self) -> None: source, _expected = read_data("trailing_comma_optional_parens2") From 461a828db26a3cb66fbaa4223fd0655845d8e2e4 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:06:49 +0000 Subject: [PATCH 18/96] Revert "Use a 'pass' statement since 'maybe_make_parens_invisible_in_atom' currently always returns False for non-atom nodes, making the 'wrap_in_parentheses' call unreachable" This reverts commit 0bb718c0ecbecc0143851d1ff19722f8427b9957. --- src/black/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 7fb49fa01dc..bc0d3d1b481 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5382,7 +5382,8 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.append_child(Leaf(token.RPAR, "")) break elif node.type in [syms.assert_stmt, syms.return_stmt]: - pass + if maybe_make_parens_invisible_in_atom(child, parent=node): + wrap_in_parentheses(node, child, visible=False) elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) From 07f5796faa8cb06ededd90f9f8a887d8cca7f7ac Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:07:06 +0000 Subject: [PATCH 19/96] Revert "Re-enable test_trailing_comma_optional_parens_stability3 since it has stabilized" This reverts commit fdff0840f9fa67aebedda6b42bd1bd99488bdb2d. --- tests/test_black.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_black.py b/tests/test_black.py index cc116939972..b422d53e284 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -253,6 +253,7 @@ def test_trailing_comma_optional_parens_stability2(self) -> None: actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) + @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability3(self) -> None: source, _expected = read_data("trailing_comma_optional_parens3") From 39caefbd823328fe8a6967b01e51ed03d4dad8fd Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:07:49 +0000 Subject: [PATCH 20/96] Revert "Fixup: handling of invisible parentheses for assert and return statements" This reverts commit 21842f22573d8da956faae01835fbf6e6ea26ae0. --- src/black/__init__.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index bc0d3d1b481..9034bf6cf77 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5381,9 +5381,6 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break - elif node.type in [syms.assert_stmt, syms.return_stmt]: - if maybe_make_parens_invisible_in_atom(child, parent=node): - wrap_in_parentheses(node, child, visible=False) elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) From 7c1482d418cfbbd63cdcbea6b555396fdda93b81 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 14:30:03 +0000 Subject: [PATCH 21/96] Remove example code that raises a Python TypeError during evaluation --- tests/data/invisible_parens_instability.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/data/invisible_parens_instability.py b/tests/data/invisible_parens_instability.py index 4fbff0e4d28..8855761bfb3 100644 --- a/tests/data/invisible_parens_instability.py +++ b/tests/data/invisible_parens_instability.py @@ -40,11 +40,6 @@ def test_foo(): ) == [{"raw": {"person": "1"}, "error": "Invalid field unknown", "status": "error"}] -# https://github.com/psf/black/issues/1629#issuecomment-707066734 -my_string = '______________ %s _________________ %s _____________________________________________' % ( - '____________', '________________', '___________________________________________') - - # https://github.com/psf/black/issues/1629#issuecomment-764562642 class Foo(object): def bar(self): From ab07b039472b3609fa72a853d441f07a2a4071f3 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 15:21:33 +0000 Subject: [PATCH 22/96] Indicate that failures are expected in the 'test_invisible_parens_instability' case until a resolution is found --- tests/test_black.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_black.py b/tests/test_black.py index b422d53e284..68aaa893db7 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -233,6 +233,7 @@ def _test_wip(self) -> None: black.assert_equivalent(source, actual) black.assert_stable(source, actual, black.FileMode()) + @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_invisible_parens_instability(self) -> None: source, _expected = read_data("invisible_parens_instability") From 1109e3307d4e66d2665a45fb58cf6dda9d6b2378 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 17:26:29 +0000 Subject: [PATCH 23/96] Brevity: rename method --- src/black/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 9034bf6cf77..9637c3bf835 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5002,7 +5002,7 @@ def bracket_split_build_line( result.append(leaf, preformatted=True) for comment_after in original.comments_after(leaf): result.append(comment_after, preformatted=True) - if is_body and should_split_body_explode(result, opening_bracket): + if is_body and should_split(result, opening_bracket): result.should_explode = True return result @@ -5785,7 +5785,7 @@ def ensure_visible(leaf: Leaf) -> None: leaf.value = ")" -def should_split_body_explode(line: Line, opening_bracket: Leaf) -> bool: +def should_split(line: Line, opening_bracket: Leaf) -> bool: """Should `line` be immediately split with `delimiter_split()` after RHS?""" if not (opening_bracket.parent and opening_bracket.value in "[{("): From f6eab7c096fd4d8c64a035b81907f8b1f655503b Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 17:30:05 +0000 Subject: [PATCH 24/96] Clarity: isolate and extract each responsibility from an overloaded variable --- src/black/__init__.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 9637c3bf835..138c901b494 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -1480,6 +1480,7 @@ class Line: bracket_tracker: BracketTracker = field(default_factory=BracketTracker) inside_brackets: bool = False should_explode: bool = False + magic_trailing_comma: Optional[Leaf] = None def append(self, leaf: Leaf, preformatted: bool = False) -> None: """Add a new `leaf` to the end of the line. @@ -1507,7 +1508,7 @@ def append(self, leaf: Leaf, preformatted: bool = False) -> None: self.bracket_tracker.mark(leaf) if self.mode.magic_trailing_comma: if self.has_magic_trailing_comma(leaf): - self.should_explode = True + self.magic_trailing_comma = leaf elif self.has_magic_trailing_comma(leaf, ensure_removable=True): self.remove_trailing_comma() if not self.append_comment(leaf): @@ -1791,6 +1792,7 @@ def clone(self) -> "Line": depth=self.depth, inside_brackets=self.inside_brackets, should_explode=self.should_explode, + magic_trailing_comma=self.magic_trailing_comma, ) def __str__(self) -> str: @@ -2707,7 +2709,7 @@ def init_st(ST: Type[StringTransformer]) -> StringTransformer: transformers: List[Transformer] if ( not line.contains_uncollapsable_type_comments() - and not line.should_explode + and not (line.should_explode or line.magic_trailing_comma) and ( is_line_short_enough(line, line_length=mode.line_length, line_str=line_str) or line.contains_unsplittable_type_ignore() @@ -4382,6 +4384,7 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]: depth=line.depth + 1, inside_brackets=True, should_explode=line.should_explode, + magic_trailing_comma=line.magic_trailing_comma, ) string_leaf = Leaf(token.STRING, string_value) insert_str_child(string_leaf) @@ -5921,7 +5924,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf """ omit: Set[LeafID] = set() - if not line.should_explode: + if not line.should_explode and not line.magic_trailing_comma: yield omit length = 4 * line.depth @@ -5943,7 +5946,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf elif leaf.type in CLOSING_BRACKETS: prev = line.leaves[index - 1] if index > 0 else None if ( - line.should_explode + (line.should_explode or line.magic_trailing_comma) and prev and prev.type == token.COMMA and not is_one_tuple_between( @@ -5971,7 +5974,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf yield omit if ( - line.should_explode + (line.should_explode or line.magic_trailing_comma) and prev and prev.type == token.COMMA and not is_one_tuple_between(leaf.opening_bracket, leaf, line.leaves) @@ -6604,7 +6607,7 @@ def can_omit_invisible_parens( penultimate = line.leaves[-2] last = line.leaves[-1] - if line.should_explode: + if line.should_explode or line.magic_trailing_comma: try: penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) except LookupError: @@ -6631,7 +6634,9 @@ def can_omit_invisible_parens( # unnecessary. return True - if line.should_explode and penultimate.type == token.COMMA: + if ( + line.should_explode or line.magic_trailing_comma + ) and penultimate.type == token.COMMA: # The rightmost non-omitted bracket pair is the one we want to explode on. return True From d4a2e3689dfc0d00492938083e0fbc1254de787a Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 17:37:25 +0000 Subject: [PATCH 25/96] Brevity: only use the variables required to convey the intended expressions --- src/black/__init__.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 138c901b494..dde63b4925b 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5946,7 +5946,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf elif leaf.type in CLOSING_BRACKETS: prev = line.leaves[index - 1] if index > 0 else None if ( - (line.should_explode or line.magic_trailing_comma) + line.magic_trailing_comma and prev and prev.type == token.COMMA and not is_one_tuple_between( @@ -5974,7 +5974,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf yield omit if ( - (line.should_explode or line.magic_trailing_comma) + line.magic_trailing_comma and prev and prev.type == token.COMMA and not is_one_tuple_between(leaf.opening_bracket, leaf, line.leaves) @@ -6634,9 +6634,7 @@ def can_omit_invisible_parens( # unnecessary. return True - if ( - line.should_explode or line.magic_trailing_comma - ) and penultimate.type == token.COMMA: + if line.magic_trailing_comma and penultimate.type == token.COMMA: # The rightmost non-omitted bracket pair is the one we want to explode on. return True From ebbd3e8ddbfe994e01f94cfa675faf745c7c141d Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 17:38:55 +0000 Subject: [PATCH 26/96] Consistency: use variable names that correspond to the methods they invoke --- src/black/__init__.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index dde63b4925b..ba3e03483dc 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -1479,7 +1479,7 @@ class Line: comments: Dict[LeafID, List[Leaf]] = field(default_factory=dict) bracket_tracker: BracketTracker = field(default_factory=BracketTracker) inside_brackets: bool = False - should_explode: bool = False + should_split: bool = False magic_trailing_comma: Optional[Leaf] = None def append(self, leaf: Leaf, preformatted: bool = False) -> None: @@ -1791,7 +1791,7 @@ def clone(self) -> "Line": mode=self.mode, depth=self.depth, inside_brackets=self.inside_brackets, - should_explode=self.should_explode, + should_split=self.should_split, magic_trailing_comma=self.magic_trailing_comma, ) @@ -2709,7 +2709,7 @@ def init_st(ST: Type[StringTransformer]) -> StringTransformer: transformers: List[Transformer] if ( not line.contains_uncollapsable_type_comments() - and not (line.should_explode or line.magic_trailing_comma) + and not (line.should_split or line.magic_trailing_comma) and ( is_line_short_enough(line, line_length=mode.line_length, line_str=line_str) or line.contains_unsplittable_type_ignore() @@ -4383,7 +4383,7 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]: mode=line.mode, depth=line.depth + 1, inside_brackets=True, - should_explode=line.should_explode, + should_split=line.should_split, magic_trailing_comma=line.magic_trailing_comma, ) string_leaf = Leaf(token.STRING, string_value) @@ -5006,7 +5006,7 @@ def bracket_split_build_line( for comment_after in original.comments_after(leaf): result.append(comment_after, preformatted=True) if is_body and should_split(result, opening_bracket): - result.should_explode = True + result.should_split = True return result @@ -5924,7 +5924,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf """ omit: Set[LeafID] = set() - if not line.should_explode and not line.magic_trailing_comma: + if not line.should_split and not line.magic_trailing_comma: yield omit length = 4 * line.depth @@ -6607,7 +6607,7 @@ def can_omit_invisible_parens( penultimate = line.leaves[-2] last = line.leaves[-1] - if line.should_explode or line.magic_trailing_comma: + if line.should_split or line.magic_trailing_comma: try: penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) except LookupError: From d80650366049ed9d33b33252e6aca7d234f8a06c Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 17:40:42 +0000 Subject: [PATCH 27/96] Clarity: special case: avoid using variables that have the same names as methods --- src/black/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index ba3e03483dc..33e65394c3b 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5005,7 +5005,7 @@ def bracket_split_build_line( result.append(leaf, preformatted=True) for comment_after in original.comments_after(leaf): result.append(comment_after, preformatted=True) - if is_body and should_split(result, opening_bracket): + if is_body and should_split_line(result, opening_bracket): result.should_split = True return result @@ -5788,7 +5788,7 @@ def ensure_visible(leaf: Leaf) -> None: leaf.value = ")" -def should_split(line: Line, opening_bracket: Leaf) -> bool: +def should_split_line(line: Line, opening_bracket: Leaf) -> bool: """Should `line` be immediately split with `delimiter_split()` after RHS?""" if not (opening_bracket.parent and opening_bracket.value in "[{("): From 7dba4ba502692409e3f99ee4344c7d3f0cd1af0a Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 17:45:45 +0000 Subject: [PATCH 28/96] Simplification: only use special-case token retrieval logic when magic trailing comma is present --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 33e65394c3b..72586b63539 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6607,7 +6607,7 @@ def can_omit_invisible_parens( penultimate = line.leaves[-2] last = line.leaves[-1] - if line.should_split or line.magic_trailing_comma: + if line.magic_trailing_comma: try: penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) except LookupError: From 27a2dfe24e7383f569198efc31f2b5ef3a7360cb Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 17:48:38 +0000 Subject: [PATCH 29/96] Simplification: only yield empty omit list when magic trailing comma is present --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 72586b63539..a0c21704b8b 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5924,7 +5924,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf """ omit: Set[LeafID] = set() - if not line.should_split and not line.magic_trailing_comma: + if not line.magic_trailing_comma: yield omit length = 4 * line.depth From 3a4501725495684f15f4c7a12260a4d2b0d2bfad Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 4 Feb 2021 18:07:43 +0000 Subject: [PATCH 30/96] Cleanup: remove unused / redundant variables from conditionals --- src/black/__init__.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index a0c21704b8b..d6d41033215 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5946,8 +5946,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf elif leaf.type in CLOSING_BRACKETS: prev = line.leaves[index - 1] if index > 0 else None if ( - line.magic_trailing_comma - and prev + prev and prev.type == token.COMMA and not is_one_tuple_between( leaf.opening_bracket, leaf, line.leaves @@ -5974,8 +5973,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf yield omit if ( - line.magic_trailing_comma - and prev + prev and prev.type == token.COMMA and not is_one_tuple_between(leaf.opening_bracket, leaf, line.leaves) ): @@ -6634,7 +6632,7 @@ def can_omit_invisible_parens( # unnecessary. return True - if line.magic_trailing_comma and penultimate.type == token.COMMA: + if penultimate.type == token.COMMA: # The rightmost non-omitted bracket pair is the one we want to explode on. return True From e9a732672de2eaa5603f353145d55f1bb53733f4 Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 5 Feb 2021 01:04:24 +0000 Subject: [PATCH 31/96] Revert "Address pre-existing trailing commas when not in the rightmost bracket pair" This reverts commit 586d24236e6b57bc3b5da85fdbe2563835021076. --- src/black/__init__.py | 243 +++++----------------- tests/data/cantfit.py | 12 +- tests/data/function_trailing_comma.py | 21 -- tests/data/function_trailing_comma_wip.py | 5 + tests/data/long_strings_flag_disabled.py | 13 +- tests/test_black.py | 32 ++- 6 files changed, 92 insertions(+), 234 deletions(-) create mode 100644 tests/data/function_trailing_comma_wip.py diff --git a/src/black/__init__.py b/src/black/__init__.py index e3fbffd4e31..47d42a468f0 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -203,7 +203,6 @@ class Feature(Enum): ASSIGNMENT_EXPRESSIONS = 8 POS_ONLY_ARGUMENTS = 9 RELAXED_DECORATORS = 10 - FORCE_OPTIONAL_PARENTHESES = 50 VERSION_TO_FEATURES: Dict[TargetVersion, Set[Feature]] = { @@ -1336,7 +1335,6 @@ class BracketTracker: previous: Optional[Leaf] = None _for_loop_depths: List[int] = field(default_factory=list) _lambda_argument_depths: List[int] = field(default_factory=list) - invisible: List[Leaf] = field(default_factory=list) def mark(self, leaf: Leaf) -> None: """Mark `leaf` with bracket-related metadata. Keep track of delimiters. @@ -1368,8 +1366,6 @@ def mark(self, leaf: Leaf) -> None: f" bracket: {leaf}" ) from e leaf.opening_bracket = opening_bracket - if not leaf.value: - self.invisible.append(leaf) leaf.bracket_depth = self.depth if self.depth == 0: delim = is_split_before_delimiter(leaf, self.previous) @@ -1382,8 +1378,6 @@ def mark(self, leaf: Leaf) -> None: if leaf.type in OPENING_BRACKETS: self.bracket_match[self.depth, BRACKET[leaf.type]] = leaf self.depth += 1 - if not leaf.value: - self.invisible.append(leaf) self.previous = leaf self.maybe_increment_lambda_arguments(leaf) self.maybe_increment_for_loop_variable(leaf) @@ -2727,31 +2721,20 @@ def init_st(ST: Type[StringTransformer]) -> StringTransformer: else: def rhs(line: Line, features: Collection[Feature]) -> Iterator[Line]: - """Wraps calls to `right_hand_split`. - - The calls increasingly `omit` right-hand trailers (bracket pairs with - content), meaning the trailers get glued together to split on another - bracket pair instead. - """ for omit in generate_trailers_to_omit(line, mode.line_length): lines = list( right_hand_split(line, mode.line_length, features, omit=omit) ) - # Note: this check is only able to figure out if the first line of the - # *current* transformation fits in the line length. This is true only - # for simple cases. All others require running more transforms via - # `transform_line()`. This check doesn't know if those would succeed. if is_line_short_enough(lines[0], line_length=mode.line_length): yield from lines return # All splits failed, best effort split with no omits. # This mostly happens to multiline strings that are by definition - # reported as not fitting a single line, as well as lines that contain - # trailing commas (those have to be exploded). - yield from right_hand_split( - line, line_length=mode.line_length, features=features - ) + # reported as not fitting a single line. + # line_length=1 here was historically a bug that somehow became a feature. + # See #762 and #781 for the full story. + yield from right_hand_split(line, line_length=1, features=features) if mode.experimental_string_processing: if line.inside_brackets: @@ -2782,8 +2765,17 @@ def rhs(line: Line, features: Collection[Feature]) -> Iterator[Line]: # We are accumulating lines in `result` because we might want to abort # mission and return the original line in the end, or attempt a different # split altogether. + result: List[Line] = [] try: - result = run_transformer(line, transform, mode, features, line_str=line_str) + for transformed_line in transform(line, features): + if str(transformed_line).strip("\n") == line_str: + raise CannotTransform( + "Line transformer returned an unchanged result" + ) + + result.extend( + transform_line(transformed_line, mode=mode, features=features) + ) except CannotTransform: continue else: @@ -2824,7 +2816,6 @@ class StringTransformer(ABC): line_length: int normalize_strings: bool - __name__ = "StringTransformer" @abstractmethod def do_match(self, line: Line) -> TMatchResult: @@ -3068,7 +3059,7 @@ def __remove_backslash_line_continuation_chars( ) new_line = line.clone() - new_line.comments = line.comments.copy() + new_line.comments = line.comments append_leaves(new_line, line, LL) new_string_leaf = new_line.leaves[string_idx] @@ -3427,13 +3418,8 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]: new_line = line.clone() new_line.comments = line.comments.copy() - try: - append_leaves(new_line, line, LL[: string_idx - 1]) - except BracketMatchError: - # HACK: I believe there is currently a bug somewhere in - # right_hand_split() that is causing brackets to not be tracked - # properly by a shared BracketTracker. - append_leaves(new_line, line, LL[: string_idx - 1], preformatted=True) + + append_leaves(new_line, line, LL[: string_idx - 1]) string_leaf = Leaf(token.STRING, LL[string_idx].value) LL[string_idx - 1].remove() @@ -4898,9 +4884,8 @@ def right_hand_split( tail = bracket_split_build_line(tail_leaves, line, opening_bracket) bracket_split_succeeded_or_raise(head, body, tail) if ( - Feature.FORCE_OPTIONAL_PARENTHESES not in features # the opening bracket is an optional paren - and opening_bracket.type == token.LPAR + opening_bracket.type == token.LPAR and not opening_bracket.value # the closing bracket is an optional paren and closing_bracket.type == token.RPAR @@ -4911,7 +4896,7 @@ def right_hand_split( # there are no standalone comments in the body and not body.contains_standalone_comments(0) # and we can actually remove the parens - and can_omit_invisible_parens(body, line_length, omit_on_explode=omit) + and can_omit_invisible_parens(body, line_length) ): omit = {id(closing_bracket), *omit} try: @@ -5818,9 +5803,6 @@ def should_split_line(line: Line, opening_bracket: Leaf) -> bool: def is_one_tuple_between(opening: Leaf, closing: Leaf, leaves: List[Leaf]) -> bool: """Return True if content between `opening` and `closing` looks like a one-tuple.""" - if opening.type != token.LPAR and closing.type != token.RPAR: - return False - depth = closing.bracket_depth + 1 for _opening_index, leaf in enumerate(leaves): if leaf is opening: @@ -5920,13 +5902,11 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf a preceding closing bracket fits in one line. Yielded sets are cumulative (contain results of previous yields, too). First - set is empty, unless the line should explode, in which case bracket pairs until - the one that needs to explode are omitted. + set is empty. """ omit: Set[LeafID] = set() - if not line.magic_trailing_comma: - yield omit + yield omit length = 4 * line.depth opening_bracket: Optional[Leaf] = None @@ -5945,22 +5925,9 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf if leaf is opening_bracket: opening_bracket = None elif leaf.type in CLOSING_BRACKETS: - prev = line.leaves[index - 1] if index > 0 else None - if ( - prev - and prev.type == token.COMMA - and not is_one_tuple_between( - leaf.opening_bracket, leaf, line.leaves - ) - ): - # Never omit bracket pairs with trailing commas. - # We need to explode on those. - break - inner_brackets.add(id(leaf)) elif leaf.type in CLOSING_BRACKETS: - prev = line.leaves[index - 1] if index > 0 else None - if prev and prev.type in OPENING_BRACKETS: + if index > 0 and line.leaves[index - 1].type in OPENING_BRACKETS: # Empty brackets would fail a split so treat them as "inner" # brackets (e.g. only add them to the `omit` set if another # pair of brackets was good enough. @@ -5973,15 +5940,6 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf inner_brackets.clear() yield omit - if ( - prev - and prev.type == token.COMMA - and not is_one_tuple_between(leaf.opening_bracket, leaf, line.leaves) - ): - # Never omit bracket pairs with trailing commas. - # We need to explode on those. - break - if leaf.value: opening_bracket = leaf.opening_bracket closing_bracket = leaf @@ -6565,11 +6523,7 @@ def can_be_split(line: Line) -> bool: return True -def can_omit_invisible_parens( - line: Line, - line_length: int, - omit_on_explode: Collection[LeafID] = (), -) -> bool: +def can_omit_invisible_parens(line: Line, line_length: int) -> bool: """Does `line` have a shape safe to reformat without optional parens around it? Returns True for only a subset of potentially nice looking formattings but @@ -6592,27 +6546,37 @@ def can_omit_invisible_parens( assert len(line.leaves) >= 2, "Stranded delimiter" - # With a single delimiter, omit if the expression starts or ends with - # a bracket. first = line.leaves[0] second = line.leaves[1] + penultimate = line.leaves[-2] + last = line.leaves[-1] + + # With a single delimiter, omit if the expression starts or ends with + # a bracket. if first.type in OPENING_BRACKETS and second.type not in CLOSING_BRACKETS: - if _can_omit_opening_paren(line, first=first, line_length=line_length): - return True + remainder = False + length = 4 * line.depth + for _index, leaf, leaf_length in enumerate_with_length(line): + if leaf.type in CLOSING_BRACKETS and leaf.opening_bracket is first: + remainder = True + if remainder: + length += leaf_length + if length > line_length: + break + + if leaf.type in OPENING_BRACKETS: + # There are brackets we can further split on. + remainder = False + + else: + # checked the entire string and line length wasn't exceeded + if len(line.leaves) == _index + 1: + return True # Note: we are not returning False here because a line might have *both* # a leading opening bracket and a trailing closing bracket. If the # opening bracket doesn't match our rule, maybe the closing will. - penultimate = line.leaves[-2] - last = line.leaves[-1] - if line.magic_trailing_comma: - try: - penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) - except LookupError: - # Turns out we'd omit everything. We cannot skip the optional parentheses. - return False - if ( last.type == token.RPAR or last.type == token.RBRACE @@ -6633,120 +6597,21 @@ def can_omit_invisible_parens( # unnecessary. return True - if penultimate.type == token.COMMA: - # The rightmost non-omitted bracket pair is the one we want to explode on. - return True - - if _can_omit_closing_paren(line, last=last, line_length=line_length): - return True - - return False - - -def _can_omit_opening_paren(line: Line, *, first: Leaf, line_length: int) -> bool: - """See `can_omit_invisible_parens`.""" - remainder = False - length = 4 * line.depth - _index = -1 - for _index, leaf, leaf_length in enumerate_with_length(line): - if leaf.type in CLOSING_BRACKETS and leaf.opening_bracket is first: - remainder = True - if remainder: + length = 4 * line.depth + seen_other_brackets = False + for _index, leaf, leaf_length in enumerate_with_length(line): length += leaf_length - if length > line_length: - break + if leaf is last.opening_bracket: + if seen_other_brackets or length <= line_length: + return True - if leaf.type in OPENING_BRACKETS: + elif leaf.type in OPENING_BRACKETS: # There are brackets we can further split on. - remainder = False - - else: - # checked the entire string and line length wasn't exceeded - if len(line.leaves) == _index + 1: - return True - - return False - - -def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool: - """See `can_omit_invisible_parens`.""" - length = 4 * line.depth - seen_other_brackets = False - for _index, leaf, leaf_length in enumerate_with_length(line): - length += leaf_length - if leaf is last.opening_bracket: - if seen_other_brackets or length <= line_length: - return True - - elif leaf.type in OPENING_BRACKETS: - # There are brackets we can further split on. - seen_other_brackets = True + seen_other_brackets = True return False -def last_two_except(leaves: List[Leaf], omit: Collection[LeafID]) -> Tuple[Leaf, Leaf]: - """Return (penultimate, last) leaves skipping brackets in `omit` and contents.""" - stop_after = None - last = None - for leaf in reversed(leaves): - if stop_after: - if leaf is stop_after: - stop_after = None - continue - - if last: - return leaf, last - - if id(leaf) in omit: - stop_after = leaf.opening_bracket - else: - last = leaf - else: - raise LookupError("Last two leaves were also skipped") - - -def run_transformer( - line: Line, - transform: Transformer, - mode: Mode, - features: Collection[Feature], - *, - line_str: str = "", -) -> List[Line]: - if not line_str: - line_str = line_to_string(line) - result: List[Line] = [] - for transformed_line in transform(line, features): - if str(transformed_line).strip("\n") == line_str: - raise CannotTransform("Line transformer returned an unchanged result") - - result.extend(transform_line(transformed_line, mode=mode, features=features)) - - if not ( - transform.__name__ == "rhs" - and line.bracket_tracker.invisible - and not any(bracket.value for bracket in line.bracket_tracker.invisible) - and not line.contains_multiline_strings() - and not result[0].contains_uncollapsable_type_comments() - and not result[0].contains_unsplittable_type_ignore() - and not is_line_short_enough(result[0], line_length=mode.line_length) - ): - return result - - line_copy = line.clone() - append_leaves(line_copy, line, line.leaves) - features_fop = set(features) | {Feature.FORCE_OPTIONAL_PARENTHESES} - second_opinion = run_transformer( - line_copy, transform, mode, features_fop, line_str=line_str - ) - if all( - is_line_short_enough(ln, line_length=mode.line_length) for ln in second_opinion - ): - result = second_opinion - return result - - def get_cache_file(mode: Mode) -> Path: return CACHE_DIR / f"cache.{mode.get_cache_key()}.pickle" diff --git a/tests/data/cantfit.py b/tests/data/cantfit.py index 0849374f776..ef9b78e09a9 100644 --- a/tests/data/cantfit.py +++ b/tests/data/cantfit.py @@ -67,15 +67,11 @@ normal_name = ( but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying() ) -normal_name = ( - but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( - arg1, arg2, arg3 - ) +normal_name = but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( + arg1, arg2, arg3 ) -normal_name = ( - but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( - [1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3 - ) +normal_name = but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( + [1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3 ) # long arguments normal_name = normal_function_name( diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index d15459cbeb5..314a56cf67b 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -9,12 +9,6 @@ def f2(a,b,): def f(a:int=1,): call(arg={'explode': 'this',}) call2(arg=[1,2,3],) - x = { - "a": 1, - "b": 2, - }["a"] - if a == {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}["a"]: - pass def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" @@ -57,21 +51,6 @@ def f( call2( arg=[1, 2, 3], ) - x = { - "a": 1, - "b": 2, - }["a"] - if a == { - "a": 1, - "b": 2, - "c": 3, - "d": 4, - "e": 5, - "f": 6, - "g": 7, - "h": 8, - }["a"]: - pass def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ diff --git a/tests/data/function_trailing_comma_wip.py b/tests/data/function_trailing_comma_wip.py new file mode 100644 index 00000000000..c41fc709d97 --- /dev/null +++ b/tests/data/function_trailing_comma_wip.py @@ -0,0 +1,5 @@ +CONFIG_FILES = [CONFIG_FILE] + SHARED_CONFIG_FILES + USER_CONFIG_FILES # type: Final + +# output + +CONFIG_FILES = [CONFIG_FILE] + SHARED_CONFIG_FILES + USER_CONFIG_FILES # type: Final \ No newline at end of file diff --git a/tests/data/long_strings_flag_disabled.py b/tests/data/long_strings_flag_disabled.py index ef3094fd779..db3954e3abd 100644 --- a/tests/data/long_strings_flag_disabled.py +++ b/tests/data/long_strings_flag_disabled.py @@ -133,11 +133,14 @@ "Use f-strings instead!", ) -old_fmt_string3 = "Whereas only the strings after the percent sign were long in the last example, this example uses a long initial string as well. This is another %s %s %s %s" % ( - "really really really really really", - "old", - "way to format strings!", - "Use f-strings instead!", +old_fmt_string3 = ( + "Whereas only the strings after the percent sign were long in the last example, this example uses a long initial string as well. This is another %s %s %s %s" + % ( + "really really really really really", + "old", + "way to format strings!", + "Use f-strings instead!", + ) ) fstring = f"f-strings definitely make things more {difficult} than they need to be for {{black}}. But boy they sure are handy. The problem is that some lines will need to have the 'f' whereas others do not. This {line}, for example, needs one." diff --git a/tests/test_black.py b/tests/test_black.py index 4d89af39c48..62b89e17cc5 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -219,16 +219,27 @@ def test_piping_diff_with_color(self) -> None: self.assertIn("\033[0m", actual) @patch("black.dump_to_file", dump_to_stderr) - def _test_wip(self) -> None: - source, expected = read_data("wip") - sys.settrace(tracefunc) - mode = replace( - DEFAULT_MODE, - experimental_string_processing=False, - target_versions={black.TargetVersion.PY38}, - ) - actual = fs(source, mode=mode) - sys.settrace(None) + def test_function(self) -> None: + source, expected = read_data("function") + actual = fs(source) + self.assertFormatEqual(expected, actual) + black.assert_equivalent(source, actual) + black.assert_stable(source, actual, DEFAULT_MODE) + + @patch("black.dump_to_file", dump_to_stderr) + def test_function2(self) -> None: + source, expected = read_data("function2") + actual = fs(source) + self.assertFormatEqual(expected, actual) + black.assert_equivalent(source, actual) + black.assert_stable(source, actual, DEFAULT_MODE) + + @patch("black.dump_to_file", dump_to_stderr) + def test_function_trailing_comma_wip(self) -> None: + source, expected = read_data("function_trailing_comma_wip") + # sys.settrace(tracefunc) + actual = fs(source) + # sys.settrace(None) self.assertFormatEqual(expected, actual) black.assert_equivalent(source, actual) black.assert_stable(source, actual, black.FileMode()) @@ -1814,7 +1825,6 @@ def tracefunc(frame: types.FrameType, event: str, arg: Any) -> Callable: return tracefunc stack = len(inspect.stack()) - 19 - stack *= 2 filename = frame.f_code.co_filename lineno = frame.f_lineno func_sig_lineno = lineno - 1 From 8b997a8f2af0a0952cd835e0f22d7a7fdb285989 Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 5 Feb 2021 01:05:39 +0000 Subject: [PATCH 32/96] Unset failure expectations on a few test cases --- tests/test_black.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/test_black.py b/tests/test_black.py index 62b89e17cc5..3654acd13a7 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -244,28 +244,24 @@ def test_function_trailing_comma_wip(self) -> None: black.assert_equivalent(source, actual) black.assert_stable(source, actual, black.FileMode()) - @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_invisible_parens_instability(self) -> None: source, _expected = read_data("invisible_parens_instability") actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) - @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability1(self) -> None: source, _expected = read_data("trailing_comma_optional_parens1") actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) - @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability2(self) -> None: source, _expected = read_data("trailing_comma_optional_parens2") actual = fs(source) black.assert_stable(source, actual, DEFAULT_MODE) - @unittest.expectedFailure @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability3(self) -> None: source, _expected = read_data("trailing_comma_optional_parens3") From ec1b0ab49918c0538bcbd18b0950c72849309248 Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 5 Feb 2021 02:51:26 +0000 Subject: [PATCH 33/96] Work-in-progress: investigate bracket matching --- src/black/__init__.py | 4 ++++ tests/data/long_strings__regression.py | 6 ++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 47d42a468f0..9eaa2db32f7 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5374,6 +5374,10 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) + elif isinstance(child, Node): + if maybe_make_parens_invisible_in_atom(child, parent=node): + wrap_in_parentheses(node, child, visible=False) + check_lpar = isinstance(child, Leaf) and child.value in parens_after diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py index 2e7f2483b63..ad44a16ccaa 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -347,7 +347,7 @@ def xxxxxxx_xxxxxx(xxxx): assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, - ], ("xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx) + ], "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx value.__dict__[ key @@ -828,9 +828,7 @@ def xxxxxxx_xxxxxx(xxxx): assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, - ], ( - "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx - ) + ], "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx value.__dict__[ From b90268e3170e8e31fbd939055f5d35587a79c33a Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 5 Feb 2021 11:14:21 +0000 Subject: [PATCH 34/96] Revert "Work-in-progress: investigate bracket matching" This reverts commit ec1b0ab49918c0538bcbd18b0950c72849309248. --- src/black/__init__.py | 4 ---- tests/data/long_strings__regression.py | 6 ++++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 9eaa2db32f7..47d42a468f0 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5374,10 +5374,6 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) - elif isinstance(child, Node): - if maybe_make_parens_invisible_in_atom(child, parent=node): - wrap_in_parentheses(node, child, visible=False) - check_lpar = isinstance(child, Leaf) and child.value in parens_after diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py index ad44a16ccaa..2e7f2483b63 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -347,7 +347,7 @@ def xxxxxxx_xxxxxx(xxxx): assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, - ], "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx + ], ("xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx) value.__dict__[ key @@ -828,7 +828,9 @@ def xxxxxxx_xxxxxx(xxxx): assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, - ], "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx + ], ( + "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx + ) value.__dict__[ From 268e3f35f31add91f8e622f67368f951ae723280 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 01:02:23 +0000 Subject: [PATCH 35/96] Fixup: handling of invisible parentheses for assert and return statements --- src/black/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 47d42a468f0..7e505a9521a 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5370,6 +5370,9 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break + elif node.type in [syms.assert_stmt, syms.return_stmt]: + if maybe_make_parens_invisible_in_atom(child, parent=node): + wrap_in_parentheses(node, child, visible=False) elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) From 0596b38f62ff6807721c987575029345e6e824c1 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Feb 2021 10:04:33 +0000 Subject: [PATCH 36/96] Use a 'pass' statement since 'maybe_make_parens_invisible_in_atom' currently always returns False for non-atom nodes, making the 'wrap_in_parentheses' call unreachable --- src/black/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 7e505a9521a..4a18890538f 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5371,8 +5371,7 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.append_child(Leaf(token.RPAR, "")) break elif node.type in [syms.assert_stmt, syms.return_stmt]: - if maybe_make_parens_invisible_in_atom(child, parent=node): - wrap_in_parentheses(node, child, visible=False) + pass elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) From 58c3cce56550d747415e35a9aca3f35be6317d8e Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 5 Feb 2021 11:19:02 +0000 Subject: [PATCH 37/96] Expectation adjustment: remove parentheses around pre-f-string formatted string --- tests/data/long_strings__regression.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py index 2e7f2483b63..ad44a16ccaa 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -347,7 +347,7 @@ def xxxxxxx_xxxxxx(xxxx): assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, - ], ("xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx) + ], "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx value.__dict__[ key @@ -828,9 +828,7 @@ def xxxxxxx_xxxxxx(xxxx): assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, - ], ( - "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx - ) + ], "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx value.__dict__[ From 235b8546cf06babd9a54d25f4a449749eaecce17 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 6 Feb 2021 16:42:08 +0000 Subject: [PATCH 38/96] Restore latest function_trailing_comma tests --- tests/data/function_trailing_comma.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index 314a56cf67b..d15459cbeb5 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -9,6 +9,12 @@ def f2(a,b,): def f(a:int=1,): call(arg={'explode': 'this',}) call2(arg=[1,2,3],) + x = { + "a": 1, + "b": 2, + }["a"] + if a == {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}["a"]: + pass def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" @@ -51,6 +57,21 @@ def f( call2( arg=[1, 2, 3], ) + x = { + "a": 1, + "b": 2, + }["a"] + if a == { + "a": 1, + "b": 2, + "c": 3, + "d": 4, + "e": 5, + "f": 6, + "g": 7, + "h": 8, + }["a"]: + pass def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ From e245b1a554f2c1e600ca0944a6369b4696431b9c Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 11:53:25 +0000 Subject: [PATCH 39/96] Add additional test coverage --- tests/data/function_trailing_comma.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index d15459cbeb5..d15decc0c37 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -15,6 +15,12 @@ def f(a:int=1,): }["a"] if a == {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}["a"]: pass + if a == {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}: + pass + assert val[-1] in ( + x, + y, + ) def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" @@ -72,6 +78,21 @@ def f( "h": 8, }["a"]: pass + if a == { + "a": 1, + "b": 2, + "c": 3, + "d": 4, + "e": 5, + "f": 6, + "g": 7, + "h": 8, + }: + pass + assert val[-1] in ( + x, + y, + ) def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[ From acffae3f675e4846317e7cee25b7aac145e93e9d Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 13:53:18 +0000 Subject: [PATCH 40/96] When a magic comma is present in the line, delay emission of empty trailer until the comma is encountered --- src/black/__init__.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 4a18890538f..afd05db2d8e 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5908,7 +5908,8 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf """ omit: Set[LeafID] = set() - yield omit + if not line.magic_trailing_comma: + yield omit length = 4 * line.depth opening_bracket: Optional[Leaf] = None @@ -5937,6 +5938,9 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf continue if closing_bracket: + if closing_bracket is line.magic_trailing_comma: + line.magic_trailing_comma = None + yield set() omit.add(id(closing_bracket)) omit.update(inner_brackets) inner_brackets.clear() @@ -5946,6 +5950,9 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf opening_bracket = leaf.opening_bracket closing_bracket = leaf + if line.magic_trailing_comma: + yield set() + def get_future_imports(node: Node) -> Set[str]: """Return a set of __future__ imports in the file.""" From d036da40842cef0d0f5f56dd2da969e11fe88446 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 13:57:28 +0000 Subject: [PATCH 41/96] Add additional test coverage --- tests/data/function_trailing_comma.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index d15decc0c37..1db4b591a7e 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -17,6 +17,9 @@ def f(a:int=1,): pass if a == {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}: pass + # TODO: Why didn't 'while' require special normalize_invisible_parens handling, like assert/return? + while a in {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}: + pass assert val[-1] in ( x, y, @@ -89,6 +92,18 @@ def f( "h": 8, }: pass + # TODO: Why didn't 'while' require special normalize_invisible_parens handling, like assert/return? + while a in { + "a": 1, + "b": 2, + "c": 3, + "d": 4, + "e": 5, + "f": 6, + "g": 7, + "h": 8, + }: + pass assert val[-1] in ( x, y, From 7da423eb8510f93e5ac5b94d5ebf4c5d43df189b Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 13:57:57 +0000 Subject: [PATCH 42/96] Add special-case handling of 'if' statements within normalize_invisible_parens method --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index afd05db2d8e..c921661e923 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5370,7 +5370,7 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break - elif node.type in [syms.assert_stmt, syms.return_stmt]: + elif node.type in [syms.assert_stmt, syms.if_stmt, syms.return_stmt]: pass elif not (isinstance(child, Leaf) and is_multiline_string(child)): From 634b1a39e15e616b076ea101b29c611ce76a5e9b Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 14:23:18 +0000 Subject: [PATCH 43/96] When a magic comma is present in the line, emit an empty trailer when we encounter closed brackets with inner brackets --- src/black/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index c921661e923..3702e236b89 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5938,9 +5938,10 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf continue if closing_bracket: - if closing_bracket is line.magic_trailing_comma: - line.magic_trailing_comma = None - yield set() + if line.magic_trailing_comma: + if closing_bracket is line.magic_trailing_comma or inner_brackets: + line.magic_trailing_comma = None + yield set() omit.add(id(closing_bracket)) omit.update(inner_brackets) inner_brackets.clear() From e8ad63182913ba958d2a2fa3c18e2917f636d44a Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 14:30:09 +0000 Subject: [PATCH 44/96] black-primer configuration: indicate that sqlalchemy would require formatting updates under this changeset --- src/black_primer/primer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index 32df01571a7..ff158ae31e6 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -98,7 +98,7 @@ }, "sqlalchemy": { "cli_arguments": [], - "expect_formatting_changes": false, + "expect_formatting_changes": true, "git_clone_url": "https://github.com/sqlalchemy/sqlalchemy.git", "long_checkout": false, "py_versions": ["all"] From 48ac43f5a5114e395236cb57a0d95b44291a2f6b Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 14:35:16 +0000 Subject: [PATCH 45/96] Include removal of work-in-progress test from 586d24236e6b57bc3b5da85fdbe2563835021076 --- tests/data/function_trailing_comma_wip.py | 5 ----- tests/test_black.py | 10 ---------- 2 files changed, 15 deletions(-) delete mode 100644 tests/data/function_trailing_comma_wip.py diff --git a/tests/data/function_trailing_comma_wip.py b/tests/data/function_trailing_comma_wip.py deleted file mode 100644 index c41fc709d97..00000000000 --- a/tests/data/function_trailing_comma_wip.py +++ /dev/null @@ -1,5 +0,0 @@ -CONFIG_FILES = [CONFIG_FILE] + SHARED_CONFIG_FILES + USER_CONFIG_FILES # type: Final - -# output - -CONFIG_FILES = [CONFIG_FILE] + SHARED_CONFIG_FILES + USER_CONFIG_FILES # type: Final \ No newline at end of file diff --git a/tests/test_black.py b/tests/test_black.py index 3654acd13a7..37c62ac9e6b 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -234,16 +234,6 @@ def test_function2(self) -> None: black.assert_equivalent(source, actual) black.assert_stable(source, actual, DEFAULT_MODE) - @patch("black.dump_to_file", dump_to_stderr) - def test_function_trailing_comma_wip(self) -> None: - source, expected = read_data("function_trailing_comma_wip") - # sys.settrace(tracefunc) - actual = fs(source) - # sys.settrace(None) - self.assertFormatEqual(expected, actual) - black.assert_equivalent(source, actual) - black.assert_stable(source, actual, black.FileMode()) - @patch("black.dump_to_file", dump_to_stderr) def test_invisible_parens_instability(self) -> None: source, _expected = read_data("invisible_parens_instability") From 62b76ca3939e4b93394dd60b61c8175e9c56ad42 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 14:40:44 +0000 Subject: [PATCH 46/96] Revert "Add additional test cases from psf/black#1629" This reverts commit 21528480a85f26c7b8192cb18892be309fe9d31c. --- tests/data/invisible_parens_instability.py | 51 ---------------------- 1 file changed, 51 deletions(-) diff --git a/tests/data/invisible_parens_instability.py b/tests/data/invisible_parens_instability.py index 8855761bfb3..d73379996ea 100644 --- a/tests/data/invisible_parens_instability.py +++ b/tests/data/invisible_parens_instability.py @@ -1,52 +1 @@ -# https://github.com/psf/black/issues/1629#issuecomment-681953135 -assert ( - xxxxxx( - xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx, - xxxxxxxxxxxxxxxxxxxxxxxxx - ) - == xxxxxx(xxxxxxxxxxx, xxxxxxxxxxxxxxxxxxxxxxxxx) -) - - -# https://github.com/psf/black/issues/1629#issuecomment-688804670 -if any( - k in t - for k in ["AAAAAAAAAA", "AAAAA", "AAAAAA", "AAAAAAAAA", "AAA", "AAAAAA", "AAAAAAAA", "AAA", "AAAAA", "AAAAA", "AAAA"] -) and not any(k in t for k in ["AAA"]): - pass - - -# https://github.com/psf/black/issues/1629#issuecomment-693816208 -aaaaaaaaaaaaaaaaaaaaaaaaaa = bbbbbbbbbbbbbbbbbbbb( # ccccccccccccccccccccccccccccccccccc - d=0 -) - - -# https://github.com/psf/black/issues/1629#issuecomment-699006156 -def f(): - return a( - b( - c(n) - # - ), - [] - ) + d(x, y) - - -# https://github.com/psf/black/issues/1629#issuecomment-704782983 -def test_foo(): - assert foo( - "foo", - ) == [{"raw": {"person": "1"}, "error": "Invalid field unknown", "status": "error"}] - - -# https://github.com/psf/black/issues/1629#issuecomment-764562642 -class Foo(object): - def bar(self): - x = 'foo ' + \ - 'subscription {} resource_group {} Site {}, foobar_name {}. Error {}'.format( - self._subscription, self._resource_group, self._site, self._foobar_name, error) - - -# https://github.com/psf/black/issues/1629#issuecomment-772691120 assert function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15) != [None] From 9ff5f98fc8690c35e3e348805f3a192be4d18722 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 14:42:21 +0000 Subject: [PATCH 47/96] black-primer configuration: indicate that flake8-bugbear would require formatting updates under this changeset --- src/black_primer/primer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index ff158ae31e6..92dce20c69a 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -40,7 +40,7 @@ }, "flake8-bugbear": { "cli_arguments": [], - "expect_formatting_changes": false, + "expect_formatting_changes": true, "git_clone_url": "https://github.com/PyCQA/flake8-bugbear.git", "long_checkout": false, "py_versions": ["all"] From 176a96a8a78a2d7c2de08174835a81e2e9ba1ce8 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 15:09:49 +0000 Subject: [PATCH 48/96] Re-apply test re-organization cleanup of 'function2' test from e6cd10e7615f4df537e2eaefcf3904a4feecad1f --- tests/test_black.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/test_black.py b/tests/test_black.py index 37c62ac9e6b..7eb9d5c8133 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -226,14 +226,6 @@ def test_function(self) -> None: black.assert_equivalent(source, actual) black.assert_stable(source, actual, DEFAULT_MODE) - @patch("black.dump_to_file", dump_to_stderr) - def test_function2(self) -> None: - source, expected = read_data("function2") - actual = fs(source) - self.assertFormatEqual(expected, actual) - black.assert_equivalent(source, actual) - black.assert_stable(source, actual, DEFAULT_MODE) - @patch("black.dump_to_file", dump_to_stderr) def test_invisible_parens_instability(self) -> None: source, _expected = read_data("invisible_parens_instability") From b93b955d8b378e28edc4a5ef535332b0881c734c Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 15:12:49 +0000 Subject: [PATCH 49/96] Apply test organization patterns from e6cd10e7615f4df537e2eaefcf3904a4feecad1f for 'invisible_parens_instability' test --- tests/data/invisible_parens_instability.py | 20 ++++++++++++++++++++ tests/test_black.py | 6 ------ tests/test_format.py | 1 + 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/data/invisible_parens_instability.py b/tests/data/invisible_parens_instability.py index d73379996ea..54fa6c33fed 100644 --- a/tests/data/invisible_parens_instability.py +++ b/tests/data/invisible_parens_instability.py @@ -1 +1,21 @@ assert function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15) != [None] + +# output + +assert function( + arg1, + arg2, + arg3, + arg4, + arg5, + arg6, + arg7, + arg8, + arg9, + arg10, + arg11, + arg12, + arg13, + arg14, + arg15, +) != [None] diff --git a/tests/test_black.py b/tests/test_black.py index 7eb9d5c8133..d2fa88987ec 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -226,12 +226,6 @@ def test_function(self) -> None: black.assert_equivalent(source, actual) black.assert_stable(source, actual, DEFAULT_MODE) - @patch("black.dump_to_file", dump_to_stderr) - def test_invisible_parens_instability(self) -> None: - source, _expected = read_data("invisible_parens_instability") - actual = fs(source) - black.assert_stable(source, actual, DEFAULT_MODE) - @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability1(self) -> None: source, _expected = read_data("trailing_comma_optional_parens1") diff --git a/tests/test_format.py b/tests/test_format.py index e4677707e3c..cb2cdb7b4c3 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -42,6 +42,7 @@ "function2", "function_trailing_comma", "import_spacing", + "invisible_parens_instability", "long_strings", "long_strings__edge_case", "long_strings__regression", From e96a683a1b1bdab6d4257682d74956736f11bcd6 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 15:14:40 +0000 Subject: [PATCH 50/96] Nitpick: rename the test; it should be considered a test of stability, not a test of instability --- ...ible_parens_instability.py => invisible_parens_stability.py} | 0 tests/test_format.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename tests/data/{invisible_parens_instability.py => invisible_parens_stability.py} (100%) diff --git a/tests/data/invisible_parens_instability.py b/tests/data/invisible_parens_stability.py similarity index 100% rename from tests/data/invisible_parens_instability.py rename to tests/data/invisible_parens_stability.py diff --git a/tests/test_format.py b/tests/test_format.py index cb2cdb7b4c3..79e8583bbfe 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -42,7 +42,7 @@ "function2", "function_trailing_comma", "import_spacing", - "invisible_parens_instability", + "invisible_parens_stability", "long_strings", "long_strings__edge_case", "long_strings__regression", From 6763046341b855f94a27595fca8e2876c502c750 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 7 Feb 2021 21:24:19 +0000 Subject: [PATCH 51/96] Include line comment copy behaviour fixup from 586d24236e6b57bc3b5da85fdbe2563835021076 --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 3702e236b89..2c118019e1f 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -3059,7 +3059,7 @@ def __remove_backslash_line_continuation_chars( ) new_line = line.clone() - new_line.comments = line.comments + new_line.comments = line.comments.copy() append_leaves(new_line, line, LL) new_string_leaf = new_line.leaves[string_idx] From 0aa8083f4deaff1d863ac030e85cb628389eb59e Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 8 Feb 2021 19:59:07 +0000 Subject: [PATCH 52/96] Cleanup: yield existing 'omit' variable rather than literal empty sets --- src/black/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 2c118019e1f..eaa0804fe23 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5941,7 +5941,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf if line.magic_trailing_comma: if closing_bracket is line.magic_trailing_comma or inner_brackets: line.magic_trailing_comma = None - yield set() + yield omit omit.add(id(closing_bracket)) omit.update(inner_brackets) inner_brackets.clear() @@ -5952,7 +5952,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf closing_bracket = leaf if line.magic_trailing_comma: - yield set() + yield omit def get_future_imports(node: Node) -> Set[str]: From efc75521ea5dabac912413dea95fbdca0b331355 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 21:13:57 +0000 Subject: [PATCH 53/96] Readability: reduce boolean nesting --- src/black/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index d6d41033215..a8f7ac7d19e 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -2709,7 +2709,8 @@ def init_st(ST: Type[StringTransformer]) -> StringTransformer: transformers: List[Transformer] if ( not line.contains_uncollapsable_type_comments() - and not (line.should_split or line.magic_trailing_comma) + and not line.should_split + and not line.magic_trailing_comma and ( is_line_short_enough(line, line_length=mode.line_length, line_str=line_str) or line.contains_unsplittable_type_ignore() From b2a4a291e3c6c70f0d428853d73a21b0e5adfca3 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 21:20:43 +0000 Subject: [PATCH 54/96] Include explanatory comments in 'rhs' method from 586d24236e6b57bc3b5da85fdbe2563835021076 --- src/black/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 3df7bd2f1f4..5ceb8cbe908 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -2722,10 +2722,19 @@ def init_st(ST: Type[StringTransformer]) -> StringTransformer: else: def rhs(line: Line, features: Collection[Feature]) -> Iterator[Line]: + """Wraps calls to `right_hand_split`. + The calls increasingly `omit` right-hand trailers (bracket pairs with + content), meaning the trailers get glued together to split on another + bracket pair instead. + """ for omit in generate_trailers_to_omit(line, mode.line_length): lines = list( right_hand_split(line, mode.line_length, features, omit=omit) ) + # Note: this check is only able to figure out if the first line of the + # *current* transformation fits in the line length. This is true only + # for simple cases. All others require running more transforms via + # `transform_line()`. This check doesn't know if those would succeed. if is_line_short_enough(lines[0], line_length=mode.line_length): yield from lines return From 934c79db4d02cf105a9fe4c04478f42884932302 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 21:22:36 +0000 Subject: [PATCH 55/96] Include short-circuit condition in 'is_one_tuple_between' method from 586d24236e6b57bc3b5da85fdbe2563835021076 --- src/black/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 5ceb8cbe908..1ca48ca4e89 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5815,6 +5815,9 @@ def should_split_line(line: Line, opening_bracket: Leaf) -> bool: def is_one_tuple_between(opening: Leaf, closing: Leaf, leaves: List[Leaf]) -> bool: """Return True if content between `opening` and `closing` looks like a one-tuple.""" + if opening.type != token.LPAR and closing.type != token.RPAR: + return False + depth = closing.bracket_depth + 1 for _opening_index, leaf in enumerate(leaves): if leaf is opening: From c16e29ab0c5a12550787a157a7ef7a9f9055035e Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 21:37:31 +0000 Subject: [PATCH 56/96] Include extracted '_can_omit_opening_paren', '_can_omit_closing_paren' methods from 586d24236e6b57bc3b5da85fdbe2563835021076 --- src/black/__init__.py | 64 ++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 1ca48ca4e89..518db3d41cc 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6577,24 +6577,8 @@ def can_omit_invisible_parens(line: Line, line_length: int) -> bool: # With a single delimiter, omit if the expression starts or ends with # a bracket. if first.type in OPENING_BRACKETS and second.type not in CLOSING_BRACKETS: - remainder = False - length = 4 * line.depth - for _index, leaf, leaf_length in enumerate_with_length(line): - if leaf.type in CLOSING_BRACKETS and leaf.opening_bracket is first: - remainder = True - if remainder: - length += leaf_length - if length > line_length: - break - - if leaf.type in OPENING_BRACKETS: - # There are brackets we can further split on. - remainder = False - - else: - # checked the entire string and line length wasn't exceeded - if len(line.leaves) == _index + 1: - return True + if _can_omit_opening_paren(line, first=first, line_length=line_length): + return True # Note: we are not returning False here because a line might have *both* # a leading opening bracket and a trailing closing bracket. If the @@ -6620,21 +6604,45 @@ def can_omit_invisible_parens(line: Line, line_length: int) -> bool: # unnecessary. return True - length = 4 * line.depth - seen_other_brackets = False - for _index, leaf, leaf_length in enumerate_with_length(line): - length += leaf_length - if leaf is last.opening_bracket: - if seen_other_brackets or length <= line_length: - return True + if _can_omit_closing_paren(line, last=last, line_length=line_length): + return True + + return False - elif leaf.type in OPENING_BRACKETS: - # There are brackets we can further split on. - seen_other_brackets = True + +def _can_omit_opening_paren(line: Line, *, first: Leaf, line_length: int) -> bool: + """See `can_omit_invisible_parens`.""" + remainder = False + length = 4 * line.depth + _index = -1 + for _index, leaf, leaf_length in enumerate_with_length(line): + if leaf.type in CLOSING_BRACKETS and leaf.opening_bracket is first: + remainder = True + if remainder: + remainder = False + else: + # checked the entire string and line length wasn't exceeded + if len(line.leaves) == _index + 1: + return True return False +def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool: + """See `can_omit_invisible_parens`.""" + length = 4 * line.depth + seen_other_brackets = False + for _index, leaf, leaf_length in enumerate_with_length(line): + length += leaf_length + if leaf is last.opening_bracket: + if seen_other_brackets or length <= line_length: + return True + + elif leaf.type in OPENING_BRACKETS: + # There are brackets we can further split on. + seen_other_brackets = True + + def get_cache_file(mode: Mode) -> Path: return CACHE_DIR / f"cache.{mode.get_cache_key()}.pickle" From 7dfb59a69b60e12e4110f61bb6bfa9f9446e7940 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 21:39:25 +0000 Subject: [PATCH 57/96] Relocate 'last', 'penultimate' variable declarations nearer to first use --- src/black/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 518db3d41cc..79b79ef7341 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6571,8 +6571,6 @@ def can_omit_invisible_parens(line: Line, line_length: int) -> bool: first = line.leaves[0] second = line.leaves[1] - penultimate = line.leaves[-2] - last = line.leaves[-1] # With a single delimiter, omit if the expression starts or ends with # a bracket. @@ -6584,6 +6582,9 @@ def can_omit_invisible_parens(line: Line, line_length: int) -> bool: # a leading opening bracket and a trailing closing bracket. If the # opening bracket doesn't match our rule, maybe the closing will. + penultimate = line.leaves[-2] + last = line.leaves[-1] + if ( last.type == token.RPAR or last.type == token.RBRACE From 6423a8d2a6626065e8433e69413ebe9d83ce3dca Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 21:48:16 +0000 Subject: [PATCH 58/96] Relocate comment to minimize review diff size --- src/black/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 79b79ef7341..afe454e0600 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6569,11 +6569,11 @@ def can_omit_invisible_parens(line: Line, line_length: int) -> bool: assert len(line.leaves) >= 2, "Stranded delimiter" + # With a single delimiter, omit if the expression starts or ends with + # a bracket. first = line.leaves[0] second = line.leaves[1] - # With a single delimiter, omit if the expression starts or ends with - # a bracket. if first.type in OPENING_BRACKETS and second.type not in CLOSING_BRACKETS: if _can_omit_opening_paren(line, first=first, line_length=line_length): return True From 834366306f579b14de10f2353f1819caf79bdd2b Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 21:57:46 +0000 Subject: [PATCH 59/96] Remove empty line --- src/black/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index afe454e0600..8ada0f62561 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6573,7 +6573,6 @@ def can_omit_invisible_parens(line: Line, line_length: int) -> bool: # a bracket. first = line.leaves[0] second = line.leaves[1] - if first.type in OPENING_BRACKETS and second.type not in CLOSING_BRACKETS: if _can_omit_opening_paren(line, first=first, line_length=line_length): return True From 66ef77a711c6731142973db9b00aace04bc0716a Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 22:00:14 +0000 Subject: [PATCH 60/96] Cleanup: undo accidental changes to remainder logic in '_can_omit_opening_paren' method --- src/black/__init__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 8ada0f62561..bf301cca8f1 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6619,7 +6619,13 @@ def _can_omit_opening_paren(line: Line, *, first: Leaf, line_length: int) -> boo if leaf.type in CLOSING_BRACKETS and leaf.opening_bracket is first: remainder = True if remainder: - remainder = False + length += leaf_length + if length > line_length: + break + + if leaf.type in OPENING_BRACKETS: + # There are brackets we can further split on. + remainder = False else: # checked the entire string and line length wasn't exceeded if len(line.leaves) == _index + 1: From 4126cdd82a90000a7c84954668e5eded112a7df6 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 22:07:16 +0000 Subject: [PATCH 61/96] Restore redundant condition in order to further minimize review diff size --- src/black/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index bf301cca8f1..04151881644 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6604,6 +6604,10 @@ def can_omit_invisible_parens(line: Line, line_length: int) -> bool: # unnecessary. return True + if line.should_split and penultimate.type == token.COMMA: + # The rightmost non-omitted bracket pair is the one we want to explode on. + return True + if _can_omit_closing_paren(line, last=last, line_length=line_length): return True From 34cce108a45766d2185dc7dfabee212e63b6dc74 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 22:09:57 +0000 Subject: [PATCH 62/96] Restore empty line --- src/black/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 04151881644..4f1d573eb38 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6630,6 +6630,7 @@ def _can_omit_opening_paren(line: Line, *, first: Leaf, line_length: int) -> boo if leaf.type in OPENING_BRACKETS: # There are brackets we can further split on. remainder = False + else: # checked the entire string and line length wasn't exceeded if len(line.leaves) == _index + 1: From 8fcfcbf069399020b947b996f91561933fb2fe40 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 22:10:41 +0000 Subject: [PATCH 63/96] Restore explicit return value --- src/black/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 4f1d573eb38..b923117ddb0 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6653,6 +6653,8 @@ def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool # There are brackets we can further split on. seen_other_brackets = True + return False + def get_cache_file(mode: Mode) -> Path: return CACHE_DIR / f"cache.{mode.get_cache_key()}.pickle" From 0adad3a029f45e07bf37336756a91f819f562561 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 22:14:44 +0000 Subject: [PATCH 64/96] Include extraction of 'prev' variable within 'generate_trailers_to_omit' method from 586d24236e6b57bc3b5da85fdbe2563835021076 --- src/black/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index b923117ddb0..7149860e0ac 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5943,7 +5943,8 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf elif leaf.type in CLOSING_BRACKETS: inner_brackets.add(id(leaf)) elif leaf.type in CLOSING_BRACKETS: - if index > 0 and line.leaves[index - 1].type in OPENING_BRACKETS: + prev = line.leaves[index - 1] if index > 0 else None + if prev and prev.type in OPENING_BRACKETS: # Empty brackets would fail a split so treat them as "inner" # brackets (e.g. only add them to the `omit` set if another # pair of brackets was good enough. From 788002a3796f929809e59d2182535f2b6c6eec05 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 23:26:16 +0000 Subject: [PATCH 65/96] Restore empty line --- src/black/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 7149860e0ac..0ff053d941d 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -2723,6 +2723,7 @@ def init_st(ST: Type[StringTransformer]) -> StringTransformer: def rhs(line: Line, features: Collection[Feature]) -> Iterator[Line]: """Wraps calls to `right_hand_split`. + The calls increasingly `omit` right-hand trailers (bracket pairs with content), meaning the trailers get glued together to split on another bracket pair instead. From 879cbe22b3e152f3e440b89381836b4b63888d3d Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 23:38:45 +0000 Subject: [PATCH 66/96] Remove empty line --- src/black/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 0ff053d941d..c4780568cc3 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -3429,7 +3429,6 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]: new_line = line.clone() new_line.comments = line.comments.copy() - append_leaves(new_line, line, LL[: string_idx - 1]) string_leaf = Leaf(token.STRING, LL[string_idx].value) From c7d7f59052e2d66ffc5d712a67eaf2fe76a81ce0 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 9 Feb 2021 23:41:56 +0000 Subject: [PATCH 67/96] Restore accurate comment --- src/black/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index c4780568cc3..1318a30ae1b 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5917,7 +5917,8 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf a preceding closing bracket fits in one line. Yielded sets are cumulative (contain results of previous yields, too). First - set is empty. + set is empty, unless the line should explode, in which case bracket pairs until + the one that needs to explode are omitted. """ omit: Set[LeafID] = set() From 9f0289ab2cc63f57d1ef77b1ee348d7614001cf0 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 12:33:50 +0000 Subject: [PATCH 68/96] Minimize changes: more closely resemble original conditional logic --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index c3b9a34c88e..7babde357e4 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6634,7 +6634,7 @@ def can_omit_invisible_parens( # unnecessary. return True - if penultimate.type == token.COMMA: + if line.magic_trailing_comma and penultimate.type == token.COMMA: # The rightmost non-omitted bracket pair is the one we want to explode on. return True From b4c92385a0e93981aa806083a72e697eb5c6df60 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 12:38:14 +0000 Subject: [PATCH 69/96] Update test expectations --- tests/data/composition.py | 25 +++++------ tests/data/composition_no_trailing_comma.py | 50 +++++++++------------ tests/data/long_strings_flag_disabled.py | 13 +++--- 3 files changed, 38 insertions(+), 50 deletions(-) diff --git a/tests/data/composition.py b/tests/data/composition.py index e429f15e669..9ccbd100b25 100644 --- a/tests/data/composition.py +++ b/tests/data/composition.py @@ -165,17 +165,14 @@ def tricky_asserts(self) -> None: _C.__init__.__code__.co_firstlineno + 1, ) - assert ( - expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect - == { - key1: value1, - key2: value2, - key3: value3, - key4: value4, - key5: value5, - key6: value6, - key7: value7, - key8: value8, - key9: value9, - } - ) + assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == { + key1: value1, + key2: value2, + key3: value3, + key4: value4, + key5: value5, + key6: value6, + key7: value7, + key8: value8, + key9: value9, + } diff --git a/tests/data/composition_no_trailing_comma.py b/tests/data/composition_no_trailing_comma.py index f17b89dea8d..1cea37d8641 100644 --- a/tests/data/composition_no_trailing_comma.py +++ b/tests/data/composition_no_trailing_comma.py @@ -165,20 +165,17 @@ def tricky_asserts(self) -> None: _C.__init__.__code__.co_firstlineno + 1, ) - assert ( - expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect - == { - key1: value1, - key2: value2, - key3: value3, - key4: value4, - key5: value5, - key6: value6, - key7: value7, - key8: value8, - key9: value9 - } - ) + assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == { + key1: value1, + key2: value2, + key3: value3, + key4: value4, + key5: value5, + key6: value6, + key7: value7, + key8: value8, + key9: value9 + } @@ -351,17 +348,14 @@ def tricky_asserts(self) -> None: _C.__init__.__code__.co_firstlineno + 1, ) - assert ( - expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect - == { - key1: value1, - key2: value2, - key3: value3, - key4: value4, - key5: value5, - key6: value6, - key7: value7, - key8: value8, - key9: value9, - } - ) + assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == { + key1: value1, + key2: value2, + key3: value3, + key4: value4, + key5: value5, + key6: value6, + key7: value7, + key8: value8, + key9: value9, + } diff --git a/tests/data/long_strings_flag_disabled.py b/tests/data/long_strings_flag_disabled.py index db3954e3abd..ef3094fd779 100644 --- a/tests/data/long_strings_flag_disabled.py +++ b/tests/data/long_strings_flag_disabled.py @@ -133,14 +133,11 @@ "Use f-strings instead!", ) -old_fmt_string3 = ( - "Whereas only the strings after the percent sign were long in the last example, this example uses a long initial string as well. This is another %s %s %s %s" - % ( - "really really really really really", - "old", - "way to format strings!", - "Use f-strings instead!", - ) +old_fmt_string3 = "Whereas only the strings after the percent sign were long in the last example, this example uses a long initial string as well. This is another %s %s %s %s" % ( + "really really really really really", + "old", + "way to format strings!", + "Use f-strings instead!", ) fstring = f"f-strings definitely make things more {difficult} than they need to be for {{black}}. But boy they sure are handy. The problem is that some lines will need to have the 'f' whereas others do not. This {line}, for example, needs one." From 6bb74e082d7e2cc6f39719378ba03fdf39ff56a1 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 12:44:24 +0000 Subject: [PATCH 70/96] Cleanup: remove redundant 'yield' statement --- src/black/__init__.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 8e43835e594..79faaf73df2 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5966,9 +5966,6 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf opening_bracket = leaf.opening_bracket closing_bracket = leaf - if line.magic_trailing_comma: - yield omit - def get_future_imports(node: Node) -> Set[str]: """Return a set of __future__ imports in the file.""" From 695ecf157528334cbfbe430c8fabc6f50abdff8c Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 12:45:31 +0000 Subject: [PATCH 71/96] Cleanup: remove redundant magic comma value unassignment --- src/black/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 79faaf73df2..ad6f955e93d 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5955,7 +5955,6 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf if closing_bracket: if line.magic_trailing_comma: if closing_bracket is line.magic_trailing_comma or inner_brackets: - line.magic_trailing_comma = None yield omit omit.add(id(closing_bracket)) omit.update(inner_brackets) From cb4ce1b03c21fd1b279325d0317a004c090eb013 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 12:49:32 +0000 Subject: [PATCH 72/96] Cleanup: remove redundant conditional check --- src/black/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index ad6f955e93d..90ebbdf5de1 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5953,9 +5953,8 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf continue if closing_bracket: - if line.magic_trailing_comma: - if closing_bracket is line.magic_trailing_comma or inner_brackets: - yield omit + if closing_bracket is line.magic_trailing_comma or inner_brackets: + yield omit omit.add(id(closing_bracket)) omit.update(inner_brackets) inner_brackets.clear() From 7f3375f0e546411ed56e5630b02ad94da1a152b3 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 12:58:49 +0000 Subject: [PATCH 73/96] black-primer configuration: indicate that tox would require formatting updates under this changeset --- src/black_primer/primer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index 92dce20c69a..6ccc8f8a6af 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -105,7 +105,7 @@ }, "tox": { "cli_arguments": [], - "expect_formatting_changes": false, + "expect_formatting_changes": true, "git_clone_url": "https://github.com/tox-dev/tox.git", "long_checkout": false, "py_versions": ["all"] From a8df5154009d10b65edb305b93133b6c8dbac10e Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 13:00:39 +0000 Subject: [PATCH 74/96] Cleanup: remove redundant 'return_stmt' value test --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 90ebbdf5de1..59fff0c344f 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5380,7 +5380,7 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break - elif node.type in [syms.assert_stmt, syms.if_stmt, syms.return_stmt]: + elif node.type in [syms.assert_stmt, syms.if_stmt]: pass elif not (isinstance(child, Leaf) and is_multiline_string(child)): From ae519cafd0020a9be4306edde68cd304d1e1494c Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 15:59:18 +0000 Subject: [PATCH 75/96] Isolate logically separate conditional paths --- src/black/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 59fff0c344f..cefc5a45714 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5953,7 +5953,9 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf continue if closing_bracket: - if closing_bracket is line.magic_trailing_comma or inner_brackets: + if closing_bracket is line.magic_trailing_comma: + yield omit + if inner_brackets: yield omit omit.add(id(closing_bracket)) omit.update(inner_brackets) From 630771afe5a90411b311e343f4c97ff12649aac7 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 16:00:06 +0000 Subject: [PATCH 76/96] Reduce visits to 'inner_brackets' conditional path --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index cefc5a45714..750092d7cd7 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5955,7 +5955,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf if closing_bracket: if closing_bracket is line.magic_trailing_comma: yield omit - if inner_brackets: + if line.magic_trailing_comma and inner_brackets: yield omit omit.add(id(closing_bracket)) omit.update(inner_brackets) From 169ef3816f5136cab5ad2f0b19e1cabbb0ce0db6 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 16:11:35 +0000 Subject: [PATCH 77/96] Add explanatory and placeholder comments --- src/black/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 750092d7cd7..aec81219cb4 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5953,8 +5953,10 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf continue if closing_bracket: + # Omit tokens to the right of brackets that contain a trailing comma if closing_bracket is line.magic_trailing_comma: yield omit + # TODO: Understand and explain this line; are there edge cases here? if line.magic_trailing_comma and inner_brackets: yield omit omit.add(id(closing_bracket)) From 3f8673dab1733369f011bc8e26bc65ba708ef474 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 16:56:14 +0000 Subject: [PATCH 78/96] Consistency: use a set to hold symbol types during membership test --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index aec81219cb4..eaa940079fd 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5380,7 +5380,7 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break - elif node.type in [syms.assert_stmt, syms.if_stmt]: + elif node.type in {syms.assert_stmt, syms.if_stmt}: pass elif not (isinstance(child, Leaf) and is_multiline_string(child)): From 4d5ba05f108070d85084ae880e4ec6e27969e148 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 10 Feb 2021 22:40:45 +0000 Subject: [PATCH 79/96] Include 'BracketMatchError' exception handler and comment from 586d24236e6b57bc3b5da85fdbe2563835021076, so that it remains clear that there is a known issue --- src/black/__init__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index eaa940079fd..c4e5539f000 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -3429,7 +3429,13 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]: new_line = line.clone() new_line.comments = line.comments.copy() - append_leaves(new_line, line, LL[: string_idx - 1]) + try: + append_leaves(new_line, line, LL[: string_idx - 1]) + except BracketMatchError: + # HACK: I believe there is currently a bug somewhere in + # right_hand_split() that is causing brackets to not be tracked + # properly by a shared BracketTracker. + append_leaves(new_line, line, LL[: string_idx - 1], preformatted=True) string_leaf = Leaf(token.STRING, LL[string_idx].value) LL[string_idx - 1].remove() From 8c19ff5bbfb0759619f669037e9ace4107ca36b4 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 11 Feb 2021 19:11:56 +0000 Subject: [PATCH 80/96] Revert incorrect test expectation updates --- tests/data/cantfit.py | 12 +++-- tests/data/composition.py | 25 ++++++----- tests/data/composition_no_trailing_comma.py | 50 ++++++++++++--------- tests/data/invisible_parens_instability.py | 1 + tests/data/long_strings__regression.py | 6 ++- 5 files changed, 55 insertions(+), 39 deletions(-) create mode 100644 tests/data/invisible_parens_instability.py diff --git a/tests/data/cantfit.py b/tests/data/cantfit.py index ef9b78e09a9..0849374f776 100644 --- a/tests/data/cantfit.py +++ b/tests/data/cantfit.py @@ -67,11 +67,15 @@ normal_name = ( but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying() ) -normal_name = but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( - arg1, arg2, arg3 +normal_name = ( + but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( + arg1, arg2, arg3 + ) ) -normal_name = but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( - [1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3 +normal_name = ( + but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( + [1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3 + ) ) # long arguments normal_name = normal_function_name( diff --git a/tests/data/composition.py b/tests/data/composition.py index 9ccbd100b25..e429f15e669 100644 --- a/tests/data/composition.py +++ b/tests/data/composition.py @@ -165,14 +165,17 @@ def tricky_asserts(self) -> None: _C.__init__.__code__.co_firstlineno + 1, ) - assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == { - key1: value1, - key2: value2, - key3: value3, - key4: value4, - key5: value5, - key6: value6, - key7: value7, - key8: value8, - key9: value9, - } + assert ( + expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect + == { + key1: value1, + key2: value2, + key3: value3, + key4: value4, + key5: value5, + key6: value6, + key7: value7, + key8: value8, + key9: value9, + } + ) diff --git a/tests/data/composition_no_trailing_comma.py b/tests/data/composition_no_trailing_comma.py index 1cea37d8641..f17b89dea8d 100644 --- a/tests/data/composition_no_trailing_comma.py +++ b/tests/data/composition_no_trailing_comma.py @@ -165,17 +165,20 @@ def tricky_asserts(self) -> None: _C.__init__.__code__.co_firstlineno + 1, ) - assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == { - key1: value1, - key2: value2, - key3: value3, - key4: value4, - key5: value5, - key6: value6, - key7: value7, - key8: value8, - key9: value9 - } + assert ( + expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect + == { + key1: value1, + key2: value2, + key3: value3, + key4: value4, + key5: value5, + key6: value6, + key7: value7, + key8: value8, + key9: value9 + } + ) @@ -348,14 +351,17 @@ def tricky_asserts(self) -> None: _C.__init__.__code__.co_firstlineno + 1, ) - assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == { - key1: value1, - key2: value2, - key3: value3, - key4: value4, - key5: value5, - key6: value6, - key7: value7, - key8: value8, - key9: value9, - } + assert ( + expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect + == { + key1: value1, + key2: value2, + key3: value3, + key4: value4, + key5: value5, + key6: value6, + key7: value7, + key8: value8, + key9: value9, + } + ) diff --git a/tests/data/invisible_parens_instability.py b/tests/data/invisible_parens_instability.py new file mode 100644 index 00000000000..d73379996ea --- /dev/null +++ b/tests/data/invisible_parens_instability.py @@ -0,0 +1 @@ +assert function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15) != [None] diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py index ad44a16ccaa..2e7f2483b63 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -347,7 +347,7 @@ def xxxxxxx_xxxxxx(xxxx): assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, - ], "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx + ], ("xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx) value.__dict__[ key @@ -828,7 +828,9 @@ def xxxxxxx_xxxxxx(xxxx): assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, - ], "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx + ], ( + "xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx + ) value.__dict__[ From 1096bc908d56a6013c82f626f08f2915a38ecd32 Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 12 Feb 2021 12:56:49 +0000 Subject: [PATCH 81/96] Remove special-case invisible parentheses logic related to magic comma presence --- src/black/__init__.py | 4 ---- tests/data/long_strings_flag_disabled.py | 13 ++++++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index fa83e1577bf..aafbc608ed8 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6634,10 +6634,6 @@ def can_omit_invisible_parens( # unnecessary. return True - if line.magic_trailing_comma and penultimate.type == token.COMMA: - # The rightmost non-omitted bracket pair is the one we want to explode on. - return True - if _can_omit_closing_paren(line, last=last, line_length=line_length): return True diff --git a/tests/data/long_strings_flag_disabled.py b/tests/data/long_strings_flag_disabled.py index ef3094fd779..db3954e3abd 100644 --- a/tests/data/long_strings_flag_disabled.py +++ b/tests/data/long_strings_flag_disabled.py @@ -133,11 +133,14 @@ "Use f-strings instead!", ) -old_fmt_string3 = "Whereas only the strings after the percent sign were long in the last example, this example uses a long initial string as well. This is another %s %s %s %s" % ( - "really really really really really", - "old", - "way to format strings!", - "Use f-strings instead!", +old_fmt_string3 = ( + "Whereas only the strings after the percent sign were long in the last example, this example uses a long initial string as well. This is another %s %s %s %s" + % ( + "really really really really really", + "old", + "way to format strings!", + "Use f-strings instead!", + ) ) fstring = f"f-strings definitely make things more {difficult} than they need to be for {{black}}. But boy they sure are handy. The problem is that some lines will need to have the 'f' whereas others do not. This {line}, for example, needs one." From 16d25057814b4a92aa9c88441a5e4715850c16b9 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 13 Feb 2021 11:33:24 +0000 Subject: [PATCH 82/96] Include removal of line_length=1 from 586d24236e6b57bc3b5da85fdbe2563835021076 --- src/black/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 761a22de781..fe3deb61733 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -2742,10 +2742,11 @@ def rhs(line: Line, features: Collection[Feature]) -> Iterator[Line]: # All splits failed, best effort split with no omits. # This mostly happens to multiline strings that are by definition - # reported as not fitting a single line. - # line_length=1 here was historically a bug that somehow became a feature. - # See #762 and #781 for the full story. - yield from right_hand_split(line, line_length=1, features=features) + # reported as not fitting a single line, as well as lines that contain + # trailing commas (those have to be exploded). + yield from right_hand_split( + line, line_length=mode.line_length, features=features + ) if mode.experimental_string_processing: if line.inside_brackets: From fe64f19cb5f5deaaaefdb874cbdf5eb93e4ff06e Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 13 Feb 2021 11:44:57 +0000 Subject: [PATCH 83/96] Update test expectations (note: this includes output that exceeds the line length limit) --- tests/data/cantfit.py | 12 +++----- tests/data/collections.py | 7 ++--- tests/data/composition.py | 34 ++++++++++----------- tests/data/composition_no_trailing_comma.py | 34 ++++++++++----------- 4 files changed, 41 insertions(+), 46 deletions(-) diff --git a/tests/data/cantfit.py b/tests/data/cantfit.py index 0849374f776..ef9b78e09a9 100644 --- a/tests/data/cantfit.py +++ b/tests/data/cantfit.py @@ -67,15 +67,11 @@ normal_name = ( but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying() ) -normal_name = ( - but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( - arg1, arg2, arg3 - ) +normal_name = but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( + arg1, arg2, arg3 ) -normal_name = ( - but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( - [1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3 - ) +normal_name = but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( + [1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3 ) # long arguments normal_name = normal_function_name( diff --git a/tests/data/collections.py b/tests/data/collections.py index 68431665211..99269c45891 100644 --- a/tests/data/collections.py +++ b/tests/data/collections.py @@ -148,10 +148,9 @@ print("foo %r", (foo.bar,)) if True: - IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = ( - Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING - | {pylons.controllers.WSGIController} - ) + IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING | { + pylons.controllers.WSGIController + } if True: ec2client.get_waiter("instance_stopped").wait( diff --git a/tests/data/composition.py b/tests/data/composition.py index e429f15e669..cee4d67fb00 100644 --- a/tests/data/composition.py +++ b/tests/data/composition.py @@ -39,9 +39,12 @@ def test(self) -> None: # Only send the first n items. items=items[:num_items] ) - return ( - 'Utterly failed doctest test for %s\n File "%s", line %s, in %s\n\n%s' - % (test.name, test.filename, lineno, lname, err) + return 'Utterly failed doctest test for %s\n File "%s", line %s, in %s\n\n%s' % ( + test.name, + test.filename, + lineno, + lname, + err, ) def omitting_trailers(self) -> None: @@ -165,17 +168,14 @@ def tricky_asserts(self) -> None: _C.__init__.__code__.co_firstlineno + 1, ) - assert ( - expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect - == { - key1: value1, - key2: value2, - key3: value3, - key4: value4, - key5: value5, - key6: value6, - key7: value7, - key8: value8, - key9: value9, - } - ) + assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == { + key1: value1, + key2: value2, + key3: value3, + key4: value4, + key5: value5, + key6: value6, + key7: value7, + key8: value8, + key9: value9, + } diff --git a/tests/data/composition_no_trailing_comma.py b/tests/data/composition_no_trailing_comma.py index f17b89dea8d..6d53cda5154 100644 --- a/tests/data/composition_no_trailing_comma.py +++ b/tests/data/composition_no_trailing_comma.py @@ -225,9 +225,12 @@ def test(self) -> None: # Only send the first n items. items=items[:num_items] ) - return ( - 'Utterly failed doctest test for %s\n File "%s", line %s, in %s\n\n%s' - % (test.name, test.filename, lineno, lname, err) + return 'Utterly failed doctest test for %s\n File "%s", line %s, in %s\n\n%s' % ( + test.name, + test.filename, + lineno, + lname, + err, ) def omitting_trailers(self) -> None: @@ -351,17 +354,14 @@ def tricky_asserts(self) -> None: _C.__init__.__code__.co_firstlineno + 1, ) - assert ( - expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect - == { - key1: value1, - key2: value2, - key3: value3, - key4: value4, - key5: value5, - key6: value6, - key7: value7, - key8: value8, - key9: value9, - } - ) + assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == { + key1: value1, + key2: value2, + key3: value3, + key4: value4, + key5: value5, + key6: value6, + key7: value7, + key8: value8, + key9: value9, + } From b06158931331b9144a4cc33ee292d271063d449b Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 13 Feb 2021 11:56:45 +0000 Subject: [PATCH 84/96] Remove special-case handling for 'assert' and 'if' statements --- src/black/__init__.py | 2 -- tests/data/function_trailing_comma.py | 23 +++++++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index fe3deb61733..c06e4a6cf26 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -5387,8 +5387,6 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None: node.insert_child(index, Leaf(token.LPAR, "")) node.append_child(Leaf(token.RPAR, "")) break - elif node.type in {syms.assert_stmt, syms.if_stmt}: - pass elif not (isinstance(child, Leaf) and is_multiline_string(child)): wrap_in_parentheses(node, child, visible=False) diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index 1db4b591a7e..4be4383b0e6 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -70,16 +70,19 @@ def f( "a": 1, "b": 2, }["a"] - if a == { - "a": 1, - "b": 2, - "c": 3, - "d": 4, - "e": 5, - "f": 6, - "g": 7, - "h": 8, - }["a"]: + if ( + a + == { + "a": 1, + "b": 2, + "c": 3, + "d": 4, + "e": 5, + "f": 6, + "g": 7, + "h": 8, + }["a"] + ): pass if a == { "a": 1, From 60d5191bc190200348ec097b7ae911af7a2cd651 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 13 Feb 2021 12:22:20 +0000 Subject: [PATCH 85/96] Restore last_two_except logic from 586d24236e6b57bc3b5da85fdbe2563835021076 (with minor modifications) --- src/black/__init__.py | 44 ++++++++++++++++++++++++--- tests/data/function_trailing_comma.py | 23 ++++++-------- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index c06e4a6cf26..eed5656d513 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -4174,9 +4174,10 @@ def _return_match(LL: List[Leaf]) -> Optional[int]: """ # If this line is apart of a return/yield statement and the first leaf # contains either the "return" or "yield" keywords... - if parent_type(LL[0]) in [syms.return_stmt, syms.yield_expr] and LL[ - 0 - ].value in ["return", "yield"]: + if ( + parent_type(LL[0]) in [syms.return_stmt, syms.yield_expr] + and LL[0].value in ["return", "yield"] + ): is_valid_index = is_valid_index_factory(LL) idx = 2 if is_valid_index(1) and is_empty_par(LL[1]) else 1 @@ -4913,7 +4914,7 @@ def right_hand_split( # there are no standalone comments in the body and not body.contains_standalone_comments(0) # and we can actually remove the parens - and can_omit_invisible_parens(body, line_length) + and can_omit_invisible_parens(body, line_length, omit) ): omit = {id(closing_bracket), *omit} try: @@ -6552,7 +6553,9 @@ def can_be_split(line: Line) -> bool: return True -def can_omit_invisible_parens(line: Line, line_length: int) -> bool: +def can_omit_invisible_parens( + line: Line, line_length: int, omit: Collection[LeafID] +) -> bool: """Does `line` have a shape safe to reformat without optional parens around it? Returns True for only a subset of potentially nice looking formattings but @@ -6589,6 +6592,16 @@ def can_omit_invisible_parens(line: Line, line_length: int) -> bool: penultimate = line.leaves[-2] last = line.leaves[-1] + if omit: + try: + penultimate, last = last_two_except(line.leaves, omit) + except LookupError: + # Turns out we'd omit everything. We cannot skip the optional parentheses. + return False + + # TODO: Is this reasonable? + if last.type in COMPARATORS: + return True if ( last.type == token.RPAR @@ -6658,6 +6671,27 @@ def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool return False +def last_two_except(leaves: List[Leaf], omit: Collection[LeafID]) -> Tuple[Leaf, Leaf]: + """Return (penultimate, last) leaves skipping brackets in `omit` and contents.""" + stop_after = None + last = None + for leaf in reversed(leaves): + if stop_after: + if leaf is stop_after: + stop_after = None + continue + + if last: + return leaf, last + + if id(leaf) in omit: + stop_after = leaf.opening_bracket + else: + last = leaf + else: + raise LookupError("Last two leaves were also skipped") + + def get_cache_file(mode: Mode) -> Path: return CACHE_DIR / f"cache.{mode.get_cache_key()}.pickle" diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index 4be4383b0e6..1db4b591a7e 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -70,19 +70,16 @@ def f( "a": 1, "b": 2, }["a"] - if ( - a - == { - "a": 1, - "b": 2, - "c": 3, - "d": 4, - "e": 5, - "f": 6, - "g": 7, - "h": 8, - }["a"] - ): + if a == { + "a": 1, + "b": 2, + "c": 3, + "d": 4, + "e": 5, + "f": 6, + "g": 7, + "h": 8, + }["a"]: pass if a == { "a": 1, From 6b10579caa117410a99aa4b1e64f4cc13c0e5474 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 13 Feb 2021 12:29:37 +0000 Subject: [PATCH 86/96] Also allow omission of invisible parentheses for math operators --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index eed5656d513..a41e2d246b1 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6600,7 +6600,7 @@ def can_omit_invisible_parens( return False # TODO: Is this reasonable? - if last.type in COMPARATORS: + if last.type in COMPARATORS or last.type in MATH_OPERATORS: return True if ( From eab9b2b38d91cccf6ea1a7270b02f7466a295969 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 13 Feb 2021 12:36:01 +0000 Subject: [PATCH 87/96] Remove outdated comment in test data --- tests/data/function_trailing_comma.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py index 1db4b591a7e..7dbbd855633 100644 --- a/tests/data/function_trailing_comma.py +++ b/tests/data/function_trailing_comma.py @@ -17,7 +17,6 @@ def f(a:int=1,): pass if a == {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}: pass - # TODO: Why didn't 'while' require special normalize_invisible_parens handling, like assert/return? while a in {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}: pass assert val[-1] in ( @@ -92,7 +91,6 @@ def f( "h": 8, }: pass - # TODO: Why didn't 'while' require special normalize_invisible_parens handling, like assert/return? while a in { "a": 1, "b": 2, From c2bf24b13dbdfde9f62c07daabc9d95bf2d5a79c Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 13 Feb 2021 12:42:11 +0000 Subject: [PATCH 88/96] black-primer configuration: indicate that tox would not require formatting updates under this changeset --- src/black_primer/primer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index 6ccc8f8a6af..92dce20c69a 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -105,7 +105,7 @@ }, "tox": { "cli_arguments": [], - "expect_formatting_changes": true, + "expect_formatting_changes": false, "git_clone_url": "https://github.com/tox-dev/tox.git", "long_checkout": false, "py_versions": ["all"] From 09560600e006ebac17621a15720e311f5eb72712 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sat, 13 Feb 2021 12:42:24 +0000 Subject: [PATCH 89/96] black-primer configuration: indicate that bandersnatch would require formatting updates under this changeset --- src/black_primer/primer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black_primer/primer.json b/src/black_primer/primer.json index 92dce20c69a..d7e998c6d0f 100644 --- a/src/black_primer/primer.json +++ b/src/black_primer/primer.json @@ -17,7 +17,7 @@ }, "bandersnatch": { "cli_arguments": [], - "expect_formatting_changes": false, + "expect_formatting_changes": true, "git_clone_url": "https://github.com/pypa/bandersnatch.git", "long_checkout": false, "py_versions": ["all"] From b0aaff051f1c1925d7654dac657b3a24450d9905 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 17 Feb 2021 12:45:06 +0000 Subject: [PATCH 90/96] Cleanup: remove 'test_function' test case; formatting of the 'function' test module is covered by the 'test_simple_format' test case --- tests/test_black.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/test_black.py b/tests/test_black.py index 845bfdfa89b..40c7cd64fa3 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -218,14 +218,6 @@ def test_piping_diff_with_color(self) -> None: self.assertIn("\033[31m", actual) self.assertIn("\033[0m", actual) - @patch("black.dump_to_file", dump_to_stderr) - def test_function(self) -> None: - source, expected = read_data("function") - actual = fs(source) - self.assertFormatEqual(expected, actual) - black.assert_equivalent(source, actual) - black.assert_stable(source, actual, DEFAULT_MODE) - @patch("black.dump_to_file", dump_to_stderr) def test_trailing_comma_optional_parens_stability1(self) -> None: source, _expected = read_data("trailing_comma_optional_parens1") From 10d56e57305b7445e21088ee938b625a9e1aec2a Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 17 Feb 2021 12:47:25 +0000 Subject: [PATCH 91/96] Cleanup: restore doubling of the 'stack' variable from 586d24236e6b57bc3b5da85fdbe2563835021076 --- tests/test_black.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_black.py b/tests/test_black.py index 40c7cd64fa3..adf2dedff89 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -1794,6 +1794,7 @@ def tracefunc(frame: types.FrameType, event: str, arg: Any) -> Callable: return tracefunc stack = len(inspect.stack()) - 19 + stack *= 2 filename = frame.f_code.co_filename lineno = frame.f_lineno func_sig_lineno = lineno - 1 From 17e95404b83eb198943c542a485a2fd541239c60 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 21 Feb 2021 11:24:19 +0000 Subject: [PATCH 92/96] Cleanup: remove unused test module --- tests/data/invisible_parens_instability.py | 1 - 1 file changed, 1 deletion(-) delete mode 100644 tests/data/invisible_parens_instability.py diff --git a/tests/data/invisible_parens_instability.py b/tests/data/invisible_parens_instability.py deleted file mode 100644 index d73379996ea..00000000000 --- a/tests/data/invisible_parens_instability.py +++ /dev/null @@ -1 +0,0 @@ -assert function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15) != [None] From 538677315dd50b846f1c96cbc9a695810ac35b79 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 21 Feb 2021 12:34:42 +0000 Subject: [PATCH 93/96] Undo method signature changes to 'can_omit_invisible_parens' since it is public library method by naming convention --- src/black/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 6a8a0da8224..996fc7c0285 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -6578,7 +6578,9 @@ def can_be_split(line: Line) -> bool: def can_omit_invisible_parens( - line: Line, line_length: int, omit: Collection[LeafID] + line: Line, + line_length: int, + omit_on_explode: Collection[LeafID] = (), ) -> bool: """Does `line` have a shape safe to reformat without optional parens around it? @@ -6616,9 +6618,9 @@ def can_omit_invisible_parens( penultimate = line.leaves[-2] last = line.leaves[-1] - if omit: + if omit_on_explode: try: - penultimate, last = last_two_except(line.leaves, omit) + penultimate, last = last_two_except(line.leaves, omit=omit_on_explode) except LookupError: # Turns out we'd omit everything. We cannot skip the optional parentheses. return False From f7bec57609106882fd99574bbc1b1d1613bb1efa Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 21 Feb 2021 12:42:54 +0000 Subject: [PATCH 94/96] Cleanup: undo change to use implicit argument name --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 996fc7c0285..fc86468437b 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -4916,7 +4916,7 @@ def right_hand_split( # there are no standalone comments in the body and not body.contains_standalone_comments(0) # and we can actually remove the parens - and can_omit_invisible_parens(body, line_length, omit) + and can_omit_invisible_parens(body, line_length, omit_on_explode=omit) ): omit = {id(closing_bracket), *omit} try: From 3cc7102137f1570d229d7237392e3a952330d6cd Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 22 Feb 2021 11:04:21 +0000 Subject: [PATCH 95/96] Add changelog entry --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 3fd7ad40496..3bb2932c2b3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -30,6 +30,8 @@ - `--diff` correctly indicates when a file doesn't end in a newline (#1662) +- Reduce second-pass formatting stability errors (#1629) + #### _Packaging_ - Self-contained native _Black_ binaries are now provided for releases via GitHub From b48e35c16a549339b9a921981d3c4c2f8a55a801 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 3 Mar 2021 14:20:39 +0000 Subject: [PATCH 96/96] Fixup for incorrect changelog merge conflict resolution --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 35273845a26..f5aa0c1d47d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -31,9 +31,9 @@ - `--diff` correctly indicates when a file doesn't end in a newline (#1662) - Added `--stdin-filename` argument to allow stdin to respect `--force-exclude` rules + (#1780) - Reduce second-pass formatting stability errors (#1629) - (#1780) #### _Packaging_