Make parentheses invisible recursively in atoms

This fixes non-deterministic formatting when multiple pairs of removable
parentheses are used.

Fixes #183

README.md

@@ -538,9 +538,14 @@ More details can be found in [CONTRIBUTING](CONTRIBUTING.md).
 
 ### 18.5a0 (unreleased)
 
-* Slices are now formatted according to PEP 8 (#178)
+* slices are now formatted according to PEP 8 (#178)
 
-* Empty parentheses in a class definition are removed (#145, #180)
+* empty parentheses in a class definition are now removed (#145, #180)
+
+* fixed an invalid trailing comma sometimes left in imports (#185)
+
+* fixed non-deterministic formatting when multiple pairs of removable parentheses
+  were used (#183)
 
 
 ### 18.4a4

black.py

@@ -2187,17 +2187,7 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None:
     for child in list(node.children):
         if check_lpar:
             if child.type == syms.atom:
-                if not (
-                    is_empty_tuple(child)
-                    or is_one_tuple(child)
-                    or max_delimiter_priority_in_atom(child) >= COMMA_PRIORITY
-                ):
-                    first = child.children[0]
-                    last = child.children[-1]
-                    if first.type == token.LPAR and last.type == token.RPAR:
-                        # make parentheses invisible
-                        first.value = ""  # type: ignore
-                        last.value = ""  # type: ignore
+                maybe_make_parens_invisible_in_atom(child)
             elif is_one_tuple(child):
                 # wrap child in visible parentheses
                 lpar = Leaf(token.LPAR, "(")
@@ -2214,6 +2204,29 @@ def normalize_invisible_parens(node: Node, parens_after: Set[str]) -> None:
         check_lpar = isinstance(child, Leaf) and child.value in parens_after
 
 
+def maybe_make_parens_invisible_in_atom(node: LN) -> bool:
+    """If it's safe, make the parens in the atom `node` invisible, recusively."""
+    if (
+        node.type != syms.atom
+        or is_empty_tuple(node)
+        or is_one_tuple(node)
+        or max_delimiter_priority_in_atom(node) >= COMMA_PRIORITY
+    ):
+        return False
+
+    first = node.children[0]
+    last = node.children[-1]
+    if first.type == token.LPAR and last.type == token.RPAR:
+        # make parentheses invisible
+        first.value = ""  # type: ignore
+        last.value = ""  # type: ignore
+        if len(node.children) > 1:
+            maybe_make_parens_invisible_in_atom(node.children[1])
+        return True
+
+    return False
+
+
 def is_empty_tuple(node: LN) -> bool:
     """Return True if `node` holds an empty tuple."""
     return (

docs/reference/reference_functions.rst

@@ -98,6 +98,8 @@ Utilities
 
 .. autofunction:: black.make_comment
 
+.. autofunction:: black.maybe_make_parens_invisible_in_atom
+
 .. autofunction:: black.max_delimiter_priority_in_atom
 
 .. autofunction:: black.normalize_prefix

tests/expression.diff

@@ -129,7 +129,7 @@
  ]
  slice[0]
  slice[0:1]
-@@ -123,88 +145,114 @@
+@@ -123,91 +145,119 @@
  numpy[-(c + 1) :, d]
  numpy[:, l[-2]]
  numpy[:, ::-1]
@@ -201,6 +201,9 @@
 +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")
+-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,): ...
 -for y in (): ...
 -for z in (i for i in (1, 2, 3)): ...
@@ -242,6 +245,11 @@
 -    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa /
 -    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 +print(*lambda x: x)
++assert not Test, "Short message"
++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,):
 +    ...
 +for y in ():

tests/expression.py

@@ -163,6 +163,9 @@ async def f():
 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")
+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,): ...
 for y in (): ...
 for z in (i for i in (1, 2, 3)): ...
@@ -419,6 +422,11 @@ async def f():
 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"
+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,):
     ...
 for y in ():