From 5c6243212c0e86401415976eede0dc4109b4ed05 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 18 Dec 2023 14:52:39 -0500 Subject: [PATCH] Add separate file; tweak docs --- .../test/fixtures/pylint/too_many_locals.py | 3 +- .../checkers/ast/analyze/deferred_scopes.rs | 22 +----------- .../src/rules/pylint/rules/too_many_locals.rs | 36 ++++++++++++++++--- ...rules__pylint__tests__too_many_locals.snap | 10 +++--- crates/ruff_workspace/src/options.rs | 2 +- 5 files changed, 39 insertions(+), 34 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/too_many_locals.py b/crates/ruff_linter/resources/test/fixtures/pylint/too_many_locals.py index 40af4a3fb1bf9..ffb30d7607e19 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/too_many_locals.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/too_many_locals.py @@ -1,4 +1,4 @@ -def func() -> None: # Ok +def func() -> None: # OK # 15 is max default first = 1 second = 2 @@ -16,6 +16,7 @@ def func() -> None: # Ok fourteenth = 14 fifteenth = 15 + def func() -> None: # PLR0914 first = 1 second = 2 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index c4597a495bdd3..2cef614ea94ed 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -1,5 +1,4 @@ use ruff_diagnostics::Diagnostic; -use ruff_python_ast::identifier::Identifier; use ruff_python_semantic::analyze::visibility; use ruff_python_semantic::{Binding, BindingKind, ScopeKind}; use ruff_text_size::Ranged; @@ -340,26 +339,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { } if checker.enabled(Rule::TooManyLocals) { - // PLR0914 - let num_locals = scope - .binding_ids() - .filter(|&id| { - let binding = checker.semantic.binding(id); - matches!(binding.kind, BindingKind::Assignment) - }) - .count(); - - if num_locals > checker.settings.pylint.max_locals { - if let ScopeKind::Function(func) = scope.kind { - diagnostics.push(Diagnostic::new( - pylint::rules::TooManyLocals { - current_amount: num_locals, - max_amount: checker.settings.pylint.max_locals, - }, - func.identifier(), - )); - }; - } + pylint::rules::too_many_locals(checker, scope, &mut diagnostics); } } } diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_locals.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_locals.rs index 88636c672ea77..e2137b637bf52 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_locals.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_locals.rs @@ -1,10 +1,14 @@ -use ruff_diagnostics::Violation; +use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::identifier::Identifier; +use ruff_python_semantic::{Scope, ScopeKind}; + +use crate::checkers::ast::Checker; /// ## What it does -/// Checks for functions/methods that include too many local variables. +/// Checks for functions that include too many local variables. /// -/// By default, this rule allows up to fifteen arguments, as configured by the +/// By default, this rule allows up to fifteen locals, as configured by the /// [`pylint.max-locals`] option. /// /// ## Why is this bad? @@ -17,8 +21,8 @@ use ruff_macros::{derive_message_formats, violation}; /// - `pylint.max-locals` #[violation] pub struct TooManyLocals { - pub(crate) current_amount: usize, - pub(crate) max_amount: usize, + current_amount: usize, + max_amount: usize, } impl Violation for TooManyLocals { @@ -31,3 +35,25 @@ impl Violation for TooManyLocals { format!("Too many local variables: ({current_amount}/{max_amount})") } } + +/// PLR0914 +pub(crate) fn too_many_locals(checker: &Checker, scope: &Scope, diagnostics: &mut Vec) { + let num_locals = scope + .binding_ids() + .filter(|id| { + let binding = checker.semantic().binding(*id); + binding.kind.is_assignment() + }) + .count(); + if num_locals > checker.settings.pylint.max_locals { + if let ScopeKind::Function(func) = scope.kind { + diagnostics.push(Diagnostic::new( + TooManyLocals { + current_amount: num_locals, + max_amount: checker.settings.pylint.max_locals, + }, + func.identifier(), + )); + }; + } +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__too_many_locals.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__too_many_locals.snap index 100255a4c8b6c..5150eac445d4e 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__too_many_locals.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__too_many_locals.snap @@ -1,14 +1,12 @@ --- source: crates/ruff_linter/src/rules/pylint/mod.rs --- -too_many_locals.py:19:5: PLR0914 Too many local variables: (16/15) +too_many_locals.py:20:5: PLR0914 Too many local variables: (16/15) | -17 | fifteenth = 15 -18 | -19 | def func() -> None: # PLR0914 +20 | def func() -> None: # PLR0914 | ^^^^ PLR0914 -20 | first = 1 -21 | second = 2 +21 | first = 1 +22 | second = 2 | diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 28bf3d36e5e29..169b8c3da5060 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -2747,7 +2747,7 @@ impl PylintOptions { max_public_methods: self .max_public_methods .unwrap_or(defaults.max_public_methods), - max_locals: defaults.max_locals, + max_locals: self.max_locals.unwrap_or(defaults.max_locals), } } }