Skip to content

Commit

Permalink
[pylint] implement rule PLR0914
Browse files Browse the repository at this point in the history
  • Loading branch information
diceroll123 committed Dec 16, 2023
1 parent 0029b4f commit 62df5a2
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
def func():
# Ok
# 11 is max default
first = 1
second = 2
third = 3
fourth = 4
fifth = 5
sixth = 6
seventh = 7
eighth = 8
ninth = 9
tenth = 10
eleventh = 11

def func():
# PLR0914
first = 1
second = 2
third = 3
fourth = 4
fifth = 5
sixth = 6
seventh = 7
eighth = 8
ninth = 9
tenth = 10
eleventh = 11
twelfth = 12
thirteenth = 13
33 changes: 32 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use std::ops::Add;

use ruff_diagnostics::Diagnostic;
use ruff_python_semantic::analyze::visibility;
use ruff_python_semantic::{Binding, BindingKind, ScopeKind};
use ruff_text_size::Ranged;
use ruff_text_size::{Ranged, TextRange, TextSize};

use crate::checkers::ast::Checker;
use crate::codes::Rule;
Expand All @@ -15,6 +17,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
Rule::RedefinedArgumentFromLocal,
Rule::RedefinedWhileUnused,
Rule::RuntimeImportInTypeCheckingBlock,
Rule::TooManyLocals,
Rule::TypingOnlyFirstPartyImport,
Rule::TypingOnlyStandardLibraryImport,
Rule::TypingOnlyThirdPartyImport,
Expand Down Expand Up @@ -336,6 +339,34 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
if checker.enabled(Rule::NoSelfUse) {
pylint::rules::no_self_use(checker, scope_id, scope, &mut diagnostics);
}

if checker.enabled(Rule::TooManyLocals) {
// PLR0914
let mut num_locals = 0;
let mut last_seen_range = TextRange::default();
for (_name, binding_id) in scope.bindings() {
let binding = checker.semantic.binding(binding_id);
if matches!(binding.kind, BindingKind::Assignment) {
num_locals += 1;

// use the beginning of the last seen assignment as the range
last_seen_range = TextRange::new(
binding.range().start(),
binding.range().start().add(TextSize::from(1)),
);
}
}

if num_locals > checker.settings.pylint.max_locals {
diagnostics.push(Diagnostic::new(
pylint::rules::TooManyLocals {
current_amount: num_locals,
max_amount: checker.settings.pylint.max_locals,
},
last_seen_range,
));
}
}
}
}
checker.diagnostics.extend(diagnostics);
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "R0911") => (RuleGroup::Stable, rules::pylint::rules::TooManyReturnStatements),
(Pylint, "R0912") => (RuleGroup::Stable, rules::pylint::rules::TooManyBranches),
(Pylint, "R0913") => (RuleGroup::Stable, rules::pylint::rules::TooManyArguments),
(Pylint, "R0914") => (RuleGroup::Stable, rules::pylint::rules::TooManyLocals),
(Pylint, "R0915") => (RuleGroup::Stable, rules::pylint::rules::TooManyStatements),
(Pylint, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions),
(Pylint, "R0917") => (RuleGroup::Preview, rules::pylint::rules::TooManyPositional),
Expand Down
16 changes: 16 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,22 @@ mod tests {
Ok(())
}

#[test]
fn too_many_locals() -> Result<()> {
let diagnostics = test_path(
Path::new("pylint/too_many_locals.py"),
&LinterSettings {
pylint: pylint::settings::Settings {
max_locals: 11,
..pylint::settings::Settings::default()
},
..LinterSettings::for_rules(vec![Rule::TooManyLocals])
},
)?;
assert_messages!(diagnostics);
Ok(())
}

#[test]
fn unspecified_encoding_python39_or_lower() -> Result<()> {
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub(crate) use sys_exit_alias::*;
pub(crate) use too_many_arguments::*;
pub(crate) use too_many_boolean_expressions::*;
pub(crate) use too_many_branches::*;
pub(crate) use too_many_locals::*;
pub(crate) use too_many_positional::*;
pub(crate) use too_many_public_methods::*;
pub(crate) use too_many_return_statements::*;
Expand Down Expand Up @@ -132,6 +133,7 @@ mod sys_exit_alias;
mod too_many_arguments;
mod too_many_boolean_expressions;
mod too_many_branches;
mod too_many_locals;
mod too_many_positional;
mod too_many_public_methods;
mod too_many_return_statements;
Expand Down
33 changes: 33 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/too_many_locals.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};

/// ## What it does
/// Checks for functions/methods that include too many local variables.
///
/// By default, this rule allows up to eleven arguments, as configured by the
/// [`pylint.max-locals`] option.
///
/// ## Why is this bad?
/// Functions with many local variables are harder to understand and maintain.
///
/// Consider refactoring functions with many local variables into smaller
/// functions with fewer assignments.
///
/// ## Options
/// - `pylint.max-locals`
#[violation]
pub struct TooManyLocals {
pub(crate) current_amount: usize,
pub(crate) max_amount: usize,
}

impl Violation for TooManyLocals {
#[derive_message_formats]
fn message(&self) -> String {
let TooManyLocals {
current_amount,
max_amount,
} = self;
format!("Too many local variables: ({current_amount}/{max_amount})")
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub struct Settings {
pub max_branches: usize,
pub max_statements: usize,
pub max_public_methods: usize,
pub max_locals: usize,
}

impl Default for Settings {
Expand All @@ -59,6 +60,7 @@ impl Default for Settings {
max_branches: 12,
max_statements: 50,
max_public_methods: 20,
max_locals: 11,
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
too_many_locals.py:30:5: PLR0914 Too many local variables: (13/11)
|
28 | eleventh = 11
29 | twelfth = 12
30 | thirteenth = 13
| ^ PLR0914
|


6 changes: 6 additions & 0 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2703,6 +2703,11 @@ pub struct PylintOptions {
#[option(default = r"3", value_type = "int", example = r"max-pos-args = 3")]
pub max_positional_args: Option<usize>,

/// Maximum number of local variables allowed for a function or method body (see:
/// `PLR0914`).
#[option(default = r"11", value_type = "int", example = r"max-locals = 11")]
pub max_locals: Option<usize>,

/// Maximum number of statements allowed for a function or method body (see:
/// `PLR0915`).
#[option(default = r"50", value_type = "int", example = r"max-statements = 50")]
Expand Down Expand Up @@ -2742,6 +2747,7 @@ impl PylintOptions {
max_public_methods: self
.max_public_methods
.unwrap_or(defaults.max_public_methods),
max_locals: defaults.max_locals,
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 62df5a2

Please sign in to comment.