Misuse of or like this seems to be extremely common among beginners:
if x == 2 or 3:
print("yes")
Can we make a SyntaxWarning for it, similar to:
>>> if x is 2:
... print("yes")
...
<stdin>:1: SyntaxWarning: "is" with 'int' literal. Did you mean "=="?
Trying to define this concretely: the warning would be when and or or is used between two literals? That seems reasonable. I can’t think of any situations where you’d want to write 2 or 3; does anyone know of any?
It’s not quite “between two literals,” since this parses as if (x == 2) or 3. I’m not sure precisely what should be considered: is the equality important? Is the if important?
@nedbat I actually sometimes temporarily append or 1 to a condition for debugging, when I definitely want the if-block executed. A syntax warning might be bothersome then.
Right, I messed up my reading of it. That makes it a bit harder.
I was assuming that it would be parsed as (2 or 3) which would be safe to test for (it’s optimized out anyway). It wouldn’t be in this case. But it wouldn’t be in the original case either.
A SyntaxWarning won’t help as soon as variables are introduced:
if x == a or b:
print('yes')
That’s true, but that’s also the case for if x is 1 and that’s enough of a consideration to have a warning for. There definitely are a lot of student programmers out there writing things like if word == "no" or "NO" that it’d be helpful to give the warning. But I’m not sure how to pin down the trigger.
I’d propose triggering it on an AST tree like (sorry for the invented meta-syntax):
test=BoolOp(
op=Or(), # or And()
values=[
Compare(
left=<ANY>,
ops=[<ANY>],
comparators=[Constant(value=<ANY>)]),
Constant(value=<ANY>)
]
)
(Typed this at the same time as Ned.)
Seems like it should be enough to detect an ast.And or ast.Or node below an ast.If or ast.While node where one of the values is an ast.Constant.
While it’s not idiomatic nowadays, x = a and b or literal_default is a common pattern, which is why I say below If and While only to avoid false positives.
Good point about x = a or "yes" as a defaulting mechanism, so yes, it should be inside if/while.
I think the thread may have already come to all the same conclusions I did, but I see value in summarizing:
I think it’s too easy for code of that form to be actually meaningful, to warn for it.
I think the if is important to the warning. If you’re hacking a conditional for debugging purposes, a warning is the least of your worries. If you have a “real” use for something like or 1, it’s IMO much more likely to be in a context like a = b() or 1.
I think the best we could do is to warn on if <EXPR> or <truthy_literal>:, with something like
SyntaxWarning: condition is always met because <truthy_literal> is always true
and similarly for if <EXPR> and <falsy_literal>.
And of course the same with operands reversed, and with multiple and/or… just applying some simple logic at the AST level with the statically known literal values. (I think this is what @davidism was getting at, but a bit more sophisticated.)
… On Second Thought…
Maybe it would be worthwhile to warn on things like if <EXPR> or <VAR>:, if the last token in the expression is an identifier as well. A case like if x == a or b: seems relatively more likely to be the beginner error than, say, x == 1 or a:. Aside from that, the natural workaround for the warning (aside from explicitly silencing it) would be to add parentheses, like if (x == a) or b:, which is arguably clearer anyway. (If the beginner did mean if x == a or x == b:, then adding such parentheses makes the problem easier to understand and debug when it subsequently arises.) Of course, we don’t want to push people into writing tons of extra, paranoid parentheses; but still.
I can’t think offhand how to phrase such warning, though.
While I agree that this kind of beginner mistake can and does crop up, I don’t agree that we need to create a syntax warning for it. It’s all down to experience and once a beginner makes this kind of a fundamental error and is made aware of the mistake, it serves as a lesson and said mistake is unlikely to be repeated.
We’ve all had to learn and the best way to learn is to make these kind of mistakes and move on.
I think this is an the territory of linters and style checkers. For now, SyntaxWarning is raised in two cases:
assert with parenthesis as in C, depending on CPython specific interning of literals, missed comma interpreted as a call or subscription.Python does not raise SyntaxWarning for constructions which can occur in real code. In the linter you have options to control what types of warnings you want to see, and it is expect to use such options. The Python interpreter is used to run Python programs, not to view warnings. If the warning is enabled by default and needs an option to disable, it will harm users of Python programs. If the warning is disabled by default and needs an option to enable, it loses the purpose. The beginner will not use option, because they do not know about it, and the more advanced user can use a linter with the same success.
Do you we shouldn’t have made a SyntaxWarning for if x is 1?
Yes, absolutely. I’m suggesting that Python help make the beginner aware of the mistake.
Do you have an example of if <CONDITION> or <LITERAL>: from real code?
Same. The syntax warning could be a good thing to make sure you don’t ship your debug code, though…
The warning wouldn’t trigger if you prepend 1 or to a condition, so that might be an alternative. Of course, that assumes you don’t mind skipping the evaluation of the condition itself.
I often add “1 or " or “0 and " before condition to make it temporary always true or always false. I do this on reflexes without much thinking, like adding a debug print. I do not know whether I added " or 1” or " and 0” after condition, but maybe I did this too. Of course this code only lives during debugging, but even in such case I do not want to see a SyntaxWarning. If I train myself to ignore SyntaxWarning, I will ignore them in other cases.
The standard Pythonic way to spell that would be or True. Would the SyntaxWarning apply to that case as well? That seems to be the one case where it could plausibly be intentional (even if temporary).
Well, the issue is that the users most likely to make this mistake are the least likely to use, or at least pay attention to and know how to read, the output of linters and style checkers (especially since they will likely be reporting many other less serious issues with their code than this).
I’m not sure I’d consider a temporary debugging hack to be “normal working code”, as the code is deliberately not functioning as normally intended. And what about if x is 1?
Different strokes for different folks, but personally I’d much rather see a SyntaxWarning, as I would not want that hack to live longer than absolutely necessary and I’d much rather have a reminder that its there and to remove it as soon as possible than risk any chance it makes it into production.