From a780faccd5ef99a3a5760f30ab6d9d78cc113def Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 11 Sep 2022 22:40:41 +0900 Subject: [PATCH] Use handle_node_store --- src/ast/checks.rs | 56 +---------------------------------------------- src/check_ast.rs | 55 ++++++++++++++-------------------------------- 2 files changed, 18 insertions(+), 93 deletions(-) diff --git a/src/ast/checks.rs b/src/ast/checks.rs index c4817558f0..d57622c5bd 100644 --- a/src/ast/checks.rs +++ b/src/ast/checks.rs @@ -97,7 +97,7 @@ pub fn is_ambiguous_name(name: &str) -> bool { name == "l" || name == "I" || name == "O" } -pub fn check_ambiguous_variable_name_str(name: &str, location: Location) -> Option { +pub fn check_ambiguous_variable_name(name: &str, location: Location) -> Option { if is_ambiguous_name(name) { Some(Check::new( CheckKind::AmbiguousVariableName(name.to_string()), @@ -108,60 +108,6 @@ pub fn check_ambiguous_variable_name_str(name: &str, location: Location) -> Opti } } -pub fn check_ambiguous_variable_name_arguments(arguments: &Arguments) -> Vec { - // Collect all the arguments into a single vector. - let mut all_arguments: Vec<&Arg> = arguments - .args - .iter() - .chain(arguments.posonlyargs.iter()) - .chain(arguments.kwonlyargs.iter()) - .collect(); - if let Some(arg) = &arguments.vararg { - all_arguments.push(arg); - } - if let Some(arg) = &arguments.kwarg { - all_arguments.push(arg); - } - all_arguments - .iter() - .filter_map(|arg| check_ambiguous_variable_name_str(&arg.node.arg, arg.location)) - .collect() -} - -fn check_target(target: &Expr) -> Option { - match &target.node { - ExprKind::Name { id, .. } => check_ambiguous_variable_name_str(id, target.location), - ExprKind::Starred { value, .. } => { - if let ExprKind::Name { id, .. } = &value.node { - check_ambiguous_variable_name_str(id, value.location) - } else { - None - } - } - _ => None, - } -} - -/// Check AmbiguousVariableName compliance. -pub fn check_ambiguous_variable_name(target: &Expr) -> Vec { - let mut checks: Vec = vec![]; - match &target.node { - ExprKind::Tuple { elts, .. } | ExprKind::List { elts, .. } => { - for elt in elts { - if let Some(check) = check_target(elt) { - checks.push(check); - }; - } - } - _ => { - if let Some(check) = check_target(target) { - checks.push(check); - }; - } - } - checks -} - /// Check UselessObjectInheritance compliance. pub fn check_useless_object_inheritance( stmt: &Stmt, diff --git a/src/check_ast.rs b/src/check_ast.rs index a8afd64d66..7e821b2d36 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -124,7 +124,7 @@ where if self.settings.select.contains(&CheckCode::E741) { self.checks.extend(names.iter().filter_map(|name| { - checks::check_ambiguous_variable_name_str(name, stmt.location) + checks::check_ambiguous_variable_name(name, stmt.location) })); } } @@ -401,22 +401,15 @@ where self.seen_non_import = true; } } - StmtKind::Assign { value, targets, .. } => { + StmtKind::Assign { value, .. } => { self.seen_non_import = true; if self.settings.select.contains(&CheckCode::E731) { if let Some(check) = checks::check_do_not_assign_lambda(value, stmt.location) { self.checks.push(check); } } - if self.settings.select.contains(&CheckCode::E741) { - self.checks.extend( - targets - .iter() - .flat_map(checks::check_ambiguous_variable_name), - ); - } } - StmtKind::AnnAssign { value, target, .. } => { + StmtKind::AnnAssign { value, .. } => { self.seen_non_import = true; if self.settings.select.contains(&CheckCode::E731) { if let Some(value) = value { @@ -427,30 +420,10 @@ where } } } - if self.settings.select.contains(&CheckCode::E741) { - self.checks - .extend(checks::check_ambiguous_variable_name(target)); - } } StmtKind::Delete { .. } => { self.seen_non_import = true; } - StmtKind::For { target, .. } => { - if self.settings.select.contains(&CheckCode::E741) { - self.checks - .extend(checks::check_ambiguous_variable_name(target)); - } - } - StmtKind::With { items, .. } | StmtKind::AsyncWith { items, .. } => { - if self.settings.select.contains(&CheckCode::E741) { - for item in items { - if let Some(vars) = &item.optional_vars { - self.checks - .extend(checks::check_ambiguous_variable_name(vars)) - } - } - } - } _ => {} } @@ -810,14 +783,6 @@ where } } - if self.settings.select.contains(&CheckCode::E741) { - if let Some(check) = - checks::check_ambiguous_variable_name_str(name, excepthandler.location) - { - self.checks.push(check); - } - } - if let Some(binding) = definition { scope.values.insert(name.to_string(), binding); } @@ -861,6 +826,13 @@ where location: arg.location, }, ); + + if self.settings.select.contains(&CheckCode::E741) { + if let Some(check) = checks::check_ambiguous_variable_name(&arg.node.arg, arg.location) + { + self.checks.push(check); + } + } } } @@ -993,6 +965,13 @@ impl<'a> Checker<'a> { } } + if self.settings.select.contains(&CheckCode::E741) && checks::is_ambiguous_name(id) { + self.checks.push(Check::new( + CheckKind::AmbiguousVariableName(id.to_string()), + expr.location, + )); + } + // TODO(charlie): Handle alternate binding types (like `Annotation`). if id == "__all__" && matches!(current.kind, ScopeKind::Module)