Repository navigation
Checks module bodies to be reachable - #11361
hauntsaninja merged 6 commits into
Conversation
Sorry, something went wrong.
| with self.tscope.module_scope(self.tree.fullname): | ||
| with self.enter_partial_types(), self.binder.top_frame_context(): | ||
| for d in self.tree.defs: | ||
| if (self.binder.is_unreachable() |
There was a problem hiding this comment.
self.accept(Block(self.tree.defs)) didn't work. Let's try explicit condition.
Sorry, something went wrong.
JukkaL
left a comment
There was a problem hiding this comment.
Sounds like a good idea! If a user asks for warnings about unreachable code, this seems like what would be expected.
Sorry, something went wrong.
| reveal_type(b) # E: Statement is unreachable | ||
|
|
||
| def foo(c: int) -> None: | ||
| def foo(c: int) -> None: # E: Statement is unreachable |
There was a problem hiding this comment.
The rest of this test case should probably be moved to a new test case, since this is no longer testing what was the original intent, as everything is unreachable beyond this point.
Sorry, something went wrong.
There was a problem hiding this comment.
Is this resolved? Afaict the if False check should be in its own test case right?
Sorry, something went wrong.
|
@JukkaL done! Now it is in sync with pylance 👍 |
Sorry, something went wrong.
| def foo(c: int) -> None: | ||
| def foo(c: int) -> None: # E: Statement is unreachable | ||
| reveal_type(c) # N: Revealed type is "builtins.int" | ||
| assert not isinstance(c, int) |
There was a problem hiding this comment.
Could be a clearer statement of intent to explicitly raise an exception here. This will not make the code below unreachable if run with assertions disabled, which you can do in Python.
Sorry, something went wrong.
There was a problem hiding this comment.
Nope, because we already have tests for explicit raise. Here we test assert False
Sorry, something went wrong.
There was a problem hiding this comment.
Ah, I see. I guess my point is that assert False isn't a safe way to block off code. But whether mypy recognizes that or not is a more high level design decision, and not a subject for this PR.
Sorry, something went wrong.
| reveal_type(b) # E: Statement is unreachable | ||
|
|
||
| def foo(c: int) -> None: | ||
| def foo(c: int) -> None: # E: Statement is unreachable |
There was a problem hiding this comment.
Is this resolved? Afaict the if False check should be in its own test case right?
Sorry, something went wrong.
|
@hauntsaninja I forgot to run git push 🤦 » git log
commit 2d51fb85261a23242f79e9ffaa4dc95cb7c0c5d0 (HEAD -> issue-11331)
Author: sobolevn <mail@sobolevn.me>
Date: Fri Oct 22 18:28:03 2021 +0300
Addresses review
|
Sorry, something went wrong.
hauntsaninja
left a comment
There was a problem hiding this comment.
I kind of suspected that :-) Thanks, looks great
Sorry, something went wrong.
This is a very simple change, but now module bodies are also checked to be reachable.
Let's see what effect it will have.
Closes #11331