Checks module bodies to be reachable #11361
Conversation
| @@ -307,6 +307,10 @@ def check_first_pass(self) -> None: | |||
| 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.
| @@ -768,13 +768,13 @@ while isinstance(b, int): | |||
| else: | |||
| 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.
There was a problem hiding this comment.
Is this resolved? Afaict the if False check should be in its own test case right?
|
@JukkaL done! Now it is in sync with |
| @@ -768,13 +768,13 @@ while isinstance(b, int): | |||
| else: | |||
| reveal_type(b) # E: Statement is unreachable | |||
|
|
|||
| 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.
There was a problem hiding this comment.
Nope, because we already have tests for explicit raise. Here we test assert False
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.
| @@ -768,13 +768,13 @@ while isinstance(b, int): | |||
| else: | |||
| 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?
|
@hauntsaninja I forgot to run |
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