Repository navigation
Warning for foo == "bar" or "baz" #92739
Description
Feature or enhancement
Emit SyntaxWarning for code like if foo == "bar" or "baz". It should be written as if foo == "bar" or foo == "baz", or perhaps if foo in ("bar", "baz"). The warning is emitted only if "bar" and "baz" here are string literals or number literals as in if foo == 1 or 2.5, but foo can be any expression. Maybe also do the same for while and assert.
Pitch
Currently there's no warning, the if statement just silently does the wrong thing. I have helped many people to learn programming in Python, and in my experience, this is a mistake that every beginner makes at some point during their learning.
In an if statement, a condition like ... or "some hard-coded string" always evaluates to a truthy value (unless the string is empty in which case the or "" can be safely removed). So in this context, or "some_string" can't really be intentional. It's an error and it shouldn't pass silently, but it does.
Alternative ideas that I thought of and rejected:
- Adding this to a linter instead of CPython: Beginners don't know what a linter is, and many beginners don't use an IDE that runs linters in the background. Whether or not they should is a matter of opinions :)
- The
oroperator raising runtime error if one of the operands is a string. Breaks correct code likereturn foobar or "some default value". Would also be ridiculously slow.
CPython already emits a similar warning for assert(condition, message), which asserts that a tuple of two elements is truthy.
Activity
nedbat commentedon May 12, 2022
I think this would be great.
sweeneyde commentedon May 12, 2022
I opened #92747
serhiy-storchaka commentedon May 13, 2022
I do not think this should be added in the compiler. It will complicate the code for very tiny benefit. Such kind of mistakes are only made by beginners which learn programming. It is easy to catch when it happen the first time, and it is unimaginative that anybody make it the second time.
In contrary, the mistake in
assert(condition, message)can be made even by experience programmer if he came from other programming language, and it is difficult to catch it, because it just "works", until it should fail, but do not fail, and in otherwise correct program it never fails.Akuli commentedon May 13, 2022
I have quite the opposite experience. I know a friend who was working on a non-trivial size program (hundreds of lines) when this happened to him for the first time. If I hadn't pointed it out in a review, the program would have behaved badly in a corner case that happens somewhat rarely. He didn't notice it himself when testing.
sweeneyde commentedon May 14, 2022
Hm, I'm sort of on the fence about this. On one hand, it is an extremely common issue on StackOverflow. This question has 1,294 questions linked to/from it including many, many duplicates.
I do buy the argument that the best tool to detect this is a linter, and needless complexity should be avoided, but I also think this might give a bit of benefit to beginners. I would defer to those with more experience teaching beginners though. I would assume that issue would be particular to those learning Python as their first programming language.
ronaldoussoren commentedon May 14, 2022
IMHO The proposed warning can only find a very limited subset of possibly problematic patterns without resulting in a high level of false positives.
The stack overflow question you link to is an example of that. That refers to "a or b or c == 2". In the context of that question it is clear that the OP should have used "a == 2 or b == 2 ...", but I've used similar code patterns where the code as written was correct.
sweeneyde commentedon May 14, 2022
That particular heavily-linked-to post isn't of a form that could be caught at compile time, but browsing the top 15 "Hottest" of those 1294 links, most of these could safely be made to emit compiler warnings without bothering anyone:
if "a" or "á" or "à" or "ã" or "â" in someList:
proceed="y" or "Y"
if name == ("Jesse" or "jesse"):
if fields[9] != ('A' or 'D' or 'E' or 'N' or 'R'):
if job == "mechanic" or "tech":, (though this is a question "How do I do this better", not a mistake)
if ('a' or 'b') in L:
if x == "monkey" or "monkeys":
if first == "a" or "e" or "i" or "o" or "u":
if (len(x) == (4 or 6)) and x.isdigit():
if user_input == "look" or "look around":
if request == "y" or "Y":
if choice == "yes" or "Yes":
if not ( userDesc==("Y") or ("H") or ("E")):
elif taxon == "Kids Earrings" or "Earrings":
Akuli commentedon May 15, 2022
There seems to be 3 different opinions about this:
For opinion 3:
I don't see how limited implies it's useless. Beginners use more hard-coded strings and ints in their if checks than experienced users, so restricting this to hard-coded values doesn't matter very much. The list above shows this quite clearly.
This one boils down to whether we want "simple is better than complex" or "practicality beats purity". To me, a warning like this seems very practical, while others prefer keeping the compiler simple.
boxed commentedon May 16, 2022
Might I propose a much simpler option that also catches much more:
Warn for any literal in an
ifsurrounded by and/or/in.serhiy-storchaka commentedon May 22, 2022
#92747 adds 60 lines of the C code! It is a much even for more useful feature.
And the benefit? If a beginner who learn programming with Python as his first language will get a warning which he will not be able to understand when he makes one specific kind of 100 kinds of errors which beginners do. In any case it will end on StackOverflow or other forum, if he just not ask a teacher or a classmate.
boxed commentedon May 22, 2022
The argument above can be rephrased:
Python is the biggest programming language in the world. It has enormous positive benefits if it helps people, especially beginners who are already having a very hard time. This is why the new error messages in 3.11 were introduced and why they are so awesome.
My suggestion above is probably shorter than 60 lines of C too, and would catch other mistakes too.
Akuli commentedon May 22, 2022
The problem with this is that experienced people don't understand how common this mistake is, and how much the warning would help beginners.
This is true, but it doesn't mean that the warning is useless. It tells the beginner that they are doing something wrong. It mentions the line number of the broken line of code. It is very easy to search on google (although this isn't a big difference, as "python or not working" already finds this problem).
This one is worse than other beginner problems, because you don't even realize that something is wrong. As I said, this could happen when your programs are already 300 lines long or so.
Akuli commentedon Jun 3, 2022
Maybe this warning could be compared to the new warning for
is:It's a similar problem: experienced programmers (coming from other languages or otherwise) will use
==as supposed to, and not try to combine strings withor, while beginners translating their thoughts from English are more likely to useisor misuseor.I think the
orwarning is slightly less important than theiswarning though. Someone could program Python for a long time and useisfor all checks between integers, and their code would stop working only when it gets an integer that's too big to be cached. Withor, at least your code won't appear to work when you test it, if you test it properly. I have even seen a tutorial that usesisin their first if statement example, but a badly writtenorwould never make its way into a tutorial (although many tutorials don't warn about it).But still, where do we draw the line? Does this really make the
iswarning acceptable and theorwarning a bad idea?boxed commentedon Jun 3, 2022
@Akuli just warning for any part of an
ifthat can be trivially determined to be static at compile time seems like a much cleaner solution to the original problem.So
if 1would warn,if foo or 1would warn, but also the original exampleif foo == "bar" or "baz".It's not about
or, orandoris. It's about a literal as a part of what should be an evaluated expression. This btw makes sense forwhiletoo!boxed commentedon Jun 3, 2022
@oda-gitso good point. No not that. I guess literal
Trueas the entire expression would have to be whitelisted for this warning. Makes it a bit less clean, but still ok I think.Akuli commentedon Jun 3, 2022
I'm closing this issue, because it just doesn't seem to be going anywhere: a few core developers say no, and you generally don't go online to change your mind. In general, I'm confused about whether the core developers think that Python is supposed to be beginner-friendly:
if x is 1warning suggests yes, this issue suggests no. I did expect this to be controversial though :)This isn't a documentation issue. When a beginner learns to program, they have:
We can't just update all tutorials, just because there's so many different tutorials. Same goes for editors and linters. If we want this problem to be detected reliably before the experienced programmer(s), it has to be done by the interpreter.