Compare commits

...

4 Commits

Author SHA1 Message Date
Charlie Marsh
ce7fc9eda7 Move to preview 2023-09-13 20:40:17 -04:00
Charlie Marsh
1dd0bd4e6e Tweaks 2023-09-13 20:35:43 -04:00
Charlie Marsh
3d1ae648ad Merge branch 'main' into pylint-too-many-public-methods 2023-09-13 20:23:19 -04:00
Jelle van der Waa
a66ec47957 [pylint] Implement too-many-public-methods rule (PLR0904)
Implement https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/too-many-public-methods.html
2023-08-30 10:23:42 +02:00
12 changed files with 283 additions and 2 deletions

View File

@@ -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

View File

@@ -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);
}

View File

@@ -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),

View File

@@ -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(())
}
}

View File

@@ -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;

View File

@@ -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(),
));
}
}

View File

@@ -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,
}
}
}

View File

@@ -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:
|

View File

@@ -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| {

View File

@@ -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(

View File

@@ -2155,6 +2155,13 @@ pub struct PylintOptions {
/// 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>,
}
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),
}
}
}

11
ruff.schema.json generated
View File

@@ -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",