Skip to content

Commit

Permalink
suggested tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
diceroll123 committed Jan 25, 2024
1 parent 43c75ab commit 514affe
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ 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
Expand Up @@ -28,3 +28,11 @@ def area(self) -> float:

class CustomException(Exception): # OK
...


class A:
class B:
...

def __init__(self):
...
6 changes: 1 addition & 5 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,11 +407,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
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,
);
pylint::rules::too_few_public_methods(checker, class_def);
}
if checker.enabled(Rule::TooManyPublicMethods) {
pylint::rules::too_many_public_methods(
Expand Down
17 changes: 1 addition & 16 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ mod tests {
#[test_case(Rule::PotentialIndexError, Path::new("potential_index_error.py"))]
#[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))]
#[test_case(Rule::TooManyNestedBlocks, Path::new("too_many_nested_blocks.py"))]
#[test_case(Rule::TooFewPublicMethods, Path::new("too_few_public_methods.py"))]
#[test_case(
Rule::UnnecessaryDictIndexLookup,
Path::new("unnecessary_dict_index_lookup.py")
Expand Down Expand Up @@ -376,22 +377,6 @@ 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
73 changes: 34 additions & 39 deletions crates/ruff_linter/src/rules/pylint/rules/too_few_public_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,14 @@ 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.
/// Checks for classes that only have a public `__init__` method,
/// as well as 0 base classes, and 0 decorators.
///
/// ## Why is this bad?
/// Classes with too few public methods are possibly better off
/// Classes with just an `__init__` 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):
Expand All @@ -35,32 +32,18 @@ use crate::checkers::ast::Checker;
/// x: float
/// y: float
/// ```
///
/// ## Options
/// - `pylint.min-public-methods`
#[violation]
pub struct TooFewPublicMethods {
methods: usize,
min_methods: usize,
}
pub struct TooFewPublicMethods;

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})")
format!("Class could be dataclass or namedtuple")
}
}

/// R0903
pub(crate) fn too_few_public_methods(
checker: &mut Checker,
class_def: &ast::StmtClassDef,
min_methods: usize,
) {
pub(crate) fn too_few_public_methods(checker: &mut Checker, class_def: &ast::StmtClassDef) {
// allow decorated classes
if !class_def.decorator_list.is_empty() {
return;
Expand All @@ -71,22 +54,34 @@ pub(crate) fn too_few_public_methods(
return;
}

let methods = class_def
.body
.iter()
.filter(|stmt| {
stmt.as_function_def_stmt()
.is_some_and(|node| matches!(visibility::method_visibility(node), Public))
})
.count();
let mut public_methods = 0;
let mut has_dunder_init = false;

for stmt in &class_def.body {
if public_methods > 1 && has_dunder_init {
// we're good to break here
break;
}
match stmt {
ast::Stmt::FunctionDef(node) => {
if !has_dunder_init && node.name.to_string() == "__init__" {
has_dunder_init = true;
}
if matches!(visibility::method_visibility(node), Public) {
public_methods += 1;
}
}
ast::Stmt::ClassDef(_) => {
// allow classes with nested classes, often used for config
return;
}
_ => {}
}
}

if methods < min_methods {
checker.diagnostics.push(Diagnostic::new(
TooFewPublicMethods {
methods,
min_methods,
},
class_def.range(),
));
if has_dunder_init && public_methods == 1 {
checker
.diagnostics
.push(Diagnostic::new(TooFewPublicMethods, class_def.range()));
}
}
3 changes: 0 additions & 3 deletions crates/ruff_linter/src/rules/pylint/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ 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,
pub max_nested_blocks: usize,
}
Expand All @@ -76,7 +75,6 @@ impl Default for Settings {
max_branches: 12,
max_statements: 50,
max_public_methods: 20,
min_public_methods: 2,
max_locals: 15,
max_nested_blocks: 5,
}
Expand All @@ -98,7 +96,6 @@ 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
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
too_few_public_methods.py:6:1: PLR0903 Too few public methods (1 < 2)
too_few_public_methods.py:6:1: PLR0903 Class could be dataclass or namedtuple
|
6 | / class Point: # PLR0903
7 | | def __init__(self, x: float, y: float) -> None:
Expand Down
9 changes: 0 additions & 9 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 514affe

Please sign in to comment.