Skip to content

Commit

Permalink
[pylint] - Implement too-few-public-methods rule (PLR0903)
Browse files Browse the repository at this point in the history
  • Loading branch information
diceroll123 committed Jan 21, 2024
1 parent 8379841 commit 366bd8e
Show file tree
Hide file tree
Showing 12 changed files with 163 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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:
...
7 changes: 7 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
16 changes: 16 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
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(),
));
}
}
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/rules/pylint/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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,
}
}
Expand All @@ -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
]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
|


2 changes: 1 addition & 1 deletion crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1198,7 +1198,7 @@ mod tests {
Rule::ManualDictComprehension,
Rule::ReimplementedStarmap,
Rule::SliceCopy,
Rule::TooManyPublicMethods,
Rule::TooFewPublicMethods,
Rule::TooManyPublicMethods,
Rule::UndocumentedWarn,
Rule::UnnecessaryEnumerate,
Expand Down
12 changes: 12 additions & 0 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize>,

/// 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<usize>,
}

impl PylintOptions {
Expand All @@ -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),
}
}
}
Expand Down
10 changes: 10 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 366bd8e

Please sign in to comment.