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 0000000000..80cd6e3329 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pylint/too_many_public_methods.py @@ -0,0 +1,60 @@ +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 + +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 diff --git a/crates/ruff/src/checkers/ast/analyze/statement.rs b/crates/ruff/src/checkers/ast/analyze/statement.rs index fabc60e3ea..f3c9596331 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 694050cc8b..c8c75ab2ef 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -269,6 +269,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W1510") => (RuleGroup::Unspecified, rules::pylint::rules::SubprocessRunWithoutCheck), #[allow(deprecated)] (Pylint, "W1641") => (RuleGroup::Nursery, rules::pylint::rules::EqWithoutHash), + (Pylint, "R0904") => (RuleGroup::Preview, rules::pylint::rules::TooManyPublicMethods), (Pylint, "W2901") => (RuleGroup::Unspecified, rules::pylint::rules::RedefinedLoopName), #[allow(deprecated)] (Pylint, "W3201") => (RuleGroup::Nursery, rules::pylint::rules::BadDunderMethodName), diff --git a/crates/ruff/src/rules/pylint/mod.rs b/crates/ruff/src/rules/pylint/mod.rs index dc2310c58e..ad3f7f3d66 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 d8306b086d..7643b55b52 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 0000000000..7e7c113d80 --- /dev/null +++ b/crates/ruff/src/rules/pylint/rules/too_many_public_methods.rs @@ -0,0 +1,126 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_semantic::analyze::visibility::{self, Visibility::Public}; +use ruff_text_size::Ranged; + +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 +/// Assuming that `pylint.max-public-settings` is 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 methods = class_def + .body + .iter() + .filter(|stmt| { + stmt.as_function_def_stmt() + .is_some_and(|node| matches!(visibility::method_visibility(node), Public)) + }) + .count(); + + if methods > max_methods { + checker.diagnostics.push(Diagnostic::new( + TooManyPublicMethods { + methods, + max_methods, + }, + class_def.range(), + )); + } +} diff --git a/crates/ruff/src/rules/pylint/settings.rs b/crates/ruff/src/rules/pylint/settings.rs index 082ed190c7..e09e4835f8 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 0000000000..c24d559c4d --- /dev/null +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__too_many_public_methods.snap @@ -0,0 +1,46 @@ +--- +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 + | |____________^ PLR0904 +36 | +37 | class Small: + | + + diff --git a/crates/ruff_python_semantic/src/analyze/visibility.rs b/crates/ruff_python_semantic/src/analyze/visibility.rs index da826db5a0..48836b9da6 100644 --- a/crates/ruff_python_semantic/src/analyze/visibility.rs +++ b/crates/ruff_python_semantic/src/analyze/visibility.rs @@ -184,7 +184,7 @@ pub(crate) fn function_visibility(function: &ast::StmtFunctionDef) -> Visibility } } -pub(crate) fn method_visibility(function: &ast::StmtFunctionDef) -> Visibility { +pub fn method_visibility(function: &ast::StmtFunctionDef) -> Visibility { // Is this a setter or deleter? if function.decorator_list.iter().any(|decorator| { collect_call_path(&decorator.expression).is_some_and(|call_path| { diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 105ef947fa..04a92c0a83 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -851,7 +851,7 @@ mod tests { Rule::QuadraticListSummation, ]; - const PREVIEW_RULES: &[Rule] = &[Rule::SliceCopy]; + const PREVIEW_RULES: &[Rule] = &[Rule::TooManyPublicMethods, Rule::SliceCopy]; #[allow(clippy::needless_pass_by_value)] fn resolve_rules( diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 6b53f94d34..3c8cc9e227 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -2155,6 +2155,13 @@ pub struct PylintOptions { /// 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, } impl PylintOptions { @@ -2168,6 +2175,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), } } } diff --git a/ruff.schema.json b/ruff.schema.json index 7daca772e0..4402f3c797 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1612,6 +1612,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": [ @@ -2296,6 +2305,8 @@ "PLR040", "PLR0402", "PLR09", + "PLR090", + "PLR0904", "PLR091", "PLR0911", "PLR0912",