From: Zsolt Dollenstein Date: Tue, 1 May 2018 05:49:30 +0000 (-0700) Subject: Format subscriptions in a PEP-8 compliant way (#178) X-Git-Url: https://git.madduck.net/etc/vim.git/commitdiff_plain/9f096d55365cb63548eef97e254c2793ae2776a0 Format subscriptions in a PEP-8 compliant way (#178) Fixes #157 --- diff --git a/.flake8 b/.flake8 index 5838390..fae93b0 100644 --- a/.flake8 +++ b/.flake8 @@ -2,7 +2,7 @@ # 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 diff --git a/README.md b/README.md index bc8977b..31f92c9 100644 --- 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. +### 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 diff --git a/black.py b/black.py index 4c5f0f0..5e087d1 100644 --- a/black.py +++ b/black.py @@ -89,11 +89,11 @@ class FormatError(Exception): 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.""" - unformatted_prefix = leaf.prefix[:self.consumed] + unformatted_prefix = leaf.prefix[: self.consumed] return Leaf(token.NEWLINE, unformatted_prefix) @@ -582,6 +582,23 @@ UNPACKING_PARENTS = { 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 @@ -698,6 +715,10 @@ class BracketTracker: 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: @@ -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. - 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) @@ -859,7 +882,7 @@ class Line: else: return False - for leaf in self.leaves[_opening_index + 1:]: + for leaf in self.leaves[_opening_index + 1 :]: if leaf is closing: break @@ -920,6 +943,24 @@ class Line: 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: @@ -1303,8 +1344,12 @@ BRACKETS = OPENING_BRACKETS | CLOSING_BRACKETS 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 = " " @@ -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}" - 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 @@ -1328,7 +1376,13 @@ def whitespace(leaf: Leaf) -> str: # noqa C901 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: @@ -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}: - return NO + return SPACE if complex_subscript else NO elif ( prevp.parent @@ -1455,7 +1509,7 @@ def whitespace(leaf: Leaf) -> str: # noqa C901 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" @@ -1464,7 +1518,7 @@ def whitespace(leaf: Leaf) -> str: # noqa C901 return NO - else: + elif not complex_subscript: return NO elif p.type == syms.atom: @@ -1534,6 +1588,14 @@ def preceding_leaf(node: Optional[LN]) -> Optional[Leaf]: 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. @@ -1994,6 +2056,7 @@ def explode_split( try: yield from delimiter_split(new_lines[1], py36) + 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}") - 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 diff --git a/tests/expression.diff b/tests/expression.diff index 11c1355..309a480 100644 --- a/tests/expression.diff +++ b/tests/expression.diff @@ -130,7 +130,7 @@ 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, :] diff --git a/tests/expression.py b/tests/expression.py index 274c150..d170a66 100644 --- a/tests/expression.py +++ b/tests/expression.py @@ -105,7 +105,7 @@ slice[:] slice[:-1] slice[1:] slice[::-1] -slice[d::d + 1] +slice[d :: d + 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[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, :] @@ -341,7 +341,7 @@ slice[:] slice[:-1] slice[1:] slice[::-1] -slice[d::d + 1] +slice[d :: d + 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[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, :] diff --git a/tests/fmtonoff.py b/tests/fmtonoff.py index a7b9bc7..0ff6672 100644 --- 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))) - assert task._cancel_stack[:len(old_stack)] == old_stack + assert task._cancel_stack[: len(old_stack)] == old_stack def spaces_types( diff --git a/tests/function.py b/tests/function.py index 9a12bf6..4ec9057 100644 --- 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))) - assert task._cancel_stack[:len(old_stack)] == old_stack + assert task._cancel_stack[: len(old_stack)] == old_stack def spaces_types( diff --git a/tests/slices.py b/tests/slices.py new file mode 100644 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 94cbe35..02926d1 100644 --- 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) + @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")