Add semantic analysis of type aliases and parameters #6109

zanieb · 2023-07-26T22:22:11Z

Requires astral-sh/RustPython-Parser#42
Related PyCQA/pyflakes#778
PEP-695
Part of #5062

Summary

Adds a scope for type parameters, a type parameter binding kind, and checker visitation of type parameters in type alias statements, function definitions, and class definitions.

A few changes were necessary to ensure correctness following the insertion of a new scope between function and class scopes and their parent.

Test Plan

Undefined name snapshots.

Unused type parameter rule will be added as follow-up.

zanieb · 2023-07-26T22:32:05Z

If interested, working with examples at https://gist.github.com/zanieb/fe1d6c32db2e9a6cc1bcb3be6feaaf05

crates/ruff/src/checkers/ast/mod.rs

charliermarsh · 2023-07-26T22:46:36Z

This is looking good to me!

github-actions · 2023-07-26T23:36:24Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                    pr
-----                                      ----                                    --
formatter/large/dataset.py                 1.19     12.9±0.38ms     3.1 MB/sec     1.00     10.9±0.47ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.19      2.5±0.12ms     6.7 MB/sec     1.00      2.1±0.11ms     8.0 MB/sec
formatter/numpy/globals.py                 1.05   262.1±11.92µs    11.3 MB/sec     1.00   249.6±23.05µs    11.8 MB/sec
formatter/pydantic/types.py                1.13      5.3±0.29ms     4.8 MB/sec     1.00      4.7±0.26ms     5.4 MB/sec
linter/all-rules/large/dataset.py          1.00     14.2±0.68ms     2.9 MB/sec     1.00     14.3±0.53ms     2.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.09ms     4.8 MB/sec     1.06      3.7±0.11ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.00   493.1±19.60µs     6.0 MB/sec     1.02   502.4±17.31µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.3±0.22ms     4.1 MB/sec     1.03      6.5±0.29ms     3.9 MB/sec
linter/default-rules/large/dataset.py      1.00      7.5±0.26ms     5.4 MB/sec     1.07      8.1±0.28ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.07  1731.7±213.19µs     9.6 MB/sec    1.00  1612.3±81.91µs    10.3 MB/sec
linter/default-rules/numpy/globals.py      1.00    177.2±6.23µs    16.7 MB/sec     1.10   194.9±17.64µs    15.1 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.3±0.14ms     7.6 MB/sec     1.04      3.5±0.14ms     7.3 MB/sec

Windows

group                                      main                                    pr
-----                                      ----                                    --
formatter/large/dataset.py                 1.00     13.7±0.71ms     3.0 MB/sec     1.01     13.9±0.65ms     2.9 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.7±0.17ms     6.3 MB/sec     1.00      2.6±0.18ms     6.3 MB/sec
formatter/numpy/globals.py                 1.02   294.9±33.43µs    10.0 MB/sec     1.00   289.6±21.05µs    10.2 MB/sec
formatter/pydantic/types.py                1.01      5.8±0.42ms     4.4 MB/sec     1.00      5.7±0.27ms     4.5 MB/sec
linter/all-rules/large/dataset.py          1.00     17.9±0.71ms     2.3 MB/sec     1.00     17.9±0.73ms     2.3 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.6±0.31ms     3.6 MB/sec     1.02      4.7±0.32ms     3.5 MB/sec
linter/all-rules/numpy/globals.py          1.00   545.8±38.41µs     5.4 MB/sec     1.01   549.6±34.32µs     5.4 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.9±0.36ms     3.2 MB/sec     1.01      8.0±0.48ms     3.2 MB/sec
linter/default-rules/large/dataset.py      1.00      9.7±0.41ms     4.2 MB/sec     1.02      9.9±0.54ms     4.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1957.4±102.10µs     8.5 MB/sec    1.01  1983.9±145.44µs     8.4 MB/sec
linter/default-rules/numpy/globals.py      1.01   215.0±18.54µs    13.7 MB/sec     1.00   212.6±13.41µs    13.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.3±0.32ms     5.9 MB/sec     1.03      4.5±0.41ms     5.7 MB/sec

Ensures that type parameter names are bound in the value expression

The parent scope of classes and functions is now always a `Type` scope and additional traversal must be done to reach the scope of interest

zanieb · 2023-07-27T21:55:11Z

Huh my latest commit is pushed but missing. Silly GitHub.

charliermarsh · 2023-07-28T01:57:58Z

crates/ruff/resources/test/fixtures/pyflakes/F821_17.py

+# Types used in aliased assignment must exist
+
+type Foo = DoesNotExist  # F821: Undefined name `DoesNotExist`
+type Foo = list[DoesNotExist]  # F821: Undefined name `DoesNotExist`


Were you able to test these against the updated version of Pyflakes?

I haven't! I'll do that and confirm they match. I tested for correctness with Pyright and Python runtime errors.

The results are identical except that pyflakes emits two incorrect warnings

10:21: undefined name 'Ts' 11:21: undefined name 'P'

They appear to handle binding of type var tuples and param specs incorrectly.

crates/ruff/src/rules/pyflakes/mod.rs

charliermarsh · 2023-07-28T02:02:12Z

crates/ruff_python_semantic/src/model.rs

+    }
+
+    /// Returns the first parent of the given scope that is not a [`ScopeKind::Type`] scope, if any.
+    pub fn scope_parent_skip_types(&self, scope: &Scope) -> Option<&Scope<'a>> {


Hmm, maybe non_type_scope_parent, so that it describes the data returned rather than the action taken? But not a strong opinion.

I'm not attached to my name but I don't like that one either haha maybe scope_non_type_parent? eek. I also wanted to try scope_first_parent_not(kind: ScopeKind) or something but making it generic felt weird too.

Maybe what you have here is just clearest... Or scope_parent_skip_type_scopes???

Maybe first_non_type_parent_scope or non_type_parent_scope?

Or we name it first_value_scope. The way I think about it is that variables can either bind a value or a type.

Unrelated: Is this a python thing that types have their own scope? In JS, they all end up in the same scope, but each binding is either a value or a type.

Yeah, surprisingly they added an entirely new lexical scope to support type parameters specifically in PEP 695: https://peps.python.org/pep-0695/#type-parameter-scopes. It's required due to some of the nuances of when these things are evaluated.

(Perhaps we should consider a more specific name for that scope kind though.)

I kind of like TypeParam scope more than Type but I didn't want to deviate in this first pass

crates/ruff_python_semantic/src/model.rs

crates/ruff/resources/test/fixtures/pyflakes/F821_17.py

charliermarsh

This looks great! I love the thoroughness of the test suite. I admittedly didn't review the actual test results for semantic correctness but am happy to take a closer look on any of the finer points if useful.

zanieb · 2023-07-28T03:49:31Z

crates/ruff/resources/test/fixtures/pyflakes/F821_17.py

+def can_access_inside[T](t: T) -> T:  # OK
+    print(T)  # OK
+    return t  # OK
+class Forward: ...  # OK


Self: Need to move Forward definition to end of file or change names so they do not collide across test cases.

Another benefit of isolating fixtures >.<

MichaReiser · 2023-07-28T06:30:14Z

crates/ruff_python_semantic/src/model.rs

+    }
+
+    /// Returns the first parent of the given scope that is not a [`ScopeKind::Type`] scope, if any.
+    pub fn scope_parent_skip_types(&self, scope: &Scope) -> Option<&Scope<'a>> {


Maybe first_non_type_parent_scope or non_type_parent_scope?

Or we name it first_value_scope. The way I think about it is that variables can either bind a value or a type.

Unrelated: Is this a python thing that types have their own scope? In JS, they all end up in the same scope, but each binding is either a value or a type.

crates/ruff_python_semantic/src/model.rs

zanieb force-pushed the zanie/695-semantic branch 3 times, most recently from 063126f to fcd5e14 Compare July 26, 2023 22:26

charliermarsh reviewed Jul 26, 2023

View reviewed changes

crates/ruff/src/checkers/ast/mod.rs Outdated Show resolved Hide resolved

zanieb force-pushed the zanie/695-semantic branch from 1636bcc to 5d3a631 Compare July 27, 2023 13:48

charliermarsh force-pushed the main branch from e87032d to 2a65e6f Compare July 27, 2023 14:25

zanieb force-pushed the zanie/695-semantic branch from 5d3a631 to b80a5b5 Compare July 27, 2023 14:28

zanieb added 12 commits July 27, 2023 13:00

Add ScopeKind::Type

958fb8d

Visit class decorators before the rest of the class definition

9c7e579

Add binding of type parameters in classes and functions

90ee170

Add deferred detection of undefined names in type variable bounds

4f8a3bd

Add "step" comments

99607e5

Visit type alias parameters before the value

103ae9c

Ensures that type parameter names are bound in the value expression

Implement visitation of type alias statements in the checker

e88c5c5

Simplify match

112a745

Clippy

7a42b3a

Add F821 test case for type parameters

2589d74

Fix classification of functions

79e9ff5

The parent scope of classes and functions is now always a `Type` scope and additional traversal must be done to reach the scope of interest

Fix scoping of class variables

093d40f

zanieb force-pushed the zanie/695-semantic branch from b80a5b5 to 093d40f Compare July 27, 2023 21:40

Implement scoping in lookup_symbol

fd6e6f4

zanieb marked this pull request as ready for review July 27, 2023 21:46

Commit snapshot file

8e94ac9

charliermarsh reviewed Jul 28, 2023

View reviewed changes

crates/ruff/src/rules/pyflakes/mod.rs Outdated Show resolved Hide resolved

charliermarsh reviewed Jul 28, 2023

View reviewed changes

crates/ruff_python_semantic/src/model.rs Show resolved Hide resolved

charliermarsh reviewed Jul 28, 2023

View reviewed changes

crates/ruff/resources/test/fixtures/pyflakes/F821_17.py Outdated Show resolved Hide resolved

charliermarsh approved these changes Jul 28, 2023

View reviewed changes

zanieb commented Jul 28, 2023

View reviewed changes

MichaReiser reviewed Jul 28, 2023

View reviewed changes

zanieb added 5 commits July 28, 2023 11:07

Use while let instead of loop

65682a1

Rename non-type scope parent method for clarity

4aaed6c

Fix whitespace

eec45ba

Rename methods (review feedback)

04cf38a

Deduplicate forward ref declarations

c35fd54

zanieb merged commit 047c211 into main Jul 28, 2023

zanieb deleted the zanie/695-semantic branch July 28, 2023 22:06

zanieb mentioned this pull request Jul 31, 2023

Add support for PEP 695 #5062

Closed

zanieb mentioned this pull request Aug 25, 2023

Implement support for PEP-695 Instagram/LibCST#922

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add semantic analysis of type aliases and parameters #6109

Add semantic analysis of type aliases and parameters #6109

zanieb commented Jul 26, 2023 •

edited

Loading

zanieb commented Jul 26, 2023

charliermarsh commented Jul 26, 2023

github-actions bot commented Jul 26, 2023 •

edited

Loading

zanieb commented Jul 27, 2023

charliermarsh Jul 28, 2023

zanieb Jul 28, 2023

zanieb Jul 28, 2023

charliermarsh Jul 28, 2023

zanieb Jul 28, 2023

charliermarsh Jul 28, 2023

MichaReiser Jul 28, 2023

charliermarsh Jul 28, 2023

charliermarsh Jul 28, 2023

zanieb Jul 28, 2023

charliermarsh left a comment

zanieb Jul 28, 2023

charliermarsh Jul 28, 2023

MichaReiser Jul 28, 2023

Add semantic analysis of type aliases and parameters #6109

Add semantic analysis of type aliases and parameters #6109

Conversation

zanieb commented Jul 26, 2023 • edited Loading

Summary

Test Plan

zanieb commented Jul 26, 2023

charliermarsh commented Jul 26, 2023

github-actions bot commented Jul 26, 2023 • edited Loading

PR Check Results

Ecosystem

Benchmark

Linux

Windows

zanieb commented Jul 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zanieb commented Jul 26, 2023 •

edited

Loading

github-actions bot commented Jul 26, 2023 •

edited

Loading