[red-knot] Consider all definitions after terminal statements unreachable #15676
Conversation
dcreager
requested review from
carljm,
MichaReiser,
AlexWaygood and
sharkdp
as code owners
|
|
There are a couple of new diagnostics in the benchmark that don't look correct to me. I need to see if I can minimize that into an mdtest to diagnose. |
| x = "test" | ||
| else: | ||
| return "early" | ||
| return x |
There was a problem hiding this comment.
Just to clarify, since it took me a moment on first read, even though you just explained it above:
| return x | |
| return x # no possibly-unresolved-reference diagnostic! |
| else: | ||
| break | ||
| return x | ||
| return "late" |
There was a problem hiding this comment.
We could return x here as well and expect the possibly-unresolved-reference error?
| reveal_type(x) # revealed: Literal["before", "loop", "break"] | ||
| ``` | ||
|
|
||
| ## `return` is terminal in nested scopes |
There was a problem hiding this comment.
I don't think "nested scopes" is the right term here; Python if statements don't establish nested scopes. This heading would suggest a test with e.g. a nested function.
| ## `return` is terminal in nested scopes | |
| ## `return` is terminal in nested conditionals |
| reveal_type(x) # revealed: Literal["continue"] | ||
| continue | ||
| reveal_type(x) # revealed: Literal["loop"] | ||
| reveal_type(x) # revealed: Literal["before", "loop"] |
There was a problem hiding this comment.
I'm curious in which combination of cond and i it'd be Literal["loop"] at here.
|
|
||
| def g(cond: bool, i: int): | ||
| x = "before" | ||
| while i < 5: |
There was a problem hiding this comment.
You may want to switch to for loop instead or increase i's value inside the loop, otherwise it's infinite loop when i<5.
* main: [red-knot] MDTests: Do not depend on precise public-symbol type inference (#15691) [red-knot] Make `infer.rs` unit tests independent of public symbol inference (#15690) Tidy knot CLI tests (#15685) [red-knot] Port comprehension tests to Markdown (#15688) Create Unknown rule diagnostics with a source range (#15648) [red-knot] Port 'deferred annotations' unit tests to Markdown (#15686) [red-knot] Support custom typeshed Markdown tests (#15683) Don't run the linter ecosystem check on PRs that only touch red-knot crates (#15687) Add `rules` table to configuration (#15645) [red-knot] Make `Diagnostic::file` optional (#15640) [red-knot] Add test for nested attribute access (#15684) [red-knot] Anchor relative paths in configurations (#15634) [`pyupgrade`] Handle multiple base classes for PEP 695 generics (`UP046`) (#15659) [`pyflakes`] Treat arguments passed to the `default=` parameter of `TypeVar` as type expressions (`F821`) (#15679) Upgrade zizmor to the latest version in CI (#15649) [`pyupgrade`] Add rules to use PEP 695 generics in classes and functions (`UP046`, `UP047`) (#15565) [red-knot] Ensure a gradual type can always be assigned to itself (#15675)
|
I don't think you need to change anything for this PR, but just so it's on your radar: def f():
x = 1
while True:
try:
break
finally:
x = 2
reveal_type(x) # revealed: Literal[2] (it gives a revealed type of |
|
Yeah, we can handle |
We can use the new statically known branches feature to address #14014, by adding an "always false" visibility constraint immediately after each terminal statement.
Test Plan
The new mdtests failed (with incorrect
reveal_typeresults, and spuriouspossibly-unresolved-referenceerrors) before adding the new visibility constraints.