From 4894b8ffafaa6fa88feaa6710ec4f825218717e6 Mon Sep 17 00:00:00 2001 From: Steve C Date: Sun, 21 Jan 2024 17:33:59 -0500 Subject: [PATCH] [`pylint`] - Implement `too-few-public-methods` rule (`PLR0903`) --- ...ow_settings__display_default_settings.snap | 1 + .../fixtures/pylint/too_few_public_methods.py | 15 ++++ .../src/checkers/ast/analyze/statement.rs | 7 ++ crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/pylint/mod.rs | 16 ++++ .../ruff_linter/src/rules/pylint/rules/mod.rs | 2 + .../pylint/rules/too_few_public_methods.rs | 81 +++++++++++++++++++ .../ruff_linter/src/rules/pylint/settings.rs | 3 + ...pylint__tests__too_few_public_methods.snap | 13 +++ crates/ruff_workspace/src/configuration.rs | 2 +- crates/ruff_workspace/src/options.rs | 12 +++ ruff.schema.json | 10 +++ 12 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/too_few_public_methods.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/too_few_public_methods.rs create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__too_few_public_methods.snap diff --git a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap index ae92e615f048fa..14bc5005e9837a 100644 --- a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap +++ b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap @@ -348,6 +348,7 @@ linter.pylint.max_bool_expr = 5 linter.pylint.max_branches = 12 linter.pylint.max_statements = 50 linter.pylint.max_public_methods = 20 +linter.pylint.min_public_methods = 2 linter.pylint.max_locals = 15 linter.pyupgrade.keep_runtime_typing = false diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/too_few_public_methods.py b/crates/ruff_linter/resources/test/fixtures/pylint/too_few_public_methods.py new file mode 100644 index 00000000000000..603459b23e0abd --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/too_few_public_methods.py @@ -0,0 +1,15 @@ +from __future__ import annotations + + +class Point: # PLR0903 + def __init__(self, x: float, y: float) -> None: + self.x = x + self.y = y + + +class Rectangle: # OK + def __init__(self, top_left: Point, bottom_right: Point) -> None: + ... + + def area(self) -> float: + ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index c31f1479976635..10b1ae019de972 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -406,6 +406,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::TooFewPublicMethods) { + pylint::rules::too_few_public_methods( + checker, + class_def, + checker.settings.pylint.min_public_methods, + ); + } if checker.enabled(Rule::TooManyPublicMethods) { pylint::rules::too_many_public_methods( checker, diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 40bbdabb659a3a..d16bb65025a646 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -292,6 +292,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { #[allow(deprecated)] (Pylint, "W1641") => (RuleGroup::Nursery, rules::pylint::rules::EqWithoutHash), (Pylint, "W2101") => (RuleGroup::Preview, rules::pylint::rules::UselessWithLock), + (Pylint, "R0903") => (RuleGroup::Preview, rules::pylint::rules::TooFewPublicMethods), (Pylint, "R0904") => (RuleGroup::Preview, rules::pylint::rules::TooManyPublicMethods), (Pylint, "W2901") => (RuleGroup::Stable, rules::pylint::rules::RedefinedLoopName), #[allow(deprecated)] diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index bf5bce4a353e37..c8757629e67d36 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -373,6 +373,22 @@ mod tests { Ok(()) } + #[test] + fn too_few_public_methods() -> Result<()> { + let diagnostics = test_path( + Path::new("pylint/too_few_public_methods.py"), + &LinterSettings { + pylint: pylint::settings::Settings { + min_public_methods: 2, + ..pylint::settings::Settings::default() + }, + ..LinterSettings::for_rules(vec![Rule::TooFewPublicMethods]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } + #[test] fn too_many_locals() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 22be5657bd22d5..eb617bd3b7c350 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -56,6 +56,7 @@ pub(crate) use subprocess_popen_preexec_fn::*; pub(crate) use subprocess_run_without_check::*; pub(crate) use super_without_brackets::*; pub(crate) use sys_exit_alias::*; +pub(crate) use too_few_public_methods::*; pub(crate) use too_many_arguments::*; pub(crate) use too_many_boolean_expressions::*; pub(crate) use too_many_branches::*; @@ -139,6 +140,7 @@ mod subprocess_popen_preexec_fn; mod subprocess_run_without_check; mod super_without_brackets; mod sys_exit_alias; +mod too_few_public_methods; mod too_many_arguments; mod too_many_boolean_expressions; mod too_many_branches; diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_few_public_methods.rs b/crates/ruff_linter/src/rules/pylint/rules/too_few_public_methods.rs new file mode 100644 index 00000000000000..37a1a5bf406f0e --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/too_few_public_methods.rs @@ -0,0 +1,81 @@ +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 few public methods +/// +/// By default, this rule allows down to 2 public methods, as configured by +/// the [`pylint.min-public-methods`] option. +/// +/// ## Why is this bad? +/// Classes with too few public methods are possibly better off +/// being a dataclass or a namedtuple, which are more lightweight. +/// +/// ## Example +/// Assuming that `pylint.min-public-settings` is set to 2: +/// ```python +/// class Point: +/// def __init__(self, x: float, y: float): +/// self.x = x +/// self.y = y +/// ``` +/// +/// Use instead: +/// ```python +/// from dataclasses import dataclass +/// +/// @dataclass +/// class Point: +/// x: float +/// y: float +/// ``` +/// +/// ## Options +/// - `pylint.min-public-methods` +#[violation] +pub struct TooFewPublicMethods { + methods: usize, + min_methods: usize, +} + +impl Violation for TooFewPublicMethods { + #[derive_message_formats] + fn message(&self) -> String { + let TooFewPublicMethods { + methods, + min_methods, + } = self; + format!("Too few public methods ({methods} < {min_methods})") + } +} + +/// R0903 +pub(crate) fn too_few_public_methods( + checker: &mut Checker, + class_def: &ast::StmtClassDef, + min_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 < min_methods { + checker.diagnostics.push(Diagnostic::new( + TooFewPublicMethods { + methods, + min_methods, + }, + class_def.range(), + )); + } +} diff --git a/crates/ruff_linter/src/rules/pylint/settings.rs b/crates/ruff_linter/src/rules/pylint/settings.rs index 71ff494bf2a7cb..3c3193c37ae5a0 100644 --- a/crates/ruff_linter/src/rules/pylint/settings.rs +++ b/crates/ruff_linter/src/rules/pylint/settings.rs @@ -59,6 +59,7 @@ pub struct Settings { pub max_branches: usize, pub max_statements: usize, pub max_public_methods: usize, + pub min_public_methods: usize, pub max_locals: usize, } @@ -74,6 +75,7 @@ impl Default for Settings { max_branches: 12, max_statements: 50, max_public_methods: 20, + min_public_methods: 2, max_locals: 15, } } @@ -94,6 +96,7 @@ impl fmt::Display for Settings { self.max_branches, self.max_statements, self.max_public_methods, + self.min_public_methods, self.max_locals ] } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__too_few_public_methods.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__too_few_public_methods.snap new file mode 100644 index 00000000000000..57d87969318227 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__too_few_public_methods.snap @@ -0,0 +1,13 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +too_few_public_methods.py:4:1: PLR0903 Too few public methods (1 < 2) + | +4 | / class Point: # PLR0903 +5 | | def __init__(self, x: float, y: float) -> None: +6 | | self.x = x +7 | | self.y = y + | |__________________^ PLR0903 + | + + diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 18e7551f556deb..e009674cc8710c 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -1198,7 +1198,7 @@ mod tests { Rule::ManualDictComprehension, Rule::ReimplementedStarmap, Rule::SliceCopy, - Rule::TooManyPublicMethods, + Rule::TooFewPublicMethods, Rule::TooManyPublicMethods, Rule::UndocumentedWarn, Rule::UnnecessaryEnumerate, diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 6aaa21113b4b55..e04c773c5cbcae 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -2728,6 +2728,15 @@ pub struct PylintOptions { /// (see: `PLR0916`). #[option(default = r"5", value_type = "int", example = r"max-bool-expr = 5")] pub max_bool_expr: Option, + + /// Minimum number of public methods allowed within a single class + /// (see: `PLR0903`). + #[option( + default = r"2", + value_type = "int", + example = r"min-public-methods = 2" + )] + pub min_public_methods: Option, } impl PylintOptions { @@ -2751,6 +2760,9 @@ impl PylintOptions { .max_public_methods .unwrap_or(defaults.max_public_methods), max_locals: self.max_locals.unwrap_or(defaults.max_locals), + min_public_methods: self + .min_public_methods + .unwrap_or(defaults.min_public_methods), } } } diff --git a/ruff.schema.json b/ruff.schema.json index 9ea851bd26b54e..a0e0214fa88842 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2454,6 +2454,15 @@ ], "format": "uint", "minimum": 0.0 + }, + "min-public-methods": { + "description": "Minimum number of public methods allowed within a single class (see: `PLR0903`).", + "type": [ + "integer", + "null" + ], + "format": "uint", + "minimum": 0.0 } }, "additionalProperties": false @@ -3183,6 +3192,7 @@ "PLR0402", "PLR09", "PLR090", + "PLR0903", "PLR0904", "PLR091", "PLR0911",