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
Run second pass of formatter #2549
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -879,24 +879,22 @@ def format_stdin_to_stdout( | |
|
||
|
||
def check_stability_and_equivalence( | ||
src_contents: str, dst_contents: str, *, mode: Mode | ||
src_contents: str, dst_contents_pass1: str, dst_contents_pass2: str, *, mode: Mode | ||
) -> None: | ||
"""Perform stability and equivalence checks. | ||
|
||
Raise AssertionError if source and destination contents are not | ||
equivalent, or if a second pass of the formatter would format the | ||
content differently. | ||
""" | ||
assert_equivalent(src_contents, dst_contents) | ||
assert_equivalent(src_contents, dst_contents_pass1) | ||
|
||
# Forced second pass to work around optional trailing commas (becoming | ||
# Require 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) | ||
if dst_contents_pass1 != dst_contents_pass2: | ||
assert_equivalent(src_contents, dst_contents_pass2, pass_num=2) | ||
assert_stable(src_contents, dst_contents_pass2, mode=mode) | ||
# Note: no need to explicitly call `assert_stable` if `dst_contents` was | ||
# the same as `dst_contents_pass2`. | ||
|
||
|
@@ -914,13 +912,18 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo | |
if mode.is_ipynb: | ||
dst_contents = format_ipynb_string(src_contents, fast=fast, mode=mode) | ||
else: | ||
dst_contents = format_str(src_contents, mode=mode) | ||
dst_contents_pass1 = format_str(src_contents, mode=mode) | ||
dst_contents = format_str(dst_contents_pass1, mode=mode) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This actually runs the second pass of the formatter and returns the results of the second pass out |
||
|
||
if src_contents == dst_contents: | ||
raise NothingChanged | ||
|
||
if not fast and not mode.is_ipynb: | ||
# Jupyter notebooks will already have been checked above. | ||
check_stability_and_equivalence(src_contents, dst_contents, mode=mode) | ||
check_stability_and_equivalence( | ||
src_contents, dst_contents_pass1, dst_contents, mode=mode | ||
) | ||
|
||
return dst_contents | ||
|
||
|
||
|
@@ -967,9 +970,12 @@ def format_cell(src: str, *, fast: bool, mode: Mode) -> str: | |
masked_src, replacements = mask_cell(src_without_trailing_semicolon) | ||
except SyntaxError: | ||
raise NothingChanged from None | ||
masked_dst = format_str(masked_src, mode=mode) | ||
masked_dst_pass1 = format_str(masked_src, mode=mode) | ||
masked_dst = format_str(masked_dst_pass1, mode=mode) | ||
if not fast: | ||
check_stability_and_equivalence(masked_src, masked_dst, mode=mode) | ||
check_stability_and_equivalence( | ||
masked_src, masked_dst_pass1, masked_dst, mode=mode | ||
) | ||
dst_without_trailing_semicolon = unmask_cell(masked_dst, replacements) | ||
dst = put_trailing_semicolon_back( | ||
dst_without_trailing_semicolon, has_trailing_semicolon | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ | |
import black | ||
from black.debug import DebugVisitor | ||
from black.mode import TargetVersion | ||
from black.output import err, out | ||
from black.output import err, out, diff | ||
|
||
THIS_DIR = Path(__file__).parent | ||
DATA_DIR = THIS_DIR / "data" | ||
|
@@ -47,6 +47,9 @@ def _assert_format_equal(expected: str, actual: str) -> None: | |
except Exception as ve: | ||
err(str(ve)) | ||
|
||
if actual != expected: | ||
out(diff(expected, actual, "expected", "actual")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found this incredibly helpful in debugging the test failures. |
||
|
||
assert actual == expected | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, the second pass only happened here in the
check_stability_and_equivalence
method, and thus wasn't actually being written out / returned fromformat_file_contents
.