Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve error message for missing : before suites #87163

Closed
pablogsal opened this issue Jan 22, 2021 · 9 comments
Closed

Improve error message for missing : before suites #87163

pablogsal opened this issue Jan 22, 2021 · 9 comments
Labels
3.10 only security fixes

Comments

@pablogsal
Copy link
Member

BPO 42997
Nosy @gvanrossum, @aroberge, @serhiy-storchaka, @lysnikolaou, @pablogsal
PRs
  • bpo-42997: Improve error message for missing : before suites #24292
  • bpo-42997: Improve error message for missing : before suites #24293
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2021-02-02.19:57:31.655>
created_at = <Date 2021-01-22.01:20:05.352>
labels = ['3.10']
title = 'Improve error message for missing : before suites'
updated_at = <Date 2021-02-02.19:57:31.654>
user = 'https://github.com/pablogsal'

    bugs.python.org fields:

    activity = <Date 2021-02-02.19:57:31.654>
actor = 'pablogsal'
assignee = 'none'
closed = True
closed_date = <Date 2021-02-02.19:57:31.655>
closer = 'pablogsal'
components = []
creation = <Date 2021-01-22.01:20:05.352>
creator = 'pablogsal'
dependencies = []
files = []
hgrepos = []
issue_num = 42997
keywords = ['patch']
message_count = 9.0
messages = ['385463', '385465', '385468', '385481', '385720', '385725', '385726', '385728', '386164']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'aroberge', 'serhiy.storchaka', 'lys.nikolaou', 'pablogsal']
pr_nums = ['24292', '24293']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue42997'
versions = ['Python 3.10']
    @pablogsal
    Copy link
    Member Author

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

    @pablogsal pablogsal added 3.10 only security fixes labels Jan 22, 2021
    @pablogsal
    Copy link
    Member Author

    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.

    @pablogsal
    Copy link
    Member Author

    PR 24293 implements the second idea (only new rules and no new machinery)

    @serhiy-storchaka
    Copy link
    Member

    See also bpo-1634034.

    @lysnikolaou
    Copy link
    Contributor

    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.

    @pablogsal
    Copy link
    Member Author

    See also bpo-1634034.

    The problem with bpo-1634034 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

    @pablogsal
    Copy link
    Member Author

    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?

    @lysnikolaou
    Copy link
    Contributor

    Yeah, but the cut is "absolute" (because of the syntax Error). IIRC the normal cut only affects that rule, no?

    Right.

    @pablogsal
    Copy link
    Member Author

    New changeset 58fb156 by Pablo Galindo in branch 'master':
    bpo-42997: Improve error message for missing : before suites (GH-24292)
    58fb156

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.10 only security fixes
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @serhiy-storchaka @pablogsal @lysnikolaou