Open jcrist opened 2 years ago
Great idea!
If possible, I'd be interested in hacking on this as an interesting side project. I took a scan through the code and couldn't figure out where such an analysis pass might fit in to mypy. If this seems like a reasonable project for someone who's new-to-mypy-but-not-new-to-python, I'd welcome some advice on where/how this feature should be added. "No, this is a tricky feature that would be best handled by the core mypy team" is also a fine response. Happy to help if possible.
@jcrist yes, this looks rather simple. I can guide you, if you wish 🙂
So, here's how I see this:
visit_try_stmt
in checker.py
: https://github.com/python/mypy/blob/fdcda966023840b854567df0643091653b179210/mypy/checker.py#L3563if self.options.warn_unreachable
we check TryStmt.types
prop: https://github.com/python/mypy/blob/fdcda966023840b854567df0643091653b179210/mypy/nodes.py#L1314 You can convert Expression
to Type
with self.expr_checker.accept(expression)
. It is a good idea to disable errors while doing that. Because we properly check expression types in a different place. Or maybe you can add this check to a place where we do check except
types 🤔 is_subtype
function: https://github.com/python/mypy/blob/fdcda966023840b854567df0643091653b179210/mypy/subtypes.py#L49self.fail
[fixture ...]
with proper exception hierarchy!I hope that this helps. Feel free to ping me if you have any questions 🙂 Btw, I am not a maintainer, so this can still be rejected during the review from real mypy experts. But, you never know untill you try! 👍
@jcrist are you still working on this? If not I would give it a try :)
No, I never gave it a go - please feel free to pick this up.
Since except blocks are checked in order, and the first matching block is executed, it's possible to accidentally write a try-except that will never execute some branches. For example:
Since
PermissionError
is a subclass ofOSError
, theexcept PermissionError
case will never execute. I'd expect/hope that these kind of mistakes would be detectable with type information. I think adding checks for this would make sense under the--warn-unreachable
config flag, so a new CLI option may not be needed.