From 739c3aa4fa6357cfa5cf46fac3ec6ebb1f309eb0 Mon Sep 17 00:00:00 2001 From: harupy Date: Mon, 12 Sep 2022 18:47:04 +0900 Subject: [PATCH] Implement E722 --- examples/generate_rules_table.rs | 1 + resources/test/fixtures/E722.py | 6 ++ src/check_ast.rs | 97 +++++++++++++++++--------------- src/checks.rs | 9 +++ src/linter.rs | 25 ++++++++ src/pyproject.rs | 1 + src/settings.rs | 1 + 7 files changed, 96 insertions(+), 44 deletions(-) create mode 100644 resources/test/fixtures/E722.py diff --git a/examples/generate_rules_table.rs b/examples/generate_rules_table.rs index 4eed9e794a..1ff212d966 100644 --- a/examples/generate_rules_table.rs +++ b/examples/generate_rules_table.rs @@ -9,6 +9,7 @@ fn main() { CheckKind::AssertTuple, CheckKind::DefaultExceptNotLast, CheckKind::DoNotAssignLambda, + CheckKind::DoNotUseBareExcept, CheckKind::DuplicateArgumentName, CheckKind::FStringMissingPlaceholders, CheckKind::FutureFeatureNotDefined("...".to_string()), diff --git a/resources/test/fixtures/E722.py b/resources/test/fixtures/E722.py new file mode 100644 index 0000000000..e90f3455f7 --- /dev/null +++ b/resources/test/fixtures/E722.py @@ -0,0 +1,6 @@ +try: + pass +except ValueError: + pass +except: + pass diff --git a/src/check_ast.rs b/src/check_ast.rs index bc2623cf8b..6f038465bd 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -813,20 +813,45 @@ where fn visit_excepthandler(&mut self, excepthandler: &'b Excepthandler) { match &excepthandler.node { - ExcepthandlerKind::ExceptHandler { name, .. } => match name { - Some(name) => { - if self.settings.select.contains(&CheckCode::E741) { - if let Some(check) = - checks::check_ambiguous_variable_name(name, excepthandler.location) - { - self.checks.push(check); + ExcepthandlerKind::ExceptHandler { type_, name, .. } => { + if self.settings.select.contains(&CheckCode::E722) && type_.is_none() { + self.checks.push(Check::new( + CheckKind::DoNotUseBareExcept, + excepthandler.location, + )); + } + match name { + Some(name) => { + if self.settings.select.contains(&CheckCode::E741) { + if let Some(check) = + checks::check_ambiguous_variable_name(name, excepthandler.location) + { + self.checks.push(check); + } } - } - let scope = - &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; - if scope.values.contains_key(name) { + let scope = &self.scopes + [*(self.scope_stack.last().expect("No current scope found."))]; + if scope.values.contains_key(name) { + let parent = self.parents + [*(self.parent_stack.last().expect("No parent found."))]; + self.handle_node_store( + &Expr::new( + excepthandler.location, + ExprKind::Name { + id: name.to_string(), + ctx: ExprContext::Store, + }, + ), + parent, + ); + self.parents.push(parent); + } + let parent = self.parents[*(self.parent_stack.last().expect("No parent found."))]; + let scope = &self.scopes + [*(self.scope_stack.last().expect("No current scope found."))]; + let definition = scope.values.get(name).cloned(); self.handle_node_store( &Expr::new( excepthandler.location, @@ -838,45 +863,29 @@ where parent, ); self.parents.push(parent); - } - let parent = - self.parents[*(self.parent_stack.last().expect("No parent found."))]; - let scope = - &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; - let definition = scope.values.get(name).cloned(); - self.handle_node_store( - &Expr::new( - excepthandler.location, - ExprKind::Name { - id: name.to_string(), - ctx: ExprContext::Store, - }, - ), - parent, - ); - self.parents.push(parent); + walk_excepthandler(self, excepthandler); - walk_excepthandler(self, excepthandler); + let scope = &mut self.scopes + [*(self.scope_stack.last().expect("No current scope found."))]; + if let Some(binding) = &scope.values.remove(name) { + if self.settings.select.contains(&CheckCode::F841) + && binding.used.is_none() + { + self.checks.push(Check::new( + CheckKind::UnusedVariable(name.to_string()), + excepthandler.location, + )); + } + } - let scope = &mut self.scopes - [*(self.scope_stack.last().expect("No current scope found."))]; - if let Some(binding) = &scope.values.remove(name) { - if self.settings.select.contains(&CheckCode::F841) && binding.used.is_none() - { - self.checks.push(Check::new( - CheckKind::UnusedVariable(name.to_string()), - excepthandler.location, - )); + if let Some(binding) = definition { + scope.values.insert(name.to_string(), binding); } } - - if let Some(binding) = definition { - scope.values.insert(name.to_string(), binding); - } + None => walk_excepthandler(self, excepthandler), } - None => walk_excepthandler(self, excepthandler), - }, + } } } diff --git a/src/checks.rs b/src/checks.rs index 564c2aecba..a792f93d8c 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -14,6 +14,7 @@ pub enum CheckCode { E712, E713, E714, + E722, E731, E741, E742, @@ -53,6 +54,7 @@ impl FromStr for CheckCode { "E711" => Ok(CheckCode::E711), "E712" => Ok(CheckCode::E712), "E713" => Ok(CheckCode::E713), + "E722" => Ok(CheckCode::E722), "E714" => Ok(CheckCode::E714), "E731" => Ok(CheckCode::E731), "E741" => Ok(CheckCode::E741), @@ -95,6 +97,7 @@ impl CheckCode { CheckCode::E712 => "E712", CheckCode::E713 => "E713", CheckCode::E714 => "E714", + CheckCode::E722 => "E722", CheckCode::E731 => "E731", CheckCode::E741 => "E741", CheckCode::E742 => "E742", @@ -134,6 +137,7 @@ impl CheckCode { CheckCode::E712 => &LintSource::AST, CheckCode::E713 => &LintSource::AST, CheckCode::E714 => &LintSource::AST, + CheckCode::E722 => &LintSource::AST, CheckCode::E731 => &LintSource::AST, CheckCode::E741 => &LintSource::AST, CheckCode::E742 => &LintSource::AST, @@ -186,6 +190,7 @@ pub enum CheckKind { AmbiguousFunctionName(String), DefaultExceptNotLast, DoNotAssignLambda, + DoNotUseBareExcept, DuplicateArgumentName, FStringMissingPlaceholders, FutureFeatureNotDefined(String), @@ -224,6 +229,7 @@ impl CheckKind { CheckKind::AmbiguousClassName(_) => "AmbiguousClassName", CheckKind::AmbiguousFunctionName(_) => "AmbiguousFunctionName", CheckKind::DefaultExceptNotLast => "DefaultExceptNotLast", + CheckKind::DoNotUseBareExcept => "DoNotUseBareExcept", CheckKind::DuplicateArgumentName => "DuplicateArgumentName", CheckKind::FStringMissingPlaceholders => "FStringMissingPlaceholders", CheckKind::FutureFeatureNotDefined(_) => "FutureFeatureNotDefined", @@ -262,6 +268,7 @@ impl CheckKind { match self { CheckKind::AssertTuple => &CheckCode::F631, CheckKind::DefaultExceptNotLast => &CheckCode::F707, + CheckKind::DoNotUseBareExcept => &CheckCode::E722, CheckKind::DuplicateArgumentName => &CheckCode::F831, CheckKind::FStringMissingPlaceholders => &CheckCode::F541, CheckKind::FutureFeatureNotDefined(_) => &CheckCode::F407, @@ -302,6 +309,7 @@ impl CheckKind { CheckKind::AssertTuple => { "Assert test is a non-empty tuple, which is always `True`".to_string() } + CheckKind::DoNotUseBareExcept => "Do not use bare `except`".to_string(), CheckKind::DefaultExceptNotLast => { "an `except:` block as not the last exception handler".to_string() } @@ -416,6 +424,7 @@ impl CheckKind { CheckKind::AssertTuple => false, CheckKind::DefaultExceptNotLast => false, CheckKind::DoNotAssignLambda => false, + CheckKind::DoNotUseBareExcept => false, CheckKind::DuplicateArgumentName => false, CheckKind::FStringMissingPlaceholders => false, CheckKind::FutureFeatureNotDefined(_) => false, diff --git a/src/linter.rs b/src/linter.rs index 15296ea24f..f580701c28 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -240,6 +240,31 @@ mod tests { Ok(()) } + #[test] + fn e722() -> Result<()> { + let mut actual = check_path( + Path::new("./resources/test/fixtures/E722.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::E722]), + }, + &fixer::Mode::Generate, + )?; + actual.sort_by_key(|check| check.location); + let expected = vec![Check { + kind: CheckKind::DoNotUseBareExcept, + location: Location::new(5, 1), + fix: None, + }]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + #[test] fn e714() -> Result<()> { let mut actual = check_path( diff --git a/src/pyproject.rs b/src/pyproject.rs index 57bed20e0f..c91313fc5f 100644 --- a/src/pyproject.rs +++ b/src/pyproject.rs @@ -265,6 +265,7 @@ other-attribute = 1 CheckCode::E712, CheckCode::E713, CheckCode::E714, + CheckCode::E722, CheckCode::E731, CheckCode::E741, CheckCode::E742, diff --git a/src/settings.rs b/src/settings.rs index d779618572..405cb7eb4b 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -50,6 +50,7 @@ impl Settings { CheckCode::E712, CheckCode::E713, CheckCode::E714, + CheckCode::E722, CheckCode::E731, CheckCode::E741, CheckCode::E742,