From 65a7a5a72b43fa1b781701aecfe41f2327a6b854 Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 18 Jul 2023 11:20:48 -0500 Subject: [PATCH 1/5] Implement `any_over_expr` for type alias and type params --- crates/ruff_python_ast/src/helpers.rs | 99 +++++++++++++++++++++++++-- 1 file changed, 95 insertions(+), 4 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 0dfad2d123ea9..d156d625c93aa 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -8,6 +8,7 @@ use rustc_hash::FxHashMap; use rustpython_ast::CmpOp; use rustpython_parser::ast::{ self, Arguments, Constant, ExceptHandler, Expr, Keyword, MatchCase, Pattern, Ranged, Stmt, + TypeParam, }; use rustpython_parser::{lexer, Mode, Tok}; use smallvec::SmallVec; @@ -265,6 +266,19 @@ where } } +pub fn any_over_type_param(type_param: &TypeParam, func: &F) -> bool +where + F: Fn(&Expr) -> bool, +{ + match type_param { + TypeParam::TypeVar(ast::TypeParamTypeVar { bound, .. }) => bound + .as_ref() + .map_or(false, |value| any_over_expr(value, func)), + TypeParam::TypeVarTuple(ast::TypeParamTypeVarTuple { .. }) => false, + TypeParam::ParamSpec(ast::TypeParamParamSpec { .. }) => false, + } +} + pub fn any_over_pattern(pattern: &Pattern, func: &F) -> bool where F: Fn(&Expr) -> bool, @@ -391,6 +405,18 @@ where targets, range: _range, }) => targets.iter().any(|expr| any_over_expr(expr, func)), + Stmt::TypeAlias(ast::StmtTypeAlias { + name, + type_params, + value, + .. + }) => { + any_over_expr(name, func) + || type_params + .iter() + .any(|type_param| any_over_type_param(type_param, func)) + || any_over_expr(value, func) + } Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { targets.iter().any(|expr| any_over_expr(expr, func)) || any_over_expr(value, func) } @@ -539,7 +565,6 @@ where range: _range, }) => any_over_expr(value, func), Stmt::Pass(_) | Stmt::Break(_) | Stmt::Continue(_) => false, - Stmt::TypeAlias(_) => todo!(), } } @@ -1566,13 +1591,16 @@ mod tests { use anyhow::Result; use ruff_text_size::{TextLen, TextRange, TextSize}; - use rustpython_ast::{CmpOp, Expr, Ranged}; + use rustpython_ast::{ + self, CmpOp, Constant, Expr, ExprConstant, Identifier, Ranged, TypeParam, + TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, + }; use rustpython_parser::ast::Suite; use rustpython_parser::Parse; use crate::helpers::{ - first_colon_range, has_trailing_content, locate_cmp_ops, resolve_imported_module_path, - LocatedCmpOp, + any_over_type_param, first_colon_range, has_trailing_content, + locate_cmp_ops, resolve_imported_module_path, LocatedCmpOp, }; use crate::source_code::Locator; @@ -1746,4 +1774,67 @@ y = 2 Ok(()) } + + + #[test] + fn any_over_type_param_type_var() { + let type_var_no_bound = TypeParam::TypeVar(TypeParamTypeVar { + range: TextRange::new(TextSize::from(0), TextSize::from(0)), + bound: None, + name: Identifier::new("x", TextRange::new(TextSize::from(0), TextSize::from(0))), + }); + assert_eq!( + any_over_type_param(&type_var_no_bound, &|_expr| true), + false + ); + + let bound = Expr::Constant(ExprConstant { + value: Constant::Int(1.into()), + kind: Some("x".to_string()), + range: TextRange::new(TextSize::from(0), TextSize::from(0)), + }); + + let type_var_with_bound = TypeParam::TypeVar(TypeParamTypeVar { + range: TextRange::new(TextSize::from(0), TextSize::from(0)), + bound: Some(Box::new(bound.clone())), + name: Identifier::new("x", TextRange::new(TextSize::from(0), TextSize::from(0))), + }); + assert_eq!( + any_over_type_param(&type_var_with_bound, &|expr| { + assert_eq!( + *expr, bound, + "the received expression should be the unwrapped bound" + ); + true + }), + true, + "if true is returned from `func` it should be respected" + ); + } + + #[test] + fn any_over_type_param_type_var_tuple() { + let type_var_tuple = TypeParam::TypeVarTuple(TypeParamTypeVarTuple { + range: TextRange::new(TextSize::from(0), TextSize::from(0)), + name: Identifier::new("x", TextRange::new(TextSize::from(0), TextSize::from(0))), + }); + assert_eq!( + any_over_type_param(&type_var_tuple, &|_expr| true), + false, + "type var tuples have no expressions to visit" + ); + } + + #[test] + fn any_over_type_param_param_spec() { + let type_param_spec = TypeParam::ParamSpec(TypeParamParamSpec { + range: TextRange::new(TextSize::from(0), TextSize::from(0)), + name: Identifier::new("x", TextRange::new(TextSize::from(0), TextSize::from(0))), + }); + assert_eq!( + any_over_type_param(&type_param_spec, &|_expr| true), + false, + "param specs have no expressions to visit" + ); + } } From a556b65a7c300f1a7063ab26981de1ee257e9b9e Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 18 Jul 2023 11:59:54 -0500 Subject: [PATCH 2/5] Add test type alias expression visits --- crates/ruff_python_ast/src/helpers.rs | 62 +++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index d156d625c93aa..d48d4685649a4 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1589,17 +1589,21 @@ pub fn locate_cmp_ops(expr: &Expr, locator: &Locator) -> Vec { mod tests { use std::borrow::Cow; + use std::cell::RefCell; + + use std::vec; + use anyhow::Result; use ruff_text_size::{TextLen, TextRange, TextSize}; use rustpython_ast::{ - self, CmpOp, Constant, Expr, ExprConstant, Identifier, Ranged, TypeParam, - TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, + self, CmpOp, Constant, Expr, ExprConstant, ExprContext, ExprName, Identifier, Ranged, Stmt, + StmtTypeAlias, TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, }; use rustpython_parser::ast::Suite; use rustpython_parser::Parse; use crate::helpers::{ - any_over_type_param, first_colon_range, has_trailing_content, + any_over_stmt, any_over_type_param, first_colon_range, has_trailing_content, locate_cmp_ops, resolve_imported_module_path, LocatedCmpOp, }; use crate::source_code::Locator; @@ -1775,6 +1779,58 @@ y = 2 Ok(()) } + #[test] + fn any_over_stmt_type_alias() { + let range = TextRange::new(TextSize::from(0), TextSize::from(0)); + let seen = RefCell::new(Vec::new()); + let name = Expr::Name(ExprName { + id: "x".to_string(), + range, + ctx: ExprContext::Load, + }); + let constant_one = Expr::Constant(ExprConstant { + value: Constant::Int(1.into()), + kind: Some("x".to_string()), + range: range.clone(), + }); + let constant_two = Expr::Constant(ExprConstant { + value: Constant::Int(2.into()), + kind: Some("y".to_string()), + range: range.clone(), + }); + let constant_three = Expr::Constant(ExprConstant { + value: Constant::Int(3.into()), + kind: Some("z".to_string()), + range: range.clone(), + }); + let type_var_one = TypeParam::TypeVar(TypeParamTypeVar { + range: range.clone(), + bound: Some(Box::new(constant_one.clone())), + name: Identifier::new("x", range.clone()), + }); + let type_var_two = TypeParam::TypeVar(TypeParamTypeVar { + range: range.clone(), + bound: Some(Box::new(constant_two.clone())), + name: Identifier::new("x", range.clone()), + }); + let type_alias = Stmt::TypeAlias(StmtTypeAlias { + name: Box::new(name.clone()), + type_params: vec![type_var_one, type_var_two], + value: Box::new(constant_three.clone()), + range: range.clone(), + }); + assert_eq!( + any_over_stmt(&type_alias, &|expr| { + seen.borrow_mut().push(expr.clone()); + false + }), + false + ); + assert_eq!( + seen.take(), + vec![name, constant_one, constant_two, constant_three] + ) + } #[test] fn any_over_type_param_type_var() { From dc33c8aa4f135a8f896e3ef7ce0724844646b9f7 Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 18 Jul 2023 12:00:46 -0500 Subject: [PATCH 3/5] Use `TextRange::default` in tests --- crates/ruff_python_ast/src/helpers.rs | 37 +++++++++++++-------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index d48d4685649a4..b3f1095ed8c13 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1781,43 +1781,42 @@ y = 2 #[test] fn any_over_stmt_type_alias() { - let range = TextRange::new(TextSize::from(0), TextSize::from(0)); let seen = RefCell::new(Vec::new()); let name = Expr::Name(ExprName { id: "x".to_string(), - range, + range: TextRange::default(), ctx: ExprContext::Load, }); let constant_one = Expr::Constant(ExprConstant { value: Constant::Int(1.into()), kind: Some("x".to_string()), - range: range.clone(), + range: TextRange::default(), }); let constant_two = Expr::Constant(ExprConstant { value: Constant::Int(2.into()), kind: Some("y".to_string()), - range: range.clone(), + range: TextRange::default(), }); let constant_three = Expr::Constant(ExprConstant { value: Constant::Int(3.into()), kind: Some("z".to_string()), - range: range.clone(), + range: TextRange::default(), }); let type_var_one = TypeParam::TypeVar(TypeParamTypeVar { - range: range.clone(), + range: TextRange::default(), bound: Some(Box::new(constant_one.clone())), - name: Identifier::new("x", range.clone()), + name: Identifier::new("x", TextRange::default()), }); let type_var_two = TypeParam::TypeVar(TypeParamTypeVar { - range: range.clone(), + range: TextRange::default(), bound: Some(Box::new(constant_two.clone())), - name: Identifier::new("x", range.clone()), + name: Identifier::new("x", TextRange::default()), }); let type_alias = Stmt::TypeAlias(StmtTypeAlias { name: Box::new(name.clone()), type_params: vec![type_var_one, type_var_two], value: Box::new(constant_three.clone()), - range: range.clone(), + range: TextRange::default(), }); assert_eq!( any_over_stmt(&type_alias, &|expr| { @@ -1835,9 +1834,9 @@ y = 2 #[test] fn any_over_type_param_type_var() { let type_var_no_bound = TypeParam::TypeVar(TypeParamTypeVar { - range: TextRange::new(TextSize::from(0), TextSize::from(0)), + range: TextRange::default(), bound: None, - name: Identifier::new("x", TextRange::new(TextSize::from(0), TextSize::from(0))), + name: Identifier::new("x", TextRange::default()), }); assert_eq!( any_over_type_param(&type_var_no_bound, &|_expr| true), @@ -1847,13 +1846,13 @@ y = 2 let bound = Expr::Constant(ExprConstant { value: Constant::Int(1.into()), kind: Some("x".to_string()), - range: TextRange::new(TextSize::from(0), TextSize::from(0)), + range: TextRange::default(), }); let type_var_with_bound = TypeParam::TypeVar(TypeParamTypeVar { - range: TextRange::new(TextSize::from(0), TextSize::from(0)), + range: TextRange::default(), bound: Some(Box::new(bound.clone())), - name: Identifier::new("x", TextRange::new(TextSize::from(0), TextSize::from(0))), + name: Identifier::new("x", TextRange::default()), }); assert_eq!( any_over_type_param(&type_var_with_bound, &|expr| { @@ -1871,8 +1870,8 @@ y = 2 #[test] fn any_over_type_param_type_var_tuple() { let type_var_tuple = TypeParam::TypeVarTuple(TypeParamTypeVarTuple { - range: TextRange::new(TextSize::from(0), TextSize::from(0)), - name: Identifier::new("x", TextRange::new(TextSize::from(0), TextSize::from(0))), + range: TextRange::default(), + name: Identifier::new("x", TextRange::default()), }); assert_eq!( any_over_type_param(&type_var_tuple, &|_expr| true), @@ -1884,8 +1883,8 @@ y = 2 #[test] fn any_over_type_param_param_spec() { let type_param_spec = TypeParam::ParamSpec(TypeParamParamSpec { - range: TextRange::new(TextSize::from(0), TextSize::from(0)), - name: Identifier::new("x", TextRange::new(TextSize::from(0), TextSize::from(0))), + range: TextRange::default(), + name: Identifier::new("x", TextRange::default()), }); assert_eq!( any_over_type_param(&type_param_spec, &|_expr| true), From d0caccab9da7e762dc7296e5a487f3a117c27dcc Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 18 Jul 2023 12:09:02 -0500 Subject: [PATCH 4/5] Clippy --- crates/ruff_python_ast/src/helpers.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index b3f1095ed8c13..53b23014f53eb 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1818,17 +1818,16 @@ y = 2 value: Box::new(constant_three.clone()), range: TextRange::default(), }); - assert_eq!( - any_over_stmt(&type_alias, &|expr| { + assert!( + !any_over_stmt(&type_alias, &|expr| { seen.borrow_mut().push(expr.clone()); false - }), - false + }) ); assert_eq!( seen.take(), vec![name, constant_one, constant_two, constant_three] - ) + ); } #[test] @@ -1838,9 +1837,8 @@ y = 2 bound: None, name: Identifier::new("x", TextRange::default()), }); - assert_eq!( - any_over_type_param(&type_var_no_bound, &|_expr| true), - false + assert!( + !any_over_type_param(&type_var_no_bound, &|_expr| true) ); let bound = Expr::Constant(ExprConstant { @@ -1854,7 +1852,7 @@ y = 2 bound: Some(Box::new(bound.clone())), name: Identifier::new("x", TextRange::default()), }); - assert_eq!( + assert!( any_over_type_param(&type_var_with_bound, &|expr| { assert_eq!( *expr, bound, @@ -1862,7 +1860,6 @@ y = 2 ); true }), - true, "if true is returned from `func` it should be respected" ); } @@ -1873,9 +1870,8 @@ y = 2 range: TextRange::default(), name: Identifier::new("x", TextRange::default()), }); - assert_eq!( - any_over_type_param(&type_var_tuple, &|_expr| true), - false, + assert!( + !any_over_type_param(&type_var_tuple, &|_expr| true), "type var tuples have no expressions to visit" ); } @@ -1886,9 +1882,8 @@ y = 2 range: TextRange::default(), name: Identifier::new("x", TextRange::default()), }); - assert_eq!( - any_over_type_param(&type_param_spec, &|_expr| true), - false, + assert!( + !any_over_type_param(&type_param_spec, &|_expr| true), "param specs have no expressions to visit" ); } From 61d74b6367cdbe37ff81e800dbc3874c907e0221 Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 18 Jul 2023 12:41:08 -0500 Subject: [PATCH 5/5] Reformat --- crates/ruff_python_ast/src/helpers.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 53b23014f53eb..e9da97b63d322 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1818,12 +1818,10 @@ y = 2 value: Box::new(constant_three.clone()), range: TextRange::default(), }); - assert!( - !any_over_stmt(&type_alias, &|expr| { - seen.borrow_mut().push(expr.clone()); - false - }) - ); + assert!(!any_over_stmt(&type_alias, &|expr| { + seen.borrow_mut().push(expr.clone()); + false + })); assert_eq!( seen.take(), vec![name, constant_one, constant_two, constant_three] @@ -1837,9 +1835,7 @@ y = 2 bound: None, name: Identifier::new("x", TextRange::default()), }); - assert!( - !any_over_type_param(&type_var_no_bound, &|_expr| true) - ); + assert!(!any_over_type_param(&type_var_no_bound, &|_expr| true)); let bound = Expr::Constant(ExprConstant { value: Constant::Int(1.into()),