Warn about always true/false isinstance tests #2395
Comments
|
Initial reactions:
|
|
To add to Reid's comment: but don't warn about an assertion that always fails if it's an explicit |
|
@ddfisher Or any false-y builtin constant, since I've seen |
|
@ilinum This issue might be interesting given your recent quest for flagging unreachable code. |
|
I have found five other issues on the tracker that are some variations of this, so I think this is high priority. |
This was referenced Jun 15, 2019
Michael0x2a
added a commit
to Michael0x2a/mypy
that referenced
this issue
Jun 24, 2019
This diff adds a `--disallow-inferred-unreachable` flag that reports an error if: 1. Any branch is inferred to be unreachable 2. Any subexpression in boolean expressions, inline if statements, and list/set/generator/dict comprehensions are always unreachable or redundant. This only takes into account the results of *type* analysis. We do not report errors for branches that are inferred to be unreachable in the semantic analysis phase (e.g. due to `sys.platform` checks and the like). Those checks are all intentional/deliberately flag certain branches as unreachable, so error messages would be annoying in those cases. A few additional notes: 1. I didn't spend a huge amount of time trying to come up with error messages. They could probably do with some polishing/rewording. 2. I thought about enabling this flag for default with mypy, but unfortunately that ended up producing ~40 to 50 (apparently legitimate) error messages. About a fourth of them were due to runtime checks checking to see if some type is None (despite not being declared to be Optional), about a fourth seemed to be due to mypy not quite understanding how to handle things like traits and Bogus[...], and about a fourth seemed to be legitimately unnecessary checks we could safely remove. The final checks were a mixture of typeshed bugs and misc errors with the reachability checks. (e.g. see python#7048) 3. For these reasons, I decided against adding this flag to `--strict` and against documenting it yet. We'll probably need to flush out a few bugs in mypy first/get mypy to the state where we can at least honestly dogfood this. Resolves python#2395; partially addresses python#7008.
JukkaL
pushed a commit
that referenced
this issue
Jun 28, 2019
This diff adds a `--warn-unreachable` flag that reports an error if: 1. Any branch is inferred to be unreachable 2. Any subexpression in boolean expressions, inline if statements, and list/set/generator/dict comprehensions are always unreachable or redundant. This only takes into account the results of *type* analysis. We do not report errors for branches that are inferred to be unreachable in the semantic analysis phase (e.g. due to `sys.platform` checks and the like). Those checks are all intentional/deliberately flag certain branches as unreachable, so error messages would be annoying in those cases. A few additional notes: 1. I thought about enabling this flag for default with mypy, but unfortunately that ended up producing ~40 to 50 (apparently legitimate) error messages. About a fourth of them were due to runtime checks checking to see if some type is None (despite not being declared to be Optional), about a fourth seemed to be due to mypy not quite understanding how to handle things like traits and Bogus[...], and about a fourth seemed to be legitimately unnecessary checks we could safely remove. The final checks were a mixture of typeshed bugs and misc errors with the reachability checks. (e.g. see #7048) 2. For these reasons, I decided against adding this flag to `--strict` and against documenting it yet. We'll probably need to flush out a few bugs in mypy first/get mypy to the state where we can at least honestly dogfood this. Resolves #2395; partially addresses #7008.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
These may result in code that mypy considers unreachable and silently doesn't type check. However, they may happen quite frequently in innocent things like
assert isinstance(x, str)so finding a good heuristic for what to report can be tricky.The text was updated successfully, but these errors were encountered: