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

Clearer errorr message for "Cannot parse" when targetting multiple version #3294

Open
plannigan opened this issue Sep 28, 2022 · 13 comments · May be fixed by #4378
Open

Clearer errorr message for "Cannot parse" when targetting multiple version #3294

plannigan opened this issue Sep 28, 2022 · 13 comments · May be fixed by #4378
Assignees
Labels
good first issue Good for newcomers T: enhancement New feature or request

Comments

@plannigan
Copy link

Is your feature request related to a problem? Please describe.

I was working on a project that is only intended to run on Python 3.10. However, the black configuration incorrectly listed multiple target versions because I copied the file from a different project.

When I tried to format a file with a match statement, the error was

error: cannot format foo.py: Cannot parse: 2:6: match bar:

I was initially confused as I knew that black had added Python 3.10 support. Then I noticed my mistake. By saying that the code should target a multiple Python versions, using a match statement would not be valid.

Describe the solution you'd like

If black encounters a parsing error and it was configured to target multiple Python versions, I think it would be helpful to mention that the syntax "might be valid, but not across all target versions". This would be a hint to the user to think about if they should be using that syntax or that black is misconfigured (as it was in my case).

Describe alternatives you've considered

It would require more work, but the parser could back track and attempt each of the parser for each of the listed target versions. That would allow black provide an even more specific error message. Something like "this is valid for Python 3.10, but targeting Python 3.10 and 3.9", while also being able to identify an invalid syntax.

@plannigan plannigan added the T: enhancement New feature or request label Sep 28, 2022
@felix-hilden
Copy link
Collaborator

Thanks for submitting the issue! I'm all for a better error message 👍

@felix-hilden felix-hilden added the good first issue Good for newcomers label Oct 7, 2022
@nnrepos
Copy link
Contributor

nnrepos commented Oct 8, 2022

hey @felix-hilden, i would like to work on this, and add the grammar version to the error message.
i believe the error occurs in src/black/parsing.py:103:

grammars = get_grammars(set(target_versions))
    errors = {}
    for grammar in grammars:
        drv = driver.Driver(grammar)
        try:
            result = drv.parse_string(src_txt, True)
            break

        except ParseError as pe:
            lineno, column = pe.context[1]
            lines = src_txt.splitlines()
            try:
                faulty_line = lines[lineno - 1]
            except IndexError:
                faulty_line = "<line number missing in source>"
            errors[grammar.version] = InvalidInput(
                f"Cannot parse: {lineno}:{column}: {faulty_line}"  # 103
            )

could you assign me?
thanks 😄

@Elvis020
Copy link

I think this issue should be marked closed, correct me if I am wrong

@nnrepos
Copy link
Contributor

nnrepos commented Oct 15, 2022

I think this issue should be marked closed, correct me if I am wrong

hey, usually an issue is closed when a PR is merged. this is done automatically if the PR contains something like closes #3294.

@Elvis020
Copy link

ok, cool. Looks like it's left for a pending reviewer to take a look

@rishidyno
Copy link

Hi @plannigan, I would like to work on this issue could you please assign me this.

@JelleZijlstra
Copy link
Collaborator

We don't usually assign issues. If you have a suggested solution, please open a PR and we'll take a look.

@plannigan
Copy link
Author

I also don't have permissions to assign people to issues 😄.

@nnrepos
Copy link
Contributor

nnrepos commented Nov 17, 2023

hey guys, i offered a solution to this issue last year:
#3323

alas - it was disapproved, despite the fact that it fixed this issue.
please consider checking my PR again.
otherwise, if this issue is a won't fix, i recommend closing this issue.
thanks

@AshishNarne
Copy link

I might try a new approach to this.

@smb55
Copy link

smb55 commented Jun 7, 2024

I've had another go at this, taking into account the feedback for the previous PR.

I was looking into the second part of the request about noting that syntax could be valid across other target versions, and I'm unable to reproduce this issue. As far as I can see, this error only occurs if ALL versions are unable to parse the line, so we shouldn't need this error to handle a case where some versions parse and others don't.

@plannigan
Copy link
Author

Given how old the ticket is, I don't remember exactly what the originally configured python versions were. But I feel like the content under "Describe alternatives you've considered" was more of "maybe X is happening" rather than "I know X is happening". So if there are different/better error messages when one configured parser can parse, but a different configured parser can't, then that is fine with me.

@smb55
Copy link

smb55 commented Jun 8, 2024

Unless there's a bigger issue here, I don't believe raising a parse error when one version parses and another does not is expected behaviour. Currently, there's a loop that stores errors and only raises the exception if all versions fail.

The pull request I've submitted will change the error message to look like this:
image

I think this should make it much clearer what is causing the error. If Black is parsing with a different version than what the user intended, this error should make them aware. Based on the feedback in the previous unsuccessful PR, I've included both the grammars and the Python versions. This should clarify the issue if a user requests a version that supports a feature but Black selects a grammar version that does not support the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers T: enhancement New feature or request
Projects
None yet
8 participants