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

Crash with preview style + line length 1 #4062

Closed
JelleZijlstra opened this issue Nov 21, 2023 · 7 comments · Fixed by #4086
Closed

Crash with preview style + line length 1 #4062

JelleZijlstra opened this issue Nov 21, 2023 · 7 comments · Fixed by #4086
Labels
C: crash Black is crashing C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: bug Something isn't working

Comments

@JelleZijlstra
Copy link
Collaborator

On Black 23.11.0:

% black -v --line-length 1 -c 'def f() -> tuple[int, int]: pass' --preview
def f() -> tuple[int, int]: pass
Traceback (most recent call last):
  File "src/black/__init__.py", line 1444, in assert_equivalent
  File "src/black/parsing.py", line 140, in parse_ast
SyntaxError: expected ':' (<unknown>, line 1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "src/black/__init__.py", line 782, in reformat_code
  File "src/black/__init__.py", line 945, in format_stdin_to_stdout
  File "src/black/__init__.py", line 1011, in format_file_contents
  File "src/black/__init__.py", line 985, in check_stability_and_equivalence
  File "src/black/__init__.py", line 1447, in assert_equivalent
AssertionError: INTERNAL ERROR: Black produced invalid code: expected ':' (<unknown>, line 1). Please report a bug on https://github.com/psf/black/issues.  This invalid output might be helpful: /var/folders/hh/hdb5qd2x3v51v6l0gknpfjfr0000gp/T/blk_wad8_c6m.log
error: cannot format <string>: INTERNAL ERROR: Black produced invalid code: expected ':' (<unknown>, line 1). Please report a bug on https://github.com/psf/black/issues.  This invalid output might be helpful: /var/folders/hh/hdb5qd2x3v51v6l0gknpfjfr0000gp/T/blk_wad8_c6m.log
Oh no! 💥 💔 💥

Works without preview:

% black -v --line-length 1 -c 'def f() -> tuple[int, int]: pass'

def f() -> (
    tuple[
        int,
        int,
    ]
):
    pass
reformatted <string>
All done! ✨ 🍰 ✨

You can also make it crash with a slightly more reasonable line length if you make the identifiers longer:

% black -v --line-length 20 -c 'def ffffffffffffff() -> tupleeeeee[int, iiiiiiint]: pass' --preview
def ffffffffffffff() -> tupleeeeee[int, iiiiiiint]: pass
Traceback (most recent call last):
  File "src/black/__init__.py", line 1444, in assert_equivalent
  File "src/black/parsing.py", line 140, in parse_ast
SyntaxError: expected ':' (<unknown>, line 1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "src/black/__init__.py", line 782, in reformat_code
  File "src/black/__init__.py", line 945, in format_stdin_to_stdout
  File "src/black/__init__.py", line 1011, in format_file_contents
  File "src/black/__init__.py", line 985, in check_stability_and_equivalence
  File "src/black/__init__.py", line 1447, in assert_equivalent
AssertionError: INTERNAL ERROR: Black produced invalid code: expected ':' (<unknown>, line 1). Please report a bug on https://github.com/psf/black/issues.  This invalid output might be helpful: /var/folders/hh/hdb5qd2x3v51v6l0gknpfjfr0000gp/T/blk_4f39fhc8.log
error: cannot format <string>: INTERNAL ERROR: Black produced invalid code: expected ':' (<unknown>, line 1). Please report a bug on https://github.com/psf/black/issues.  This invalid output might be helpful: /var/folders/hh/hdb5qd2x3v51v6l0gknpfjfr0000gp/T/blk_4f39fhc8.log

You can see with --fast why it crashes: it puts an illegal newline after the ->:

% black --fast --line-length 1 -c 'def f() -> tuple[int, int]: pass' --preview
def f() -> 
    tuple[
    int,
    int,
]:
    pass

This is from the parenthesize_long_type_hints preview feature (#3899).

@JelleZijlstra JelleZijlstra added T: bug Something isn't working C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. C: crash Black is crashing labels Nov 21, 2023
@JelleZijlstra
Copy link
Collaborator Author

Correction: This is due to respect_magic_trailing_comma_in_return_type (#3916), not parenthesize_long_type_hints.

@JelleZijlstra
Copy link
Collaborator Author

Looked into this a little today but couldn't find a fix. This variant of the test case is more effective: def f() -> tuple[int, int,]: ... since it doesn't rely on Black inserting a trailing comma itself first.

@jakkdl
Copy link
Contributor

jakkdl commented Dec 5, 2023

I'll try looking at it.

It's a shame that this didn't get caught at the time, since assert_format doesn't care about checking it for preview features. Maybe better if preview features had to explicitly mark themselves as not compatible with line-length=1 somewhere to skip the check?

@JelleZijlstra
Copy link
Collaborator Author

I did put some logic in the test suite that runs all tests in a couple of modes and looks for crashes, but it might not do the combination --preview --line-length=1.

@jakkdl
Copy link
Contributor

jakkdl commented Dec 5, 2023

I did put some logic in the test suite that runs all tests in a couple of modes and looks for crashes, but it might not do the combination --preview --line-length=1.

black/tests/util.py

Lines 126 to 128 in 3416b2c

# Similarly, setting line length to 1 is a good way to catch
# stability bugs. But only in non-preview mode because preview mode
# currently has a lot of line length 1 bugs.

@jakkdl
Copy link
Contributor

jakkdl commented Dec 5, 2023

undoing that exception brings up two failing cases, fmtskip2 and comments2. Dug into comments2 a little bit and it was turning

a = "type comment with trailing space"  # type: str

into

a = (
  "type comment with trailing space"
)

and losing the type comment. Did not look at the other one, but could maybe split those out into separate files and mark them as vulnerable to line-length, or fiddle with the Preview enums in the Mode to remove known-bad preview features from it when doing the line-length check.

@JelleZijlstra
Copy link
Collaborator Author

Oops, I totally forgot I wrote that comment :). Yes, it would be good to put some mechanism in that lets us run --preview --line-length=1 for all except the known-bad cases. My preferred implementation would be a custom flag we add to the --flags at the top of the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: crash Black is crashing C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants