diff --git a/README.md b/README.md index 91ea5cbebf..1969b0b719 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,7 @@ OPTIONS: | F704 | YieldOutsideFunction | a `yield` or `yield from` statement outside of a function/method | | F706 | ReturnOutsideFunction | a `return` statement outside of a function/method | | F821 | UndefinedName | Undefined name `...` | +| F822 | UndefinedExport | Undefined name `...` in __all__ | | F823 | UndefinedLocal | Local variable `...` referenced before assignment | | F831 | DuplicateArgumentName | Duplicate argument name in function definition | | F841 | UnusedVariable | Local variable `...` is assigned to but never used | diff --git a/examples/generate_rules_table.rs b/examples/generate_rules_table.rs index 5f920d2217..65aed6504a 100644 --- a/examples/generate_rules_table.rs +++ b/examples/generate_rules_table.rs @@ -12,6 +12,7 @@ fn main() { CheckKind::ReturnOutsideFunction, CheckKind::UndefinedLocal("...".to_string()), CheckKind::UndefinedName("...".to_string()), + CheckKind::UndefinedExport("...".to_string()), CheckKind::UnusedImport("...".to_string()), CheckKind::UnusedVariable("...".to_string()), CheckKind::UselessObjectInheritance("...".to_string()), diff --git a/resources/test/fixtures/F822.py b/resources/test/fixtures/F822.py new file mode 100644 index 0000000000..f1d40ed728 --- /dev/null +++ b/resources/test/fixtures/F822.py @@ -0,0 +1,3 @@ +a = 1 + +__all__ = ["a", "b"] diff --git a/resources/test/fixtures/pyproject.toml b/resources/test/fixtures/pyproject.toml index b036ed57b7..80c93adb39 100644 --- a/resources/test/fixtures/pyproject.toml +++ b/resources/test/fixtures/pyproject.toml @@ -10,6 +10,7 @@ select = [ "F704", "F706", "F821", + "F822", "F823", "F831", "F841", diff --git a/src/check_ast.rs b/src/check_ast.rs index ca7a6e258f..ec8efab33c 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1,4 +1,5 @@ use std::collections::BTreeSet; +use std::path::Path; use rustpython_parser::ast::{ Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Stmt, @@ -15,6 +16,7 @@ use crate::visitor::{walk_excepthandler, Visitor}; struct Checker<'a> { settings: &'a Settings, + path: &'a str, checks: Vec, scopes: Vec, dead_scopes: Vec, @@ -24,9 +26,10 @@ struct Checker<'a> { } impl Checker<'_> { - pub fn new(settings: &Settings) -> Checker { + pub fn new<'a>(settings: &'a Settings, path: &'a str) -> Checker<'a> { Checker { settings, + path, checks: vec![], scopes: vec![], dead_scopes: vec![], @@ -673,19 +676,40 @@ impl Checker<'_> { } fn check_dead_scopes(&mut self) { - if self.settings.select.contains(&CheckCode::F401) { - for scope in &self.dead_scopes { - let all_binding = match scope.values.get("__all__") { - Some(binding) => match &binding.kind { - BindingKind::Export(names) => Some(names), - _ => None, - }, - _ => None, - }; + if !self.settings.select.contains(&CheckCode::F822) + && !self.settings.select.contains(&CheckCode::F401) + { + return; + } + for scope in &self.dead_scopes { + let all_binding = scope.values.get("__all__"); + let all_names = all_binding.and_then(|binding| match &binding.kind { + BindingKind::Export(names) => Some(names), + _ => None, + }); + + if self.settings.select.contains(&CheckCode::F822) + && !Path::new(self.path).ends_with("__init__.py") + { + if let Some(binding) = all_binding { + if let Some(names) = all_names { + for name in names { + if !scope.values.contains_key(name) { + self.checks.push(Check { + kind: CheckKind::UndefinedExport(name.to_string()), + location: binding.location, + }); + } + } + } + } + } + + if self.settings.select.contains(&CheckCode::F401) { for (name, binding) in scope.values.iter().rev() { let used = binding.used.is_some() - || all_binding + || all_names .map(|names| names.contains(name)) .unwrap_or_default(); @@ -708,7 +732,7 @@ impl Checker<'_> { } pub fn check_ast(python_ast: &Suite, settings: &Settings, path: &str) -> Vec { - let mut checker = Checker::new(settings); + let mut checker = Checker::new(settings, path); checker.push_scope(Scope::new(ScopeKind::Module)); checker.bind_builtins(); diff --git a/src/checks.rs b/src/checks.rs index 0c35052e38..7b4443a722 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -16,6 +16,7 @@ pub enum CheckCode { F704, F706, F821, + F822, F823, F831, F841, @@ -36,6 +37,7 @@ impl FromStr for CheckCode { "F704" => Ok(CheckCode::F704), "F706" => Ok(CheckCode::F706), "F821" => Ok(CheckCode::F821), + "F822" => Ok(CheckCode::F822), "F823" => Ok(CheckCode::F823), "F831" => Ok(CheckCode::F831), "F841" => Ok(CheckCode::F841), @@ -58,6 +60,7 @@ impl CheckCode { CheckCode::F706 => "F706", CheckCode::F821 => "F821", CheckCode::F823 => "F823", + CheckCode::F822 => "F822", CheckCode::F831 => "F831", CheckCode::F841 => "F841", CheckCode::F901 => "F901", @@ -76,6 +79,7 @@ impl CheckCode { CheckCode::F704 => &LintSource::AST, CheckCode::F706 => &LintSource::AST, CheckCode::F821 => &LintSource::AST, + CheckCode::F822 => &LintSource::AST, CheckCode::F823 => &LintSource::AST, CheckCode::F831 => &LintSource::AST, CheckCode::F841 => &LintSource::AST, @@ -101,6 +105,7 @@ pub enum CheckKind { RaiseNotImplemented, ReturnOutsideFunction, UndefinedLocal(String), + UndefinedExport(String), UndefinedName(String), UnusedImport(String), UnusedVariable(String), @@ -120,6 +125,7 @@ impl CheckKind { CheckKind::RaiseNotImplemented => "RaiseNotImplemented", CheckKind::ReturnOutsideFunction => "ReturnOutsideFunction", CheckKind::UndefinedLocal(_) => "UndefinedLocal", + CheckKind::UndefinedExport(_) => "UndefinedExport", CheckKind::UndefinedName(_) => "UndefinedName", CheckKind::UnusedImport(_) => "UnusedImport", CheckKind::UnusedVariable(_) => "UnusedVariable", @@ -138,6 +144,7 @@ impl CheckKind { CheckKind::LineTooLong => &CheckCode::E501, CheckKind::RaiseNotImplemented => &CheckCode::F901, CheckKind::ReturnOutsideFunction => &CheckCode::F706, + CheckKind::UndefinedExport(_) => &CheckCode::F822, CheckKind::UndefinedLocal(_) => &CheckCode::F823, CheckKind::UndefinedName(_) => &CheckCode::F821, CheckKind::UnusedImport(_) => &CheckCode::F401, @@ -165,6 +172,9 @@ impl CheckKind { CheckKind::ReturnOutsideFunction => { "a `return` statement outside of a function/method".to_string() } + CheckKind::UndefinedExport(name) => { + format!("Undefined name `{name}` in __all__") + } CheckKind::UndefinedName(name) => { format!("Undefined name `{name}`") } diff --git a/src/linter.rs b/src/linter.rs index 63be1d0fea..59c5571c4f 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -330,6 +330,54 @@ mod tests { Ok(()) } + #[test] + fn f822() -> Result<()> { + let actual = check_path( + Path::new("./resources/test/fixtures/F822.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::F822]), + }, + &cache::Mode::None, + )?; + let expected = vec![Message { + kind: CheckKind::UndefinedExport("b".to_string()), + location: Location::new(3, 1), + filename: "./resources/test/fixtures/F822.py".to_string(), + }]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + + #[test] + fn f823() -> Result<()> { + let actual = check_path( + Path::new("./resources/test/fixtures/F823.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::F823]), + }, + &cache::Mode::None, + )?; + let expected = vec![Message { + kind: CheckKind::UndefinedLocal("my_var".to_string()), + location: Location::new(6, 5), + filename: "./resources/test/fixtures/F823.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( @@ -366,30 +414,6 @@ mod tests { Ok(()) } - #[test] - fn f823() -> Result<()> { - let actual = check_path( - Path::new("./resources/test/fixtures/F823.py"), - &settings::Settings { - line_length: 88, - exclude: vec![], - select: BTreeSet::from([CheckCode::F823]), - }, - &cache::Mode::None, - )?; - let expected = vec![Message { - kind: CheckKind::UndefinedLocal("my_var".to_string()), - location: Location::new(6, 5), - filename: "./resources/test/fixtures/F823.py".to_string(), - }]; - assert_eq!(actual.len(), expected.len()); - for i in 0..actual.len() { - assert_eq!(actual[i], expected[i]); - } - - Ok(()) - } - #[test] fn f841() -> Result<()> { let actual = check_path( diff --git a/src/pyproject.rs b/src/pyproject.rs index f34ee5d300..c18fc9be92 100644 --- a/src/pyproject.rs +++ b/src/pyproject.rs @@ -245,6 +245,7 @@ other-attribute = 1 CheckCode::F704, CheckCode::F706, CheckCode::F821, + CheckCode::F822, CheckCode::F823, CheckCode::F831, CheckCode::F841,