]> git.madduck.net Git - etc/vim.git/commitdiff

madduck's git repository

Every one of the projects in this repository is available at the canonical URL git://git.madduck.net/madduck/pub/<projectpath> — see each project's metadata for the exact URL.

All patches and comments are welcome. Please squash your changes to logical commits before using git-format-patch and git-send-email to patches@git.madduck.net. If you'd read over the Git project's submission guidelines and adhered to them, I'd be especially grateful.

SSH access, as well as push access can be individually arranged.

If you use my repositories frequently, consider adding the following snippet to ~/.gitconfig and using the third clone URL listed for each project:

[url "git://git.madduck.net/madduck/"]
  insteadOf = madduck:

Tweak collection literals to explode with trailing comma (#826)
authorAugie Fackler <raf@durin42.com>
Sun, 20 Oct 2019 14:08:34 +0000 (10:08 -0400)
committerŁukasz Langa <lukasz@langa.pl>
Sun, 20 Oct 2019 14:08:34 +0000 (16:08 +0200)
black.py
tests/data/collections.py [new file with mode: 0644]
tests/data/comments2.py
tests/data/comments7.py
tests/data/expression.diff
tests/data/expression.py
tests/data/fmtonoff.py
tests/data/function.py
tests/data/function2.py
tests/data/function_trailing_comma.py
tests/test_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)
         ]
 
+    @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 @@ class Line:
 
     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()
diff --git a/tests/data/collections.py b/tests/data/collections.py
new file mode 100644 (file)
index 0000000..d408bf9
--- /dev/null
@@ -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,},
+    )
index e928f6d5e163a55373e7858756e58a0b317c7ebc..489661e01446ff5dec0bd709fd92936f67e304f9 100644 (file)
@@ -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 @@ short
         syms.simple_stmt,
         [
             Node(statement, result),
-            Leaf(token.NEWLINE, '\n'),  # FIXME: \r\n?
+            Leaf(token.NEWLINE, '\n')  # FIXME: \r\n?
         ],
     )
 
index 088dc9979d36145604b3e1d2693b1c74de46755f..948b3b03a2bfc139f09bdba5839a8194d4a72e05 100644 (file)
@@ -94,7 +94,7 @@ result = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
 
 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
index eff98f9ecac1be1db50391db386bd8355770965a..da20094cc1ea688ae4a2a0748c54f10a3d872c86 100644 (file)
@@ -11,7 +11,7 @@
  True
  False
  1
-@@ -29,63 +29,84 @@
+@@ -29,63 +29,96 @@
  ~great
  +value
  -1
  []
  [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,
  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]]]],
  ]
  slice[0:1:2]
  slice[:]
  slice[:-1]
-@@ -134,113 +155,171 @@
+@@ -134,113 +169,171 @@
  numpy[-(c + 1) :, d]
  numpy[:, l[-2]]
  numpy[:, ::-1]
 +    .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()
      return True
  last_call()
  # standalone comment at ENDMARKER
index 3851249e4573d461fb1cee6f68f6a238df2848d4..c9b149ffa80044f5afe88cb8da6dcacf3623bfe4 100644 (file)
@@ -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,
+]
 [*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 @@ call.me(maybe)
 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 @@ result = (
     .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()
index 5f62af025220258d2a63a6518bcd0f929be6a4c4..fff751e37248dcaa131b5c8883538ccc86ce1420 100644 (file)
@@ -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",
index 4754588e38d602b4af5afe7470ac62fd1b2879cd..51234a1e9b406c9d6bd576bf227e9bbd54f027ce 100644 (file)
@@ -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,
index 0c9da12c38845c529287d05492e3b8a8546483e6..f57a3f5213a6ad7c900e2c1a70f09d100a7aa8bc 100644 (file)
@@ -24,7 +24,7 @@ def h():
 
 # output
 
-def f(a, **kwargs) -> A:
+def f(a, **kwargs,) -> A:
     with cache_dir():
         if something:
             result = CliRunner().invoke(
index 29fd99b7d91e1fc5d8f216cf755ff32635703a09..f2594c68ae246743348851d860ccdd8d78fd2ede 100644 (file)
@@ -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,):
     ...
index 107f77d368e3f92b15c7479612f084d331117c82..a92798da12cbd8005c7e445d08df7cbe9b013126 100644 (file)
@@ -1369,6 +1369,13 @@ class BlackTestCase(unittest.TestCase):
                 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(