Don't use optional parentheses in unnecessary situations

If an expression starts or ends with a bracket and only contains a single
delimiter, don't wrap it in additional optional parentheses.  We can use the
brackets for the split.

Fixes #177

Fixes #193

diff --git a/README.md b/README.md
index 6c99fd32a7c5053fa084d8ecf906acd55ba29d38..1570348ade6060f93d6192e9185233c09cf35d74 100644 (file)
--- a/README.md
+++ b/README.md
@@ -597,6 +597,9 @@ More details can be found in [CONTRIBUTING](CONTRIBUTING.md).
 * math operators now use their respective priorities for delimiting multiline
   expressions (#148)
 
 * math operators now use their respective priorities for delimiting multiline
   expressions (#148)
 

+* optional parentheses are now omitted on expressions that start or end
+  with a bracket and only contain a single operator (#177)
+

 * empty parentheses in a class definition are now removed (#145, #180)
 
 * string prefixes are now standardized to lowercase and `u` is removed
 * empty parentheses in a class definition are now removed (#145, #180)
 
 * string prefixes are now standardized to lowercase and `u` is removed


@@ -621,6 +624,8 @@ More details can be found in [CONTRIBUTING](CONTRIBUTING.md).
   where used both in function signatures with stars and function calls
   with stars but the former would be reformatted to a single line.
 
   where used both in function signatures with stars and function calls
   with stars but the former would be reformatted to a single line.
 

+* fixed crash on dealing with optional parentheses (#193)
+

 * fixed crash when dead symlinks where encountered
 
 
 * fixed crash when dead symlinks where encountered
 
 

diff --git a/black.py b/black.py
index a21b51aea04b9ceecb37436b49b81b60a4522b07..e7a7aa8585836c72a5d24dfa35fda0089462578a 100644 (file)
--- a/black.py
+++ b/black.py
@@ -239,11 +239,8 @@ def reformat_one(
                 src = src.resolve()
                 if src in cache and cache[src] == get_cache_info(src):
                     changed = Changed.CACHED
                 src = src.resolve()
                 if src in cache and cache[src] == get_cache_info(src):
                     changed = Changed.CACHED

-            if (
-                changed is not Changed.CACHED
-                and format_file_in_place(
-                    src, line_length=line_length, fast=fast, write_back=write_back
-                )
+            if changed is not Changed.CACHED and format_file_in_place(
+                src, line_length=line_length, fast=fast, write_back=write_back

             ):
                 changed = Changed.YES
             if write_back == WriteBack.YES and changed is not Changed.NO:
             ):
                 changed = Changed.YES
             if write_back == WriteBack.YES and changed is not Changed.NO:


@@ -860,14 +857,11 @@ class Line:
             second_leaf: Optional[Leaf] = self.leaves[1]
         except IndexError:
             second_leaf = None
             second_leaf: Optional[Leaf] = self.leaves[1]
         except IndexError:
             second_leaf = None

-        return (
-            (first_leaf.type == token.NAME and first_leaf.value == "def")
-            or (
-                first_leaf.type == token.ASYNC
-                and second_leaf is not None
-                and second_leaf.type == token.NAME
-                and second_leaf.value == "def"
-            )
+        return (first_leaf.type == token.NAME and first_leaf.value == "def") or (
+            first_leaf.type == token.ASYNC
+            and second_leaf is not None
+            and second_leaf.type == token.NAME
+            and second_leaf.value == "def"

         )
 
     @property
         )
 
     @property


@@ -1032,9 +1026,8 @@ class Line:
             and subscript_start.type == syms.subscriptlist
         ):
             subscript_start = child_towards(subscript_start, leaf)
             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())
+        return subscript_start is not None and any(
+            n.type in TEST_DESCENDANTS for n in subscript_start.pre_order()

         )
 
     def __str__(self) -> str:
         )
 
     def __str__(self) -> str:


@@ -1999,8 +1992,9 @@ def right_hand_split(
     # Since body is a new indent level, remove spurious leading whitespace.
     if body_leaves:
         normalize_prefix(body_leaves[0], inside_brackets=True)
     # Since body is a new indent level, remove spurious leading whitespace.
     if body_leaves:
         normalize_prefix(body_leaves[0], inside_brackets=True)

-    elif not head_leaves:
-        # No `head` and no `body` means the split failed. `tail` has all content.
+    if not head_leaves:
+        # No `head` means the split failed. Either `tail` has all content or
+        # the matching `opening_bracket` wasn't available on `line` anymore.

         raise CannotSplit("No brackets found")
 
     # Build the new lines.
         raise CannotSplit("No brackets found")
 
     # Build the new lines.


@@ -2018,19 +2012,27 @@ def right_hand_split(
         # the closing bracket is an optional paren
         and closing_bracket.type == token.RPAR
         and not closing_bracket.value
         # the closing bracket is an optional paren
         and closing_bracket.type == token.RPAR
         and not closing_bracket.value

-        # there are no delimiters or standalone comments in the body
-        and not body.bracket_tracker.delimiters
+        # there are no standalone comments in the body

         and not line.contains_standalone_comments(0)
         # and it's not an import (optional parens are the only thing we can split
         # on in this case; attempting a split without them is a waste of time)
         and not line.is_import
     ):
         omit = {id(closing_bracket), *omit}
         and not line.contains_standalone_comments(0)
         # and it's not an import (optional parens are the only thing we can split
         # on in this case; attempting a split without them is a waste of time)
         and not line.is_import
     ):
         omit = {id(closing_bracket), *omit}

-        try:
-            yield from right_hand_split(line, py36=py36, omit=omit)
-            return
-        except CannotSplit:
-            pass
+        delimiter_count = len(body.bracket_tracker.delimiters)
+        if (
+            delimiter_count == 0
+            or delimiter_count == 1
+            and (
+                body.leaves[0].type in OPENING_BRACKETS
+                or body.leaves[-1].type in CLOSING_BRACKETS
+            )
+        ):
+            try:
+                yield from right_hand_split(line, py36=py36, omit=omit)
+                return
+            except CannotSplit:
+                pass

 
     ensure_visible(opening_bracket)
     ensure_visible(closing_bracket)
 
     ensure_visible(opening_bracket)
     ensure_visible(closing_bracket)

diff --git a/tests/composition.py b/tests/composition.py
index ea213cff274e4965f0d8f1b95b82a4cb3af725c8..c3dfd3db042de048c26e43ad368d049557db4e8c 100644 (file)
--- a/tests/composition.py
+++ b/tests/composition.py
@@ -33,7 +33,7 @@ class C:
                     ):
                         print(i)
 
                     ):
                         print(i)
 

-    def omitting_trailers() -> None:
+    def omitting_trailers(self) -> 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]


@@ -43,3 +43,111 @@ class C:
         d[0][1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20][21][
             22
         ]
         d[0][1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20][21][
             22
         ]

+
+    def easy_asserts(self) -> None:
+        assert {
+            key1: value1,
+            key2: value2,
+            key3: value3,
+            key4: value4,
+            key5: value5,
+            key6: value6,
+            key7: value7,
+            key8: value8,
+            key9: value9,
+        } == expected, "Not what we expected"
+
+        assert expected == {
+            key1: value1,
+            key2: value2,
+            key3: value3,
+            key4: value4,
+            key5: value5,
+            key6: value6,
+            key7: value7,
+            key8: value8,
+            key9: value9,
+        }, "Not what we expected"
+
+        assert expected == {
+            key1: value1,
+            key2: value2,
+            key3: value3,
+            key4: value4,
+            key5: value5,
+            key6: value6,
+            key7: value7,
+            key8: value8,
+            key9: value9,
+        }
+
+    def tricky_asserts(self) -> None:
+        assert {
+            key1: value1,
+            key2: value2,
+            key3: value3,
+            key4: value4,
+            key5: value5,
+            key6: value6,
+            key7: value7,
+            key8: value8,
+            key9: value9,
+        } == expected(
+            value, is_going_to_be="too long to fit in a single line", srsly=True
+        ), "Not what we expected"
+
+        assert {
+            key1: value1,
+            key2: value2,
+            key3: value3,
+            key4: value4,
+            key5: value5,
+            key6: value6,
+            key7: value7,
+            key8: value8,
+            key9: value9,
+        } == expected, (
+            "Not what we expected and the message is too long to fit in one line"
+        )
+
+        assert expected(
+            value, is_going_to_be="too long to fit in a single line", srsly=True
+        ) == {
+            key1: value1,
+            key2: value2,
+            key3: value3,
+            key4: value4,
+            key5: value5,
+            key6: value6,
+            key7: value7,
+            key8: value8,
+            key9: value9,
+        }, "Not what we expected"
+
+        assert expected == {
+            key1: value1,
+            key2: value2,
+            key3: value3,
+            key4: value4,
+            key5: value5,
+            key6: value6,
+            key7: value7,
+            key8: value8,
+            key9: value9,
+        }, (
+            "Not what we expected and the message is too long to fit "
+            "in one line because it's too long"
+        )
+
+        # This is weird but true.
+        assert expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect == {
+            key1: value1,
+            key2: value2,
+            key3: value3,
+            key4: value4,
+            key5: value5,
+            key6: value6,
+            key7: value7,
+            key8: value8,
+            key9: value9,
+        }

diff --git a/tests/empty_lines.py b/tests/empty_lines.py
index 5b7ce928a783ed27c4fd23446680a4989a8a3c0c..4c03e432383c174127c803dea8076488a09a8daf 100644 (file)
--- a/tests/empty_lines.py
+++ b/tests/empty_lines.py
@@ -123,29 +123,23 @@ def f():
             return NO
 
         if prevp.type == token.EQUAL:
             return NO
 
         if prevp.type == token.EQUAL:

-            if (
-                prevp.parent
-                and prevp.parent.type in {
-                    syms.typedargslist,
-                    syms.varargslist,
-                    syms.parameters,
-                    syms.arglist,
-                    syms.argument,
-                }
-            ):
+            if prevp.parent and prevp.parent.type in {
+                syms.typedargslist,
+                syms.varargslist,
+                syms.parameters,
+                syms.arglist,
+                syms.argument,
+            }:

                 return NO
 
         elif prevp.type == token.DOUBLESTAR:
                 return NO
 
         elif prevp.type == token.DOUBLESTAR:

-            if (
-                prevp.parent
-                and prevp.parent.type in {
-                    syms.typedargslist,
-                    syms.varargslist,
-                    syms.parameters,
-                    syms.arglist,
-                    syms.dictsetmaker,
-                }
-            ):
+            if prevp.parent and prevp.parent.type in {
+                syms.typedargslist,
+                syms.varargslist,
+                syms.parameters,
+                syms.arglist,
+                syms.dictsetmaker,
+            }:

                 return NO
 
 
                 return NO
 
 


@@ -183,14 +177,11 @@ def g():
             return NO
 
         if prevp.type == token.EQUAL:
             return NO
 
         if prevp.type == token.EQUAL:

-            if (
-                prevp.parent
-                and prevp.parent.type in {
-                    syms.typedargslist,
-                    syms.varargslist,
-                    syms.parameters,
-                    syms.arglist,
-                    syms.argument,
-                }
-            ):
+            if prevp.parent and prevp.parent.type in {
+                syms.typedargslist,
+                syms.varargslist,
+                syms.parameters,
+                syms.arglist,
+                syms.argument,
+            }:

                 return NO
                 return NO

diff --git a/tests/expression.diff b/tests/expression.diff
index 835b7b0de2f31def2e45eeefb721f0552e37dd33..5ad905f0f616febc48d1b38c26e30dd8635a2903 100644 (file)
--- a/tests/expression.diff
+++ b/tests/expression.diff
@@ -167,11 +167,11 @@
 -what_is_up_with_those_new_coord_names = (coord_names + set(vars_to_create)) + set(vars_to_remove)
 -what_is_up_with_those_new_coord_names = (coord_names | set(vars_to_create)) - set(vars_to_remove)
 -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()
 -what_is_up_with_those_new_coord_names = (coord_names + set(vars_to_create)) + set(vars_to_remove)
 -what_is_up_with_those_new_coord_names = (coord_names | set(vars_to_create)) - set(vars_to_remove)
 -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()

-+what_is_up_with_those_new_coord_names = (
-+    (coord_names + set(vars_to_create)) + set(vars_to_remove)
++what_is_up_with_those_new_coord_names = (coord_names + set(vars_to_create)) + set(
++    vars_to_remove

 +)
 +)

-+what_is_up_with_those_new_coord_names = (
-+    (coord_names | set(vars_to_create)) - set(vars_to_remove)
++what_is_up_with_those_new_coord_names = (coord_names | set(vars_to_create)) - set(
++    vars_to_remove

 +)
 +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


@@ -256,8 +256,8 @@
 -    ~ aaaaaaaaaaaaaaaa.a + aaaaaaaaaaaaaaaa.b - aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e | aaaaaaaaaaaaaaaa.f & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h ^ aaaaaaaaaaaaaaaa.i << aaaaaaaaaaaaaaaa.k >> aaaaaaaaaaaaaaaa.l ** aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n
 +print(*lambda x: x)
 +assert not Test, "Short message"
 -    ~ aaaaaaaaaaaaaaaa.a + aaaaaaaaaaaaaaaa.b - aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e | aaaaaaaaaaaaaaaa.f & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h ^ aaaaaaaaaaaaaaaa.i << aaaaaaaaaaaaaaaa.k >> aaaaaaaaaaaaaaaa.l ** aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n
 +print(*lambda x: x)
 +assert not Test, "Short message"

-+assert (
-+    this is ComplexTest and not requirements.fit_in_a_single_line(force=False)
++assert this is ComplexTest and not requirements.fit_in_a_single_line(
++    force=False

 +), "Short message"
 +assert parens is TooMany
 +for (x,) in (1,), (2,), (3,):
 +), "Short message"
 +assert parens is TooMany
 +for (x,) in (1,), (2,), (3,):

diff --git a/tests/expression.py b/tests/expression.py
index 7bea9a7f2aa14bc8fcd78cb68e290b1c8f93695d..65833c12efcb30c327eb56da3c04d850992a8b75 100644 (file)
--- a/tests/expression.py
+++ b/tests/expression.py
@@ -401,11 +401,11 @@ b = (1,)
 c = 1
 d = (1,) + a + (2,)
 e = (1,).count(1)
 c = 1
 d = (1,) + a + (2,)
 e = (1,).count(1)

-what_is_up_with_those_new_coord_names = (
-    (coord_names + set(vars_to_create)) + set(vars_to_remove)
+what_is_up_with_those_new_coord_names = (coord_names + set(vars_to_create)) + set(
+    vars_to_remove

 )
 )

-what_is_up_with_those_new_coord_names = (
-    (coord_names | set(vars_to_create)) - set(vars_to_remove)
+what_is_up_with_those_new_coord_names = (coord_names | set(vars_to_create)) - set(
+    vars_to_remove

 )
 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


@@ -433,8 +433,8 @@ print(*[] or [1])
 print(**{1: 3} if False else {x: x for x in range(3)})
 print(*lambda x: x)
 assert not Test, "Short message"
 print(**{1: 3} if False else {x: x for x in range(3)})
 print(*lambda x: x)
 assert not Test, "Short message"

-assert (
-    this is ComplexTest and not requirements.fit_in_a_single_line(force=False)
+assert this is ComplexTest and not requirements.fit_in_a_single_line(
+    force=False

 ), "Short message"
 assert parens is TooMany
 for (x,) in (1,), (2,), (3,):
 ), "Short message"
 assert parens is TooMany
 for (x,) in (1,), (2,), (3,):