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 f294ac6c0eecbb..59901906c7e93f 100644 --- a/crates/ruff/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff/src/checkers/ast/analyze/statement.rs @@ -399,6 +399,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 9018dcc946980e..bd2396a701f20a 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -226,6 +226,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W1508") => (RuleGroup::Unspecified, rules::pylint::rules::InvalidEnvvarDefault), (Pylint, "W1509") => (RuleGroup::Unspecified, rules::pylint::rules::SubprocessPopenPreexecFn), (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, "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 4e0a8f7e1b4621..43e06759a9a129 100644 --- a/crates/ruff/src/rules/pylint/mod.rs +++ b/crates/ruff/src/rules/pylint/mod.rs @@ -250,4 +250,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 1cc36b22937ebe..c5dc5dcc90029c 100644 --- a/crates/ruff/src/rules/pylint/rules/mod.rs +++ b/crates/ruff/src/rules/pylint/rules/mod.rs @@ -39,6 +39,7 @@ pub(crate) use subprocess_popen_preexec_fn::*; 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::*; @@ -93,6 +94,7 @@ mod subprocess_popen_preexec_fn; 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..2fe375e83c1087 --- /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, Ranged, 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 { name, body, .. } = 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, + }, + name.range(), + )); + } +} diff --git a/crates/ruff/src/rules/pylint/settings.rs b/crates/ruff/src/rules/pylint/settings.rs index 6aab5503fab7cc..d7ebbc9081db6e 100644 --- a/crates/ruff/src/rules/pylint/settings.rs +++ b/crates/ruff/src/rules/pylint/settings.rs @@ -70,6 +70,13 @@ pub struct Options { /// Maximum number of statements allowed for a function or method body (see: /// `PLR0915`). pub max_statements: Option, + #[option( + default = r"20", + value_type = "int", + example = r"max-public-methods = 20" + )] + /// Maximum number of public methods allowed for a class (see: `PLR0904`). + pub max_public_methods: Option, } #[derive(Debug, CacheKey)] @@ -79,6 +86,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 { @@ -89,6 +97,7 @@ impl Default for Settings { max_returns: 6, max_branches: 12, max_statements: 50, + max_public_methods: 20, } } } @@ -104,6 +113,9 @@ impl From for Settings { max_returns: options.max_returns.unwrap_or(defaults.max_returns), max_branches: options.max_branches.unwrap_or(defaults.max_branches), max_statements: options.max_statements.unwrap_or(defaults.max_statements), + max_public_methods: options + .max_public_methods + .unwrap_or(defaults.max_public_methods), } } } @@ -116,6 +128,7 @@ impl From for Options { max_returns: Some(settings.max_returns), max_branches: Some(settings.max_branches), max_statements: Some(settings.max_statements), + max_public_methods: Some(settings.max_public_methods), } } } 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..68e18d0a3c06b7 --- /dev/null +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__too_many_public_methods.snap @@ -0,0 +1,11 @@ +--- +source: crates/ruff/src/rules/pylint/mod.rs +--- +too_many_public_methods.py:1:7: PLR0904 Too many public methods (10 > 7) + | +1 | class Everything: + | ^^^^^^^^^^ PLR0904 +2 | foo = 1 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 87ddf7002cc55b..22f62585d29282 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1578,6 +1578,15 @@ "format": "uint", "minimum": 0.0 }, + "max-public-methods": { + "description": "Maximum number of public methods allowed for a class (see: `PLR0904`).", + "type": [ + "integer", + "null" + ], + "format": "uint", + "minimum": 0.0 + }, "max-returns": { "description": "Maximum number of return statements allowed for a function or method body (see `PLR0911`)", "type": [ @@ -2236,6 +2245,8 @@ "PLR040", "PLR0402", "PLR09", + "PLR090", + "PLR0904", "PLR091", "PLR0911", "PLR0912",