Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stability fixup: address stability issues relating to interaction between magic commas and invisible parentheses #1958

Closed
wants to merge 114 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
114 commits
Select commit Hold shift + click to select a range
fa0b64d
Add illustrative test data
jayaddison Feb 3, 2021
21842f2
Fixup: handling of invisible parentheses for assert and return statem…
jayaddison Feb 3, 2021
fdff084
Re-enable test_trailing_comma_optional_parens_stability3 since it has…
jayaddison Feb 3, 2021
0bb718c
Use a 'pass' statement since 'maybe_make_parens_invisible_in_atom' cu…
jayaddison Feb 3, 2021
82ab09d
Do not omit any tokens when selecting the last two leaves
jayaddison Feb 3, 2021
f154042
Cleanup: remove slightly arbitrary 'pass' handling for assert and ret…
jayaddison Feb 3, 2021
d262d29
Update output expectation for trailing comma reformatting test case
jayaddison Feb 3, 2021
23facce
Cleanup: remove redundant code path
jayaddison Feb 3, 2021
543fc7e
Update primer.json to permit formatting changes to be passed by conti…
jayaddison Feb 3, 2021
27e1f05
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Feb 4, 2021
6c6d262
Temporarily revert src directory changes
jayaddison Feb 4, 2021
2152848
Add additional test cases from psf/black#1629
jayaddison Feb 4, 2021
a23e87d
Revert "Temporarily revert src directory changes"
jayaddison Feb 4, 2021
065abbf
Revert "Update primer.json to permit formatting changes to be passed …
jayaddison Feb 4, 2021
10015f6
Revert "Cleanup: remove redundant code path"
jayaddison Feb 4, 2021
16bf728
Revert "Update output expectation for trailing comma reformatting tes…
jayaddison Feb 4, 2021
f4ee9dc
Revert "Cleanup: remove slightly arbitrary 'pass' handling for assert…
jayaddison Feb 4, 2021
4c2947f
Revert "Do not omit any tokens when selecting the last two leaves"
jayaddison Feb 4, 2021
461a828
Revert "Use a 'pass' statement since 'maybe_make_parens_invisible_in_…
jayaddison Feb 4, 2021
07f5796
Revert "Re-enable test_trailing_comma_optional_parens_stability3 sinc…
jayaddison Feb 4, 2021
39caefb
Revert "Fixup: handling of invisible parentheses for assert and retur…
jayaddison Feb 4, 2021
7c1482d
Remove example code that raises a Python TypeError during evaluation
jayaddison Feb 4, 2021
ab07b03
Indicate that failures are expected in the 'test_invisible_parens_ins…
jayaddison Feb 4, 2021
1109e33
Brevity: rename method
jayaddison Feb 4, 2021
f6eab7c
Clarity: isolate and extract each responsibility from an overloaded v…
jayaddison Feb 4, 2021
d4a2e36
Brevity: only use the variables required to convey the intended expre…
jayaddison Feb 4, 2021
ebbd3e8
Consistency: use variable names that correspond to the methods they i…
jayaddison Feb 4, 2021
d806503
Clarity: special case: avoid using variables that have the same names…
jayaddison Feb 4, 2021
7dba4ba
Simplification: only use special-case token retrieval logic when magi…
jayaddison Feb 4, 2021
27a2dfe
Simplification: only yield empty omit list when magic trailing comma …
jayaddison Feb 4, 2021
3a45017
Cleanup: remove unused / redundant variables from conditionals
jayaddison Feb 4, 2021
4ff708f
Merge branch 'refactor/distinguished-variable-responsibilities' into …
jayaddison Feb 4, 2021
95677ff
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Feb 5, 2021
e9a7326
Revert "Address pre-existing trailing commas when not in the rightmos…
jayaddison Feb 5, 2021
8b997a8
Unset failure expectations on a few test cases
jayaddison Feb 5, 2021
ec1b0ab
Work-in-progress: investigate bracket matching
jayaddison Feb 5, 2021
b90268e
Revert "Work-in-progress: investigate bracket matching"
jayaddison Feb 5, 2021
268e3f3
Fixup: handling of invisible parentheses for assert and return statem…
jayaddison Feb 3, 2021
0596b38
Use a 'pass' statement since 'maybe_make_parens_invisible_in_atom' cu…
jayaddison Feb 3, 2021
58c3cce
Expectation adjustment: remove parentheses around pre-f-string format…
jayaddison Feb 5, 2021
235b854
Restore latest function_trailing_comma tests
jayaddison Feb 6, 2021
e245b1a
Add additional test coverage
jayaddison Feb 7, 2021
acffae3
When a magic comma is present in the line, delay emission of empty tr…
jayaddison Feb 7, 2021
d036da4
Add additional test coverage
jayaddison Feb 7, 2021
7da423e
Add special-case handling of 'if' statements within normalize_invisib…
jayaddison Feb 7, 2021
634b1a3
When a magic comma is present in the line, emit an empty trailer when…
jayaddison Feb 7, 2021
e8ad631
black-primer configuration: indicate that sqlalchemy would require fo…
jayaddison Feb 7, 2021
48ac43f
Include removal of work-in-progress test from 586d24236e6b57bc3b5da85…
jayaddison Feb 7, 2021
62b76ca
Revert "Add additional test cases from psf/black#1629"
jayaddison Feb 7, 2021
9ff5f98
black-primer configuration: indicate that flake8-bugbear would requir…
jayaddison Feb 7, 2021
176a96a
Re-apply test re-organization cleanup of 'function2' test from e6cd10…
jayaddison Feb 7, 2021
b93b955
Apply test organization patterns from e6cd10e7615f4df537e2eaefcf3904a…
jayaddison Feb 7, 2021
e96a683
Nitpick: rename the test; it should be considered a test of stability…
jayaddison Feb 7, 2021
6763046
Include line comment copy behaviour fixup from 586d24236e6b57bc3b5da8…
jayaddison Feb 7, 2021
0aa8083
Cleanup: yield existing 'omit' variable rather than literal empty sets
jayaddison Feb 8, 2021
efc7552
Readability: reduce boolean nesting
jayaddison Feb 9, 2021
0666fc1
Merge branch 'refactor/distinguished-variable-responsibilities' into …
jayaddison Feb 9, 2021
b2a4a29
Include explanatory comments in 'rhs' method from 586d24236e6b57bc3b5…
jayaddison Feb 9, 2021
934c79d
Include short-circuit condition in 'is_one_tuple_between' method from…
jayaddison Feb 9, 2021
c16e29a
Include extracted '_can_omit_opening_paren', '_can_omit_closing_paren…
jayaddison Feb 9, 2021
7dfb59a
Relocate 'last', 'penultimate' variable declarations nearer to first use
jayaddison Feb 9, 2021
a62b47a
Merge branch 'master' into refactor/distinguished-variable-responsibi…
jayaddison Feb 9, 2021
6423a8d
Relocate comment to minimize review diff size
jayaddison Feb 9, 2021
82ca108
Merge branch 'refactor/distinguished-variable-responsibilities' into …
jayaddison Feb 9, 2021
8343663
Remove empty line
jayaddison Feb 9, 2021
66ef77a
Cleanup: undo accidental changes to remainder logic in '_can_omit_ope…
jayaddison Feb 9, 2021
4126cdd
Restore redundant condition in order to further minimize review diff …
jayaddison Feb 9, 2021
34cce10
Restore empty line
jayaddison Feb 9, 2021
8fcfcbf
Restore explicit return value
jayaddison Feb 9, 2021
0adad3a
Include extraction of 'prev' variable within 'generate_trailers_to_om…
jayaddison Feb 9, 2021
788002a
Restore empty line
jayaddison Feb 9, 2021
879cbe2
Remove empty line
jayaddison Feb 9, 2021
c7d7f59
Restore accurate comment
jayaddison Feb 9, 2021
9f0289a
Minimize changes: more closely resemble original conditional logic
jayaddison Feb 10, 2021
4d39520
Merge branch 'refactor/distinguished-variable-responsibilities' into …
jayaddison Feb 10, 2021
b4c9238
Update test expectations
jayaddison Feb 10, 2021
6bb74e0
Cleanup: remove redundant 'yield' statement
jayaddison Feb 10, 2021
695ecf1
Cleanup: remove redundant magic comma value unassignment
jayaddison Feb 10, 2021
cb4ce1b
Cleanup: remove redundant conditional check
jayaddison Feb 10, 2021
7f3375f
black-primer configuration: indicate that tox would require formattin…
jayaddison Feb 10, 2021
a8df515
Cleanup: remove redundant 'return_stmt' value test
jayaddison Feb 10, 2021
ae519ca
Isolate logically separate conditional paths
jayaddison Feb 10, 2021
630771a
Reduce visits to 'inner_brackets' conditional path
jayaddison Feb 10, 2021
169ef38
Add explanatory and placeholder comments
jayaddison Feb 10, 2021
c74a1c5
Merge branch 'master' into refactor/distinguished-variable-responsibi…
jayaddison Feb 10, 2021
15788c5
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Feb 10, 2021
3f8673d
Consistency: use a set to hold symbol types during membership test
jayaddison Feb 10, 2021
4d5ba05
Include 'BracketMatchError' exception handler and comment from 586d24…
jayaddison Feb 10, 2021
8c19ff5
Revert incorrect test expectation updates
jayaddison Feb 11, 2021
7c4a6a0
Merge branch 'master' into refactor/distinguished-variable-responsibi…
jayaddison Feb 12, 2021
1096bc9
Remove special-case invisible parentheses logic related to magic comm…
jayaddison Feb 12, 2021
e04ed95
Merge branch 'cleanup/remove-special-case-invisible-parens-handling' …
jayaddison Feb 13, 2021
16d2505
Include removal of line_length=1 from 586d24236e6b57bc3b5da85fdbe2563…
jayaddison Feb 13, 2021
fe64f19
Update test expectations (note: this includes output that exceeds the…
jayaddison Feb 13, 2021
b061589
Remove special-case handling for 'assert' and 'if' statements
jayaddison Feb 13, 2021
60d5191
Restore last_two_except logic from 586d24236e6b57bc3b5da85fdbe2563835…
jayaddison Feb 13, 2021
6b10579
Also allow omission of invisible parentheses for math operators
jayaddison Feb 13, 2021
eab9b2b
Remove outdated comment in test data
jayaddison Feb 13, 2021
c2bf24b
black-primer configuration: indicate that tox would not require forma…
jayaddison Feb 13, 2021
0956060
black-primer configuration: indicate that bandersnatch would require …
jayaddison Feb 13, 2021
ae45ee3
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Feb 16, 2021
b0aaff0
Cleanup: remove 'test_function' test case; formatting of the 'functio…
jayaddison Feb 17, 2021
10d56e5
Cleanup: restore doubling of the 'stack' variable from 586d24236e6b57…
jayaddison Feb 17, 2021
3dabcc3
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Feb 20, 2021
17e9540
Cleanup: remove unused test module
jayaddison Feb 21, 2021
5386773
Undo method signature changes to 'can_omit_invisible_parens' since it…
jayaddison Feb 21, 2021
f7bec57
Cleanup: undo change to use implicit argument name
jayaddison Feb 21, 2021
5a2111a
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Feb 22, 2021
3cc7102
Add changelog entry
jayaddison Feb 22, 2021
d868154
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Feb 27, 2021
47fc731
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Feb 28, 2021
372b74c
Merge branch 'master' into stability/invisible-parens-assert-return
jayaddison Mar 3, 2021
b48e35c
Fixup for incorrect changelog merge conflict resolution
jayaddison Mar 3, 2021
bab0899
Merge branch 'master' into stability/invisible-parens-assert-return
ambv Apr 1, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/black/__init__.py
Expand Up @@ -6606,7 +6606,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
Expand Down Expand Up @@ -6683,7 +6683,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
Expand All @@ -6696,10 +6696,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")

Expand Down
24 changes: 14 additions & 10 deletions tests/data/function_trailing_comma.py
Expand Up @@ -23,6 +23,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[

# output


def f(
a,
):
Expand Down Expand Up @@ -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"]
):
jayaddison marked this conversation as resolved.
Show resolved Hide resolved
pass


Expand Down
1 change: 1 addition & 0 deletions 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]
9 changes: 6 additions & 3 deletions tests/test_black.py
Expand Up @@ -233,21 +233,24 @@ 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")
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")
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")
Expand Down