classification
Title: Improve error message for missing : before suites
Type: Stage: resolved
Components: Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: aroberge, gvanrossum, lys.nikolaou, pablogsal, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2021-01-22 01:20 by pablogsal, last changed 2021-02-02 19:57 by pablogsal. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 24292 merged pablogsal, 2021-01-22 01:24
PR 24293 closed pablogsal, 2021-01-22 01:46
Messages (9)
msg385463 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-01-22 01:20
Instead of displaying a generic syntax error:

Python 3.8.6 (default, Oct 10 2020, 18:31:21)
[GCC 10.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> for x in range
  File "<stdin>", line 1
    for x in range
                 ^
SyntaxError: invalid syntax
>>>

we could display:

>>> for x in range
  File "<stdin>", line 1
    for x in range
                  ^
SyntaxError: expected ':'

The same idea applies for every suite that has a missing ':'
msg385465 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-01-22 01:27
There are several ways to implement this:

In PR24292 I implemented this idea by adding a new element to the grammar: '&&'. This allows to hard-expect a token: if the token is not there the parsing hard-fails immediately without trying anything else and displays an appropriate error message

The other possibility if we think that PR24292 is overkill is manually adding an "invalid_wathever" for every possible compound statement option that parses the statement with a !':' and then raises appropriately. This option is more verbose in the grammar but requires no new machinery.
msg385468 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-01-22 01:48
PR 24293 implements the second idea (only new rules and no new machinery)
msg385481 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-01-22 07:38
See also issue1634034.
msg385720 - (view) Author: Lysandros Nikolaou (lys.nikolaou) * (Python committer) Date: 2021-01-26 15:49
I propose to go with the first approach. I really like that and I can see us using it in various place in the future. It's basically a positive lookahead with a cut, but the extra feature that it raises a SyntaxError, right?

I'll have a look at the PR now.
msg385725 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-01-26 16:46
> See also issue1634034.

The problem with issue1634034 is that is not anymore possible without manual changes to the grammar with the PEG parser (or new infrastructure). The new parser does not have a defined concept of "i expected this token and now I am going to hard fail because is not there", but instead it will backtrack and try something else. We also did some work on something similar and we deemed it not useful as it was:

bugs.python.org/issue40599
msg385726 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-01-26 16:47
> It's basically a positive lookahead with a cut, but the extra feature that it raises a SyntaxError, right?

Yeah, but the cut is "absolute" (because of the syntax Error). IIRC the normal cut only affects that rule, no?
msg385728 - (view) Author: Lysandros Nikolaou (lys.nikolaou) * (Python committer) Date: 2021-01-26 18:18
> Yeah, but the cut is "absolute" (because of the syntax Error). IIRC the normal cut only affects that rule, no?

Right.
msg386164 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-02-02 19:54
New changeset 58fb156edda1a0e924a38bfed494bd06cb09c9a3 by Pablo Galindo in branch 'master':
bpo-42997: Improve error message for missing : before suites (GH-24292)
https://github.com/python/cpython/commit/58fb156edda1a0e924a38bfed494bd06cb09c9a3
History
Date User Action Args
2021-02-02 19:57:31pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-02-02 19:54:31pablogsalsetmessages: + msg386164
2021-01-26 18:18:09lys.nikolaousetmessages: + msg385728
2021-01-26 16:47:09pablogsalsetmessages: + msg385726
2021-01-26 16:46:27pablogsalsetmessages: + msg385725
2021-01-26 15:49:51lys.nikolaousetmessages: + msg385720
2021-01-22 18:49:35arobergesetnosy: + aroberge
2021-01-22 07:38:40serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg385481
2021-01-22 01:48:35pablogsalsetmessages: + msg385468
2021-01-22 01:46:45pablogsalsetstage: patch review
pull_requests: + pull_request23115
2021-01-22 01:27:30pablogsalsetnosy: + gvanrossum
2021-01-22 01:27:20pablogsalsetmessages: + msg385465
stage: patch review -> (no value)
2021-01-22 01:24:11pablogsalsetkeywords: + patch
stage: patch review
pull_requests: + pull_request23113
2021-01-22 01:24:00pablogsalsetnosy: + lys.nikolaou
2021-01-22 01:20:05pablogsalcreate