From 6f8e0224d03fbe7a25735ce15c6eb909c76bc042 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 10 Dec 2022 11:33:24 -0500 Subject: [PATCH] Implement W0602 (global-variable-not-assigned) (#1179) --- .../pylint/global_variable_not_assigned.py | 32 +++++++++++++++++++ src/check_ast.rs | 14 ++++++++ src/checks.rs | 10 +++++- src/checks_gen.rs | 13 ++++++-- src/pylint/mod.rs | 1 + ...W0602_global_variable_not_assigned.py.snap | 23 +++++++++++++ 6 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 resources/test/fixtures/pylint/global_variable_not_assigned.py create mode 100644 src/pylint/snapshots/ruff__pylint__tests__PLW0602_global_variable_not_assigned.py.snap diff --git a/resources/test/fixtures/pylint/global_variable_not_assigned.py b/resources/test/fixtures/pylint/global_variable_not_assigned.py new file mode 100644 index 0000000000..28e0e11c31 --- /dev/null +++ b/resources/test/fixtures/pylint/global_variable_not_assigned.py @@ -0,0 +1,32 @@ +### +# Errors. +### +def f(): + global x + + +def f(): + global x + + print(x) + + +### +# Non-errors. +### +def f(): + global x + + x = 1 + + +def f(): + global x + + (x, y) = (1, 2) + + +def f(): + global x + + del x diff --git a/src/check_ast.rs b/src/check_ast.rs index 8251638fc7..6e349e15e8 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -3174,6 +3174,7 @@ impl<'a> Checker<'a> { && !self.settings.enabled.contains(&CheckCode::F405) && !self.settings.enabled.contains(&CheckCode::F811) && !self.settings.enabled.contains(&CheckCode::F822) + && !self.settings.enabled.contains(&CheckCode::PLW0602) { return; } @@ -3185,6 +3186,19 @@ impl<'a> Checker<'a> { .rev() .map(|index| &self.scopes[*index]) { + // PLW0602 + if self.settings.enabled.contains(&CheckCode::PLW0602) { + for (name, index) in &scope.values { + let binding = &self.bindings[*index]; + if matches!(binding.kind, BindingKind::Global) { + checks.push(Check::new( + CheckKind::GlobalVariableNotAssigned((*name).to_string()), + binding.range, + )); + } + } + } + // Imports in classes are public members. if matches!(scope.kind, ScopeKind::Class(..)) { continue; diff --git a/src/checks.rs b/src/checks.rs index 932d28fa5a..51d7880282 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -106,6 +106,7 @@ pub enum CheckCode { PLR1701, PLR1722, PLW0120, + PLW0602, // flake8-builtins A001, A002, @@ -639,6 +640,7 @@ pub enum CheckKind { AwaitOutsideAsync, ConsiderMergingIsinstance(String, Vec), ConsiderUsingFromImport(String, String), + GlobalVariableNotAssigned(String), MisplacedComparisonConstant(String), PropertyWithParameters, UnnecessaryDirectLambdaCall, @@ -963,6 +965,7 @@ impl CheckCode { } CheckCode::PLR1722 => CheckKind::UseSysExit("exit".to_string()), CheckCode::PLW0120 => CheckKind::UselessElseOnLoop, + CheckCode::PLW0602 => CheckKind::GlobalVariableNotAssigned("...".to_string()), // flake8-builtins CheckCode::A001 => CheckKind::BuiltinVariableShadowing("...".to_string()), CheckCode::A002 => CheckKind::BuiltinArgumentShadowing("...".to_string()), @@ -1412,6 +1415,7 @@ impl CheckCode { CheckCode::PLR1701 => CheckCategory::Pylint, CheckCode::PLR1722 => CheckCategory::Pylint, CheckCode::PLW0120 => CheckCategory::Pylint, + CheckCode::PLW0602 => CheckCategory::Pylint, CheckCode::Q000 => CheckCategory::Flake8Quotes, CheckCode::Q001 => CheckCategory::Flake8Quotes, CheckCode::Q002 => CheckCategory::Flake8Quotes, @@ -1537,11 +1541,12 @@ impl CheckKind { CheckKind::AwaitOutsideAsync => &CheckCode::PLE1142, CheckKind::ConsiderMergingIsinstance(..) => &CheckCode::PLR1701, CheckKind::ConsiderUsingFromImport(..) => &CheckCode::PLR0402, + CheckKind::GlobalVariableNotAssigned(..) => &CheckCode::PLW0602, CheckKind::MisplacedComparisonConstant(..) => &CheckCode::PLC2201, - CheckKind::UsedPriorGlobalDeclaration(..) => &CheckCode::PLE0118, CheckKind::PropertyWithParameters => &CheckCode::PLR0206, CheckKind::UnnecessaryDirectLambdaCall => &CheckCode::PLC3002, CheckKind::UseSysExit(_) => &CheckCode::PLR1722, + CheckKind::UsedPriorGlobalDeclaration(..) => &CheckCode::PLE0118, CheckKind::UselessElseOnLoop => &CheckCode::PLW0120, CheckKind::UselessImportAlias => &CheckCode::PLC0414, // flake8-builtins @@ -1964,6 +1969,9 @@ impl CheckKind { CheckKind::UsedPriorGlobalDeclaration(name, line) => { format!("Name `{name}` is used prior to global declaration on line {line}") } + CheckKind::GlobalVariableNotAssigned(name) => { + format!("Using global for `{name}` but no assignment is done") + } CheckKind::AwaitOutsideAsync => { "`await` should be used within an async function".to_string() } diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 6370d7cd63..39bf4f7c51 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -342,6 +342,9 @@ pub enum CheckCodePrefix { PLW01, PLW012, PLW0120, + PLW06, + PLW060, + PLW0602, Q, Q0, Q00, @@ -1375,11 +1378,14 @@ impl CheckCodePrefix { CheckCodePrefix::PLR1701 => vec![CheckCode::PLR1701], CheckCodePrefix::PLR172 => vec![CheckCode::PLR1722], CheckCodePrefix::PLR1722 => vec![CheckCode::PLR1722], - CheckCodePrefix::PLW => vec![CheckCode::PLW0120], - CheckCodePrefix::PLW0 => vec![CheckCode::PLW0120], + CheckCodePrefix::PLW => vec![CheckCode::PLW0120, CheckCode::PLW0602], + CheckCodePrefix::PLW0 => vec![CheckCode::PLW0120, CheckCode::PLW0602], CheckCodePrefix::PLW01 => vec![CheckCode::PLW0120], CheckCodePrefix::PLW012 => vec![CheckCode::PLW0120], CheckCodePrefix::PLW0120 => vec![CheckCode::PLW0120], + CheckCodePrefix::PLW06 => vec![CheckCode::PLW0602], + CheckCodePrefix::PLW060 => vec![CheckCode::PLW0602], + CheckCodePrefix::PLW0602 => vec![CheckCode::PLW0602], CheckCodePrefix::Q => vec![ CheckCode::Q000, CheckCode::Q001, @@ -2153,6 +2159,9 @@ impl CheckCodePrefix { CheckCodePrefix::PLW01 => SuffixLength::Two, CheckCodePrefix::PLW012 => SuffixLength::Three, CheckCodePrefix::PLW0120 => SuffixLength::Four, + CheckCodePrefix::PLW06 => SuffixLength::Two, + CheckCodePrefix::PLW060 => SuffixLength::Three, + CheckCodePrefix::PLW0602 => SuffixLength::Four, CheckCodePrefix::Q => SuffixLength::Zero, CheckCodePrefix::Q0 => SuffixLength::One, CheckCodePrefix::Q00 => SuffixLength::Two, diff --git a/src/pylint/mod.rs b/src/pylint/mod.rs index 1b1e5f54af..090b18e734 100644 --- a/src/pylint/mod.rs +++ b/src/pylint/mod.rs @@ -27,6 +27,7 @@ mod tests { #[test_case(CheckCode::PLR1722, Path::new("consider_using_sys_exit_5.py"); "PLR1722_5")] #[test_case(CheckCode::PLR1722, Path::new("consider_using_sys_exit_6.py"); "PLR1722_6")] #[test_case(CheckCode::PLW0120, Path::new("useless_else_on_loop.py"); "PLW0120")] + #[test_case(CheckCode::PLW0602, Path::new("global_variable_not_assigned.py"); "PLW0602")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); let mut checks = test_path( diff --git a/src/pylint/snapshots/ruff__pylint__tests__PLW0602_global_variable_not_assigned.py.snap b/src/pylint/snapshots/ruff__pylint__tests__PLW0602_global_variable_not_assigned.py.snap new file mode 100644 index 0000000000..6cf60c146f --- /dev/null +++ b/src/pylint/snapshots/ruff__pylint__tests__PLW0602_global_variable_not_assigned.py.snap @@ -0,0 +1,23 @@ +--- +source: src/pylint/mod.rs +expression: checks +--- +- kind: + GlobalVariableNotAssigned: x + location: + row: 5 + column: 4 + end_location: + row: 5 + column: 12 + fix: ~ +- kind: + GlobalVariableNotAssigned: x + location: + row: 9 + column: 4 + end_location: + row: 9 + column: 12 + fix: ~ +