diff --git a/crates/ruff/resources/test/fixtures/pylint/too_many_public_methods.py b/crates/ruff/resources/test/fixtures/pylint/too_many_public_methods.py new file mode 100644 index 00000000000000..b287fb6c927a4d --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pylint/too_many_public_methods.py @@ -0,0 +1,66 @@ +class Everything: + foo = 1 + + def __init__(self): + pass + + def _private(self): + pass + + def method1(self): + pass + + def method2(self): + pass + + def method3(self): + pass + + def method4(self): + pass + + def method5(self): + pass + + def method6(self): + pass + + def method7(self): + pass + + def method8(self): + pass + + def method9(self): + pass + + def method10(self): + pass + +class Small: + def __init__(self): + pass + + def _private(self): + pass + + def method1(self): + pass + + def method2(self): + pass + + def method3(self): + pass + + def method4(self): + pass + + def method5(self): + pass + + def method6(self): + pass + + def method7(self): + pass diff --git a/crates/ruff/src/checkers/ast/analyze/statement.rs b/crates/ruff/src/checkers/ast/analyze/statement.rs index fabc60e3eaf58e..f3c9596331a061 100644 --- a/crates/ruff/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff/src/checkers/ast/analyze/statement.rs @@ -411,6 +411,13 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::EqWithoutHash) { pylint::rules::object_without_hash_method(checker, class_def); } + if checker.enabled(Rule::TooManyPublicMethods) { + pylint::rules::too_many_public_methods( + checker, + class_def, + checker.settings.pylint.max_public_methods, + ); + } if checker.enabled(Rule::GlobalStatement) { pylint::rules::global_statement(checker, name); } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 67eb3887db769d..2c5b3c7bad50d5 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -229,6 +229,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W1509") => (RuleGroup::Unspecified, rules::pylint::rules::SubprocessPopenPreexecFn), (Pylint, "W1510") => (RuleGroup::Unspecified, rules::pylint::rules::SubprocessRunWithoutCheck), (Pylint, "W1641") => (RuleGroup::Nursery, rules::pylint::rules::EqWithoutHash), + (Pylint, "R0904") => (RuleGroup::Unspecified, rules::pylint::rules::TooManyPublicMethods), (Pylint, "W2901") => (RuleGroup::Unspecified, rules::pylint::rules::RedefinedLoopName), (Pylint, "W3201") => (RuleGroup::Nursery, rules::pylint::rules::BadDunderMethodName), (Pylint, "W3301") => (RuleGroup::Unspecified, rules::pylint::rules::NestedMinMax), diff --git a/crates/ruff/src/rules/pylint/mod.rs b/crates/ruff/src/rules/pylint/mod.rs index 5d18261946bcdf..670797b2ac170d 100644 --- a/crates/ruff/src/rules/pylint/mod.rs +++ b/crates/ruff/src/rules/pylint/mod.rs @@ -256,4 +256,20 @@ mod tests { assert_messages!(diagnostics); Ok(()) } + + #[test] + fn too_many_public_methods() -> Result<()> { + let diagnostics = test_path( + Path::new("pylint/too_many_public_methods.py"), + &Settings { + pylint: pylint::settings::Settings { + max_public_methods: 7, + ..pylint::settings::Settings::default() + }, + ..Settings::for_rules(vec![Rule::TooManyPublicMethods]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } } diff --git a/crates/ruff/src/rules/pylint/rules/mod.rs b/crates/ruff/src/rules/pylint/rules/mod.rs index 532f267ce4ed6a..b45b857f46b886 100644 --- a/crates/ruff/src/rules/pylint/rules/mod.rs +++ b/crates/ruff/src/rules/pylint/rules/mod.rs @@ -43,6 +43,7 @@ pub(crate) use subprocess_run_without_check::*; pub(crate) use sys_exit_alias::*; pub(crate) use too_many_arguments::*; pub(crate) use too_many_branches::*; +pub(crate) use too_many_public_methods::*; pub(crate) use too_many_return_statements::*; pub(crate) use too_many_statements::*; pub(crate) use type_bivariance::*; @@ -101,6 +102,7 @@ mod subprocess_run_without_check; mod sys_exit_alias; mod too_many_arguments; mod too_many_branches; +mod too_many_public_methods; mod too_many_return_statements; mod too_many_statements; mod type_bivariance; diff --git a/crates/ruff/src/rules/pylint/rules/too_many_public_methods.rs b/crates/ruff/src/rules/pylint/rules/too_many_public_methods.rs new file mode 100644 index 00000000000000..4d59605a4cb314 --- /dev/null +++ b/crates/ruff/src/rules/pylint/rules/too_many_public_methods.rs @@ -0,0 +1,126 @@ +use ruff_python_ast::{self as ast, Stmt}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for classes with too many public methods +/// +/// By default, this rule allows up to 20 statements, as configured by the +/// `pylint.max-public-methods` option. +/// +/// ## Why is this bad? +/// Classes with many public methods are harder to understand +/// and maintain. +/// +/// Instead, consider refactoring the class into separate classes. +/// +/// ## Example +/// With `pylint.max-public-settings` set to 5 +/// +/// ```python +/// class Linter: +/// def __init__(self): +/// pass +/// +/// def pylint(self): +/// pass +/// +/// def pylint_settings(self): +/// pass +/// +/// def flake8(self): +/// pass +/// +/// def flake8_settings(self): +/// pass +/// +/// def pydocstyle(self): +/// pass +/// +/// def pydocstyle_settings(self): +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// class Linter: +/// def __init__(self): +/// self.pylint = Pylint() +/// self.flake8 = Flake8() +/// self.pydocstyle = Pydocstyle() +/// +/// def lint(self): +/// pass +/// +/// +/// class Pylint: +/// def lint(self): +/// pass +/// +/// def settings(self): +/// pass +/// +/// +/// class Flake8: +/// def lint(self): +/// pass +/// +/// def settings(self): +/// pass +/// +/// +/// class Pydocstyle: +/// def lint(self): +/// pass +/// +/// def settings(self): +/// pass +/// ``` +/// +/// ## Options +/// - `pylint.max-public-methods` +#[violation] +pub struct TooManyPublicMethods { + methods: usize, + max_methods: usize, +} + +impl Violation for TooManyPublicMethods { + #[derive_message_formats] + fn message(&self) -> String { + let TooManyPublicMethods { + methods, + max_methods, + } = self; + format!("Too many public methods ({methods} > {max_methods})") + } +} + +/// R0904 +pub(crate) fn too_many_public_methods( + checker: &mut Checker, + class_def: &ast::StmtClassDef, + max_methods: usize, +) { + let ast::StmtClassDef { body, range, .. } = class_def; + let methods = body + .iter() + .filter(|stmt| match stmt { + Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => !name.starts_with('_'), + _ => false, + }) + .count(); + + if methods > max_methods { + checker.diagnostics.push(Diagnostic::new( + TooManyPublicMethods { + methods, + max_methods, + }, + *range, + )); + } +} diff --git a/crates/ruff/src/rules/pylint/settings.rs b/crates/ruff/src/rules/pylint/settings.rs index 082ed190c76a3d..e09e4835f82799 100644 --- a/crates/ruff/src/rules/pylint/settings.rs +++ b/crates/ruff/src/rules/pylint/settings.rs @@ -42,6 +42,7 @@ pub struct Settings { pub max_returns: usize, pub max_branches: usize, pub max_statements: usize, + pub max_public_methods: usize, } impl Default for Settings { @@ -52,6 +53,7 @@ impl Default for Settings { max_returns: 6, max_branches: 12, max_statements: 50, + max_public_methods: 20, } } } diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__too_many_public_methods.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__too_many_public_methods.snap new file mode 100644 index 00000000000000..c0318f795a90bd --- /dev/null +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__too_many_public_methods.snap @@ -0,0 +1,49 @@ +--- +source: crates/ruff/src/rules/pylint/mod.rs +--- +too_many_public_methods.py:1:1: PLR0904 Too many public methods (10 > 7) + | + 1 | / class Everything: + 2 | | foo = 1 + 3 | | + 4 | | def __init__(self): + 5 | | pass + 6 | | + 7 | | def _private(self): + 8 | | pass + 9 | | +10 | | def method1(self): +11 | | pass +12 | | +13 | | def method2(self): +14 | | pass +15 | | +16 | | def method3(self): +17 | | pass +18 | | +19 | | def method4(self): +20 | | pass +21 | | +22 | | def method5(self): +23 | | pass +24 | | +25 | | def method6(self): +26 | | pass +27 | | +28 | | def method7(self): +29 | | pass +30 | | +31 | | def method8(self): +32 | | pass +33 | | +34 | | def method9(self): +35 | | pass +36 | | +37 | | def method10(self): +38 | | pass + | |____________^ PLR0904 +39 | +40 | class Small: + | + + diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 599f4626393d33..c126623863c74f 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -2134,6 +2134,8 @@ pub struct PylintOptions { /// Maximum number of statements allowed for a function or method body (see: /// `PLR0915`). pub max_statements: Option, + /// Maximum number of public methods allowed for a class (see: `PLR0904`). + pub max_public_methods: Option, } impl PylintOptions { @@ -2147,6 +2149,9 @@ impl PylintOptions { max_returns: self.max_returns.unwrap_or(defaults.max_returns), max_branches: self.max_branches.unwrap_or(defaults.max_branches), max_statements: self.max_statements.unwrap_or(defaults.max_statements), + max_public_methods: self + .max_public_methods + .unwrap_or(defaults.max_public_methods), } } }