From: Łukasz Langa <lukasz@langa.pl>
Date: Tue, 5 Jun 2018 04:27:51 +0000 (-0700)
Subject: Fix unnecessary parentheses when a line contains multiline strings
X-Git-Url: https://git.madduck.net/etc/vim.git/commitdiff_plain/d638d56e0e383caeeff048e76b02b50b29367c3d

Fix unnecessary parentheses when a line contains multiline strings

Fixes #232
---

diff --git a/README.md b/README.md
index bfd77f9..ac25db1 100644
--- a/README.md
+++ b/README.md
@@ -721,6 +721,8 @@ More details can be found in [CONTRIBUTING](CONTRIBUTING.md).
 
 * fixed long trivial assignments being wrapped in unnecessary parentheses (#273)
 
+* fixed unnecessary parentheses when a line contained multiline strings (#232)
+
 * fixed stdin handling not working correctly if an old version of Click was
   used (#276)
 
diff --git a/black.py b/black.py
index 35af598..551d3c1 100644
--- a/black.py
+++ b/black.py
@@ -1094,6 +1094,13 @@ class Line:
 
         return False
 
+    def contains_multiline_strings(self) -> bool:
+        for leaf in self.leaves:
+            if is_multiline_string(leaf):
+                return True
+
+        return False
+
     def maybe_remove_trailing_comma(self, closing: Leaf) -> bool:
         """Remove trailing comma if there is one and it's safe."""
         if not (
@@ -2225,24 +2232,35 @@ def right_hand_split(
         # the closing bracket is an optional paren
         and closing_bracket.type == token.RPAR
         and not closing_bracket.value
-        # 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)
+        # 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
+        # there are no standalone comments in the body
+        and not body.contains_standalone_comments(0)
+        # and we can actually remove the parens
+        and can_omit_invisible_parens(body, line_length)
     ):
         omit = {id(closing_bracket), *omit}
-        if can_omit_invisible_parens(body, line_length):
-            try:
-                yield from right_hand_split(line, line_length, py36=py36, omit=omit)
-                return
-            except CannotSplit:
-                if len(body.leaves) == 1 and not is_line_short_enough(
-                    body, line_length=line_length
-                ):
-                    raise CannotSplit(
-                        "Splitting failed, body is still too long and can't be split."
-                    )
+        try:
+            yield from right_hand_split(line, line_length, py36=py36, omit=omit)
+            return
+
+        except CannotSplit:
+            if not (
+                can_be_split(body)
+                or is_line_short_enough(body, line_length=line_length)
+            ):
+                raise CannotSplit(
+                    "Splitting failed, body is still too long and can't be split."
+                )
+
+            elif head.contains_multiline_strings() or tail.contains_multiline_strings():
+                raise CannotSplit(
+                    "The current optional pair of parentheses is bound to fail to "
+                    "satisfy the splitting algorithm becase the head or the tail "
+                    "contains multiline strings which by definition never fit one "
+                    "line."
+                )
 
     ensure_visible(opening_bracket)
     ensure_visible(closing_bracket)
@@ -3190,6 +3208,42 @@ def is_line_short_enough(line: Line, *, line_length: int, line_str: str = "") ->
     )
 
 
+def can_be_split(line: Line) -> bool:
+    """Return False if the line cannot be split *for sure*.
+
+    This is not an exhaustive search but a cheap heuristic that we can use to
+    avoid some unfortunate formattings (mostly around wrapping unsplittable code
+    in unnecessary parentheses).
+    """
+    leaves = line.leaves
+    if len(leaves) < 2:
+        return False
+
+    if leaves[0].type == token.STRING and leaves[1].type == token.DOT:
+        call_count = 0
+        dot_count = 0
+        next = leaves[-1]
+        for leaf in leaves[-2::-1]:
+            if leaf.type in OPENING_BRACKETS:
+                if next.type not in CLOSING_BRACKETS:
+                    return False
+
+                call_count += 1
+            elif leaf.type == token.DOT:
+                dot_count += 1
+            elif leaf.type == token.NAME:
+                if not (next.type == token.DOT or next.type in OPENING_BRACKETS):
+                    return False
+
+            elif leaf.type not in CLOSING_BRACKETS:
+                return False
+
+            if dot_count > 1 and call_count > 1:
+                return False
+
+    return True
+
+
 def can_omit_invisible_parens(line: Line, line_length: int) -> bool:
     """Does `line` have a shape safe to reformat without optional parens around it?
 
diff --git a/tests/cantfit.py b/tests/cantfit.py
index 816cdef..e15e69c 100644
--- a/tests/cantfit.py
+++ b/tests/cantfit.py
@@ -28,6 +28,13 @@ normal_name = normal_function_name(
 string_variable_name = (
     "a string that is waaaaaaaayyyyyyyy too long, even in parens, there's nothing you can do"  # noqa
 )
+for key in """
+    hostname
+    port
+    username
+""".split():
+    if key in self.connect_kwargs:
+        raise ValueError(err.format(key))
 
 
 # output
@@ -71,3 +78,10 @@ normal_name = normal_function_name(
     this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it=0,
 )
 string_variable_name = "a string that is waaaaaaaayyyyyyyy too long, even in parens, there's nothing you can do"  # noqa
+for key in """
+    hostname
+    port
+    username
+""".split():
+    if key in self.connect_kwargs:
+        raise ValueError(err.format(key))