Skip to content

[red-knot] Reachability analysis #17199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Apr 4, 2025

Summary

This PR proposes a new approach to silencing unresolved-reference diagnostics by keeping track of the reachability of each use of a symbol. The changes merged in #17169 are still needed for the "Use of variable in nested function" test case, but that could also be solved in another way eventually (see astral-sh/ty#210). We can use the same technique to silence unresolved-import and unresolved-attribute false-positives, but I think this could be merged in isolation.

Test Plan

New Markdown tests, ecosystem tests

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Apr 4, 2025
Copy link
Contributor

github-actions bot commented Apr 4, 2025

mypy_primer results

Changes were detected when running on open source projects
packaging (https://github.com/pypa/packaging)
- warning[lint:unresolved-reference] /tmp/mypy_primer/projects/packaging/src/packaging/metadata.py:28:22: Name `ExceptionGroup` used when not defined
- Found 96 diagnostics
+ Found 95 diagnostics

werkzeug (https://github.com/pallets/werkzeug)
- warning[lint:possibly-unresolved-reference] /tmp/mypy_primer/projects/werkzeug/src/werkzeug/formparser.py:64:36: Name `TemporaryFile` used when possibly not defined
- Found 683 diagnostics
+ Found 682 diagnostics

rich (https://github.com/Textualize/rich)
- warning[lint:unresolved-reference] /tmp/mypy_primer/projects/rich/rich/traceback.py:462:43: Name `BaseExceptionGroup` used when not defined
- warning[lint:unresolved-reference] /tmp/mypy_primer/projects/rich/rich/traceback.py:462:63: Name `ExceptionGroup` used when not defined
- warning[lint:unresolved-reference] /tmp/mypy_primer/projects/rich/tests/test_progress.py:186:13: Name `_time` used when not defined
- warning[lint:unresolved-reference] /tmp/mypy_primer/projects/rich/tests/test_progress.py:499:20: Name `time` used when not defined
- Found 382 diagnostics
+ Found 378 diagnostics

@sharkdp sharkdp force-pushed the david/silence-unresolved-reference-in-unreachable-code-pt2 branch 3 times, most recently from 751b05e to 05a808b Compare April 4, 2025 18:37
/// use(y)
/// ```
/// Depending on the value of `test`, the `y = 1`, `y = 2`, or both bindings may be visible.
/// The use of `x` is recorded with a reachability constraint of `[test]`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm completely fine with keeping this more concise.

Comment on lines +392 to +433
### Attributes

When attribute expressions appear in unreachable code, we should not emit `unresolved-attribute`
diagnostics:

```py
import sys
import builtins

if sys.version_info >= (3, 11):
# TODO
# error: [unresolved-attribute]
builtins.ExceptionGroup
```

### Imports

When import statements appear in unreachable code, we should not emit `unresolved-import`
diagnostics:

```py
import sys

if sys.version_info >= (3, 11):
# TODO
# error: [unresolved-import]
from builtins import ExceptionGroup

# TODO
# error: [unresolved-import]
import builtins.ExceptionGroup

# See https://docs.python.org/3/whatsnew/3.11.html#new-modules

# TODO
# error: [unresolved-import]
import tomllib

# TODO
# error: [unresolved-import]
import wsgiref.types
```
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will leave attributes and imports for a follow up. I want to get some feedback on this first.

@sharkdp sharkdp force-pushed the david/silence-unresolved-reference-in-unreachable-code-pt2 branch from 05a808b to 7ae5d31 Compare April 4, 2025 18:43
@sharkdp sharkdp marked this pull request as ready for review April 4, 2025 18:47
@sharkdp sharkdp force-pushed the david/silence-unresolved-reference-in-unreachable-code-pt2 branch from 7ae5d31 to 100d88b Compare April 4, 2025 19:29
@@ -218,26 +255,24 @@ def f():

In the example below, since we use `x` in the `inner` function, we use the "public" type of `x`,
which currently refers to the end-of-scope type of `x`. Since the end of the `outer` scope is
unreachable, we treat `x` as if it was not defined. This behavior can certainly be improved.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing changed in this test, but this explanation was outdated. And the return x # … below as confusing.


```py
def outer():
x = 1

def inner():
return x # Name `x` used when not defined
reveal_type(x) # revealed: Unknown
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that we reveal Unknown here is tracked in https://github.com/astral-sh/ruff/issues/15777

FEATURE_X_ACTIVATED = False
from typing import Literal

FEATURE_X_ACTIVATED: Literal[False] = False
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't support Final yet, so I needed to declare this as Literal[False]. Otherwise, the public type (which we use below) would be Unknown | Literal[False].

Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@sharkdp sharkdp force-pushed the david/silence-unresolved-reference-in-unreachable-code-pt2 branch from 100d88b to 03a2dcf Compare April 8, 2025 06:21
@sharkdp sharkdp merged commit 60f2e67 into main Apr 8, 2025
23 checks passed
@sharkdp sharkdp deleted the david/silence-unresolved-reference-in-unreachable-code-pt2 branch April 8, 2025 06:37
dcreager added a commit that referenced this pull request Apr 9, 2025
* origin/main:
  [red-knot] Default `python-platform` to current platform (#17183)
  [red-knot] Add new 'unreachable code' test case (#17306)
  [red-knot] mypy_primer: Run on `async-utils` (#17303)
  [red-knot] Add custom `__setattr__` support (#16748)
  [red-knot] Add `__init__` arguments check when doing `try_call` on a class literal (#16512)
  [`flake8-pie`] Avoid false positive for multiple assignment with `auto()` (`PIE796`) (#17274)
  [syntax-errors] Async comprehension in sync comprehension (#17177)
  [`airflow`] Expand module path check to individual symbols (`AIR302`) (#17278)
  [syntax-errors] Check annotations in annotated assignments (#17283)
  [syntax-errors] Extend annotation checks to `await` (#17282)
  [red-knot] Add support for `assert_never` (#17287)
  [`flake8-pytest-style`] Avoid false positive for legacy form of `pytest.raises` (`PT011`) (#17231)
  [red-knot] Do not show types for literal expressions on hover (#17290)
  [red-knot] Fix dead-code clippy warning (#17291)
  [red-knot] Reachability analysis (#17199)
  [red-knot] Don't use latency-sensitive for handlers (#17227)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ty Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants