search

snok / flake8-type-checking

Flake8 plugin for managing type-checking imports & forward references.
BSD 3-Clause "New" or "Revised" License
113 stars 16 forks source link

Detect incorrect use of `|` in type annotations #181

Closed Daverball closed 10 months ago

Daverball commented 10 months ago

A fairly common mistake when partially wrapping/unwrapping annotations are expressions like:

x: 'SomeType' | None

These are always invalid at runtime and generate a TypeError (they're fine at type checking time, since it is treated the same as Union['SomeType', None])

I think it would be fairly easy to catch this mistake and add a new rule for it. Essentially any time we encounter a ast.BinOp/ast.BitOr inside a type annotation and either one of the operands is an ast.Constant, it should generate an error.

sondrelg commented 10 months ago

This sounds good, but what exactly causes it to fail at runtime?

Daverball commented 10 months ago

Neither str nor NoneType implement __or__ or __ror__, also both type.__ror__ and type.__or__ reject str as an operand. The reason being probably, that they didn't want to add __or__/__ror__ to NoneType or str, which would be necessary to allow all the possible permutations, so it's better to just always reject str.

The main reason why type checkers don't catch this currently, is probably because __or__/__ror__ need to support special form types, such as Optional and there's no way to spell this in the type system, so currently the operand's annotation is Any, even though it technically should reject anything that isn't a valid type.