Repository navigation
Minor improvement to errors with (no) missing colon #92858
Description
Some 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 syntaxProposed 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!)
Activity
hugovk commented on May 17, 2022
cc @pablogsal
pablogsal commented on May 17, 2022
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 :)
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 syntaxwookie184 commented on May 18, 2022
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
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.
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.
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
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.
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
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 :)
gh-92858: Improve error message for some suites with syntax error bef…
[3.11] pythongh-92858: Improve error message for some suites with syn…
[3.10] pythongh-92858: Improve error message for some suites with syn…
[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…
Revert 30935 defer bpo45162 to 312 (#94285)