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

Work around stability errors due to optional trailing commas #2126

Merged
merged 2 commits into from Apr 25, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 17 additions & 7 deletions src/black/__init__.py
Expand Up @@ -1016,7 +1016,17 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo

if not fast:
assert_equivalent(src_contents, dst_contents)
assert_stable(src_contents, dst_contents, mode=mode)

# Forced second pass to work around optional trailing commas (becoming
# forced trailing commas on pass 2) interacting differently with optional
# parentheses. Admittedly ugly.
dst_contents_pass2 = format_str(dst_contents, mode=mode)
if dst_contents != dst_contents_pass2:
dst_contents = dst_contents_pass2
assert_equivalent(src_contents, dst_contents, pass_num=2)
assert_stable(src_contents, dst_contents, mode=mode)
# Note: no need to explicitly call `assert_stable` if `dst_contents` was
Copy link
Collaborator

Choose a reason for hiding this comment

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

We still need to call assert_equivalent though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We did, above in line 1018.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So we do. Sorry for the noise!

# the same as `dst_contents_pass2`.
return dst_contents


Expand Down Expand Up @@ -6489,7 +6499,7 @@ def _stringify_ast(
yield f"{' ' * depth}) # /{node.__class__.__name__}"


def assert_equivalent(src: str, dst: str) -> None:
def assert_equivalent(src: str, dst: str, *, pass_num: int = 1) -> None:
"""Raise AssertionError if `src` and `dst` aren't equivalent."""
try:
src_ast = parse_ast(src)
Expand All @@ -6504,9 +6514,9 @@ def assert_equivalent(src: str, dst: str) -> None:
except Exception as exc:
log = dump_to_file("".join(traceback.format_tb(exc.__traceback__)), dst)
raise AssertionError(
f"INTERNAL ERROR: Black produced invalid code: {exc}. Please report a bug"
" on https://github.com/psf/black/issues. This invalid output might be"
f" helpful: {log}"
f"INTERNAL ERROR: Black produced invalid code on pass {pass_num}: {exc}. "
"Please report a bug on https://github.com/psf/black/issues. "
f"This invalid output might be helpful: {log}"
) from None

src_ast_str = "\n".join(_stringify_ast(src_ast))
Expand All @@ -6515,8 +6525,8 @@ def assert_equivalent(src: str, dst: str) -> None:
log = dump_to_file(diff(src_ast_str, dst_ast_str, "src", "dst"))
raise AssertionError(
"INTERNAL ERROR: Black produced code that is not equivalent to the"
" source. Please report a bug on https://github.com/psf/black/issues. "
f" This diff might be helpful: {log}"
f" source on pass {pass_num}. Please report a bug on "
f"https://github.com/psf/black/issues. This diff might be helpful: {log}"
) from None


Expand Down
18 changes: 18 additions & 0 deletions tests/test_black.py
Expand Up @@ -254,6 +254,24 @@ def test_trailing_comma_optional_parens_stability3(self) -> None:
actual = fs(source)
black.assert_stable(source, actual, DEFAULT_MODE)

@patch("black.dump_to_file", dump_to_stderr)
def test_trailing_comma_optional_parens_stability1_pass2(self) -> None:
source, _expected = read_data("trailing_comma_optional_parens1")
actual = fs(fs(source)) # this is what `format_file_contents` does with --safe
black.assert_stable(source, actual, DEFAULT_MODE)

@patch("black.dump_to_file", dump_to_stderr)
def test_trailing_comma_optional_parens_stability2_pass2(self) -> None:
source, _expected = read_data("trailing_comma_optional_parens2")
actual = fs(fs(source)) # this is what `format_file_contents` does with --safe
black.assert_stable(source, actual, DEFAULT_MODE)

@patch("black.dump_to_file", dump_to_stderr)
def test_trailing_comma_optional_parens_stability3_pass2(self) -> None:
source, _expected = read_data("trailing_comma_optional_parens3")
actual = fs(fs(source)) # this is what `format_file_contents` does with --safe
black.assert_stable(source, actual, DEFAULT_MODE)

@patch("black.dump_to_file", dump_to_stderr)
def test_pep_572(self) -> None:
source, expected = read_data("pep_572")
Expand Down