Generalize star expression handling

Fixes #132

diff --git a/README.md b/README.md
index bd088bc40d4d21c3b7cf676bb228b0b17cc6502d..e65d9810c84658fef86561c17f252f0693177721 100644 (file)
--- 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 dccda9108df4741aa49570f96ffaca049cf5fa90..c3610c74b12a43ecf654a9e08cadaa3cc8f9a5f7 100644 (file)
--- 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 19128ba7098bb2226d81a4e7026b41318a6150cf..8e9936dfaa50d28a024142a19fe8871ded38838c 100644 (file)
--- 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 02bdc2ced3076c26d58ab48fd586f593b7b80962..bc34c0ed496e4f289d7f468be5fef61eccd9b85e 100644 (file)
--- 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 357cbb63d248cf4497ab3c2527f950a9d1e75bf7..9c071771f7676ba528d5f98c480e874f759f99a5 100644 (file)
--- 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 6cf6dbc9a1b4d16da5874ee3fbd0e5b870c2771c..08f9414a7b5041452573b7c6bc6e4c2f21e3cce3 100644 (file)
--- 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 6e6aad8c90d4826926bfba58edd3370c9721a229..a4d23823c9d2164cfb0176d8544ca01e6d23fc47 100644 (file)
--- 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)