From a764f1bb3b459ee6f2e752e3d67793b119a2144a Mon Sep 17 00:00:00 2001 From: =?utf8?q?=C5=81ukasz=20Langa?= Date: Mon, 16 Apr 2018 01:32:09 -0700 Subject: [PATCH] Generalize star expression handling Fixes #132 --- README.md | 5 ++ black.py | 63 +++++++++++++++++--------- docs/reference/reference_functions.rst | 2 + tests/expression.diff | 34 +++++++++++--- tests/expression.py | 37 +++++++++++++-- tests/function.py | 16 +++++++ tests/test_black.py | 2 +- 7 files changed, 127 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index bd088bc..e65d981 100644 --- a/README.md +++ b/README.md @@ -491,6 +491,11 @@ More details can be found in [CONTRIBUTING](CONTRIBUTING.md). ## Change Log +### 18.4a3 + +* generalized star expression handling, including double stars; this + fixes multiplication making expressions "unsafe" for trailing commas (#132) + ### 18.4a2 * fixed parsing of unaligned standalone comments (#99, #112) diff --git a/black.py b/black.py index dccda91..c3610c7 100644 --- a/black.py +++ b/black.py @@ -522,7 +522,20 @@ MATH_OPERATORS = { token.DOUBLESTAR, token.DOUBLESLASH, } -VARARGS = {token.STAR, token.DOUBLESTAR} +STARS = {token.STAR, token.DOUBLESTAR} +VARARGS_PARENTS = { + syms.arglist, + syms.argument, # double star in arglist + syms.trailer, # single argument to call + syms.typedargslist, + syms.varargslist, # lambdas +} +UNPACKING_PARENTS = { + syms.atom, # single element of a list or set literal + syms.dictsetmaker, + syms.listmaker, + syms.testlist_gexp, +} COMPREHENSION_PRIORITY = 20 COMMA_PRIORITY = 10 LOGIC_PRIORITY = 5 @@ -1255,18 +1268,8 @@ def whitespace(leaf: Leaf) -> str: # noqa C901 # that, too. return prevp.prefix - elif prevp.type == token.DOUBLESTAR: - if ( - prevp.parent - and prevp.parent.type in { - syms.arglist, - syms.argument, - syms.dictsetmaker, - syms.parameters, - syms.typedargslist, - syms.varargslist, - } - ): + elif prevp.type in STARS: + if is_vararg(prevp, within=VARARGS_PARENTS | UNPACKING_PARENTS): return NO elif prevp.type == token.COLON: @@ -1275,7 +1278,7 @@ def whitespace(leaf: Leaf) -> str: # noqa C901 elif ( prevp.parent - and prevp.parent.type in {syms.factor, syms.star_expr} + and prevp.parent.type == syms.factor and prevp.type in MATH_OPERATORS ): return NO @@ -1496,11 +1499,7 @@ def is_split_before_delimiter(leaf: Leaf, previous: Leaf = None) -> int: Higher numbers are higher priority. """ - if ( - leaf.type in VARARGS - and leaf.parent - and leaf.parent.type in {syms.argument, syms.typedargslist, syms.dictsetmaker} - ): + if is_vararg(leaf, within=VARARGS_PARENTS | UNPACKING_PARENTS): # * and ** might also be MATH_OPERATORS but in this case they are not. # Don't treat them as a delimiter. return 0 @@ -1878,8 +1877,7 @@ def delimiter_split(line: Line, py36: bool = False) -> Iterator[Line]: lowest_depth = min(lowest_depth, leaf.bracket_depth) if ( leaf.bracket_depth == lowest_depth - and leaf.type == token.STAR - or leaf.type == token.DOUBLESTAR + and is_vararg(leaf, within=VARARGS_PARENTS) ): trailing_comma_safe = trailing_comma_safe and py36 leaf_priority = delimiters.get(id(leaf)) @@ -2090,6 +2088,29 @@ def is_one_tuple(node: LN) -> bool: ) +def is_vararg(leaf: Leaf, within: Set[NodeType]) -> bool: + """Return True if `leaf` is a star or double star in a vararg or kwarg. + + If `within` includes VARARGS_PARENTS, this applies to function signatures. + If `within` includes COLLECTION_LIBERALS_PARENTS, it applies to right + hand-side extended iterable unpacking (PEP 3132) and additional unpacking + generalizations (PEP 448). + """ + if leaf.type not in STARS or not leaf.parent: + return False + + p = leaf.parent + if p.type == syms.star_expr: + # Star expressions are also used as assignment targets in extended + # iterable unpacking (PEP 3132). See what its parent is instead. + if not p.parent: + return False + + p = p.parent + + return p.type in within + + def max_delimiter_priority_in_atom(node: LN) -> int: if node.type != syms.atom: return 0 diff --git a/docs/reference/reference_functions.rst b/docs/reference/reference_functions.rst index 19128ba..8e9936d 100644 --- a/docs/reference/reference_functions.rst +++ b/docs/reference/reference_functions.rst @@ -26,6 +26,8 @@ Assertions and checks .. autofunction:: black.is_python36 +.. autofunction:: black.is_vararg + Formatting ---------- diff --git a/tests/expression.diff b/tests/expression.diff index 02bdc2c..bc34c0e 100644 --- a/tests/expression.diff +++ b/tests/expression.diff @@ -11,7 +11,7 @@ True False 1 -@@ -29,65 +29,74 @@ +@@ -29,59 +29,73 @@ ~great +value -1 @@ -48,6 +48,19 @@ [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,] +-[this_is_a_very_long_variable_which_will_force_a_delimiter_split, element, another, *more] ++[*a, 4, 5] ++[4, *a, 5] ++[ ++ this_is_a_very_long_variable_which_will_force_a_delimiter_split, ++ element, ++ another, ++ *more, ++] {i for i in (1, 2, 3)} {(i ** 2) for i in (1, 2, 3)} -{(i ** 2) for i, _ in ((1, 'a'), (2, 'b'), (3, 'c'))} @@ -87,10 +100,11 @@ + **kwargs +) # note: no trailing comma pre-3.6 call(*gidgets[:2]) + call(a, *gidgets[:2]) call(**self.screen_kwargs) + call(b, **self.screen_kwargs) lukasz.langa.pl - call.me(maybe) - 1 .real +@@ -90,11 +104,11 @@ 1.0 .real ....__class__ list[str] @@ -103,7 +117,7 @@ ] slice[0] slice[0:1] -@@ -114,79 +123,113 @@ +@@ -121,85 +135,119 @@ numpy[-(c + 1):, d] numpy[:, l[-2]] numpy[:, ::-1] @@ -123,7 +137,7 @@ +((i ** 2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))) (((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3)) (*starred) --{"id": "1","type": "type","started_at": now(),"ended_at": now() + timedelta(days=10),"priority": 1,"import_session_id": 1,**kwargs} # no trailing comma, this file is not 3.6+ +-{"id": "1","type": "type","started_at": now(),"ended_at": now() + timedelta(days=10),"priority": 1,"import_session_id": 1,**kwargs} +{ + "id": "1", + "type": "type", @@ -131,8 +145,8 @@ + "ended_at": now() + timedelta(days=10), + "priority": 1, + "import_session_id": 1, -+ **kwargs -+} # no trailing comma, this file is not 3.6+ ++ **kwargs, ++} a = (1,) b = 1, c = 1 @@ -154,6 +168,12 @@ +).all() Ø = set() authors.łukasz.say_thanks() + mapping = { + A: 0.25 * (10.0 / 12), + B: 0.1 * (10.0 / 12), + C: 0.1 * (10.0 / 12), + D: 0.1 * (10.0 / 12), + } + def gen(): diff --git a/tests/expression.py b/tests/expression.py index 357cbb6..9c07177 100644 --- a/tests/expression.py +++ b/tests/expression.py @@ -54,6 +54,11 @@ str or None if (1 if True else 2) else str or bytes or None [] [1, 2, 3, 4, 5, 6, 7, 8, 9, (10 or A), (11 or B), (12 or C)] [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] {i for i in (1, 2, 3)} {(i ** 2) for i in (1, 2, 3)} {(i ** 2) for i, _ in ((1, 'a'), (2, 'b'), (3, 'c'))} @@ -76,7 +81,9 @@ call(arg, kwarg='hey') call(arg, another, kwarg='hey', **kwargs) call(this_is_a_very_long_variable_which_will_force_a_delimiter_split, arg, another, kwarg='hey', **kwargs) # note: no trailing comma pre-3.6 call(*gidgets[:2]) +call(a, *gidgets[:2]) call(**self.screen_kwargs) +call(b, **self.screen_kwargs) lukasz.langa.pl call.me(maybe) 1 .real @@ -127,7 +134,7 @@ SomeName ((i ** 2) for i, _ in ((1, 'a'), (2, 'b'), (3, 'c'))) (((i ** 2) + j) for i in (1, 2, 3) for j in (1, 2, 3)) (*starred) -{"id": "1","type": "type","started_at": now(),"ended_at": now() + timedelta(days=10),"priority": 1,"import_session_id": 1,**kwargs} # no trailing comma, this file is not 3.6+ +{"id": "1","type": "type","started_at": now(),"ended_at": now() + timedelta(days=10),"priority": 1,"import_session_id": 1,**kwargs} a = (1,) b = 1, c = 1 @@ -138,6 +145,12 @@ what_is_up_with_those_new_coord_names = (coord_names | set(vars_to_create)) - se result = session.query(models.Customer.id).filter(models.Customer.account_id == account_id, models.Customer.email == email_address).order_by(models.Customer.id.asc(),).all() Ø = set() authors.łukasz.say_thanks() +mapping = { + A: 0.25 * (10.0 / 12), + B: 0.1 * (10.0 / 12), + C: 0.1 * (10.0 / 12), + D: 0.1 * (10.0 / 12), +} def gen(): yield from outside_of_generator @@ -250,6 +263,16 @@ str or None if (1 if True else 2) else str or bytes or None [] [1, 2, 3, 4, 5, 6, 7, 8, 9, (10 or A), (11 or B), (12 or C)] [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, +] {i for i in (1, 2, 3)} {(i ** 2) for i in (1, 2, 3)} {(i ** 2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))} @@ -281,7 +304,9 @@ call( **kwargs ) # note: no trailing comma pre-3.6 call(*gidgets[:2]) +call(a, *gidgets[:2]) call(**self.screen_kwargs) +call(b, **self.screen_kwargs) lukasz.langa.pl call.me(maybe) 1 .real @@ -339,8 +364,8 @@ SomeName "ended_at": now() + timedelta(days=10), "priority": 1, "import_session_id": 1, - **kwargs -} # no trailing comma, this file is not 3.6+ + **kwargs, +} a = (1,) b = 1, c = 1 @@ -359,6 +384,12 @@ result = session.query(models.Customer.id).filter( ).all() Ø = set() authors.łukasz.say_thanks() +mapping = { + A: 0.25 * (10.0 / 12), + B: 0.1 * (10.0 / 12), + C: 0.1 * (10.0 / 12), + D: 0.1 * (10.0 / 12), +} def gen(): diff --git a/tests/function.py b/tests/function.py index 6cf6dbc..08f9414 100644 --- a/tests/function.py +++ b/tests/function.py @@ -74,6 +74,13 @@ def long_lines(): $ """, re.MULTILINE | re.VERBOSE ) +def trailing_comma(): + mapping = { + A: 0.25 * (10.0 / 12), + B: 0.1 * (10.0 / 12), + C: 0.1 * (10.0 / 12), + D: 0.1 * (10.0 / 12), +} # output @@ -198,3 +205,12 @@ def long_lines(): """, re.MULTILINE | re.VERBOSE, ) + + +def trailing_comma(): + mapping = { + A: 0.25 * (10.0 / 12), + B: 0.1 * (10.0 / 12), + C: 0.1 * (10.0 / 12), + D: 0.1 * (10.0 / 12), + } diff --git a/tests/test_black.py b/tests/test_black.py index 6e6aad8..a4d2382 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -179,7 +179,7 @@ class BlackTestCase(unittest.TestCase): msg = ( f"Expected diff isn't equal to the actual. If you made changes " f"to expression.py and this is an anticipated difference, " - f"overwrite tests/expression.diff with {dump}." + f"overwrite tests/expression.diff with {dump}" ) self.assertEqual(expected, actual, msg) -- 2.39.5