Minor improvement to errors with (no) missing colon #92858

wookie184 · 2022-05-16T19:38:35Z

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 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!)

The text was updated successfully, but these errors were encountered:

hugovk · 2022-05-17T08:02:20Z

cc @pablogsal

pablogsal · 2022-05-17T08:43:44Z

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 :)

hauntsaninja · 2022-05-17T23:29:22Z

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 · 2022-05-18T06:56:34Z

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 · 2022-05-18T11:01:32Z

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 · 2022-05-18T12:33:24Z

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 · 2022-05-18T15:45:44Z

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 :)

…ore ':' (#92894)

…tax error before ':' (pythonGH-92894) (cherry picked from commit 2fc83ac) Co-authored-by: wookie184 <wookie1840@gmail.com>

…tax error before ':' (pythonGH-92894). (cherry picked from commit 2fc83ac) Co-authored-by: wookie184 <wookie1840@gmail.com>

…ror before ':' (GH-92894). (#94183) (cherry picked from commit 2fc83ac) Co-authored-by: wookie184 <wookie1840@gmail.com> Co-authored-by: wookie184 <wookie1840@gmail.com>

…ror before ':' (GH-92894) (#94180) (cherry picked from commit 2fc83ac) Co-authored-by: wookie184 <wookie1840@gmail.com> Co-authored-by: wookie184 <wookie1840@gmail.com>

wookie184 added the type-bug label May 16, 2022

AlexWaygood added the interpreter-core label May 16, 2022

wookie184 mentioned this issue May 17, 2022

gh-92858: Improve error message for some suites with syntax error before ':' #92894

Merged

wookie184 mentioned this issue May 26, 2022

SyntaxError location for *args in class bases is inconsistent #93165

Closed

brandtbucher mentioned this issue Jun 20, 2022

Unhelpful SyntaxError with unclosed call before colon #94034

Closed

pablogsal closed this as completed in #92894 Jun 23, 2022

pablogsal pushed a commit that referenced this issue Jun 23, 2022

gh-92858: Improve error message for some suites with syntax error bef…

2fc83ac

…ore ':' (#92894)

bedevere-bot mentioned this issue Jun 23, 2022

[3.11] gh-92858: Improve error message for some suites with syntax error before ':' (GH-92894) #94180

Merged

bedevere-bot mentioned this issue Jun 23, 2022

[3.10] gh-92858: Improve error message for some suites with syntax error before ':' (GH-92894). #94183

Merged

Minor improvement to errors with (no) missing colon #92858

Minor improvement to errors with (no) missing colon #92858

wookie184 commented May 16, 2022

hugovk commented May 17, 2022

pablogsal commented May 17, 2022

hauntsaninja commented May 17, 2022 •

edited

wookie184 commented May 18, 2022 •

edited

pablogsal commented May 18, 2022

wookie184 commented May 18, 2022 •

edited

pablogsal commented May 18, 2022

Minor improvement to errors with (no) missing colon #92858

Minor improvement to errors with (no) missing colon #92858

Comments

wookie184 commented May 16, 2022

hugovk commented May 17, 2022

pablogsal commented May 17, 2022

hauntsaninja commented May 17, 2022 • edited

wookie184 commented May 18, 2022 • edited

pablogsal commented May 18, 2022

wookie184 commented May 18, 2022 • edited

pablogsal commented May 18, 2022

hauntsaninja commented May 17, 2022 •

edited

wookie184 commented May 18, 2022 •

edited

wookie184 commented May 18, 2022 •

edited