Generalize star expression handling

Fixes #132

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)

black.py

@@ -522,7 +522,20 @@ def show(cls, code: str) -> None:
     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 append_to_line(leaf: Leaf) -> 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

docs/reference/reference_functions.rst

@@ -26,6 +26,8 @@ Assertions and checks
 
 .. autofunction:: black.is_python36
 
+.. autofunction:: black.is_vararg
+
 Formatting
 ----------
 

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():

tests/expression.py

@@ -54,6 +54,11 @@
 []
 [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, 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 @@
 ((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 @@
 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 @@ async def f():
 []
 [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 @@ async def f():
     **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 @@ async def f():
     "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 @@ async def f():
 ).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():

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),
+    }

tests/test_black.py

@@ -179,7 +179,7 @@ def test_expression_diff(self) -> None:
             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)