Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash on assert and parenthesized % format (fixes #1597, fixes #1605) #1681

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 23 additions & 4 deletions src/black/__init__.py
Expand Up @@ -112,6 +112,10 @@ class InvalidInput(ValueError):
"""Raised when input source code fails all parse attempts."""


class BracketMatchError(KeyError):
"""Raised when an opening bracket is unable to be matched to a closing bracket."""


T = TypeVar("T")
E = TypeVar("E", bound=Exception)

Expand Down Expand Up @@ -1308,7 +1312,13 @@ def mark(self, leaf: Leaf) -> None:
self.maybe_decrement_after_lambda_arguments(leaf)
if leaf.type in CLOSING_BRACKETS:
self.depth -= 1
opening_bracket = self.bracket_match.pop((self.depth, leaf.type))
try:
opening_bracket = self.bracket_match.pop((self.depth, leaf.type))
except KeyError as e:
raise BracketMatchError(
"Unable to match a closing bracket to the following opening"
f" bracket: {leaf}"
) from e
leaf.opening_bracket = opening_bracket
if not leaf.value:
self.invisible.append(leaf)
Expand Down Expand Up @@ -3324,10 +3334,17 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]:
yield TErr(
"Will not strip parentheses which have comments attached to them."
)
return

new_line = line.clone()
new_line.comments = line.comments.copy()
append_leaves(new_line, line, LL[: string_idx - 1])
try:
append_leaves(new_line, line, LL[: string_idx - 1])
except BracketMatchError:
# HACK: I believe there is currently a bug somewhere in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunate, but it's better than a crash, and the formatting in the newly added test cases looks reasonable.

Copy link
Contributor Author

@bbugyi200 bbugyi200 Sep 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I don't think this will do. The append_leaves() function modifies the Node structure of the leaves and so must be atomic. I will have to come up with a different solution.

Copy link
Contributor Author

@bbugyi200 bbugyi200 Sep 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. This is fixed. The corrected version of this fix, though still not ideal, has the benefit of stripping the parens as is consistent with blacks style after #1132.

# right_hand_split() that is causing brackets to not be tracked
# properly by a shared BracketTracker.
append_leaves(new_line, line, LL[: string_idx - 1], preformatted=True)

string_leaf = Leaf(token.STRING, LL[string_idx].value)
LL[string_idx - 1].remove()
Expand Down Expand Up @@ -4598,7 +4615,9 @@ def line_to_string(line: Line) -> str:
return str(line).strip("\n")


def append_leaves(new_line: Line, old_line: Line, leaves: List[Leaf]) -> None:
def append_leaves(
new_line: Line, old_line: Line, leaves: List[Leaf], preformatted: bool = False
) -> None:
"""
Append leaves (taken from @old_line) to @new_line, making sure to fix the
underlying Node structure where appropriate.
Expand All @@ -4614,7 +4633,7 @@ def append_leaves(new_line: Line, old_line: Line, leaves: List[Leaf]) -> None:
for old_leaf in leaves:
new_leaf = Leaf(old_leaf.type, old_leaf.value)
replace_child(old_leaf, new_leaf)
new_line.append(new_leaf)
new_line.append(new_leaf, preformatted=preformatted)

for comment_leaf in old_line.comments_after(old_leaf):
new_line.append(comment_leaf, preformatted=True)
Expand Down
42 changes: 42 additions & 0 deletions tests/data/long_strings__regression.py
Expand Up @@ -330,6 +330,25 @@ def who(self):
% (i)
)

def A():
def B():
def C():
def D():
def E():
def F():
def G():
assert (
c_float(val[0][0] / val[0][1]).value
== c_float(value[0][0] / value[0][1]).value
), "%s didn't roundtrip" % tag

class xxxxxxxxxxxxxxxxxxxxx(xxxx.xxxxxxxxxxxxx):
def xxxxxxx_xxxxxx(xxxx):
assert xxxxxxx_xxxx in [
x.xxxxx.xxxxxx.xxxxx.xxxxxx,
x.xxxxx.xxxxxx.xxxxx.xxxx,
], ("xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx)

# output


Expand Down Expand Up @@ -742,3 +761,26 @@ def who(self):
r"for pid in $(ps aux | grep paster | grep -v grep | grep '\-%d' | awk"
r" '{print $2}'); do kill $pid; done" % (i)
)


def A():
def B():
def C():
def D():
def E():
def F():
def G():
assert (
c_float(val[0][0] / val[0][1]).value
== c_float(value[0][0] / value[0][1]).value
), "%s didn't roundtrip" % tag


class xxxxxxxxxxxxxxxxxxxxx(xxxx.xxxxxxxxxxxxx):
def xxxxxxx_xxxxxx(xxxx):
assert xxxxxxx_xxxx in [
x.xxxxx.xxxxxx.xxxxx.xxxxxx,
x.xxxxx.xxxxxx.xxxxx.xxxx,
], (
"xxxxxxxxxxx xxxxxxx xxxx (xxxxxx xxxx) %x xxx xxxxx" % xxxxxxx_xxxx
)