[pylint] - implement R0202 and R0203 with autofixes

crates/ruff_linter/resources/test/fixtures/pylint/no_method_decorator.py

@@ -0,0 +1,19 @@
+from random import choice
+
+class Fruit:
+    COLORS = []
+
+    def __init__(self, color):
+        self.color = color
+
+    def pick_colors(cls, *args):  # [no-classmethod-decorator]
+        """classmethod to pick fruit colors"""
+        cls.COLORS = args
+
+    pick_colors = classmethod(pick_colors)
+
+    def pick_one_color():  # [no-staticmethod-decorator]
+        """staticmethod to pick one fruit color"""
+        return choice(Fruit.COLORS)
+
+    pick_one_color = staticmethod(pick_one_color)

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

@@ -384,6 +384,12 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 range: _,
             },
         ) => {
+            if checker.enabled(Rule::NoClassmethodDecorator) {
+                pylint::rules::no_classmethod_decorator(checker, class_def);
+            }
+            if checker.enabled(Rule::NoStaticmethodDecorator) {
+                pylint::rules::no_staticmethod_decorator(checker, class_def);
+            }
             if checker.enabled(Rule::DjangoNullableModelStringField) {
                 flake8_django::rules::nullable_model_string_field(checker, body);
             }

crates/ruff_linter/src/codes.rs

@@ -245,6 +245,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "E2515") => (RuleGroup::Stable, rules::pylint::rules::InvalidCharacterZeroWidthSpace),
         (Pylint, "R0124") => (RuleGroup::Stable, rules::pylint::rules::ComparisonWithItself),
         (Pylint, "R0133") => (RuleGroup::Stable, rules::pylint::rules::ComparisonOfConstant),
+        (Pylint, "R0202") => (RuleGroup::Preview, rules::pylint::rules::NoClassmethodDecorator),
+        (Pylint, "R0203") => (RuleGroup::Preview, rules::pylint::rules::NoStaticmethodDecorator),
         (Pylint, "R0206") => (RuleGroup::Stable, rules::pylint::rules::PropertyWithParameters),
         (Pylint, "R0402") => (RuleGroup::Stable, rules::pylint::rules::ManualFromImport),
         (Pylint, "R0911") => (RuleGroup::Stable, rules::pylint::rules::TooManyReturnStatements),

crates/ruff_linter/src/rules/pylint/mod.rs

@@ -154,6 +154,8 @@ mod tests {
         Rule::RepeatedKeywordArgument,
         Path::new("repeated_keyword_argument.py")
     )]
+    #[test_case(Rule::NoClassmethodDecorator, Path::new("no_method_decorator.py"))]
+    #[test_case(Rule::NoStaticmethodDecorator, Path::new("no_method_decorator.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/pylint/rules/mod.rs

@@ -35,6 +35,7 @@ pub(crate) use manual_import_from::*;
 pub(crate) use misplaced_bare_raise::*;
 pub(crate) use named_expr_without_context::*;
 pub(crate) use nested_min_max::*;
+pub(crate) use no_method_decorator::*;
 pub(crate) use no_self_use::*;
 pub(crate) use non_ascii_module_import::*;
 pub(crate) use non_ascii_name::*;
@@ -108,6 +109,7 @@ mod manual_import_from;
 mod misplaced_bare_raise;
 mod named_expr_without_context;
 mod nested_min_max;
+mod no_method_decorator;
 mod no_self_use;
 mod non_ascii_module_import;
 mod non_ascii_name;

crates/ruff_linter/src/rules/pylint/rules/no_method_decorator.rs

@@ -0,0 +1,202 @@
+use std::collections::HashMap;
+
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, DiagnosticKind, Edit, Fix};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{self as ast, Expr, Stmt};
+use ruff_python_trivia::indentation_at_offset;
+use ruff_text_size::{Ranged, TextRange, TextSize};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for the use of a classmethod being made without the decorator.
+///
+/// ## Why is this bad?
+/// When it comes to consistency and readability, it's preferred to use the decorator.
+///
+/// ## Example
+/// ```python
+/// class Foo:
+///     def bar(cls):
+///         ...
+///
+///     bar = classmethod(bar)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// class Foo:
+///     @classmethod
+///     def bar(cls):
+///         ...
+/// ```
+#[violation]
+pub struct NoClassmethodDecorator;
+
+impl AlwaysFixableViolation for NoClassmethodDecorator {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Class method defined without decorator")
+    }
+
+    fn fix_title(&self) -> String {
+        format!("Add @classmethod decorator")
+    }
+}
+
+/// ## What it does
+/// Checks for the use of a staticmethod being made without the decorator.
+///
+/// ## Why is this bad?
+/// When it comes to consistency and readability, it's preferred to use the decorator.
+///
+/// ## Example
+/// ```python
+/// class Foo:
+///     def bar(cls):
+///         ...
+///
+///     bar = staticmethod(bar)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// class Foo:
+///     @staticmethod
+///     def bar(cls):
+///         ...
+/// ```
+#[violation]
+pub struct NoStaticmethodDecorator;
+
+impl AlwaysFixableViolation for NoStaticmethodDecorator {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Static method defined without decorator")
+    }
+
+    fn fix_title(&self) -> String {
+        format!("Add @staticmethod decorator")
+    }
+}
+
+enum MethodType {
+    Classmethod,
+    Staticmethod,
+}
+
+/// PLR0202
+pub(crate) fn no_classmethod_decorator(checker: &mut Checker, class_def: &ast::StmtClassDef) {
+    get_undecorated_methods(checker, class_def, &MethodType::Classmethod);
+}
+
+/// PLR0203
+pub(crate) fn no_staticmethod_decorator(checker: &mut Checker, class_def: &ast::StmtClassDef) {
+    get_undecorated_methods(checker, class_def, &MethodType::Staticmethod);
+}
+
+fn get_undecorated_methods(
+    checker: &mut Checker,
+    class_def: &ast::StmtClassDef,
+    method_type: &MethodType,
+) {
+    let mut explicit_decorator_calls: HashMap<String, TextRange> = HashMap::default();
+
+    let (method_name, diagnostic_type): (&str, DiagnosticKind) = match method_type {
+        MethodType::Classmethod => ("classmethod", NoClassmethodDecorator.into()),
+        MethodType::Staticmethod => ("staticmethod", NoStaticmethodDecorator.into()),
+    };
+
+    // gather all explicit *method calls
+    for stmt in &class_def.body {
+        if let Stmt::Assign(ast::StmtAssign { targets, value, .. }) = stmt {
+            if let Expr::Call(ast::ExprCall {
+                func, arguments, ..
+            }) = value.as_ref()
+            {
+                if let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() {
+                    if id == method_name && checker.semantic().is_builtin(method_name) {
+                        if arguments.args.len() != 1 {
+                            continue;
+                        }
+
+                        if targets.len() != 1 {
+                            continue;
+                        }
+
+                        let target_name = match targets.first() {
+                            Some(Expr::Name(ast::ExprName { id, .. })) => id.to_string(),
+                            _ => continue,
+                        };
+
+                        if let Expr::Name(ast::ExprName { id, .. }) = &arguments.args[0] {
+                            if target_name == *id {
+                                explicit_decorator_calls.insert(id.clone(), stmt.range());
+                            }
+                        };
+                    }
+                }
+            }
+        };
+    }
+
+    if explicit_decorator_calls.is_empty() {
+        return;
+    };
+
+    for stmt in &class_def.body {
+        if let Stmt::FunctionDef(ast::StmtFunctionDef {
+            name,
+            decorator_list,
+            ..
+        }) = stmt
+        {
+            if !explicit_decorator_calls.contains_key(name.as_str()) {
+                continue;
+            };
+
+            // if we find the decorator we're looking for, skip
+            if decorator_list.iter().any(|decorator| {
+                if let Expr::Name(ast::ExprName { id, .. }) = &decorator.expression {
+                    if id == method_name && checker.semantic().is_builtin(method_name) {
+                        return true;
+                    }
+                }
+
+                false
+            }) {
+                continue;
+            }
+
+            let mut diagnostic = Diagnostic::new(
+                diagnostic_type.clone(),
+                TextRange::new(stmt.range().start(), stmt.range().start()),
+            );
+
+            let indentation = indentation_at_offset(stmt.range().start(), checker.locator());
+
+            match indentation {
+                Some(indentation) => {
+                    let range = &explicit_decorator_calls[name.as_str()];
+
+                    // SAFETY: Ruff only supports formatting files <= 4GB
+                    #[allow(clippy::cast_possible_truncation)]
+                    diagnostic.set_fix(Fix::safe_edits(
+                        Edit::insertion(
+                            format!("@{method_name}\n{indentation}"),
+                            stmt.range().start(),
+                        ),
+                        [Edit::deletion(
+                            range.start() - TextSize::from(indentation.len() as u32),
+                            range.end(),
+                        )],
+                    ));
+                    checker.diagnostics.push(diagnostic);
+                }
+                None => {
+                    continue;
+                }
+            };
+        };
+    }
+}

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0202_no_method_decorator.py.snap

@@ -0,0 +1,31 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+no_method_decorator.py:9:5: PLR0202 [*] Class method defined without decorator
+   |
+ 7 |         self.color = color
+ 8 | 
+ 9 |     def pick_colors(cls, *args):  # [no-classmethod-decorator]
+   |      PLR0202
+10 |         """classmethod to pick fruit colors"""
+11 |         cls.COLORS = args
+   |
+   = help: Add @classmethod decorator
+
+ℹ Safe fix
+6  6  |     def __init__(self, color):
+7  7  |         self.color = color
+8  8  | 
+   9  |+    @classmethod
+9  10 |     def pick_colors(cls, *args):  # [no-classmethod-decorator]
+10 11 |         """classmethod to pick fruit colors"""
+11 12 |         cls.COLORS = args
+12 13 | 
+13    |-    pick_colors = classmethod(pick_colors)
+14 14 | 
+   15 |+
+15 16 |     def pick_one_color():  # [no-staticmethod-decorator]
+16 17 |         """staticmethod to pick one fruit color"""
+17 18 |         return choice(Fruit.COLORS)
+
+

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0203_no_method_decorator.py.snap

@@ -0,0 +1,27 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+no_method_decorator.py:15:5: PLR0203 [*] Static method defined without decorator
+   |
+13 |     pick_colors = classmethod(pick_colors)
+14 | 
+15 |     def pick_one_color():  # [no-staticmethod-decorator]
+   |      PLR0203
+16 |         """staticmethod to pick one fruit color"""
+17 |         return choice(Fruit.COLORS)
+   |
+   = help: Add @staticmethod decorator
+
+ℹ Safe fix
+12 12 | 
+13 13 |     pick_colors = classmethod(pick_colors)
+14 14 | 
+   15 |+    @staticmethod
+15 16 |     def pick_one_color():  # [no-staticmethod-decorator]
+16 17 |         """staticmethod to pick one fruit color"""
+17 18 |         return choice(Fruit.COLORS)
+18 19 | 
+19    |-    pick_one_color = staticmethod(pick_one_color)
+   20 |+
+
+