Skip to content

bpo-34850: Emit a warning for "is" and "is not" with a literal. #9642

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

Merged

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Sep 30, 2018

}
PyObject *value = e->v.Constant.value;
return (value == Py_None
|| value == Py_False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't special case True and False, those are not normally identity checked. I don't doubt that someone may have create APIs returning multiple possible types that requires this to differentiate between "real" values and bools... but yuck.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@gpshead
Copy link
Member

gpshead commented Sep 30, 2018

i plopped a do not merge label on here as the discussion in the bug over whether we even want this isn't settled.

Python/compile.c Outdated
if (!right || !left) {
const char *msg = (op == Is)
? "\"is\" with a literal. Did you mean \"==\""
: "\"is not\" with a literal. Did you mean \"!=\"";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this warning looks good. I'd like to, but can't come up with a good way to work in the phrase "identity check".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have suggestions for the wording of the news or What's New entry?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

The compiler now produces a SyntaxWarning when identity checks (is and is not) are used with string literals. These can often work by accident in CPython, but are not guaranteed by the language spec. The warning advises users to use equality tests (== and !=) instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! But a warning will be emitted not just for string literals, but for numerical literals and tuples (empty tuple is a singleton in CPython, this is an implementation detail).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can say "certain types of literals (e.g. strings, ints)" rather than "string literals".

Revert change here; will make separate PR to be applied and backported regardless of fate of this PR.
@terryjreedy terryjreedy removed their request for review September 30, 2018 21:15
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 30, 2018
Patch by Serhiy Storchaka (in PR pythonGH-9642).
(cherry picked from commit 5fa247d)

Co-authored-by: Terry Jan Reedy <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 30, 2018
Patch by Serhiy Storchaka (in PR pythonGH-9642).
(cherry picked from commit 5fa247d)

Co-authored-by: Terry Jan Reedy <[email protected]>
terryjreedy added a commit that referenced this pull request Sep 30, 2018
miss-islington added a commit that referenced this pull request Sep 30, 2018
Patch by Serhiy Storchaka (in PR GH-9642).
(cherry picked from commit 5fa247d)

Co-authored-by: Terry Jan Reedy <[email protected]>
miss-islington added a commit that referenced this pull request Sep 30, 2018
Patch by Serhiy Storchaka (in PR GH-9642).
(cherry picked from commit 5fa247d)

Co-authored-by: Terry Jan Reedy <[email protected]>
@serhiy-storchaka serhiy-storchaka merged commit 3bcbedc into python:master Jan 18, 2019
@serhiy-storchaka serhiy-storchaka deleted the is-with-literal-warning branch January 18, 2019 05:47
aptalca added a commit to aptalca/python-cloudflare that referenced this pull request Jan 10, 2020
aptalca added a commit to aptalca/python-digitalocean that referenced this pull request Jan 10, 2020
Python 3.8 is now displaying a warning when is is used with a literal.

Here are the relevant errors with 2.4.0:
```
/usr/lib/python3.8/site-packages/digitalocean/LoadBalancer.py:19: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if type is 'cookies':
```

See python/cpython#9642 for more info
koalalorenzo pushed a commit to koalalorenzo/python-digitalocean that referenced this pull request Jan 20, 2020
…302)

Python 3.8 is now displaying a warning when is is used with a literal.

Here are the relevant errors with 2.4.0:
```
/usr/lib/python3.8/site-packages/digitalocean/LoadBalancer.py:19: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if type is 'cookies':
```

See python/cpython#9642 for more info
SoulStar7116 pushed a commit to SoulStar7116/python-digitalocean that referenced this pull request Nov 16, 2024
…(#302)

Python 3.8 is now displaying a warning when is is used with a literal.

Here are the relevant errors with 2.4.0:
```
/usr/lib/python3.8/site-packages/digitalocean/LoadBalancer.py:19: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if type is 'cookies':
```

See python/cpython#9642 for more info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO-NOT-MERGE type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants