Skip to content

Fix SyntaxError messages for invalid assignment targets #127

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

Closed
wants to merge 1 commit into from

Conversation

lysnikolaou
Copy link
Member

Just wanted to show you what it takes to provide correct error messages for invalid targets without generating too much noise on the upstream repo or the bpo issue.

IMHO this is very ugly, but it might be the only way to do it, until we've found a better solution and since we did a similar thing for del_targets.

Thoughts?

@lysnikolaou lysnikolaou requested a review from pablogsal as a code owner May 11, 2020 23:16
| '(' a=star_target ')' { _PyPegen_set_expr_context(p, a, Store) }
| '(' a=[star_targets_seq] ')' { _Py_Tuple(a, Store, EXTRA) }
| '[' a=[star_targets_seq] ']' { _Py_List(a, Store, EXTRA) }
star_target_end: ')' | ']' | ',' | '=' | 'in'

Choose a reason for hiding this comment

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

I presume in is here because of for <target> in <expr>, but this would mean that you'd get a different error for

a in b = 42

than you'd get for

a < b = 42

Copy link
Member Author

@lysnikolaou lysnikolaou May 12, 2020

Choose a reason for hiding this comment

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

Actually not, because when parsing the invalid target the whole expression gets parsed, which means that cannot assign to comparison is displayed for both.

You're right. What I said above was me talking about 1 in a = 42 for some reason, which would actually display the correct error message, since 1 is not a valid assignment target.

Another problem with this is that it isn't really usable in for-statements, because the in always gets parsed as the comparison operator in, which means that even something like for f() in a: pass displays the cannot assign to comparison error.

@gvanrossum
Copy link

Yeah, this is really awful. Not sure what to do yet.

@lysnikolaou
Copy link
Member Author

I guess this can be closed, since python#20076 is now merged.

@lysnikolaou lysnikolaou deleted the star-target-errors branch May 15, 2020 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants