From e9a3484edfe4bc77baec8499e4b6d7d2e14feccf Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 30 Aug 2022 14:37:06 -0400 Subject: [PATCH] Implement F821 (#49) --- resources/test/src/F821.py | 14 ++++++++++++ resources/test/src/pyproject.toml | 1 + src/check_ast.rs | 24 ++++++++++++++++++++- src/checks.rs | 9 ++++++++ src/linter.rs | 36 +++++++++++++++++++++++++++++++ src/pyproject.rs | 1 + src/settings.rs | 1 + 7 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 resources/test/src/F821.py diff --git a/resources/test/src/F821.py b/resources/test/src/F821.py new file mode 100644 index 0000000000..aaef818990 --- /dev/null +++ b/resources/test/src/F821.py @@ -0,0 +1,14 @@ +def get_name(): + return self.name + + +def get_name(): + return (self.name,) + + +def get_name(): + del self.name + + +def get_name(self): + return self.name diff --git a/resources/test/src/pyproject.toml b/resources/test/src/pyproject.toml index fae64b5a36..c39231f50c 100644 --- a/resources/test/src/pyproject.toml +++ b/resources/test/src/pyproject.toml @@ -8,6 +8,7 @@ select = [ "F541", "F634", "F706", + "F821", "F831", "F832", "F901", diff --git a/src/check_ast.rs b/src/check_ast.rs index 4d990c0e92..bd7770aff7 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -299,7 +299,7 @@ impl Visitor for Checker<'_> { ExprKind::Name { ctx, .. } => match ctx { ExprContext::Load => self.handle_node_load(expr), ExprContext::Store => self.handle_node_store(expr), - ExprContext::Del => {} + ExprContext::Del => self.handle_node_delete(expr), }, ExprKind::GeneratorExp { .. } | ExprKind::ListComp { .. } @@ -439,8 +439,16 @@ impl Checker<'_> { } if let Some(binding) = scope.values.get_mut(id) { binding.used = Some(scope_id); + return; } } + + if self.settings.select.contains(&CheckCode::F821) { + self.checks.push(Check { + kind: CheckKind::UndefinedName(id.clone()), + location: expr.location, + }) + } } } @@ -480,6 +488,20 @@ impl Checker<'_> { } } + fn handle_node_delete(&mut self, expr: &Expr) { + if let ExprKind::Name { id, .. } = &expr.node { + let current = self.scopes.last_mut().expect("No current scope found."); + if current.values.remove(id).is_none() + && self.settings.select.contains(&CheckCode::F821) + { + self.checks.push(Check { + kind: CheckKind::UndefinedName(id.clone()), + location: expr.location, + }) + } + } + } + fn check_deferred(&mut self, path: &str) { for value in self.deferred.clone() { if let Ok(expr) = &parser::parse_expression(&value, path) { diff --git a/src/checks.rs b/src/checks.rs index dfa84ed63b..314d0e3b4d 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -13,6 +13,7 @@ pub enum CheckCode { F541, F634, F706, + F821, F831, F832, F901, @@ -29,6 +30,7 @@ impl FromStr for CheckCode { "F541" => Ok(CheckCode::F541), "F634" => Ok(CheckCode::F634), "F706" => Ok(CheckCode::F706), + "F821" => Ok(CheckCode::F821), "F831" => Ok(CheckCode::F831), "F832" => Ok(CheckCode::F832), "F901" => Ok(CheckCode::F901), @@ -46,6 +48,7 @@ impl CheckCode { CheckCode::F541 => "F541", CheckCode::F634 => "F634", CheckCode::F706 => "F706", + CheckCode::F821 => "F821", CheckCode::F831 => "F831", CheckCode::F832 => "F832", CheckCode::F901 => "F901", @@ -61,6 +64,7 @@ impl CheckCode { CheckCode::F541 => &LintSource::AST, CheckCode::F634 => &LintSource::AST, CheckCode::F706 => &LintSource::AST, + CheckCode::F821 => &LintSource::AST, CheckCode::F831 => &LintSource::AST, CheckCode::F832 => &LintSource::AST, CheckCode::F901 => &LintSource::AST, @@ -83,6 +87,7 @@ pub enum CheckKind { LineTooLong, RaiseNotImplemented, ReturnOutsideFunction, + UndefinedName(String), UndefinedLocal(String), UnusedImport(String), } @@ -98,6 +103,7 @@ impl CheckKind { CheckKind::LineTooLong => &CheckCode::E501, CheckKind::RaiseNotImplemented => &CheckCode::F901, CheckKind::ReturnOutsideFunction => &CheckCode::F706, + CheckKind::UndefinedName(_) => &CheckCode::F821, CheckKind::UndefinedLocal(_) => &CheckCode::F832, CheckKind::UnusedImport(_) => &CheckCode::F401, } @@ -123,6 +129,9 @@ impl CheckKind { CheckKind::ReturnOutsideFunction => { "a `return` statement outside of a function/method".to_string() } + CheckKind::UndefinedName(name) => { + format!("Undefined name `{name}`") + } CheckKind::UndefinedLocal(name) => { format!("Local variable `{name}` referenced before assignment") } diff --git a/src/linter.rs b/src/linter.rs index 15fe3de2f1..2cb1d77258 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -248,6 +248,42 @@ mod tests { Ok(()) } + #[test] + fn f821() -> Result<()> { + let actual = check_path( + &Path::new("./resources/test/src/F821.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::F821]), + }, + &cache::Mode::None, + )?; + let expected = vec![ + Message { + kind: CheckKind::UndefinedName("self".to_string()), + location: Location::new(2, 12), + filename: "./resources/test/src/F821.py".to_string(), + }, + Message { + kind: CheckKind::UndefinedName("self".to_string()), + location: Location::new(6, 13), + filename: "./resources/test/src/F821.py".to_string(), + }, + Message { + kind: CheckKind::UndefinedName("self".to_string()), + location: Location::new(10, 9), + filename: "./resources/test/src/F821.py".to_string(), + }, + ]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + #[test] fn f831() -> Result<()> { let actual = check_path( diff --git a/src/pyproject.rs b/src/pyproject.rs index f06f9ff5e1..eb9eb8b80c 100644 --- a/src/pyproject.rs +++ b/src/pyproject.rs @@ -233,6 +233,7 @@ other-attribute = 1 CheckCode::F541, CheckCode::F634, CheckCode::F706, + CheckCode::F821, CheckCode::F831, CheckCode::F832, CheckCode::F901, diff --git a/src/settings.rs b/src/settings.rs index f4488b6845..8f4a888d48 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -48,6 +48,7 @@ impl Settings { CheckCode::F541, CheckCode::F634, CheckCode::F706, + CheckCode::F821, CheckCode::F831, CheckCode::F832, CheckCode::F901,