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

if/elif/else statement need better error handling #22

Closed
Rabbit994 opened this issue Jul 1, 2020 · 5 comments
Closed

if/elif/else statement need better error handling #22

Rabbit994 opened this issue Jul 1, 2020 · 5 comments
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@Rabbit994
Copy link

Rabbit994 commented Jul 1, 2020

Environment data

  • Language Server version: Pylance language server 2020.6.1
  • OS and version: Windows 10 Enterprise
  • Python version (& distribution if applicable, e.g. Anaconda): Python 3.7.7

Expected behaviour

When doing improper if/elif/else statements (see code Snippet), I would expect Pylance to report syntax errors in similar fashion to pylint which returns following for code snippet: invalid syntax (, line 2) (syntax-error)

Actual behaviour

Pylance reports errors around missing : or Type annotation not supported for this type of expression for else statement.

Code Snippet / Additional information

spam = 'eggs'
if spam = 'eggs':
    print('Green Eggs and Spam')
else spam == 'ham':
    print('Green Eggs and Ham')
@gramster
Copy link
Member

gramster commented Jul 2, 2020

There is no one right way to report a syntax error; it usually depends on the parser. In pylance's case it knows that '=' is not appropriate at that location so it wants to terminate the if after if spam and reports a missing ':'.

But if I read you correctly, what you are saying is that you would prefer the error message to be "Syntax error" instead of "Expected <token>"; maybe even better would be "Syntax error: expected <token>".

@Rabbit994
Copy link
Author

pylint behavior is to report "Syntax Error" which is more clear. It's a syntax error for sure and I think calling it out as syntax error and leaving it at that would be acceptable. It would be nicer for pylance to go step beyond and report "Syntax Error, == expected" or "Syntax Error, else may not have evaluation"

It's understand the difficulty of building linter for language that has such a diverse crowd from professional programmers powering backends, data scientists using it to crunch data, Machine Learning and those starting out with their first language in the world of programming. It was friend learning programming who sent me something similar and couldn't figure it out. Just trying to be the voice of a beginner.

@erictraut
Copy link
Contributor

There are two issues here.

The first is that we were not doing a better job with parse recovery once a parse error is detected. Your example code resulted in about eight different errors. I've improved the parse recovery in this case to avoid the cascading errors. With my improvements, you will now see only two error messages.

The second issue is with the error message. I disagree that "syntax error" is a better message. This is a very generic term, and many beginning Python users won't even know what "syntax error" means. Good error messages are understandable, specific, and provide the user with information about what was expected or what they can do to fix it. As such, I think the current error message is an appropriate and useful message.

@erictraut erictraut added bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version labels Jul 2, 2020
@Rabbit994
Copy link
Author

I think syntax error is learned quickly but again, error messages are such a Bike shed problem that I’m glad error count has been reduced. Thanks for quick turnaround.

@jakebailey
Copy link
Member

This issue has been fixed in version 2020.7.0, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#202070-9-july-2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

4 participants