[`pylint`] Implement `no-self-use` (`R6301`) (#6574)

crates/ruff/resources/test/fixtures/pylint/no_self_use.py

@@ -0,0 +1,62 @@
+import abc
+
+
+class Person:
+    def developer_greeting(self, name):  # [no-self-use]
+        print(f"Greetings {name}!")
+
+    def greeting_1(self):  # [no-self-use]
+        print("Hello!")
+
+    def greeting_2(self):  # [no-self-use]
+        print("Hi!")
+
+
+# OK
+def developer_greeting():
+    print("Greetings developer!")
+
+
+# OK
+class Person:
+    name = "Paris"
+
+    def __init__(self):
+        pass
+
+    def __cmp__(self, other):
+        print(24)
+
+    def __repr__(self):
+        return "Person"
+
+    def func(self):
+        ...
+
+    def greeting_1(self):
+        print(f"Hello from {self.name} !")
+
+    @staticmethod
+    def greeting_2():
+        print("Hi!")
+
+
+class Base(abc.ABC):
+    """abstract class"""
+
+    @abstractmethod
+    def abstract_method(self):
+        """abstract method could not be a function"""
+        raise NotImplementedError
+
+
+class Sub(Base):
+    @override
+    def abstract_method(self):
+        print("concret method")
+
+
+class Prop:
+    @property
+    def count(self):
+        return 24

crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs

@@ -30,6 +30,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
         Rule::UnusedPrivateTypedDict,
         Rule::UnusedStaticMethodArgument,
         Rule::UnusedVariable,
+        Rule::NoSelfUse,
     ]) {
         return;
     }
@@ -302,6 +303,12 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
                 pyflakes::rules::unused_import(checker, scope, &mut diagnostics);
             }
         }
+
+        if scope.kind.is_function() {
+            if checker.enabled(Rule::NoSelfUse) {
+                pylint::rules::no_self_use(checker, scope, &mut diagnostics);
+            }
+        }
     }
     checker.diagnostics.extend(diagnostics);
 }

crates/ruff/src/codes.rs

@@ -216,6 +216,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "R1722") => (RuleGroup::Unspecified, rules::pylint::rules::SysExitAlias),
         (Pylint, "R2004") => (RuleGroup::Unspecified, rules::pylint::rules::MagicValueComparison),
         (Pylint, "R5501") => (RuleGroup::Unspecified, rules::pylint::rules::CollapsibleElseIf),
+        (Pylint, "R6301") => (RuleGroup::Nursery, rules::pylint::rules::NoSelfUse),
         (Pylint, "W0120") => (RuleGroup::Unspecified, rules::pylint::rules::UselessElseOnLoop),
         (Pylint, "W0127") => (RuleGroup::Unspecified, rules::pylint::rules::SelfAssigningVariable),
         (Pylint, "W0129") => (RuleGroup::Unspecified, rules::pylint::rules::AssertOnStringLiteral),

crates/ruff/src/rules/flake8_unused_arguments/mod.rs

@@ -1,5 +1,5 @@
 //! Rules from [flake8-unused-arguments](https://pypi.org/project/flake8-unused-arguments/).
-mod helpers;
+pub(crate) mod helpers;
 pub(crate) mod rules;
 pub mod settings;
 

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

@@ -132,6 +132,7 @@ mod tests {
         Path::new("subprocess_run_without_check.py")
     )]
     #[test_case(Rule::BadDunderMethodName, Path::new("bad_dunder_method_name.py"))]
+    #[test_case(Rule::NoSelfUse, Path::new("no_self_use.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/src/rules/pylint/rules/mod.rs

@@ -29,6 +29,7 @@ pub(crate) use magic_value_comparison::*;
 pub(crate) use manual_import_from::*;
 pub(crate) use named_expr_without_context::*;
 pub(crate) use nested_min_max::*;
+pub(crate) use no_self_use::*;
 pub(crate) use nonlocal_without_binding::*;
 pub(crate) use property_with_parameters::*;
 pub(crate) use redefined_loop_name::*;
@@ -86,6 +87,7 @@ mod magic_value_comparison;
 mod manual_import_from;
 mod named_expr_without_context;
 mod nested_min_max;
+mod no_self_use;
 mod nonlocal_without_binding;
 mod property_with_parameters;
 mod redefined_loop_name;

crates/ruff/src/rules/pylint/rules/no_self_use.rs

@@ -0,0 +1,120 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::call_path::{from_qualified_name, CallPath};
+use ruff_python_ast::{self as ast, ParameterWithDefault, Ranged};
+use ruff_python_semantic::{
+    analyze::{function_type, visibility},
+    Scope, ScopeKind,
+};
+
+use crate::{checkers::ast::Checker, rules::flake8_unused_arguments::helpers};
+
+/// ## What it does
+/// Checks for the presence of unused `self` parameter in methods definitions.
+///
+/// ## Why is this bad?
+/// Unused `self` parameters are usually a sign of a method that could be
+/// replaced by a function or a static method.
+///
+/// ## Example
+/// ```python
+/// class Person:
+///     def greeting(self):
+///         print("Greetings friend!")
+/// ```
+///
+/// Use instead:
+/// ```python
+/// class Person:
+///     @staticmethod
+///     def greeting():
+///         print(f"Greetings friend!")
+/// ```
+#[violation]
+pub struct NoSelfUse {
+    method_name: String,
+}
+
+impl Violation for NoSelfUse {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let NoSelfUse { method_name } = self;
+        format!("Method `{method_name}` could be a function or static method")
+    }
+}
+
+/// PLR6301
+pub(crate) fn no_self_use(checker: &Checker, scope: &Scope, diagnostics: &mut Vec<Diagnostic>) {
+    let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else {
+        return;
+    };
+
+    let ScopeKind::Function(ast::StmtFunctionDef {
+        name,
+        parameters,
+        body,
+        decorator_list,
+        ..
+    }) = scope.kind
+    else {
+        return;
+    };
+
+    if !matches!(
+        function_type::classify(
+            name,
+            decorator_list,
+            parent,
+            checker.semantic(),
+            &checker.settings.pep8_naming.classmethod_decorators,
+            &checker.settings.pep8_naming.staticmethod_decorators,
+        ),
+        function_type::FunctionType::Method
+    ) {
+        return;
+    }
+
+    let property_decorators = checker
+        .settings
+        .pydocstyle
+        .property_decorators
+        .iter()
+        .map(|decorator| from_qualified_name(decorator))
+        .collect::<Vec<CallPath>>();
+
+    if helpers::is_empty(body)
+        || visibility::is_magic(name)
+        || visibility::is_abstract(decorator_list, checker.semantic())
+        || visibility::is_override(decorator_list, checker.semantic())
+        || visibility::is_overload(decorator_list, checker.semantic())
+        || visibility::is_property(decorator_list, &property_decorators, checker.semantic())
+    {
+        return;
+    }
+
+    // Identify the `self` parameter.
+    let Some(parameter) = parameters
+        .posonlyargs
+        .iter()
+        .chain(&parameters.args)
+        .chain(&parameters.kwonlyargs)
+        .next()
+        .map(ParameterWithDefault::as_parameter)
+    else {
+        return;
+    };
+
+    if parameter.name.as_str() == "self"
+        && scope
+            .get("self")
+            .map(|binding_id| checker.semantic().binding(binding_id))
+            .is_some_and(|binding| binding.kind.is_argument() && !binding.is_used())
+    {
+        diagnostics.push(Diagnostic::new(
+            NoSelfUse {
+                method_name: name.to_string(),
+            },
+            parameter.range(),
+        ));
+    }
+}

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR6301_no_self_use.py.snap

@@ -0,0 +1,39 @@
+---
+source: crates/ruff/src/rules/pylint/mod.rs
+---
+no_self_use.py:5:28: PLR6301 Method `developer_greeting` could be a function or static method
+  |
+4 | class Person:
+5 |     def developer_greeting(self, name):  # [no-self-use]
+  |                            ^^^^ PLR6301
+6 |         print(f"Greetings {name}!")
+  |
+
+no_self_use.py:8:20: PLR6301 Method `greeting_1` could be a function or static method
+  |
+6 |         print(f"Greetings {name}!")
+7 | 
+8 |     def greeting_1(self):  # [no-self-use]
+  |                    ^^^^ PLR6301
+9 |         print("Hello!")
+  |
+
+no_self_use.py:11:20: PLR6301 Method `greeting_2` could be a function or static method
+   |
+ 9 |         print("Hello!")
+10 | 
+11 |     def greeting_2(self):  # [no-self-use]
+   |                    ^^^^ PLR6301
+12 |         print("Hi!")
+   |
+
+no_self_use.py:55:25: PLR6301 Method `abstract_method` could be a function or static method
+   |
+53 | class Sub(Base):
+54 |     @override
+55 |     def abstract_method(self):
+   |                         ^^^^ PLR6301
+56 |         print("concret method")
+   |
+
+

ruff.schema.json

@@ -2270,6 +2270,7 @@
         "PLR55",
         "PLR550",
         "PLR5501",
+        "PLR6301",
         "PLW",
         "PLW0",
         "PLW01",