Tweak collection literals to explode with trailing comma (#826)

diff --git a/black.py b/black.py
index 077698311087d75ad54b4090c8db231ccb10c55f..40b4d2b12876013b0925c9a1c706110befeb9b28 100644 (file)
--- a/black.py
+++ b/black.py
@@ -1240,6 +1240,61 @@ class Line:
             Leaf(token.DOT, ".") for _ in range(3)
         ]
 
             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.)"""
     @property
     def is_def(self) -> bool:
         """Is this a function definition? (Also returns True for async defs.)"""


@@ -1365,60 +1420,39 @@ class Line:
 
     def maybe_remove_trailing_comma(self, closing: Leaf) -> bool:
         """Remove trailing comma if there is one and it's safe."""
 
     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
         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[-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
 
         ):
             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."""
 
     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
     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()
         and (
             is_line_short_enough(line, line_length=line_length, line_str=line_str)
             or line.contains_unsplittable_type_ignore()

diff --git a/tests/data/collections.py b/tests/data/collections.py
new file mode 100644 (file)
index 0000000..d408bf9
--- /dev/null
+++ b/tests/data/collections.py
@@ -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,},
+    )

diff --git a/tests/data/comments2.py b/tests/data/comments2.py
index e928f6d5e163a55373e7858756e58a0b317c7ebc..489661e01446ff5dec0bd709fd92936f67e304f9 100644 (file)
--- a/tests/data/comments2.py
+++ b/tests/data/comments2.py
@@ -63,7 +63,7 @@ def inline_comments_in_brackets_ruin_everything():
         parameters.children = [
             children[0],  # (1
             body,
         parameters.children = [
             children[0],  # (1
             body,

-            children[-1],  # )1
+            children[-1]  # )1

         ]
         parameters.children = [
             children[0],
         ]
         parameters.children = [
             children[0],


@@ -142,7 +142,7 @@ short
         syms.simple_stmt,
         [
             Node(statement, result),
         syms.simple_stmt,
         [
             Node(statement, result),

-            Leaf(token.NEWLINE, '\n'),  # FIXME: \r\n?
+            Leaf(token.NEWLINE, '\n')  # FIXME: \r\n?

         ],
     )
 
         ],
     )
 

diff --git a/tests/data/comments7.py b/tests/data/comments7.py
index 088dc9979d36145604b3e1d2693b1c74de46755f..948b3b03a2bfc139f09bdba5839a8194d4a72e05 100644 (file)
--- a/tests/data/comments7.py
+++ b/tests/data/comments7.py
@@ -94,7 +94,7 @@ result = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
 
 def func():
     c = call(
 
 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
     )
 
     # The type: ignore exception only applies to line length, not

diff --git a/tests/data/expression.diff b/tests/data/expression.diff
index eff98f9ecac1be1db50391db386bd8355770965a..da20094cc1ea688ae4a2a0748c54f10a3d872c86 100644 (file)
--- a/tests/data/expression.diff
+++ b/tests/data/expression.diff
@@ -11,7 +11,7 @@
  True
  False
  1
  True
  False
  1

-@@ -29,63 +29,84 @@
+@@ -29,63 +29,96 @@

  ~great
  +value
  -1
  ~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, 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]
  [*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,
 +[
 +    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
  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],]
  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]]]],
  ]
  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]
  slice[0:1:2]
  slice[:]
  slice[:-1]

-@@ -134,113 +155,171 @@
+@@ -134,113 +169,171 @@

  numpy[-(c + 1) :, d]
  numpy[:, l[-2]]
  numpy[:, ::-1]
  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
 +    )
 +    .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()
 +    .all()
 +)
  Ø = set()


@@ -391,3 +405,4 @@
      return True
  last_call()
  # standalone comment at ENDMARKER
      return True
  last_call()
  # standalone comment at ENDMARKER

+ 

diff --git a/tests/data/expression.py b/tests/data/expression.py
index 3851249e4573d461fb1cee6f68f6a238df2848d4..c9b149ffa80044f5afe88cb8da6dcacf3623bfe4 100644 (file)
--- a/tests/data/expression.py
+++ b/tests/data/expression.py
@@ -314,11 +314,23 @@ str or None if (1 if True else 2) else str or bytes or None
 (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, 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]
 [*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,
 [
     this_is_a_very_long_variable_which_will_force_a_delimiter_split,
     element,


@@ -367,7 +379,9 @@ call.me(maybe)
 list[str]
 dict[str, int]
 tuple[str, ...]
 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]]]],
 ]
 very_long_variable_name_filters: t.List[
     t.Tuple[str, t.Union[str, t.List[t.Optional[str]]]],
 ]


@@ -445,7 +459,7 @@ result = (
     .filter(
         models.Customer.account_id == account_id, models.Customer.email == email_address
     )
     .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()
     .all()
 )
 Ø = set()

diff --git a/tests/data/fmtonoff.py b/tests/data/fmtonoff.py
index 5f62af025220258d2a63a6518bcd0f929be6a4c4..fff751e37248dcaa131b5c8883538ccc86ce1420 100644 (file)
--- a/tests/data/fmtonoff.py
+++ b/tests/data/fmtonoff.py
@@ -150,7 +150,7 @@ def single_literal_yapf_disable():
     BAZ = {
         (1, 2, 3, 4),
         (5, 6, 7, 8),
     BAZ = {
         (1, 2, 3, 4),
         (5, 6, 7, 8),

-        (9, 10, 11, 12),
+        (9, 10, 11, 12)

     }  # yapf: disable
 cfg.rule(
     "Default", "address",
     }  # yapf: disable
 cfg.rule(
     "Default", "address",

diff --git a/tests/data/function.py b/tests/data/function.py
index 4754588e38d602b4af5afe7470ac62fd1b2879cd..51234a1e9b406c9d6bd576bf227e9bbd54f027ce 100644 (file)
--- a/tests/data/function.py
+++ b/tests/data/function.py
@@ -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,
     return (
         yield from A(
             very_long_argument_name1=very_long_value_for_the_argument,

diff --git a/tests/data/function2.py b/tests/data/function2.py
index 0c9da12c38845c529287d05492e3b8a8546483e6..f57a3f5213a6ad7c900e2c1a70f09d100a7aa8bc 100644 (file)
--- a/tests/data/function2.py
+++ b/tests/data/function2.py
@@ -24,7 +24,7 @@ def h():
 
 # output
 
 
 # output
 

-def f(a, **kwargs) -> A:
+def f(a, **kwargs,) -> A:

     with cache_dir():
         if something:
             result = CliRunner().invoke(
     with cache_dir():
         if something:
             result = CliRunner().invoke(

diff --git a/tests/data/function_trailing_comma.py b/tests/data/function_trailing_comma.py
index 29fd99b7d91e1fc5d8f216cf755ff32635703a09..f2594c68ae246743348851d860ccdd8d78fd2ede 100644 (file)
--- a/tests/data/function_trailing_comma.py
+++ b/tests/data/function_trailing_comma.py
@@ -6,9 +6,9 @@ def f(a:int=1,):
 
 # output
 
 
 # output
 

-def f(a):
+def f(a,):

     ...
 
 
     ...
 
 

-def f(a: int = 1):
+def f(a: int = 1,):

     ...
     ...

diff --git a/tests/test_black.py b/tests/test_black.py
index 107f77d368e3f92b15c7479612f084d331117c82..a92798da12cbd8005c7e445d08df7cbe9b013126 100644 (file)
--- a/tests/test_black.py
+++ b/tests/test_black.py
@@ -1369,6 +1369,13 @@ class BlackTestCase(unittest.TestCase):
                 self.assertIn(path, pyi_cache)
                 self.assertNotIn(path, normal_cache)
 
                 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(
     def test_pipe_force_py36(self) -> None:
         source, expected = read_data("force_py36")
         result = CliRunner().invoke(