Skip to content

Commit

Permalink
Fix unnecessary parentheses when a line contains multiline strings
Browse files Browse the repository at this point in the history
Fixes psf#232
  • Loading branch information
ambv committed Jun 5, 2018
1 parent 23a00f0 commit d638d56
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 15 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
84 changes: 69 additions & 15 deletions black.py
Original file line number Diff line number Diff line change
Expand Up @@ -1094,6 +1094,13 @@ def contains_standalone_comments(self, depth_limit: int = sys.maxsize) -> bool:

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 (
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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?
Expand Down
14 changes: 14 additions & 0 deletions tests/cantfit.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@
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
Expand Down Expand Up @@ -71,3 +78,10 @@
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))

0 comments on commit d638d56

Please sign in to comment.