Don't explode trailers that fit in a single line

diff --git a/README.md b/README.md
index 3297f8797292aff5ab9e95aaabf289786afed9f6..69d423a694a5bbb26ad2de7b566ef49bde04c4e6 100644 (file)
--- a/README.md
+++ b/README.md
@@ -566,6 +566,9 @@ More details can be found in [CONTRIBUTING](CONTRIBUTING.md).
   on Python 3.6+ only code and Python 2.7+ code with the `unicode_literals`
   future import (#188, #198, #199)
 
   on Python 3.6+ only code and Python 2.7+ code with the `unicode_literals`
   future import (#188, #198, #199)
 

+* fixed trailers (content with brackets) being unnecessarily exploded
+  into their own lines after a dedented closing bracket
+

 * fixed an invalid trailing comma sometimes left in imports (#185)
 
 * fixed non-deterministic formatting when multiple pairs of removable parentheses
 * fixed an invalid trailing comma sometimes left in imports (#185)
 
 * fixed non-deterministic formatting when multiple pairs of removable parentheses

diff --git a/black.py b/black.py
index 43b9bd17f6f2dcd3b52648908e54239620ddd4b3..7823ae0afe2e809bdb2f5ca208ec9cc0565b4ced 100644 (file)
--- a/black.py
+++ b/black.py
@@ -24,6 +24,7 @@ from typing import (
     List,
     Optional,
     Pattern,
     List,
     Optional,
     Pattern,

+    Sequence,

     Set,
     Tuple,
     Type,
     Set,
     Tuple,
     Type,


@@ -41,6 +42,7 @@ from blib2to3 import pygram, pytree
 from blib2to3.pgen2 import driver, token
 from blib2to3.pgen2.parse import ParseError
 
 from blib2to3.pgen2 import driver, token
 from blib2to3.pgen2.parse import ParseError
 

+

 __version__ = "18.4a6"
 DEFAULT_LINE_LENGTH = 88
 
 __version__ = "18.4a6"
 DEFAULT_LINE_LENGTH = 88
 


@@ -1828,11 +1830,7 @@ def split_line(
         return
 
     line_str = str(line).strip("\n")
         return
 
     line_str = str(line).strip("\n")

-    if (
-        len(line_str) <= line_length
-        and "\n" not in line_str  # multiline strings
-        and not line.contains_standalone_comments()
-    ):
+    if is_line_short_enough(line, line_length=line_length, line_str=line_str):

         yield line
         return
 
         yield line
         return
 


@@ -1841,10 +1839,22 @@ def split_line(
         split_funcs = [left_hand_split]
     elif line.is_import:
         split_funcs = [explode_split]
         split_funcs = [left_hand_split]
     elif line.is_import:
         split_funcs = [explode_split]

-    elif line.inside_brackets:
-        split_funcs = [delimiter_split, standalone_comment_split, right_hand_split]

     else:
     else:

-        split_funcs = [right_hand_split]
+
+        def rhs(line: Line, py36: bool = False) -> Iterator[Line]:
+            for omit in generate_trailers_to_omit(line, line_length):
+                lines = list(right_hand_split(line, py36, omit=omit))
+                if is_line_short_enough(lines[0], line_length=line_length):
+                    yield from lines
+                    return
+
+            # All splits failed, best effort split with no omits.
+            yield from right_hand_split(line, py36)
+
+        if line.inside_brackets:
+            split_funcs = [delimiter_split, standalone_comment_split, rhs]
+        else:
+            split_funcs = [rhs]

     for split_func in split_funcs:
         # We are accumulating lines in `result` because we might want to abort
         # mission and return the original line in the end, or attempt a different
     for split_func in split_funcs:
         # We are accumulating lines in `result` because we might want to abort
         # mission and return the original line in the end, or attempt a different


@@ -1917,6 +1927,8 @@ def right_hand_split(
     """Split line into many lines, starting with the last matching bracket pair.
 
     If the split was by optional parentheses, attempt splitting without them, too.
     """Split line into many lines, starting with the last matching bracket pair.
 
     If the split was by optional parentheses, attempt splitting without them, too.

+    `omit` is a collection of closing bracket IDs that shouldn't be considered for
+    this split.

     """
     head = Line(depth=line.depth)
     body = Line(depth=line.depth + 1, inside_brackets=True)
     """
     head = Line(depth=line.depth)
     body = Line(depth=line.depth + 1, inside_brackets=True)


@@ -2446,6 +2458,67 @@ def is_python36(node: Node) -> bool:
     return False
 
 
     return False
 
 

+def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[LeafID]]:
+    """Generate sets of closing bracket IDs that should be omitted in a RHS.
+
+    Brackets can be omitted if the entire trailer up to and including
+    a preceding closing bracket fits in one line.
+
+    Yielded sets are cumulative (contain results of previous yields, too).  First
+    set is empty.
+    """
+
+    omit: Set[LeafID] = set()
+    yield omit
+
+    length = 4 * line.depth
+    opening_bracket = None
+    closing_bracket = None
+    optional_brackets: Set[LeafID] = set()
+    inner_brackets: Set[LeafID] = set()
+    for index, leaf in enumerate_reversed(line.leaves):
+        length += len(leaf.prefix) + len(leaf.value)
+        if length > line_length:
+            break
+
+        comment: Optional[Leaf]
+        for comment in line.comments_after(leaf, index):
+            if "\n" in comment.prefix:
+                break  # Oops, standalone comment!
+
+            length += len(comment.value)
+        else:
+            comment = None
+        if comment is not None:
+            break  # There was a standalone comment, we can't continue.
+
+        optional_brackets.discard(id(leaf))
+        if opening_bracket:
+            if leaf is opening_bracket:
+                opening_bracket = None
+            elif leaf.type in CLOSING_BRACKETS:
+                inner_brackets.add(id(leaf))
+        elif leaf.type in CLOSING_BRACKETS:
+            if not leaf.value:
+                optional_brackets.add(id(opening_bracket))
+                continue
+
+            if index > 0 and line.leaves[index - 1].type in OPENING_BRACKETS:
+                # Empty brackets would fail a split so treat them as "inner"
+                # brackets (e.g. only add them to the `omit` set if another
+                # pair of brackets was good enough.
+                inner_brackets.add(id(leaf))
+                continue
+
+            opening_bracket = leaf.opening_bracket
+            if closing_bracket:
+                omit.add(id(closing_bracket))
+                omit.update(inner_brackets)
+                inner_brackets.clear()
+                yield omit
+            closing_bracket = leaf
+
+

 def get_future_imports(node: Node) -> Set[str]:
     """Return a set of __future__ imports in the file."""
     imports = set()
 def get_future_imports(node: Node) -> Set[str]:
     """Return a set of __future__ imports in the file."""
     imports = set()


@@ -2723,6 +2796,28 @@ def sub_twice(regex: Pattern[str], replacement: str, original: str) -> str:
     return regex.sub(replacement, regex.sub(replacement, original))
 
 
     return regex.sub(replacement, regex.sub(replacement, original))
 
 

+def enumerate_reversed(sequence: Sequence[T]) -> Iterator[Tuple[Index, T]]:
+    """Like `reversed(enumerate(sequence))` if that were possible."""
+    index = len(sequence) - 1
+    for element in reversed(sequence):
+        yield (index, element)
+        index -= 1
+
+
+def is_line_short_enough(line: Line, *, line_length: int, line_str: str = "") -> bool:
+    """Return True if `line` is no longer than `line_length`.
+
+    Uses the provided `line_str` rendering, if any, otherwise computes a new one.
+    """
+    if not line_str:
+        line_str = str(line).strip("\n")
+    return (
+        len(line_str) <= line_length
+        and "\n" not in line_str  # multiline strings
+        and not line.contains_standalone_comments()
+    )
+
+

 CACHE_DIR = Path(user_cache_dir("black", version=__version__))
 
 
 CACHE_DIR = Path(user_cache_dir("black", version=__version__))
 
 

diff --git a/docs/reference/reference_functions.rst b/docs/reference/reference_functions.rst
index ede46a4507b418ddeed6103e989c9e37d533eb98..03604113b287ff9c0b0012b55a5997e757152ff4 100644 (file)
--- a/docs/reference/reference_functions.rst
+++ b/docs/reference/reference_functions.rst
@@ -20,6 +20,8 @@ Assertions and checks
 
 .. autofunction:: black.is_import
 
 
 .. autofunction:: black.is_import
 

+.. autofunction:: black.is_line_short_enough
+

 .. autofunction:: black.is_one_tuple
 
 .. autofunction:: black.is_python36
 .. autofunction:: black.is_one_tuple
 
 .. autofunction:: black.is_python36


@@ -94,6 +96,8 @@ Utilities
 
 .. autofunction:: black.ensure_visible
 
 
 .. autofunction:: black.ensure_visible
 

+.. autofunction:: black.enumerate_reversed
+

 .. autofunction:: black.generate_comments
 
 .. autofunction:: black.make_comment
 .. autofunction:: black.generate_comments
 
 .. autofunction:: black.make_comment

diff --git a/tests/comments4.py b/tests/comments4.py
index 241b6cee72e11e8e8b8b6d937a2f5aaea10b0a68..95299900af7df6ffd49fabe6074722665e52a8e9 100644 (file)
--- a/tests/comments4.py
+++ b/tests/comments4.py
@@ -63,14 +63,26 @@ def foo(list_a, list_b):
     results = (
         User.query.filter(User.foo == "bar").filter(  # Because foo.
             db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b))
     results = (
         User.query.filter(User.foo == "bar").filter(  # Because foo.
             db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b))

-        ).filter(
-            User.xyz.is_(None)
-        )
+        ).filter(User.xyz.is_(None))

         # Another comment about the filtering on is_quux goes here.
         .filter(db.not_(User.is_pending.astext.cast(db.Boolean).is_(True))).order_by(
             User.created_at.desc()
         # Another comment about the filtering on is_quux goes here.
         .filter(db.not_(User.is_pending.astext.cast(db.Boolean).is_(True))).order_by(
             User.created_at.desc()

-        ).with_for_update(
-            key_share=True
-        ).all()
+        ).with_for_update(key_share=True).all()

     )
     return results
     )
     return results

+
+
+def foo2(list_a, list_b):
+    # Standalone comment reasonably placed.
+    return User.query.filter(User.foo == "bar").filter(
+        db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b))
+    ).filter(User.xyz.is_(None))
+
+
+def foo3(list_a, list_b):
+    return (
+        # Standlone comment but weirdly placed.
+        User.query.filter(User.foo == "bar").filter(
+            db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b))
+        ).filter(User.xyz.is_(None))
+    )

diff --git a/tests/composition.py b/tests/composition.py
index 287888d984e086251610070df4e2316cac7dadf7..ea213cff274e4965f0d8f1b95b82a4cb3af725c8 100644 (file)
--- a/tests/composition.py
+++ b/tests/composition.py
@@ -32,3 +32,14 @@ class C:
                         # Another
                     ):
                         print(i)
                         # Another
                     ):
                         print(i)

+
+    def omitting_trailers() -> None:
+        get_collection(
+            hey_this_is_a_very_long_call, it_has_funny_attributes, really=True
+        )[OneLevelIndex]
+        get_collection(
+            hey_this_is_a_very_long_call, it_has_funny_attributes, really=True
+        )[OneLevelIndex][TwoLevelIndex][ThreeLevelIndex][FourLevelIndex]
+        d[0][1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20][21][
+            22
+        ]

diff --git a/tests/expression.diff b/tests/expression.diff
index 423cf5783efd418487fd6fa3456c236950e03174..835b7b0de2f31def2e45eeefb721f0552e37dd33 100644 (file)
--- a/tests/expression.diff
+++ b/tests/expression.diff
@@ -11,7 +11,7 @@
  True
  False
  1
  True
  False
  1

-@@ -29,62 +29,84 @@
+@@ -29,62 +29,82 @@

  ~great
  +value
  -1
  ~great
  +value
  -1


@@ -30,9 +30,7 @@
 -foo = (lambda port_id, ignore_missing: {"port1": port1_resource, "port2": port2_resource}[port_id])
 +foo = lambda port_id, ignore_missing: {
 +    "port1": port1_resource, "port2": port2_resource
 -foo = (lambda port_id, ignore_missing: {"port1": port1_resource, "port2": port2_resource}[port_id])
 +foo = lambda port_id, ignore_missing: {
 +    "port1": port1_resource, "port2": port2_resource

-+}[
-+    port_id
-+]
++}[port_id]

  1 if True else 2
  str or None if True else str or bytes or None
  (str or None) if True else (str or bytes or None)
  1 if True else 2
  str or None if True else str or bytes or None
  (str or None) if True else (str or bytes or None)


@@ -117,7 +115,7 @@
  call(**self.screen_kwargs)
  call(b, **self.screen_kwargs)
  lukasz.langa.pl
  call(**self.screen_kwargs)
  call(b, **self.screen_kwargs)
  lukasz.langa.pl

-@@ -93,11 +115,11 @@
+@@ -93,11 +113,11 @@

  1.0 .real
  ....__class__
  list[str]
  1.0 .real
  ....__class__
  list[str]


@@ -130,7 +128,7 @@
  ]
  slice[0]
  slice[0:1]
  ]
  slice[0]
  slice[0:1]

-@@ -124,103 +146,140 @@
+@@ -124,103 +144,138 @@

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


@@ -177,9 +175,7 @@
 +)
 +result = session.query(models.Customer.id).filter(
 +    models.Customer.account_id == account_id, models.Customer.email == email_address
 +)
 +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()
++).order_by(models.Customer.id.asc()).all()

  Ø = set()
  authors.łukasz.say_thanks()
  mapping = {
  Ø = set()
  authors.łukasz.say_thanks()
  mapping = {

diff --git a/tests/expression.py b/tests/expression.py
index 17b77c429d9d47d2244735f39725da8a88f26834..7bea9a7f2aa14bc8fcd78cb68e290b1c8f93695d 100644 (file)
--- a/tests/expression.py
+++ b/tests/expression.py
@@ -270,9 +270,7 @@ lambda a, b, c=True, *vararg, d=(v1 << 2), e="str", **kwargs: a + b
 manylambdas = lambda x=lambda y=lambda z=1: z: y(): x()
 foo = lambda port_id, ignore_missing: {
     "port1": port1_resource, "port2": port2_resource
 manylambdas = lambda x=lambda y=lambda z=1: z: y(): x()
 foo = lambda port_id, ignore_missing: {
     "port1": port1_resource, "port2": port2_resource

-}[
-    port_id
-]
+}[port_id]

 1 if True else 2
 str or None if True else str or bytes or None
 (str or None) if True else (str or bytes or None)
 1 if True else 2
 str or None if True else str or bytes or None
 (str or None) if True else (str or bytes or None)


@@ -411,9 +409,7 @@ what_is_up_with_those_new_coord_names = (
 )
 result = session.query(models.Customer.id).filter(
     models.Customer.account_id == account_id, models.Customer.email == email_address
 )
 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()
+).order_by(models.Customer.id.asc()).all()

 Ø = set()
 authors.łukasz.say_thanks()
 mapping = {
 Ø = set()
 authors.łukasz.say_thanks()
 mapping = {

diff --git a/tests/function.py b/tests/function.py
index ea27ebb9c7f6cfb20fdc540b43d96c5bf6e34062..4cfc945c6b2af62f19f61ed4244a87b37328c89a 100644 (file)
--- a/tests/function.py
+++ b/tests/function.py
@@ -169,9 +169,7 @@ def spaces2(result=_core.Value(None)):
 def example(session):
     result = session.query(models.Customer.id).filter(
         models.Customer.account_id == account_id, models.Customer.email == email_address
 def example(session):
     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()
+    ).order_by(models.Customer.id.asc()).all()

 
 
 def long_lines():
 
 
 def long_lines():

diff --git a/tests/function2.py b/tests/function2.py
index 1b9d7b64a0476cc2212452fb615cf8ddffa503cc..e262e0588c8373e4cbab50f389e4c2c5d6c48db9 100644 (file)
--- a/tests/function2.py
+++ b/tests/function2.py
@@ -2,6 +2,11 @@ def f(
   a,
   **kwargs,
 ) -> A:
   a,
   **kwargs,
 ) -> A:

+    with cache_dir():
+        if something:
+            result = (
+                CliRunner().invoke(black.main, [str(src1), str(src2), "--diff", "--check"])
+            )

     return A(
         very_long_argument_name1=very_long_value_for_the_argument,
         very_long_argument_name2=very_long_value_for_the_argument,
     return A(
         very_long_argument_name1=very_long_value_for_the_argument,
         very_long_argument_name2=very_long_value_for_the_argument,


@@ -11,6 +16,11 @@ def f(
 # output
 
 def f(a, **kwargs) -> A:
 # output
 
 def f(a, **kwargs) -> A:

+    with cache_dir():
+        if something:
+            result = CliRunner().invoke(
+                black.main, [str(src1), str(src2), "--diff", "--check"]
+            )

     return A(
         very_long_argument_name1=very_long_value_for_the_argument,
         very_long_argument_name2=very_long_value_for_the_argument,
     return A(
         very_long_argument_name1=very_long_value_for_the_argument,
         very_long_argument_name2=very_long_value_for_the_argument,