Implement visitation of type aliases and parameters #5927

zanieb · 2023-07-20T17:17:05Z

Summary

Part of #5062
Requires astral-sh/RustPython-Parser#32

Adds visitation of type alias statements and type parameters in class and function definitions.

Duplicates tests for PreorderVisitor into Visitor with new snapshots. Testing required node implementations for the TypeParam enum, which is a chunk of the diff and the reason we need Ranged implementations in astral-sh/RustPython-Parser#32.

Test Plan

Adds unit tests with snapshots.

github-actions · 2023-07-20T17:55:42Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.3±0.61ms     4.0 MB/sec    1.06     10.9±0.15ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.03      2.2±0.03ms     7.6 MB/sec    1.00      2.1±0.05ms     7.8 MB/sec
formatter/numpy/globals.py                 1.00    236.4±6.55µs    12.5 MB/sec    1.08   254.7±16.18µs    11.6 MB/sec
formatter/pydantic/types.py                1.00      4.5±0.15ms     5.6 MB/sec    1.00      4.5±0.13ms     5.6 MB/sec
linter/all-rules/large/dataset.py          1.00     14.6±0.43ms     2.8 MB/sec    1.01     14.8±0.21ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.8±0.07ms     4.4 MB/sec    1.00      3.8±0.04ms     4.4 MB/sec
linter/all-rules/numpy/globals.py          1.00    493.3±6.55µs     6.0 MB/sec    1.00    492.4±9.35µs     6.0 MB/sec
linter/all-rules/pydantic/types.py         1.04      6.8±0.10ms     3.8 MB/sec    1.00      6.5±0.24ms     3.9 MB/sec
linter/default-rules/large/dataset.py      1.00      7.6±0.12ms     5.3 MB/sec    1.00      7.6±0.18ms     5.4 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.03  1652.6±17.93µs    10.1 MB/sec    1.00  1609.1±42.32µs    10.3 MB/sec
linter/default-rules/numpy/globals.py      1.00    173.8±7.02µs    17.0 MB/sec    1.01    175.3±6.08µs    16.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.4±0.13ms     7.6 MB/sec    1.02      3.4±0.12ms     7.5 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.8±0.05ms     3.8 MB/sec    1.00     10.8±0.08ms     3.8 MB/sec
formatter/numpy/ctypeslib.py               1.01      2.1±0.02ms     7.8 MB/sec    1.00      2.1±0.02ms     7.9 MB/sec
formatter/numpy/globals.py                 1.00    229.7±2.97µs    12.8 MB/sec    1.00    229.2±3.09µs    12.9 MB/sec
formatter/pydantic/types.py                1.00      4.7±0.03ms     5.4 MB/sec    1.00      4.7±0.04ms     5.4 MB/sec
linter/all-rules/large/dataset.py          1.00     15.2±0.13ms     2.7 MB/sec    1.01     15.4±0.11ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.0±0.03ms     4.1 MB/sec    1.01      4.1±0.04ms     4.1 MB/sec
linter/all-rules/numpy/globals.py          1.00    409.1±6.98µs     7.2 MB/sec    1.01    413.8±6.73µs     7.1 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.9±0.06ms     3.7 MB/sec    1.03      7.1±0.09ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      8.0±0.05ms     5.1 MB/sec    1.01      8.1±0.05ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1616.8±12.96µs    10.3 MB/sec    1.02  1641.9±14.93µs    10.1 MB/sec
linter/default-rules/numpy/globals.py      1.00    173.7±1.62µs    17.0 MB/sec    1.00    174.5±2.06µs    16.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.6±0.08ms     7.0 MB/sec    1.01      3.7±0.04ms     7.0 MB/sec

zanieb · 2023-07-20T18:04:36Z

Cargo.toml

+ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "86f4dc8869bbab5786a39cc6d1e6083c12d65e6e" }
+rustpython-ast = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "86f4dc8869bbab5786a39cc6d1e6083c12d65e6e" , default-features = false, features = ["num-bigint"]}
+rustpython-format = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "86f4dc8869bbab5786a39cc6d1e6083c12d65e6e", default-features = false, features = ["num-bigint"] }
+rustpython-literal = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "86f4dc8869bbab5786a39cc6d1e6083c12d65e6e", default-features = false }
+rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "86f4dc8869bbab5786a39cc6d1e6083c12d65e6e" , default-features = false, features = ["full-lexer", "num-bigint"] }


This pin is temporary and will need to be amended on merge of astral-sh/RustPython-Parser#32

zanieb · 2023-07-20T18:05:31Z

...es/ruff_python_ast/src/snapshots/ruff_python_ast__visitor__tests__class_type_parameters.snap

@@ -0,0 +1,13 @@
+---
+source: crates/ruff_python_ast/src/visitor.rs


relevant test

crates/ruff_python_ast/src/visitor/preorder.rs

zanieb · 2023-07-20T18:09:26Z

...ruff_python_ast/src/snapshots/ruff_python_ast__visitor__tests__function_type_parameters.snap

@@ -0,0 +1,14 @@
+---
+source: crates/ruff_python_ast/src/visitor.rs


relevant test

zanieb · 2023-07-20T18:09:47Z

crates/ruff_python_ast/src/snapshots/ruff_python_ast__visitor__tests__compare.snap

@@ -0,0 +1,12 @@
+---
+source: crates/ruff_python_ast/src/visitor.rs


Most of these snapshots are new coverage of previous behavior with tests duplicated from the pre-order visitor.

zanieb · 2023-07-20T18:09:57Z

crates/ruff_python_ast/src/snapshots/ruff_python_ast__visitor__tests__type_aliases.snap

@@ -0,0 +1,15 @@
+---
+source: crates/ruff_python_ast/src/visitor.rs


relevant test

Required for astral-sh/ruff#5927

crates/ruff_python_ast/src/visitor.rs

MichaReiser

LGTM.

Can you search for all Visitor (preorder or normal) implementations and add the overrides for visit_type_params for visitors that must visit all nodes or do you plan to do this as separate PRS?

For example, it's important for the correctness of CommentsVisitor to override every node visiting function:

ruff/crates/ruff_python_formatter/src/comments/visitor.rs

Line 154 in 59f5940

impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast> {

crates/ruff_python_ast/src/visitor.rs

zanieb · 2023-07-24T17:56:00Z

Can you search for all Visitor (preorder or normal) implementations and add the overrides for visit_type_params for visitors that must visit all nodes or do you plan to do this as separate PRS?

I'll do this here!

zanieb mentioned this pull request Jul 20, 2023

Implement Ranged for TypeParam astral-sh/RustPython-Parser#32

Merged

zanieb marked this pull request as draft July 20, 2023 17:57

zanieb commented Jul 20, 2023

View reviewed changes

crates/ruff_python_ast/src/visitor/preorder.rs Outdated Show resolved Hide resolved

zanieb commented Jul 20, 2023

View reviewed changes

zanieb requested a review from charliermarsh July 20, 2023 18:17

charliermarsh pushed a commit to astral-sh/RustPython-Parser that referenced this pull request Jul 21, 2023

Implement Ranged for TypeParam (#32)

fb365c6

Required for astral-sh/ruff#5927

charliermarsh reviewed Jul 21, 2023

View reviewed changes

crates/ruff_python_ast/src/visitor.rs Show resolved Hide resolved

MichaReiser approved these changes Jul 21, 2023

View reviewed changes

crates/ruff_python_ast/src/visitor.rs Outdated Show resolved Hide resolved

crates/ruff_python_ast/src/visitor.rs Show resolved Hide resolved

crates/ruff_python_ast/src/visitor.rs Show resolved Hide resolved

zanieb force-pushed the zanie/695-visitor branch from 903a3a6 to e9cac63 Compare July 21, 2023 14:31

zanieb force-pushed the zanie/695-visitor branch from e9cac63 to f560538 Compare July 24, 2023 20:26

zanieb marked this pull request as ready for review July 24, 2023 20:29

zanieb added 13 commits July 25, 2023 10:42

Implement visitation of type aliases

fabc2c4

Implement visit of type parameters for classes and functions

3167ff0

Add test cases for preorder visitor

5ae307e

WIP: Add type parameters to AnyNode

21f9c9e

Commit snapshots

fdb8bb5

Update parser commit to include changes from main

a80c7dd

Add snapshot tests for visitor

9fa34f6

Format

0f5cede

Fixup

769daf0

Bump Parser pin following merge of dependency

0b20fe2

Shorten expression with |

dffc124

Implement visit_type_param for CommentsVisitor

4efff8e

Fix cargo deps

c8f962a

zanieb force-pushed the zanie/695-visitor branch from f560538 to c8f962a Compare July 25, 2023 15:45

Fix fuzz dependency

be33a90

zanieb enabled auto-merge (squash) July 25, 2023 17:02

zanieb merged commit 389fe13 into main Jul 25, 2023

zanieb deleted the zanie/695-visitor branch July 25, 2023 17:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement visitation of type aliases and parameters #5927

Implement visitation of type aliases and parameters #5927

zanieb commented Jul 20, 2023 •

edited

Loading

github-actions bot commented Jul 20, 2023 •

edited

Loading

zanieb Jul 20, 2023

zanieb Jul 20, 2023 •

edited

Loading

zanieb Jul 20, 2023

zanieb Jul 20, 2023

zanieb Jul 20, 2023

MichaReiser left a comment

zanieb commented Jul 24, 2023

		@@ -0,0 +1,13 @@
		---
		source: crates/ruff_python_ast/src/visitor.rs

		@@ -0,0 +1,14 @@
		---
		source: crates/ruff_python_ast/src/visitor.rs

		@@ -0,0 +1,12 @@
		---
		source: crates/ruff_python_ast/src/visitor.rs

		@@ -0,0 +1,15 @@
		---
		source: crates/ruff_python_ast/src/visitor.rs

Implement visitation of type aliases and parameters #5927

Implement visitation of type aliases and parameters #5927

Conversation

zanieb commented Jul 20, 2023 • edited Loading

Summary

Test Plan

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

PR Check Results

Ecosystem

Benchmark

Linux

Windows

zanieb Jul 20, 2023

Choose a reason for hiding this comment

zanieb Jul 20, 2023 • edited Loading

Choose a reason for hiding this comment

zanieb Jul 20, 2023

Choose a reason for hiding this comment

zanieb Jul 20, 2023

Choose a reason for hiding this comment

zanieb Jul 20, 2023

Choose a reason for hiding this comment

MichaReiser left a comment

Choose a reason for hiding this comment

zanieb commented Jul 24, 2023

zanieb commented Jul 20, 2023 •

edited

Loading

github-actions bot commented Jul 20, 2023 •

edited

Loading

zanieb Jul 20, 2023 •

edited

Loading