actions/black
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix instability due to trailing comma logic (#2572)

Browse Source 
It was causing stability issues because the first pass
could cause a "magic trailing comma" to appear, meaning
that the second pass might get a different result. It's
not critical.

Some things format differently (with extra parens)
This commit is contained in:
Nipunn Koorapati 2022-01-28 18:13:18 -08:00 committed by GitHub
parent 95e77cb559
commit a24e1f7959
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 72 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGES.md
View File

@ -139,6 +139,7 @@ and the first release covered by our new stability policy.
when `--target-version py310` is explicitly specified (#2586)
- Add support for parenthesized with (#2586)
- Declare support for Python 3.10 for running Black (#2562)
- Fix unstable black runs around magic trailing comma (#2572)
### Integrations

6
src/black/__init__.py
View File

@ -1332,7 +1332,7 @@ def get_imports_from_children(children: List[LN]) -> Generator[str, None, None]:
return imports
def assert_equivalent(src: str, dst: str, *, pass_num: int = 1) -> None:
def assert_equivalent(src: str, dst: str) -> None:
"""Raise AssertionError if `src` and `dst` aren't equivalent."""
try:
src_ast = parse_ast(src)
@ -1349,7 +1349,7 @@ def assert_equivalent(src: str, dst: str, *, pass_num: int = 1) -> None:
except Exception as exc:
log = dump_to_file("".join(traceback.format_tb(exc.__traceback__)), dst)
raise AssertionError(
f"INTERNAL ERROR: Black produced invalid code on pass {pass_num}: {exc}. "
f"INTERNAL ERROR: Black produced invalid code: {exc}. "
"Please report a bug on https://github.com/psf/black/issues. "
f"This invalid output might be helpful: {log}"
) from None
@ -1360,7 +1360,7 @@ def assert_equivalent(src: str, dst: str, *, pass_num: int = 1) -> None:
log = dump_to_file(diff(src_ast_str, dst_ast_str, "src", "dst"))
raise AssertionError(
"INTERNAL ERROR: Black produced code that is not equivalent to the"
f" source on pass {pass_num}. Please report a bug on "
f" source. Please report a bug on "
f"https://github.com/psf/black/issues. This diff might be helpful: {log}"
) from None

2
src/black/linegen.py
View File

@ -543,7 +543,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:

14
src/black/lines.py
View File

@ -3,7 +3,6 @@
import sys
from typing import (
Callable,
Collection,
Dict,
Iterator,
List,
@ -22,7 +21,7 @@
from black.nodes import STANDALONE_COMMENT, TEST_DESCENDANTS
from black.nodes import BRACKETS, OPENING_BRACKETS, CLOSING_BRACKETS
from black.nodes import syms, whitespace, replace_child, child_towards
from black.nodes import is_multiline_string, is_import, is_type_comment, last_two_except
from black.nodes import is_multiline_string, is_import, is_type_comment
from black.nodes import is_one_tuple_between
# types
@ -645,7 +644,6 @@ def can_be_split(line: Line) -> bool:
def can_omit_invisible_parens(
line: Line,
line_length: int,
omit_on_explode: Collection[LeafID] = (),
) -> bool:
"""Does `line` have a shape safe to reformat without optional parens around it?
@ -683,12 +681,6 @@ def can_omit_invisible_parens(
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
@ -710,10 +702,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

23
src/black/nodes.py
View File

@ -4,13 +4,11 @@
import sys
from typing import (
Collection,
Generic,
Iterator,
List,
Optional,
Set,
Tuple,
TypeVar,
Union,
)
@ -439,27 +437,6 @@ def prev_siblings_are(node: Optional[LN], tokens: List[Optional[NodeType]]) -> b
return prev_siblings_are(node.prev_sibling, tokens[:-1])
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: Optional[Leaf] = None
last: Optional[Leaf] = 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 parent_type(node: Optional[LN]) -> Optional[NodeType]:
"""
Returns:

23
tests/data/function_trailing_comma.py
View File

@ -89,16 +89,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

13
tests/data/long_strings_flag_disabled.py
View File

@ -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."

25
tests/data/torture.py
View File

@ -31,20 +31,17 @@ def test(self, othr):
** 101234234242352525425252352352525234890264906820496920680926538059059209922523523525
) #
assert (
sort_by_dependency(
{
"1": {"2", "3"},
"2": {"2a", "2b"},
"3": {"3a", "3b"},
"2a": set(),
"2b": set(),
"3a": set(),
"3b": set(),
}
)
== ["2a", "2b", "2", "3a", "3b", "3", "1"]
)
assert sort_by_dependency(
{
"1": {"2", "3"},
"2": {"2a", "2b"},
"3": {"3a", "3b"},
"2a": set(),
"2b": set(),
"3a": set(),
"3b": set(),
}
) == ["2a", "2b", "2", "3a", "3b", "3", "1"]
importA
0

54
tests/data/trailing_comma_optional_parens1.py
View File

@ -2,6 +2,10 @@
_winapi.ERROR_PIPE_BUSY) or _check_timeout(t):
pass
if x:
if y:
new_id = max(Vegetable.objects.order_by('-id')[0].id,
Mineral.objects.order_by('-id')[0].id) + 1
class X:
def get_help_text(self):
@ -23,39 +27,37 @@ def b(self):
# output
if (
e1234123412341234.winerror
not in (
_winapi.ERROR_SEM_TIMEOUT,
_winapi.ERROR_PIPE_BUSY,
)
or _check_timeout(t)
):
if e1234123412341234.winerror not in (
_winapi.ERROR_SEM_TIMEOUT,
_winapi.ERROR_PIPE_BUSY,
) or _check_timeout(t):
pass
if x:
if y:
new_id = (
max(
Vegetable.objects.order_by("-id")[0].id,
Mineral.objects.order_by("-id")[0].id,
)
+ 1
)
class X:
def get_help_text(self):
return (
ngettext(
"Your password must contain at least %(min_length)d character.",
"Your password must contain at least %(min_length)d characters.",
self.min_length,
)
% {"min_length": self.min_length}
)
return ngettext(
"Your password must contain at least %(min_length)d character.",
"Your password must contain at least %(min_length)d characters.",
self.min_length,
) % {"min_length": self.min_length}
class A:
def b(self):
if (
self.connection.mysql_is_mariadb
and (
10,
4,
3,
)
< self.connection.mysql_version
< (10, 5, 2)
):
if self.connection.mysql_is_mariadb and (
10,
4,
3,
) < self.connection.mysql_version < (10, 5, 2):
pass

15
tests/data/trailing_comma_optional_parens2.py
View File

@ -4,14 +4,9 @@
# output
if (
e123456.get_tk_patchlevel() >= (8, 6, 0, "final")
or (
8,
5,
8,
)
<= get_tk_patchlevel()
< (8, 6)
):
if e123456.get_tk_patchlevel() >= (8, 6, 0, "final") or (
8,
5,
8,
) <= get_tk_patchlevel() < (8, 6):
pass

5
tests/data/trailing_comma_optional_parens3.py
View File

@ -18,7 +18,4 @@
"qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweas "
+ "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwegqweasdzxcqweasdzxc.",
"qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwe",
) % {
"reported_username": reported_username,
"report_reason": report_reason,
}
) % {"reported_username": reported_username, "report_reason": report_reason}