Skip to content

Commit

Permalink
[pylint] Implement too-many-public-methods rule (PLR0904)
Browse files Browse the repository at this point in the history
  • Loading branch information
jelly committed Aug 1, 2023
1 parent de898c5 commit 0629206
Show file tree
Hide file tree
Showing 9 changed files with 253 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions crates/ruff/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
16 changes: 16 additions & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
126 changes: 126 additions & 0 deletions crates/ruff/src/rules/pylint/rules/too_many_public_methods.rs
Original file line number Diff line number Diff line change
@@ -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(),
));
}
}
13 changes: 13 additions & 0 deletions crates/ruff/src/rules/pylint/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ pub struct Options {
/// Maximum number of statements allowed for a function or method body (see:
/// `PLR0915`).
pub max_statements: Option<usize>,
#[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<usize>,
}

#[derive(Debug, CacheKey)]
Expand All @@ -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 {
Expand All @@ -89,6 +97,7 @@ impl Default for Settings {
max_returns: 6,
max_branches: 12,
max_statements: 50,
max_public_methods: 20,
}
}
}
Expand All @@ -104,6 +113,9 @@ impl From<Options> 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),
}
}
}
Expand All @@ -116,6 +128,7 @@ impl From<Settings> 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),
}
}
}
Original file line number Diff line number Diff line change
@@ -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
|


11 changes: 11 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 0629206

Please sign in to comment.