Implement any_over_expr for type alias and type params

[zanieb]

Part of #5062

[github-actions]

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      9.8±0.03ms     4.2 MB/sec    1.00      9.8±0.02ms     4.2 MB/sec
formatter/numpy/ctypeslib.py               1.00  1891.5±10.15µs     8.8 MB/sec    1.00   1893.0±3.42µs     8.8 MB/sec
formatter/numpy/globals.py                 1.00    208.3±8.56µs    14.2 MB/sec    1.00    209.0±2.25µs    14.1 MB/sec
formatter/pydantic/types.py                1.00      4.2±0.01ms     6.1 MB/sec    1.00      4.2±0.00ms     6.1 MB/sec
linter/all-rules/large/dataset.py          1.00     13.7±0.07ms     3.0 MB/sec    1.00     13.7±0.08ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      3.5±0.02ms     4.8 MB/sec    1.00      3.5±0.01ms     4.8 MB/sec
linter/all-rules/numpy/globals.py          1.00    375.9±0.90µs     7.8 MB/sec    1.00    375.9±3.06µs     7.8 MB/sec
linter/all-rules/pydantic/types.py         1.01      6.2±0.02ms     4.1 MB/sec    1.00      6.2±0.02ms     4.1 MB/sec
linter/default-rules/large/dataset.py      1.00      7.1±0.01ms     5.7 MB/sec    1.00      7.1±0.02ms     5.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1452.3±5.11µs    11.5 MB/sec    1.00   1449.1±9.97µs    11.5 MB/sec
linter/default-rules/numpy/globals.py      1.00    150.1±0.96µs    19.7 MB/sec    1.01    151.2±0.19µs    19.5 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.2±0.00ms     8.0 MB/sec    1.00      3.2±0.00ms     8.1 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.09     11.8±0.10ms     3.4 MB/sec    1.00     10.9±0.17ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.08      2.3±0.04ms     7.1 MB/sec    1.00      2.2±0.03ms     7.7 MB/sec
formatter/numpy/globals.py                 1.03    257.8±5.53µs    11.4 MB/sec    1.00   249.6±12.28µs    11.8 MB/sec
formatter/pydantic/types.py                1.09      5.1±0.06ms     5.0 MB/sec    1.00      4.7±0.06ms     5.4 MB/sec
linter/all-rules/large/dataset.py          1.00     15.2±0.14ms     2.7 MB/sec    1.01     15.4±0.18ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.0±0.06ms     4.1 MB/sec    1.00      4.0±0.05ms     4.2 MB/sec
linter/all-rules/numpy/globals.py          1.00    485.0±7.58µs     6.1 MB/sec    1.00    483.9±5.31µs     6.1 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.9±0.07ms     3.7 MB/sec    1.00      6.9±0.06ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.00      7.9±0.06ms     5.2 MB/sec    1.00      7.9±0.07ms     5.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1652.0±14.25µs    10.1 MB/sec    1.00  1650.4±28.73µs    10.1 MB/sec
linter/default-rules/numpy/globals.py      1.01    189.6±2.88µs    15.6 MB/sec    1.00    187.8±2.75µs    15.7 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.6±0.05ms     7.2 MB/sec    1.00      3.5±0.04ms     7.2 MB/sec

[zanieb]

This test is a bit contrived but I'd appreciate any general Rust feedback as I'm using it for practice!

[MichaReiser]

I think it's fine. Or let's say, it's what the API allows you to do. You could have considered changing the Fn to FnMut so that you don't need to use RefCell (I guess it was a good learning opportunity ;)).

I'm normally too lazy to bother with building the tree manually and instead parse the source code. Now, using the source code makes asserting a bit annoying because you don't have access to the node instances. That's where you could collect NodeKinds instead (you can get the kind by calling AnyNodeRef::from(expr).kind()). However, NodeKind only works if you never have to distinguish between two nodes of the same kind. Here some tests that I wrote that use NodeKind.

ruff/crates/ruff_python_ast/src/visitor/preorder.rs

Lines 955 to 1039 in 5d41c83

	#[test]
	fn function_arguments() {
	let source = r#"def a(b, c,/, d, e = 20, *args, named=5, other=20, **kwargs): pass"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn function_positional_only_with_default() {
	let source = r#"def a(b, c = 34,/, e = 20, *args): pass"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn compare() {
	let source = r#"4 < x < 5"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn list_comprehension() {
	let source = "[x for x in numbers]";
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn dict_comprehension() {
	let source = "{x: x**2 for x in numbers}";
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn set_comprehension() {
	let source = "{x for x in numbers}";
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn match_class_pattern() {
	let source = r#"
	match x:
	case Point2D(0, 0):
	...
	case Point3D(x=0, y=0, z=0):
	...
	"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn decorators() {
	let source = r#"
	@decorator
	def a():
	pass
	@test
	class A:
	pass
	"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}

With an example snapshot

ruff/crates/ruff_python_ast/src/visitor/snapshots/ruff_python_ast__visitor__preorder__tests__compare.snap

Lines 1 to 15 in 5d41c83

	---
	source: crates/ruff_python_ast/src/visitor/preorder.rs
	expression: trace
	---
	- ModModule
	- StmtExpr
	- ExprCompare
	- ExprConstant
	- Int(4)
	- Lt
	- ExprName
	- Lt
	- ExprConstant
	- Int(5)

That said. I think what you have is good. The only thing I'm not a 100% sure of is if I would bother refactoring this test if I ever end up breaking it (thinking about added value / maintenance cost)

[zanieb]

Thanks for taking a look!

You could have considered changing the Fn to FnMut

It seems wrong to change the API just for testing.

I'm not a 100% sure of is if I would bother refactoring this test if I ever end up breaking it

Definitely. That's kind of what I was getting at with it being a bit contrived.

I'm normally too lazy to bother with building the tree manually and instead parse the source code.

Ah that's very wise. I feel like that's much more maintainable. I'll look into doing that.

[zanieb]

Eh this is kind of hard to do. I'd rather address it in a follow-up or next time someone wants to add a test here.

[MichaReiser]

I think it's fine. Or let's say, it's what the API allows you to do. You could have considered changing the Fn to FnMut so that you don't need to use RefCell (I guess it was a good learning opportunity ;)).

I'm normally too lazy to bother with building the tree manually and instead parse the source code. Now, using the source code makes asserting a bit annoying because you don't have access to the node instances. That's where you could collect NodeKinds instead (you can get the kind by calling AnyNodeRef::from(expr).kind()). However, NodeKind only works if you never have to distinguish between two nodes of the same kind. Here some tests that I wrote that use NodeKind.

ruff/crates/ruff_python_ast/src/visitor/preorder.rs

Lines 955 to 1039 in 5d41c83

	#[test]
	fn function_arguments() {
	let source = r#"def a(b, c,/, d, e = 20, *args, named=5, other=20, **kwargs): pass"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn function_positional_only_with_default() {
	let source = r#"def a(b, c = 34,/, e = 20, *args): pass"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn compare() {
	let source = r#"4 < x < 5"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn list_comprehension() {
	let source = "[x for x in numbers]";
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn dict_comprehension() {
	let source = "{x: x**2 for x in numbers}";
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn set_comprehension() {
	let source = "{x for x in numbers}";
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn match_class_pattern() {
	let source = r#"
	match x:
	case Point2D(0, 0):
	...
	case Point3D(x=0, y=0, z=0):
	...
	"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}
	#[test]
	fn decorators() {
	let source = r#"
	@decorator
	def a():
	pass
	@test
	class A:
	pass
	"#;
	let trace = trace_preorder_visitation(source);
	assert_snapshot!(trace);
	}

With an example snapshot

ruff/crates/ruff_python_ast/src/visitor/snapshots/ruff_python_ast__visitor__preorder__tests__compare.snap

Lines 1 to 15 in 5d41c83

	---
	source: crates/ruff_python_ast/src/visitor/preorder.rs
	expression: trace
	---
	- ModModule
	- StmtExpr
	- ExprCompare
	- ExprConstant
	- Int(4)
	- Lt
	- ExprName
	- Lt
	- ExprConstant
	- Int(5)

That said. I think what you have is good. The only thing I'm not a 100% sure of is if I would bother refactoring this test if I ever end up breaking it (thinking about added value / maintenance cost)