8000 bpo-43833: Emit warnings for numeric literals followed by keyword by serhiy-storchaka · Pull Request #25466 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-43833: Emit warnings for numeric literals followed by keyword #25466

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.

Sign up for GitHub

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
Merged
merged 4 commits into from
Jun 8, 2021
Merged

bpo-43833: Emit warnings for numeric literals followed by keyword #25466

merged 4 commits into from
Jun 8, 2021

Conversation

serhiy-storchaka
Copy link
Member
@serhiy-storchaka serhiy-storchaka commented Apr 18, 2021 •
edited by miss-islington
Loading

Emit a deprecation warning if the numeric literal is immediately followed by
one of keywords: and, else, for, if, in, is, or. Raise a syntax error with
more informative message if it is immediately followed by other keyword or
identifier.

https://bugs.python.org/issue43833

Automerge-Triggered-By: GH:pablogsal

@serhiy-storchaka
bpo-43833: Emit warnings for numeric literals followed by keyword
56ae489 
Emit a deprecation warning if the numeric literal is immediately followed by
one of keywords: and, else, for, if, in, is, or. Raise a syntax error with
more informative message if it is immediately followed by other keyword or
identifier.
@pablogsal
Copy link
Member
pablogsal commented Apr 18, 2021 •
edited
Loading

I am not a fan of this solution as this is coupling the tokenizer and the grammar, which is a known problematic behaviour.

In particular, I don't like the fact that we need to manually check against every keyword explicitly.

Cannot we just error if we find any character that is not a whitespace or a valid numeric literal?

We should try to fix this in a way they is keyword-agnostic

@serhiy-storchaka
Copy link
Member Author

Checking against the list of valid keywords is a temporary code. It is because we want to do gradual deprecation and only emit a warning for currently valid code. At the end, there will be only syntax error and no list of keywords.

If emit a warning unconditionally, this will break some tests for invalid numeric literals (test_bad_numerical_literals in test_grammar.py and test_literals_with_leading_zeroes in test_compile.py). Some test cases will produce warning and then error, so we would need to rewrite these tests significantly, and several releases later after converting warnings to error restore original tests. Double warning-error can also confuse users.

@pablogsal
Copy link
Member
pablogsal commented Apr 18, 2021 •
edited
Loading

Checking against the list of valid keywords is a temporary code. It is because we want to do gradual deprecation and only emit a warning for currently valid code. At the end, there will be only syntax error and no list of keywords.

If emit a warning unconditionally, this will break some tests for invalid numeric literals (test_bad_numerical_literals in test_grammar.py and test_literals_with_leading_zeroes in test_compile.py). Some test cases will produce warning and then error, so we would need to rewrite these tests significantly, and several releases later after converting warnings to error restore original tests. Double warning-error can also confuse users.

Thanks for the clarification. Could you add some comments regarding this plans to the source code of the change to make sure this context is there when reading the code?

In that case I assume that the main thing to do here is to all agree on the plan.

I personally find the plan reasonable so I'm on board, but I would go directly to SyntaxWarning and in 1 or 2 releases making it a SyntaxError.

@serhiy-storchaka
Copy link
Member Author

Sure. I added a comment in verify_end_of_number(), but seems it is not enough. I'll expand by adding explanation of the intention of this behavior.

As for SyntaxWarning vs DeprecationWarning, we still emit DeprecationWarning for invalid character escapes (like in "C:\Program Files"). Initially SyntaxWarning was emitted, but after public discussion it was changed to DeprecationWarning.

@serhiy-storchaka
Copy link
Member Author

@pablogsal, do you want to add these warnings in 3.10 or 3.11 if add them at all?

@pablogsal
Copy link
Member

@pablogsal, do you want to add these warnings in 3.10 or 3.11 if add them at all?

I would like them to be included in Python 3.10 so we can make them illegal sooner. What do you think?

@pablogsal pablogsal added the needs backport to 3.10 only security fixes label Jun 8, 2021
pablogsal
pablogsal reviewed Jun 8, 2021
View reviewed changes
@miss-islington
Copy link
Contributor

@serhiy-storchaka: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 2ea6d89 into python:main Jun 8, 2021
@miss-islington
Copy link
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 8, 2021
@serhiy-storchaka @miss-islington
bpo-43833: Emit warnings for numeric literals followed by keyword (py…
6e70771 
…thonGH-25466)

Emit a deprecation warning if the numeric literal is immediately followed by
one of keywords: and, else, for, if, in, is, or. Raise a syntax error with
more informative message if it is immediately followed by other keyword or
identifier.

Automerge-Triggered-By: GH:pablogsal
(cherry picked from commit 2ea6d89)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-bot
Copy link

GH-26614 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jun 8, 2021
miss-islington added a commit that referenced this pull request Jun 8, 2021
@miss-islington @serhiy-storchaka
bpo-43833: Emit warnings for numeric literals followed by keyword (GH…
eeefa7f 
…-25466)

Emit a deprecation warning if the numeric literal is immediately followed by
one of keywords: and, else, for, if, in, is, or. Raise a syntax error with
more informative message if it is immediately followed by other keyword or
identifier.

Automerge-Triggered-By: GH:pablogsal
(cherry picked from commit 2ea6d89)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@pablogsal pablogsal mentioned this pull request Jul 18, 2021
Closed
@sco1 sco1 mannequin mentioned this pull request Apr 25, 2022
Closed
@serhiy-storchaka serhiy-storchaka deleted the end-of-numeric-literal branch January 28, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablogsal pablogsal pablogsal approved these changes

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@serhiy-storchaka @pablogsal @miss-islington @bedevere-bot @the-knights-who-say-ni
0