Tweak collection literals to explode with trailing comma (#826)
This commit is contained in:
parent
84e22b75c6
commit
9854d4b375
129
black.py
129
black.py
@ -1240,6 +1240,61 @@ def is_stub_class(self) -> bool:
|
||||
Leaf(token.DOT, ".") for _ in range(3)
|
||||
]
|
||||
|
||||
@property
|
||||
def is_collection_with_optional_trailing_comma(self) -> bool:
|
||||
"""Is this line a collection literal with a trailing comma that's optional?
|
||||
|
||||
Note that the trailing comma in a 1-tuple is not optional.
|
||||
"""
|
||||
if not self.leaves or len(self.leaves) < 4:
|
||||
return False
|
||||
# Look for and address a trailing colon.
|
||||
if self.leaves[-1].type == token.COLON:
|
||||
closer = self.leaves[-2]
|
||||
close_index = -2
|
||||
else:
|
||||
closer = self.leaves[-1]
|
||||
close_index = -1
|
||||
if closer.type not in CLOSING_BRACKETS or self.inside_brackets:
|
||||
return False
|
||||
if closer.type == token.RPAR:
|
||||
# Tuples require an extra check, because if there's only
|
||||
# one element in the tuple removing the comma unmakes the
|
||||
# tuple.
|
||||
#
|
||||
# We also check for parens before looking for the trailing
|
||||
# comma because in some cases (eg assigning a dict
|
||||
# literal) the literal gets wrapped in temporary parens
|
||||
# during parsing. This case is covered by the
|
||||
# collections.py test data.
|
||||
opener = closer.opening_bracket
|
||||
for _open_index, leaf in enumerate(self.leaves):
|
||||
if leaf is opener:
|
||||
break
|
||||
else:
|
||||
# Couldn't find the matching opening paren, play it safe.
|
||||
return False
|
||||
commas = 0
|
||||
comma_depth = self.leaves[close_index - 1].bracket_depth
|
||||
for leaf in self.leaves[_open_index + 1 : close_index]:
|
||||
if leaf.bracket_depth == comma_depth and leaf.type == token.COMMA:
|
||||
commas += 1
|
||||
if commas > 1:
|
||||
# We haven't looked yet for the trailing comma because
|
||||
# we might also have caught noop parens.
|
||||
return self.leaves[close_index - 1].type == token.COMMA
|
||||
elif commas == 1:
|
||||
return False # it's either a one-tuple or didn't have a trailing comma
|
||||
if self.leaves[close_index - 1].type in CLOSING_BRACKETS:
|
||||
close_index -= 1
|
||||
closer = self.leaves[close_index]
|
||||
if closer.type == token.RPAR:
|
||||
# TODO: this is a gut feeling. Will we ever see this?
|
||||
return False
|
||||
if self.leaves[close_index - 1].type != token.COMMA:
|
||||
return False
|
||||
return True
|
||||
|
||||
@property
|
||||
def is_def(self) -> bool:
|
||||
"""Is this a function definition? (Also returns True for async defs.)"""
|
||||
@ -1365,60 +1420,39 @@ def contains_multiline_strings(self) -> bool:
|
||||
|
||||
def maybe_remove_trailing_comma(self, closing: Leaf) -> bool:
|
||||
"""Remove trailing comma if there is one and it's safe."""
|
||||
if not (self.leaves and self.leaves[-1].type == token.COMMA):
|
||||
return False
|
||||
# We remove trailing commas only in the case of importing a
|
||||
# single name from a module.
|
||||
if not (
|
||||
self.leaves
|
||||
and self.is_import
|
||||
and len(self.leaves) > 4
|
||||
and self.leaves[-1].type == token.COMMA
|
||||
and closing.type in CLOSING_BRACKETS
|
||||
and self.leaves[-4].type == token.NAME
|
||||
and (
|
||||
# regular `from foo import bar,`
|
||||
self.leaves[-4].value == "import"
|
||||
# `from foo import (bar as baz,)
|
||||
or (
|
||||
len(self.leaves) > 6
|
||||
and self.leaves[-6].value == "import"
|
||||
and self.leaves[-3].value == "as"
|
||||
)
|
||||
# `from foo import bar as baz,`
|
||||
or (
|
||||
len(self.leaves) > 5
|
||||
and self.leaves[-5].value == "import"
|
||||
and self.leaves[-3].value == "as"
|
||||
)
|
||||
)
|
||||
and closing.type == token.RPAR
|
||||
):
|
||||
return False
|
||||
|
||||
if closing.type == token.RBRACE:
|
||||
self.remove_trailing_comma()
|
||||
return True
|
||||
|
||||
if closing.type == token.RSQB:
|
||||
comma = self.leaves[-1]
|
||||
if comma.parent and comma.parent.type == syms.listmaker:
|
||||
self.remove_trailing_comma()
|
||||
return True
|
||||
|
||||
# For parens let's check if it's safe to remove the comma.
|
||||
# Imports are always safe.
|
||||
if self.is_import:
|
||||
self.remove_trailing_comma()
|
||||
return True
|
||||
|
||||
# Otherwise, if the trailing one is the only one, we might mistakenly
|
||||
# change a tuple into a different type by removing the comma.
|
||||
depth = closing.bracket_depth + 1
|
||||
commas = 0
|
||||
opening = closing.opening_bracket
|
||||
for _opening_index, leaf in enumerate(self.leaves):
|
||||
if leaf is opening:
|
||||
break
|
||||
|
||||
else:
|
||||
return False
|
||||
|
||||
for leaf in self.leaves[_opening_index + 1 :]:
|
||||
if leaf is closing:
|
||||
break
|
||||
|
||||
bracket_depth = leaf.bracket_depth
|
||||
if bracket_depth == depth and leaf.type == token.COMMA:
|
||||
commas += 1
|
||||
if leaf.parent and leaf.parent.type in {
|
||||
syms.arglist,
|
||||
syms.typedargslist,
|
||||
}:
|
||||
commas += 1
|
||||
break
|
||||
|
||||
if commas > 1:
|
||||
self.remove_trailing_comma()
|
||||
return True
|
||||
|
||||
return False
|
||||
self.remove_trailing_comma()
|
||||
return True
|
||||
|
||||
def append_comment(self, comment: Leaf) -> bool:
|
||||
"""Add an inline or standalone comment to the line."""
|
||||
@ -2363,6 +2397,7 @@ def split_line(
|
||||
if (
|
||||
not line.contains_uncollapsable_type_comments()
|
||||
and not line.should_explode
|
||||
and not line.is_collection_with_optional_trailing_comma
|
||||
and (
|
||||
is_line_short_enough(line, line_length=line_length, line_str=line_str)
|
||||
or line.contains_unsplittable_type_ignore()
|
||||
|
160
tests/data/collections.py
Normal file
160
tests/data/collections.py
Normal file
@ -0,0 +1,160 @@
|
||||
import core, time, a
|
||||
|
||||
from . import A, B, C
|
||||
|
||||
# unwraps
|
||||
from foo import (
|
||||
bar,
|
||||
)
|
||||
|
||||
# stays wrapped
|
||||
from foo import (
|
||||
baz,
|
||||
qux,
|
||||
)
|
||||
|
||||
# as doesn't get confusing when unwrapped
|
||||
from foo import (
|
||||
xyzzy as magic,
|
||||
)
|
||||
|
||||
a = {1,2,3,}
|
||||
b = {
|
||||
1,2,
|
||||
3}
|
||||
c = {
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
}
|
||||
x = 1,
|
||||
y = narf(),
|
||||
nested = {(1,2,3),(4,5,6),}
|
||||
nested_no_trailing_comma = {(1,2,3),(4,5,6)}
|
||||
nested_long_lines = ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "cccccccccccccccccccccccccccccccccccccccc", (1, 2, 3), "dddddddddddddddddddddddddddddddddddddddd"]
|
||||
{"oneple": (1,),}
|
||||
{"oneple": (1,)}
|
||||
['ls', 'lsoneple/%s' % (foo,)]
|
||||
x = {"oneple": (1,)}
|
||||
y = {"oneple": (1,),}
|
||||
assert False, ("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa wraps %s" % bar)
|
||||
|
||||
# looping over a 1-tuple should also not get wrapped
|
||||
for x in (1,):
|
||||
pass
|
||||
for (x,) in (1,), (2,), (3,):
|
||||
pass
|
||||
|
||||
[1, 2, 3,]
|
||||
|
||||
division_result_tuple = (6/2,)
|
||||
print("foo %r", (foo.bar,))
|
||||
|
||||
if True:
|
||||
IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = (
|
||||
Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING
|
||||
| {pylons.controllers.WSGIController}
|
||||
)
|
||||
|
||||
if True:
|
||||
ec2client.get_waiter('instance_stopped').wait(
|
||||
InstanceIds=[instance.id],
|
||||
WaiterConfig={
|
||||
'Delay': 5,
|
||||
})
|
||||
ec2client.get_waiter("instance_stopped").wait(
|
||||
InstanceIds=[instance.id],
|
||||
WaiterConfig={"Delay": 5,},
|
||||
)
|
||||
ec2client.get_waiter("instance_stopped").wait(
|
||||
InstanceIds=[instance.id], WaiterConfig={"Delay": 5,},
|
||||
)
|
||||
|
||||
# output
|
||||
|
||||
|
||||
import core, time, a
|
||||
|
||||
from . import A, B, C
|
||||
|
||||
# unwraps
|
||||
from foo import bar
|
||||
|
||||
# stays wrapped
|
||||
from foo import (
|
||||
baz,
|
||||
qux,
|
||||
)
|
||||
|
||||
# as doesn't get confusing when unwrapped
|
||||
from foo import xyzzy as magic
|
||||
|
||||
a = {
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
}
|
||||
b = {1, 2, 3}
|
||||
c = {
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
}
|
||||
x = (1,)
|
||||
y = (narf(),)
|
||||
nested = {
|
||||
(1, 2, 3),
|
||||
(4, 5, 6),
|
||||
}
|
||||
nested_no_trailing_comma = {(1, 2, 3), (4, 5, 6)}
|
||||
nested_long_lines = [
|
||||
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
|
||||
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
|
||||
"cccccccccccccccccccccccccccccccccccccccc",
|
||||
(1, 2, 3),
|
||||
"dddddddddddddddddddddddddddddddddddddddd",
|
||||
]
|
||||
{
|
||||
"oneple": (1,),
|
||||
}
|
||||
{"oneple": (1,)}
|
||||
["ls", "lsoneple/%s" % (foo,)]
|
||||
x = {"oneple": (1,)}
|
||||
y = {
|
||||
"oneple": (1,),
|
||||
}
|
||||
assert False, (
|
||||
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa wraps %s"
|
||||
% bar
|
||||
)
|
||||
|
||||
# looping over a 1-tuple should also not get wrapped
|
||||
for x in (1,):
|
||||
pass
|
||||
for (x,) in (1,), (2,), (3,):
|
||||
pass
|
||||
|
||||
[
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
]
|
||||
|
||||
division_result_tuple = (6 / 2,)
|
||||
print("foo %r", (foo.bar,))
|
||||
|
||||
if True:
|
||||
IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING | {
|
||||
pylons.controllers.WSGIController
|
||||
}
|
||||
|
||||
if True:
|
||||
ec2client.get_waiter("instance_stopped").wait(
|
||||
InstanceIds=[instance.id], WaiterConfig={"Delay": 5,}
|
||||
)
|
||||
ec2client.get_waiter("instance_stopped").wait(
|
||||
InstanceIds=[instance.id], WaiterConfig={"Delay": 5,},
|
||||
)
|
||||
ec2client.get_waiter("instance_stopped").wait(
|
||||
InstanceIds=[instance.id], WaiterConfig={"Delay": 5,},
|
||||
)
|
@ -63,7 +63,7 @@ def inline_comments_in_brackets_ruin_everything():
|
||||
parameters.children = [
|
||||
children[0], # (1
|
||||
body,
|
||||
children[-1], # )1
|
||||
children[-1] # )1
|
||||
]
|
||||
parameters.children = [
|
||||
children[0],
|
||||
@ -142,7 +142,7 @@ def inline_comments_in_brackets_ruin_everything():
|
||||
syms.simple_stmt,
|
||||
[
|
||||
Node(statement, result),
|
||||
Leaf(token.NEWLINE, '\n'), # FIXME: \r\n?
|
||||
Leaf(token.NEWLINE, '\n') # FIXME: \r\n?
|
||||
],
|
||||
)
|
||||
|
||||
|
@ -94,7 +94,7 @@ def func():
|
||||
|
||||
def func():
|
||||
c = call(
|
||||
0.0123, 0.0456, 0.0789, 0.0123, 0.0789, a[-1] # type: ignore
|
||||
0.0123, 0.0456, 0.0789, 0.0123, 0.0789, a[-1], # type: ignore
|
||||
)
|
||||
|
||||
# The type: ignore exception only applies to line length, not
|
||||
|
@ -11,7 +11,7 @@
|
||||
True
|
||||
False
|
||||
1
|
||||
@@ -29,63 +29,84 @@
|
||||
@@ -29,63 +29,96 @@
|
||||
~great
|
||||
+value
|
||||
-1
|
||||
@ -61,14 +61,26 @@
|
||||
[]
|
||||
[1, 2, 3, 4, 5, 6, 7, 8, 9, (10 or A), (11 or B), (12 or C)]
|
||||
-[1, 2, 3,]
|
||||
+[1, 2, 3]
|
||||
+[
|
||||
+ 1,
|
||||
+ 2,
|
||||
+ 3,
|
||||
+]
|
||||
[*a]
|
||||
[*range(10)]
|
||||
-[*a, 4, 5,]
|
||||
-[4, *a, 5,]
|
||||
-[this_is_a_very_long_variable_which_will_force_a_delimiter_split, element, another, *more]
|
||||
+[*a, 4, 5]
|
||||
+[4, *a, 5]
|
||||
+[
|
||||
+ *a,
|
||||
+ 4,
|
||||
+ 5,
|
||||
+]
|
||||
+[
|
||||
+ 4,
|
||||
+ *a,
|
||||
+ 5,
|
||||
+]
|
||||
+[
|
||||
+ this_is_a_very_long_variable_which_will_force_a_delimiter_split,
|
||||
+ element,
|
||||
@ -118,14 +130,16 @@
|
||||
call(**self.screen_kwargs)
|
||||
call(b, **self.screen_kwargs)
|
||||
lukasz.langa.pl
|
||||
@@ -94,23 +115,23 @@
|
||||
@@ -94,23 +127,25 @@
|
||||
1.0 .real
|
||||
....__class__
|
||||
list[str]
|
||||
dict[str, int]
|
||||
tuple[str, ...]
|
||||
-tuple[str, int, float, dict[str, int],]
|
||||
+tuple[str, int, float, dict[str, int]]
|
||||
+tuple[
|
||||
+ str, int, float, dict[str, int],
|
||||
+]
|
||||
very_long_variable_name_filters: t.List[
|
||||
t.Tuple[str, t.Union[str, t.List[t.Optional[str]]]],
|
||||
]
|
||||
@ -146,7 +160,7 @@
|
||||
slice[0:1:2]
|
||||
slice[:]
|
||||
slice[:-1]
|
||||
@@ -134,113 +155,171 @@
|
||||
@@ -134,113 +169,171 @@
|
||||
numpy[-(c + 1) :, d]
|
||||
numpy[:, l[-2]]
|
||||
numpy[:, ::-1]
|
||||
@ -199,7 +213,7 @@
|
||||
+ .filter(
|
||||
+ models.Customer.account_id == account_id, models.Customer.email == email_address
|
||||
+ )
|
||||
+ .order_by(models.Customer.id.asc())
|
||||
+ .order_by(models.Customer.id.asc(),)
|
||||
+ .all()
|
||||
+)
|
||||
Ø = set()
|
||||
@ -391,3 +405,4 @@
|
||||
return True
|
||||
last_call()
|
||||
# standalone comment at ENDMARKER
|
||||
|
||||
|
@ -314,11 +314,23 @@ async def f():
|
||||
(1, 2, 3)
|
||||
[]
|
||||
[1, 2, 3, 4, 5, 6, 7, 8, 9, (10 or A), (11 or B), (12 or C)]
|
||||
[1, 2, 3]
|
||||
[
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
]
|
||||
[*a]
|
||||
[*range(10)]
|
||||
[*a, 4, 5]
|
||||
[4, *a, 5]
|
||||
[
|
||||
*a,
|
||||
4,
|
||||
5,
|
||||
]
|
||||
[
|
||||
4,
|
||||
*a,
|
||||
5,
|
||||
]
|
||||
[
|
||||
this_is_a_very_long_variable_which_will_force_a_delimiter_split,
|
||||
element,
|
||||
@ -367,7 +379,9 @@ async def f():
|
||||
list[str]
|
||||
dict[str, int]
|
||||
tuple[str, ...]
|
||||
tuple[str, int, float, dict[str, int]]
|
||||
tuple[
|
||||
str, int, float, dict[str, int],
|
||||
]
|
||||
very_long_variable_name_filters: t.List[
|
||||
t.Tuple[str, t.Union[str, t.List[t.Optional[str]]]],
|
||||
]
|
||||
@ -445,7 +459,7 @@ async def f():
|
||||
.filter(
|
||||
models.Customer.account_id == account_id, models.Customer.email == email_address
|
||||
)
|
||||
.order_by(models.Customer.id.asc())
|
||||
.order_by(models.Customer.id.asc(),)
|
||||
.all()
|
||||
)
|
||||
Ø = set()
|
||||
|
@ -150,7 +150,7 @@ def single_literal_yapf_disable():
|
||||
BAZ = {
|
||||
(1, 2, 3, 4),
|
||||
(5, 6, 7, 8),
|
||||
(9, 10, 11, 12),
|
||||
(9, 10, 11, 12)
|
||||
} # yapf: disable
|
||||
cfg.rule(
|
||||
"Default", "address",
|
||||
|
@ -230,7 +230,7 @@ def trailing_comma():
|
||||
}
|
||||
|
||||
|
||||
def f(a, **kwargs) -> A:
|
||||
def f(a, **kwargs,) -> A:
|
||||
return (
|
||||
yield from A(
|
||||
very_long_argument_name1=very_long_value_for_the_argument,
|
||||
|
@ -24,7 +24,7 @@ def inner():
|
||||
|
||||
# output
|
||||
|
||||
def f(a, **kwargs) -> A:
|
||||
def f(a, **kwargs,) -> A:
|
||||
with cache_dir():
|
||||
if something:
|
||||
result = CliRunner().invoke(
|
||||
|
@ -6,9 +6,9 @@ def f(a:int=1,):
|
||||
|
||||
# output
|
||||
|
||||
def f(a):
|
||||
def f(a,):
|
||||
...
|
||||
|
||||
|
||||
def f(a: int = 1):
|
||||
def f(a: int = 1,):
|
||||
...
|
||||
|
@ -1369,6 +1369,13 @@ def test_multi_file_force_py36(self) -> None:
|
||||
self.assertIn(path, pyi_cache)
|
||||
self.assertNotIn(path, normal_cache)
|
||||
|
||||
def test_collections(self) -> None:
|
||||
source, expected = read_data("collections")
|
||||
actual = fs(source)
|
||||
self.assertFormatEqual(expected, actual)
|
||||
black.assert_equivalent(source, actual)
|
||||
black.assert_stable(source, actual, black.FileMode())
|
||||
|
||||
def test_pipe_force_py36(self) -> None:
|
||||
source, expected = read_data("force_py36")
|
||||
result = CliRunner().invoke(
|
||||
|
Loading…
Reference in New Issue
Block a user