Check expr that are implicitly true for lack dunder bool/len

[ikonst]

Optional check which disallows using implicitly true expressions (whose type has no __bool__ nor __len__) in boolean context. The check is only enabled when using strict optionals.

This does not affect type narrowing / reachability which would continue seeing such types as "can be true / can be false". The rationale is that it's not an error per the data model to evaluate any object as boolean, and an always-true behavior could be overridden in a derived type.

Some builtin fixtures are updated to make sure common types (str, bool, int) are not considered to be always true.

[ikonst]

Do we have to check for Python 2 __nonzero__?

[JukkaL]

Teaches mypy to recognize instances that don't implement __bool__ or __len__ as always true.

Since subclasses can freely add __bool__, this doesn't sounds right. This is something we could perhaps infer for final classes, but the benefit would be marginal, since most users don't use @final much.

[ikonst]

Since subclasses can freely add bool, this doesn't sounds right.

I see that. Factoring such "knowledge" into unreachable code detection would be practically incorrect.

def f(a: object) -> None:
   a.foobar()
   _ = len(a)

For the cases above it's easy to justify a type error since, while foobar and __len__ could be added in a subclass, those operations would raise an error on object, while using object in boolean context succeeds "blindly".

I wonder how it'll be practical to add such check, perhaps through an optional warning, i.e. make it more of a linting rule and not something that affects analysis?

I keep seeing this in real world code, e.g.

def f(a: Iterable) -> None:
   if a:  # signature says Iterable but we're really expecting Collection and up
       ...

and while it's not technically incorrect, it's likely an error in typing (s/Iterable/Collection/).

Here's another favorite:

class MyMessage(google.protobuf.Message):
    ...

def f(a: MyMessage) -> None:
    if a.sub_message:  # user assumes missing Protobuf field manifests as None but it's an eagerly created empty message
        ...

Per Protobuf guidance, one shouldn't inherit from messages, so those could indeed be marked @final (e.g. in mypy-protobuf) but I agree @final is not widely used and not very practical.

[JukkaL]

This sounds like a good match for an optional check that can be activated via enabling some error code. Also --strict could enable it. If it's an optional check, it should have no impact on type checking or type inference when disabled, but that seems doable.

[ikonst]

@JukkaL Would it be OK to bolt this unto find_isinstance_check_helper like in 801f274? This checker seems to be already in the business of figuring out types in boolean context.

Also, is there anything that can print a node in a way that's short enough for messages, e.g.

note: NameExpr(m [l]) has type 'typing.Match[builtins.str*]' which does not implement __bool__ or __len__ so it will always be truthy in boolean context

Obviously I'd like something friendlier than NameExpr(m [l]). For some nodes getting a "name" is more obvious than others. Perhaps the source code of the node is the closest to what I'd expect. It's also possible that I'm worrying too much and that printing the type in context (line, column) is enough to explain what's going on.

[hauntsaninja]

I don't know that anything else in mypy prints nodes, and in general it looks pretty noisy to me. Most mypy error messages just print out a type and it seems to be fine. If you want, consider special casing RefExpr, and not printing the node for anything else.

[ikonst]

Thanks for updating the copy @hauntsaninja.

[hauntsaninja]

@ikonst Just made some more copy changes (including changing the error code name) and added some test case for if not truthy_expression. If it looks good to you, I'll merge.

My concerns with the phrasing:

1. The phrasing of "always" wasn't quite true, since subclasses could implement __bool__, so I tried to fudge that a little.
2. I don't know that "implicit bool" clearly describes the feature. I was also a little concerned about the potential for confusion with another requested mypy feature: to not treat bool as a subtype of int. Lmk if you have other suggestions.

[hauntsaninja]

Oh actually, I think I like the error code "bool-conversion" for this. Thoughts?

[ikonst]

Didn't yet review all changes, but "bool-conversion" rings like that "strict bool" feature we used to have. That is, it insinuates that relying on bool conversion is wrong perhaps even in cases where it's idiomatic Python.

[ikonst]

"You should always take an umbrella since it could always rain" -- not exactly what we mean :)

Maybe something like "since by default objects evaluate as true"?

The thing with subtypes is correct but I wonder if we can position it differently? Essentially we're warning due to lack of type-based guarantee of meaningful bool. What would we say if the python data model would be defined in such a way that len(object()) == 0)? What would our warning, if any, be like?

[ikonst]

@hauntsaninja ping :)

[hauntsaninja]

Yeah, something like "since by default objects always evaluate to true (this warning assumes that no subtype implements bool ..." seems good to me.
Feel free to make phrasing changes, or if you like it as is, let me know and I'll merge. I'm happy with most versions of the error code and docs now that we have the subtype thing mentioned.

[hauntsaninja]

Thanks for making this happen!