Format subscriptions in a PEP-8 compliant way (#178)

Fixes #157

diff --git a/.flake8 b/.flake8
index 583839065ad1b764bd7dd2f5089dd1bc5f75692a..fae93b09498fcb080a830bbb8a0cc2a3007f861a 100644 (file)
--- a/.flake8
+++ b/.flake8
@@ -2,7 +2,7 @@
 # Keep in sync with setup.cfg which is used for source packages.
 
 [flake8]
 # Keep in sync with setup.cfg which is used for source packages.
 
 [flake8]

-ignore = E266, E501, W503
+ignore = E203, E266, E501, W503

 max-line-length = 80
 max-complexity = 15
 select = B,C,E,F,W,T4,B9
 max-line-length = 80
 max-complexity = 15
 select = B,C,E,F,W,T4,B9

diff --git a/README.md b/README.md
index bc8977b7a9acff4b8a2a1de39b7abb49f3183fb2..31f92c91f14ab3f5f47af15bd561aa4b0994f148 100644 (file)
--- a/README.md
+++ b/README.md
@@ -302,6 +302,19 @@ This behaviour may raise ``W503 line break before binary operator`` warnings in
 style guide enforcement tools like Flake8. Since ``W503`` is not PEP 8 compliant,
 you should tell Flake8 to ignore these warnings.
 
 style guide enforcement tools like Flake8. Since ``W503`` is not PEP 8 compliant,
 you should tell Flake8 to ignore these warnings.
 

+### Slices
+
+PEP 8 [recommends](https://www.python.org/dev/peps/pep-0008/#whitespace-in-expressions-and-statements)
+to treat ``:`` in slices as a binary operator with the lowest priority, and to
+leave an equal amount of space on either side, except if a parameter is omitted
+(e.g. ``ham[1 + 1 :]``). It also states that for extended slices, both ``:``
+operators have to have the same amount of spacing, except if a parameter is
+omitted (``ham[1 + 1 ::]``). *Black* enforces these rules consistently.
+
+This behaviour may raise ``E203 whitespace before ':'`` warnings in style guide
+enforcement tools like Flake8. Since ``E203`` is not PEP 8 compliant, you should
+tell Flake8 to ignore these warnings.
+

 ### Parentheses
 
 Some parentheses are optional in the Python grammar.  Any expression can
 ### Parentheses
 
 Some parentheses are optional in the Python grammar.  Any expression can

diff --git a/black.py b/black.py
index 4c5f0f0c2590d476e53fdb3f5d19ed3152928a06..5e087d164e4da95b1bad894d64b9b12a5b169748 100644 (file)
--- a/black.py
+++ b/black.py
@@ -89,11 +89,11 @@ class FormatError(Exception):
         self.consumed = consumed
 
     def trim_prefix(self, leaf: Leaf) -> None:
         self.consumed = consumed
 
     def trim_prefix(self, leaf: Leaf) -> None:

-        leaf.prefix = leaf.prefix[self.consumed:]
+        leaf.prefix = leaf.prefix[self.consumed :]

 
     def leaf_from_consumed(self, leaf: Leaf) -> Leaf:
         """Returns a new Leaf from the consumed part of the prefix."""
 
     def leaf_from_consumed(self, leaf: Leaf) -> Leaf:
         """Returns a new Leaf from the consumed part of the prefix."""

-        unformatted_prefix = leaf.prefix[:self.consumed]
+        unformatted_prefix = leaf.prefix[: self.consumed]

         return Leaf(token.NEWLINE, unformatted_prefix)
 
 
         return Leaf(token.NEWLINE, unformatted_prefix)
 
 


@@ -582,6 +582,23 @@ UNPACKING_PARENTS = {
     syms.listmaker,
     syms.testlist_gexp,
 }
     syms.listmaker,
     syms.testlist_gexp,
 }

+TEST_DESCENDANTS = {
+    syms.test,
+    syms.lambdef,
+    syms.or_test,
+    syms.and_test,
+    syms.not_test,
+    syms.comparison,
+    syms.star_expr,
+    syms.expr,
+    syms.xor_expr,
+    syms.and_expr,
+    syms.shift_expr,
+    syms.arith_expr,
+    syms.trailer,
+    syms.term,
+    syms.power,
+}

 COMPREHENSION_PRIORITY = 20
 COMMA_PRIORITY = 10
 TERNARY_PRIORITY = 7
 COMPREHENSION_PRIORITY = 20
 COMMA_PRIORITY = 10
 TERNARY_PRIORITY = 7


@@ -698,6 +715,10 @@ class BracketTracker:
 
         return False
 
 
         return False
 

+    def get_open_lsqb(self) -> Optional[Leaf]:
+        """Return the most recent opening square bracket (if any)."""
+        return self.bracket_match.get((self.depth - 1, token.RSQB))
+

 
 @dataclass
 class Line:
 
 @dataclass
 class Line:


@@ -726,7 +747,9 @@ class Line:
         if self.leaves and not preformatted:
             # Note: at this point leaf.prefix should be empty except for
             # imports, for which we only preserve newlines.
         if self.leaves and not preformatted:
             # Note: at this point leaf.prefix should be empty except for
             # imports, for which we only preserve newlines.

-            leaf.prefix += whitespace(leaf)
+            leaf.prefix += whitespace(
+                leaf, complex_subscript=self.is_complex_subscript(leaf)
+            )

         if self.inside_brackets or not preformatted:
             self.bracket_tracker.mark(leaf)
             self.maybe_remove_trailing_comma(leaf)
         if self.inside_brackets or not preformatted:
             self.bracket_tracker.mark(leaf)
             self.maybe_remove_trailing_comma(leaf)


@@ -859,7 +882,7 @@ class Line:
         else:
             return False
 
         else:
             return False
 

-        for leaf in self.leaves[_opening_index + 1:]:
+        for leaf in self.leaves[_opening_index + 1 :]:

             if leaf is closing:
                 break
 
             if leaf is closing:
                 break
 


@@ -920,6 +943,24 @@ class Line:
                 self.comments[i] = (comma_index - 1, comment)
         self.leaves.pop()
 
                 self.comments[i] = (comma_index - 1, comment)
         self.leaves.pop()
 

+    def is_complex_subscript(self, leaf: Leaf) -> bool:
+        """Return True iff `leaf` is part of a slice with non-trivial exprs."""
+        open_lsqb = (
+            leaf if leaf.type == token.LSQB else self.bracket_tracker.get_open_lsqb()
+        )
+        if open_lsqb is None:
+            return False
+
+        subscript_start = open_lsqb.next_sibling
+        if (
+            isinstance(subscript_start, Node)
+            and subscript_start.type == syms.subscriptlist
+        ):
+            subscript_start = child_towards(subscript_start, leaf)
+        return subscript_start is not None and any(
+            n.type in TEST_DESCENDANTS for n in subscript_start.pre_order()
+        )
+

     def __str__(self) -> str:
         """Render the line."""
         if not self:
     def __str__(self) -> str:
         """Render the line."""
         if not self:


@@ -1303,8 +1344,12 @@ BRACKETS = OPENING_BRACKETS | CLOSING_BRACKETS
 ALWAYS_NO_SPACE = CLOSING_BRACKETS | {token.COMMA, STANDALONE_COMMENT}
 
 
 ALWAYS_NO_SPACE = CLOSING_BRACKETS | {token.COMMA, STANDALONE_COMMENT}
 
 

-def whitespace(leaf: Leaf) -> str:  # noqa C901
-    """Return whitespace prefix if needed for the given `leaf`."""
+def whitespace(leaf: Leaf, *, complex_subscript: bool) -> str:  # noqa C901
+    """Return whitespace prefix if needed for the given `leaf`.
+
+    `complex_subscript` signals whether the given leaf is part of a subscription
+    which has non-trivial arguments, like arithmetic expressions or function calls.
+    """

     NO = ""
     SPACE = " "
     DOUBLESPACE = "  "
     NO = ""
     SPACE = " "
     DOUBLESPACE = "  "


@@ -1318,7 +1363,10 @@ def whitespace(leaf: Leaf) -> str:  # noqa C901
         return DOUBLESPACE
 
     assert p is not None, f"INTERNAL ERROR: hand-made leaf without parent: {leaf!r}"
         return DOUBLESPACE
 
     assert p is not None, f"INTERNAL ERROR: hand-made leaf without parent: {leaf!r}"

-    if t == token.COLON and p.type not in {syms.subscript, syms.subscriptlist}:
+    if (
+        t == token.COLON
+        and p.type not in {syms.subscript, syms.subscriptlist, syms.sliceop}
+    ):

         return NO
 
     prev = leaf.prev_sibling
         return NO
 
     prev = leaf.prev_sibling


@@ -1328,7 +1376,13 @@ def whitespace(leaf: Leaf) -> str:  # noqa C901
             return NO
 
         if t == token.COLON:
             return NO
 
         if t == token.COLON:

-            return SPACE if prevp.type == token.COMMA else NO
+            if prevp.type == token.COLON:
+                return NO
+
+            elif prevp.type != token.COMMA and not complex_subscript:
+                return NO
+
+            return SPACE

 
         if prevp.type == token.EQUAL:
             if prevp.parent:
 
         if prevp.type == token.EQUAL:
             if prevp.parent:


@@ -1349,7 +1403,7 @@ def whitespace(leaf: Leaf) -> str:  # noqa C901
 
         elif prevp.type == token.COLON:
             if prevp.parent and prevp.parent.type in {syms.subscript, syms.sliceop}:
 
         elif prevp.type == token.COLON:
             if prevp.parent and prevp.parent.type in {syms.subscript, syms.sliceop}:

-                return NO
+                return SPACE if complex_subscript else NO

 
         elif (
             prevp.parent
 
         elif (
             prevp.parent


@@ -1455,7 +1509,7 @@ def whitespace(leaf: Leaf) -> str:  # noqa C901
         if prev and prev.type == token.LPAR:
             return NO
 
         if prev and prev.type == token.LPAR:
             return NO
 

-    elif p.type == syms.subscript:
+    elif p.type in {syms.subscript, syms.sliceop}:

         # indexing
         if not prev:
             assert p.parent is not None, "subscripts are always parented"
         # indexing
         if not prev:
             assert p.parent is not None, "subscripts are always parented"


@@ -1464,7 +1518,7 @@ def whitespace(leaf: Leaf) -> str:  # noqa C901
 
             return NO
 
 
             return NO
 

-        else:
+        elif not complex_subscript:

             return NO
 
     elif p.type == syms.atom:
             return NO
 
     elif p.type == syms.atom:


@@ -1534,6 +1588,14 @@ def preceding_leaf(node: Optional[LN]) -> Optional[Leaf]:
     return None
 
 
     return None
 
 

+def child_towards(ancestor: Node, descendant: LN) -> Optional[LN]:
+    """Return the child of `ancestor` that contains `descendant`."""
+    node: Optional[LN] = descendant
+    while node and node.parent != ancestor:
+        node = node.parent
+    return node
+
+

 def is_split_after_delimiter(leaf: Leaf, previous: Leaf = None) -> int:
     """Return the priority of the `leaf` delimiter, given a line break after it.
 
 def is_split_after_delimiter(leaf: Leaf, previous: Leaf = None) -> int:
     """Return the priority of the `leaf` delimiter, given a line break after it.
 


@@ -1994,6 +2056,7 @@ def explode_split(
 
     try:
         yield from delimiter_split(new_lines[1], py36)
 
     try:
         yield from delimiter_split(new_lines[1], py36)

+

     except CannotSplit:
         yield new_lines[1]
 
     except CannotSplit:
         yield new_lines[1]
 


@@ -2061,7 +2124,7 @@ def normalize_string_quotes(leaf: Leaf) -> None:
     unescaped_new_quote = re.compile(rf"(([^\\]|^)(\\\\)*){new_quote}")
     escaped_new_quote = re.compile(rf"([^\\]|^)\\(\\\\)*{new_quote}")
     escaped_orig_quote = re.compile(rf"([^\\]|^)\\(\\\\)*{orig_quote}")
     unescaped_new_quote = re.compile(rf"(([^\\]|^)(\\\\)*){new_quote}")
     escaped_new_quote = re.compile(rf"([^\\]|^)\\(\\\\)*{new_quote}")
     escaped_orig_quote = re.compile(rf"([^\\]|^)\\(\\\\)*{orig_quote}")

-    body = leaf.value[first_quote_pos + len(orig_quote):-len(orig_quote)]
+    body = leaf.value[first_quote_pos + len(orig_quote) : -len(orig_quote)]

     if "r" in prefix.casefold():
         if unescaped_new_quote.search(body):
             # There's at least one unescaped new_quote in this raw string
     if "r" in prefix.casefold():
         if unescaped_new_quote.search(body):
             # There's at least one unescaped new_quote in this raw string

diff --git a/tests/expression.diff b/tests/expression.diff
index 11c135546cac7e14fed19dd971870c367a156d98..309a480c2b0219c6fb53001ecdbc09ed35f2800d 100644 (file)
--- a/tests/expression.diff
+++ b/tests/expression.diff
@@ -130,7 +130,7 @@
  slice[0]
  slice[0:1]
 @@ -123,88 +145,114 @@
  slice[0]
  slice[0:1]
 @@ -123,88 +145,114 @@

- numpy[-(c + 1):, d]
+ numpy[-(c + 1) :, d]

  numpy[:, l[-2]]
  numpy[:, ::-1]
  numpy[np.newaxis, :]
  numpy[:, l[-2]]
  numpy[:, ::-1]
  numpy[np.newaxis, :]

diff --git a/tests/expression.py b/tests/expression.py
index 274c150fcde4aae43d18c7101d0fa5bfa72643e0..d170a665554dff8c8c8348b90eb9348c0547390a 100644 (file)
--- a/tests/expression.py
+++ b/tests/expression.py
@@ -105,7 +105,7 @@ slice[:]
 slice[:-1]
 slice[1:]
 slice[::-1]
 slice[:-1]
 slice[1:]
 slice[::-1]

-slice[d::d + 1]
+slice[d :: d + 1]

 slice[:c, c - 1]
 numpy[:, 0:1]
 numpy[:, :-1]
 slice[:c, c - 1]
 numpy[:, 0:1]
 numpy[:, :-1]


@@ -119,8 +119,8 @@ numpy[4:, 2:]
 numpy[:, (0, 1, 2, 5)]
 numpy[0, [0]]
 numpy[:, [i]]
 numpy[:, (0, 1, 2, 5)]
 numpy[0, [0]]
 numpy[:, [i]]

-numpy[1:c + 1, c]
-numpy[-(c + 1):, d]
+numpy[1 : c + 1, c]
+numpy[-(c + 1) :, d]

 numpy[:, l[-2]]
 numpy[:, ::-1]
 numpy[np.newaxis, :]
 numpy[:, l[-2]]
 numpy[:, ::-1]
 numpy[np.newaxis, :]


@@ -341,7 +341,7 @@ slice[:]
 slice[:-1]
 slice[1:]
 slice[::-1]
 slice[:-1]
 slice[1:]
 slice[::-1]

-slice[d::d + 1]
+slice[d :: d + 1]

 slice[:c, c - 1]
 numpy[:, 0:1]
 numpy[:, :-1]
 slice[:c, c - 1]
 numpy[:, 0:1]
 numpy[:, :-1]


@@ -355,8 +355,8 @@ numpy[4:, 2:]
 numpy[:, (0, 1, 2, 5)]
 numpy[0, [0]]
 numpy[:, [i]]
 numpy[:, (0, 1, 2, 5)]
 numpy[0, [0]]
 numpy[:, [i]]

-numpy[1:c + 1, c]
-numpy[-(c + 1):, d]
+numpy[1 : c + 1, c]
+numpy[-(c + 1) :, d]

 numpy[:, l[-2]]
 numpy[:, ::-1]
 numpy[np.newaxis, :]
 numpy[:, l[-2]]
 numpy[:, ::-1]
 numpy[np.newaxis, :]

diff --git a/tests/fmtonoff.py b/tests/fmtonoff.py
index a7b9bc744ed07148c3f7d445e22244b6ac90238c..0ff6672c86bb6aacf1b591fb607b81a9f664b8a1 100644 (file)
--- a/tests/fmtonoff.py
+++ b/tests/fmtonoff.py
@@ -121,7 +121,7 @@ def function_signature_stress_test(number:int,no_annotation=None,text:str='defau
 # fmt: on
 def spaces(a=1, b=(), c=[], d={}, e=True, f=-1, g=1 if False else 2, h="", i=r""):
     offset = attr.ib(default=attr.Factory(lambda: _r.uniform(10000, 200000)))
 # fmt: on
 def spaces(a=1, b=(), c=[], d={}, e=True, f=-1, g=1 if False else 2, h="", i=r""):
     offset = attr.ib(default=attr.Factory(lambda: _r.uniform(10000, 200000)))

-    assert task._cancel_stack[:len(old_stack)] == old_stack
+    assert task._cancel_stack[: len(old_stack)] == old_stack

 
 
 def spaces_types(
 
 
 def spaces_types(

diff --git a/tests/function.py b/tests/function.py
index 9a12bf60a9116a41649bd3d73cfee3ee3f90080e..4ec90571298f60abc862df0ee68c57b52a3be7b7 100644 (file)
--- a/tests/function.py
+++ b/tests/function.py
@@ -133,7 +133,7 @@ def function_signature_stress_test(
 
 def spaces(a=1, b=(), c=[], d={}, e=True, f=-1, g=1 if False else 2, h="", i=r""):
     offset = attr.ib(default=attr.Factory(lambda: _r.uniform(10000, 200000)))
 
 def spaces(a=1, b=(), c=[], d={}, e=True, f=-1, g=1 if False else 2, h="", i=r""):
     offset = attr.ib(default=attr.Factory(lambda: _r.uniform(10000, 200000)))

-    assert task._cancel_stack[:len(old_stack)] == old_stack
+    assert task._cancel_stack[: len(old_stack)] == old_stack

 
 
 def spaces_types(
 
 
 def spaces_types(

diff --git a/tests/slices.py b/tests/slices.py
new file mode 100644 (file)
index 0000000..7a42678
--- /dev/null
+++ b/tests/slices.py
@@ -0,0 +1,31 @@
+slice[a.b : c.d]
+slice[d :: d + 1]
+slice[d + 1 :: d]
+slice[d::d]
+slice[0]
+slice[-1]
+slice[:-1]
+slice[::-1]
+slice[:c, c - 1]
+slice[c, c + 1, d::]
+slice[ham[c::d] :: 1]
+slice[ham[cheese ** 2 : -1] : 1 : 1, ham[1:2]]
+slice[:-1:]
+slice[lambda: None : lambda: None]
+slice[lambda x, y, *args, really=2, **kwargs: None :, None::]
+slice[1 or 2 : True and False]
+slice[not so_simple : 1 < val <= 10]
+slice[(1 for i in range(42)) : x]
+slice[:: [i for i in range(42)]]
+
+
+async def f():
+    slice[await x : [i async for i in arange(42)] : 42]
+
+
+# These are from PEP-8:
+ham[1:9], ham[1:9:3], ham[:9:3], ham[1::3], ham[1:9:]
+ham[lower:upper], ham[lower:upper:], ham[lower::step]
+# ham[lower+offset : upper+offset]
+ham[: upper_fn(x) : step_fn(x)], ham[:: step_fn(x)]
+ham[lower + offset : upper + offset]

diff --git a/tests/test_black.py b/tests/test_black.py
index 94cbe35da391aca617be1dbde47a98cd797a4a9f..02926d1ae4a9fb5248390a35a2c57ceda8539f67 100644 (file)
--- a/tests/test_black.py
+++ b/tests/test_black.py
@@ -229,6 +229,14 @@ class BlackTestCase(unittest.TestCase):
         black.assert_equivalent(source, actual)
         black.assert_stable(source, actual, line_length=ll)
 
         black.assert_equivalent(source, actual)
         black.assert_stable(source, actual, line_length=ll)
 

+    @patch("black.dump_to_file", dump_to_stderr)
+    def test_slices(self) -> None:
+        source, expected = read_data("slices")
+        actual = fs(source)
+        self.assertFormatEqual(expected, actual)
+        black.assert_equivalent(source, actual)
+        black.assert_stable(source, actual, line_length=ll)
+

     @patch("black.dump_to_file", dump_to_stderr)
     def test_comments(self) -> None:
         source, expected = read_data("comments")
     @patch("black.dump_to_file", dump_to_stderr)
     def test_comments(self) -> None:
         source, expected = read_data("comments")