GeezerVison is showing github.com — Minor improvement to errors with (no) missing colon · Issue #92858 · python/cpython
Minor improvement to errors with (no) missing colon #92858Labels interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error DescriptionSome invalid for statements currently give the specific error expected ':' even if there is a colon on the line. I think it would be a slight improvement to raise a more general error in these cases. This is how while and if already behave.
Current behaviour:
>>> for x in range(10)
... pass
Traceback (most recent call last):
SyntaxError: expected ':'
>>> for x in range 10:
... pass
Traceback (most recent call last):
SyntaxError: expected ':'
>>> for x in range 10
... pass
Traceback (most recent call last):
SyntaxError: expected ':' Proposed behaviour:
>>> for x in range(10)
... pass
Traceback (most recent call last):
SyntaxError: expected ':'
>>> for x in range 10:
... pass
Traceback (most recent call last):
SyntaxError: invalid syntax
>>> for x in range 10
... pass
Traceback (most recent call last):
SyntaxError: invalid syntax Proposed Implementation:
Would do it the same way while and if do it. For the for loop example it would be something like this:
--- a/Grammar/python.gram
+++ b/Grammar/python.gram
@@ -379,7 +379,7 @@ while_stmt[stmt_ty]:
for_stmt[stmt_ty]:
| invalid_for_stmt
- | 'for' t=star_targets 'in' ~ ex=star_expressions &&':' tc=[TYPE_COMMENT] b=block el=[else_block] {
+ | 'for' t=star_targets 'in' ~ ex=star_expressions ':' tc=[TYPE_COMMENT] b=block el=[else_block] {
_PyAST_For(t, ex, b, el, NEW_TYPE_COMMENT(p, tc), EXTRA) }
| ASYNC 'for' t=star_targets 'in' ~ ex=star_expressions &&':' tc=[TYPE_COMMENT] b=block el=[else_block] {
CHECK_VERSION(stmt_ty, 5, "Async for loops are", _PyAST_AsyncFor(t, ex, b, el, NEW_TYPE_COMMENT(p, tc), EXTRA)) }
@@ -1295,6 +1295,7 @@ invalid_while_stmt:
| a='while' named_expression ':' NEWLINE !INDENT {
RAISE_INDENTATION_ERROR("expected an indented block after 'while' statement on line %d", a->lineno) }
invalid_for_stmt:
+ | 'for' star_targets 'in' star_expressions NEWLINE { RAISE_SYNTAX_ERROR("expected ':'") }
| [ASYNC] a='for' star_targets 'in' star_expressions ':' NEWLINE !INDENT {
RAISE_INDENTATION_ERROR("expected an indented block after 'for' statement on line %d", a->lineno) }
invalid_def_raw:I'd also apply this to other suites where it makes sense (e.g. with, match/case, and try)
Would it be OK for me to submit a PR making these changes? (this is my first time looking into the CPython parser so apologies if I made any obvious mistakes!) Activityhugovk commented on May 17, 2022 pablogsal commented on May 17, 2022
Would it be OK for me to submit a PR making these changes?
Yeah, go ahead! Thanks for looking into this :)
Take care because the reason this may not be already made is because there may be some hairy incompatibility down the line (I cannot remember any so it is likely that I just forgot) so make sure to run the test suite :) Reacted by wookie184 and ajoino hauntsaninja commented on May 17, 2022 Note that the error is much friendlier with Python 3.11's (edit: meant 3.10) better tracebacks, since the carets point the location of the error, so you can't confuse it for another colon on the line:
λ python3.11
Python 3.11.0b1+ (heads/3.11:6546af3, May 10 2022, 12:01:20) [Clang 13.0.0 (clang-1300.0.27.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> with block ad something: pass
File "<stdin>", line 1
with block ad something: pass
^^
SyntaxError: expected ':'
which I think I prefer compared to the error in the proposed PR:
λ ./python.exe
Python 3.12.0a0 (heads/improve-error-message:c2e406f851, May 17 2022, 16:27:27) [Clang 13.0.0 (clang-1300.0.27.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> with block ad something: pass
File "<stdin>", line 1
with block ad something: pass
^^
SyntaxError: invalid syntax
wookie184 commented on May 18, 2022 ContributorAuthor More actions I read SyntaxError: expected ':' as "you probably meant to put a colon there", so felt it makes most sense to only use it where that's likely to be the issue.
In cases other than that I don't think the fact that the parser expected a colon adds much information over a plain syntax error (and may be slightly confusing). pablogsal commented on May 18, 2022
Note that the error is much friendlier with Python 3.11's better tracebacks
Unless I am missing something very fundamental, there is nothing in 3.11 for this, this should work also on Python 3.10 as I added the changes required for range highlighting in syntax errors early in the 3.10 dev cycle.
I read SyntaxError: expected ':' as "you probably meant to put a colon there", so felt it makes most sense to only use it where that's likely to be the issue.
Technically means "the parser expected a colon here" which can be compatible with a colon being late in the line. Is not technically a suggestion but more than an explanation on what's goin on.
In cases other than that
Sorry, not sure I follow. What cases are you referring to? The example that you showed? In general, missing colon errors have been one of the most requested specialized error messages so I think it does make sense to have them appear instead of "invalid syntax" as much as possible.
Regarding this case that you show: the problem is that currently we have some suites working like you show and others not working like that (the ones using the mode that you are using in your PR). In general, I prefer to be consistent if we can, because that simplifies expectations, which is an important (although not fundamenteal) part of the UX here. wookie184 commented on May 18, 2022 ContributorAuthor More actions
Sorry, not sure I follow. What cases are you referring to? The example that you showed? In general, missing colon errors have been one of the most requested specialized error messages so I think it does make sense to have them appear instead of "invalid syntax" as much as possible.
Sorry, I misspoke there (remove 'in cases'). I was trying to say that I think the missing colon error isn't really helpful when the solution isn't to add/move a colon.
In the case mentioned, with block ad something: pass there is a syntax error before the colon (ad should be as) and the fact the parser expected a colon isn't really relevant (there's already a colon).
It also seemed to be a similar case for pretty much all cases that my PR changes (see added test cases on the PR). The only exception I could think of is where there is a colon missing and the body is on the same line, e.g. for i in range(10) print(i) but that's ambiguous and sort of ugly anyway.
There are cases such as for i in range 10 which are missing a colon and also otherwise syntactically invalid. I think the more general error is also nicer here as the issue isn't just a missing colon.
Regarding this case that you show: the problem is that currently we have some suites working like you show and others not working like that (the ones using the mode that you are using in your PR). In general, I prefer to be consistent if we can, because that simplifies expectations, which is an important (although not fundamenteal) part of the UX here.
Agreed. I felt changing the for-like behaviour to match the while-like behaviour made more sense rather than the other way round, although it seems others may disagree. I'd be happy to change to whatever the consensus is (if it's decided any change should be made). pablogsal commented on May 18, 2022
Agreed. I felt changing the for-like behaviour to match the while-like behaviour made more sense rather than the other way round, although it seems others may disagree. I'd be happy to change to whatever the consensus is (if it's decided any change should be made).
Yeah, I think is going to be better to do follow what while loops are doing. The errors that way are less surprising 👍
I will review your PR soon. Thanks a lot for working on this :) added 5 commits that reference this issue on Jun 23, 2022gh-92858: Improve error message for some suites with syntax error bef… [3.10] gh-92858: Improve error message for some suites with syntax er… [3.11] gh-92858: Improve error message for some suites with syntax er…
MetadataMetadatainterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error DevelopmentNo branches or pull requestsIssue actions
|