bpo-40599: Improve error messages with expected keywords#20039
bpo-40599: Improve error messages with expected keywords#20039pablogsal wants to merge 1 commit intopython:masterfrom
Conversation
|
I open this PR as a draft because I still have some questions about the approach but we can start a discussion to see how to improve it :) Some examples of the errors with this PR: >>> from x impirt l
File "<stdin>", line 1
from x impirt l
^
SyntaxError: Invalid syntax. Expected one of: import
>>> with x os y
File "<stdin>", line 1
with x os y
^
SyntaxError: Invalid syntax. Expected one of: as, or, if, in, is, not, and
>>> l = 3 if x elso 4
File "<stdin>", line 1
l = 3 if x elso 4
^
SyntaxError: Invalid syntax. Expected one of: or, in, is, else, not, and
>>> if x:
... ...
... elif y:
... ...
... elso
File "<stdin>", line 5
elso
^
SyntaxError: Invalid syntax. Expected one of: elif, elseWith some work, we could extend this to general tokens (although it may be very verbose). |
|
This looks like scope creep for the original issue. Even if you can solve this using the new parser, please open a new bpo issue and have a discussion about it there (or perhaps on python-ideas or python-dev?). |
Ok, will do. In any case, I opened the draft PR to get some initial opinions from you and @lysnikolaou to see if is worth pursuing. |
|
I have linked this against this new issue: https://bugs.python.org/issue40599 and unlinked it from the original issue. |
There was a problem hiding this comment.
I have a bit of feedback on some of the examples. When/if I find the time, I'll experiment further with the PR locally and try to provide more in-depth feedback.
>>> from x impirt l
File "<stdin>", line 1
from x impirt l
^
SyntaxError: Invalid syntax. Expected one of: importHuge +1 for offering a specific suggestion when it's unambiguous. However, I think that it could look a significantly better to omit the "one of" when there is only when possible suggestion (assuming it's not too much of an implementation issue). E.g. SyntaxError: Invalid syntax. Expected: import.
>>> with x os y
File "<stdin>", line 1
with x os y
^
SyntaxError: Invalid syntax. Expected one of: as, or, if, in, is, not, andI'm -0 on the behavior in the above example. Offering 5+ possible suggestions (7 in this case) for keywords doesn't seem especially helpful and results in a rather long exception message.
My personal preference would be to only display the possible suggestion(s) if there are only 4 or less (maybe 3?) possible keywords, and omit "one of" when there's only one.
As for the implementation details, my knowledge of the C-API and especially the parser is rather limited, but would it be possible to check the number of possible keywords with something like Py_ssize_t err_tokens_size = PySet_Size(p->err_tokens)?
If the above or something along those lines would be feasible, I would be +1 on the PR overall. Otherwise, I'm +0. It seems like a good idea, but I think the error message would be a bit too bloated when there's too many possible keywords.
|
Thanks for the feedback @aeros ! I think that the problem is that, as Guido mentions, without tokens this is not very helpful and once we add tokens the possible items in the set will increase a lot. For instance, if you do As I mentioned in the issue, I opened this draft to start a discussion but I and more and more unsure that this approach will give us better debugability than the noise it adds :( |
I would personally be okay with that, it could be quite useful at occasionally detecting unambiguous misspellings, such as the import example. Although one may argue that it's more of a job of a linter to pick up on that.
Either way, I think this discussion is a good start and gives a general direction (and perhaps more importantly, establishes what it shouldn't do). Thanks for starting the discussion. |
The current errors are due to tests that don't expect the extra information in the error messages.
https://bugs.python.org/issue40599