diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/bad_staticmethod_argument.py b/crates/ruff_linter/resources/test/fixtures/pylint/bad_staticmethod_argument.py new file mode 100644 index 0000000000000..6de4d74ee485f --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/bad_staticmethod_argument.py @@ -0,0 +1,44 @@ +class Wolf: + @staticmethod + def eat(self): # [bad-staticmethod-argument] + pass + + +class Wolf: + @staticmethod + def eat(sheep): + pass + + +class Sheep: + @staticmethod + def eat(cls, x, y, z): # [bad-staticmethod-argument] + pass + + @staticmethod + def sleep(self, x, y, z): # [bad-staticmethod-argument] + pass + + def grow(self, x, y, z): + pass + + @classmethod + def graze(cls, x, y, z): + pass + + +class Foo: + @staticmethod + def eat(x, self, z): + pass + + @staticmethod + def sleep(x, cls, z): + pass + + def grow(self, x, y, z): + pass + + @classmethod + def graze(cls, x, y, z): + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index 0a85c041b0bdf..c4d0ee7944b78 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -15,15 +15,19 @@ use crate::rules::{ pub(crate) fn deferred_scopes(checker: &mut Checker) { if !checker.any_enabled(&[ Rule::AsyncioDanglingTask, + Rule::BadStaticmethodArgument, + Rule::BuiltinAttributeShadowing, Rule::GlobalVariableNotAssigned, Rule::ImportPrivateName, Rule::ImportShadowedByLoopVar, - Rule::InvalidFirstArgumentNameForMethod, Rule::InvalidFirstArgumentNameForClassMethod, + Rule::InvalidFirstArgumentNameForMethod, Rule::NoSelfUse, Rule::RedefinedArgumentFromLocal, Rule::RedefinedWhileUnused, Rule::RuntimeImportInTypeCheckingBlock, + Rule::SingledispatchMethod, + Rule::SingledispatchmethodFunction, Rule::TooManyLocals, Rule::TypingOnlyFirstPartyImport, Rule::TypingOnlyStandardLibraryImport, @@ -31,19 +35,16 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { Rule::UndefinedLocal, Rule::UnusedAnnotation, Rule::UnusedClassMethodArgument, - Rule::BuiltinAttributeShadowing, Rule::UnusedFunctionArgument, Rule::UnusedImport, Rule::UnusedLambdaArgument, Rule::UnusedMethodArgument, Rule::UnusedPrivateProtocol, Rule::UnusedPrivateTypeAlias, - Rule::UnusedPrivateTypeVar, Rule::UnusedPrivateTypedDict, + Rule::UnusedPrivateTypeVar, Rule::UnusedStaticMethodArgument, Rule::UnusedVariable, - Rule::SingledispatchMethod, - Rule::SingledispatchmethodFunction, ]) { return; } @@ -424,6 +425,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { pylint::rules::singledispatchmethod_function(checker, scope, &mut diagnostics); } + if checker.enabled(Rule::BadStaticmethodArgument) { + pylint::rules::bad_staticmethod_argument(checker, scope, &mut diagnostics); + } + if checker.any_enabled(&[ Rule::InvalidFirstArgumentNameForClassMethod, Rule::InvalidFirstArgumentNameForMethod, diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 75cb2966bc30f..10a7fb752592f 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -225,12 +225,12 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "C0208") => (RuleGroup::Stable, rules::pylint::rules::IterationOverSet), (Pylint, "C0414") => (RuleGroup::Stable, rules::pylint::rules::UselessImportAlias), (Pylint, "C0415") => (RuleGroup::Preview, rules::pylint::rules::ImportOutsideTopLevel), - (Pylint, "C2401") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiName), - (Pylint, "C2403") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiImportName), - (Pylint, "C2801") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDunderCall), #[allow(deprecated)] (Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString), + (Pylint, "C2401") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiName), + (Pylint, "C2403") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiImportName), (Pylint, "C2701") => (RuleGroup::Preview, rules::pylint::rules::ImportPrivateName), + (Pylint, "C2801") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDunderCall), (Pylint, "C3002") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryDirectLambdaCall), (Pylint, "E0100") => (RuleGroup::Stable, rules::pylint::rules::YieldInInit), (Pylint, "E0101") => (RuleGroup::Stable, rules::pylint::rules::ReturnInInit), @@ -272,6 +272,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (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, "R0904") => (RuleGroup::Preview, rules::pylint::rules::TooManyPublicMethods), (Pylint, "R0911") => (RuleGroup::Stable, rules::pylint::rules::TooManyReturnStatements), (Pylint, "R0912") => (RuleGroup::Stable, rules::pylint::rules::TooManyBranches), (Pylint, "R0913") => (RuleGroup::Stable, rules::pylint::rules::TooManyArguments), @@ -282,9 +283,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "R1701") => (RuleGroup::Stable, rules::pylint::rules::RepeatedIsinstanceCalls), (Pylint, "R1702") => (RuleGroup::Preview, rules::pylint::rules::TooManyNestedBlocks), (Pylint, "R1704") => (RuleGroup::Preview, rules::pylint::rules::RedefinedArgumentFromLocal), + (Pylint, "R1706") => (RuleGroup::Removed, rules::pylint::rules::AndOrTernary), (Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn), (Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison), - (Pylint, "R1706") => (RuleGroup::Removed, rules::pylint::rules::AndOrTernary), (Pylint, "R1722") => (RuleGroup::Stable, rules::pylint::rules::SysExitAlias), (Pylint, "R1733") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDictIndexLookup), (Pylint, "R1736") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryListIndexLookup), @@ -302,11 +303,12 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W0129") => (RuleGroup::Stable, rules::pylint::rules::AssertOnStringLiteral), (Pylint, "W0131") => (RuleGroup::Stable, rules::pylint::rules::NamedExprWithoutContext), (Pylint, "W0133") => (RuleGroup::Preview, rules::pylint::rules::UselessExceptionStatement), + (Pylint, "W0211") => (RuleGroup::Preview, rules::pylint::rules::BadStaticmethodArgument), (Pylint, "W0245") => (RuleGroup::Preview, rules::pylint::rules::SuperWithoutBrackets), (Pylint, "W0406") => (RuleGroup::Stable, rules::pylint::rules::ImportSelf), (Pylint, "W0602") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableNotAssigned), - (Pylint, "W0604") => (RuleGroup::Preview, rules::pylint::rules::GlobalAtModuleLevel), (Pylint, "W0603") => (RuleGroup::Stable, rules::pylint::rules::GlobalStatement), + (Pylint, "W0604") => (RuleGroup::Preview, rules::pylint::rules::GlobalAtModuleLevel), (Pylint, "W0711") => (RuleGroup::Stable, rules::pylint::rules::BinaryOpException), (Pylint, "W1501") => (RuleGroup::Preview, rules::pylint::rules::BadOpenMode), (Pylint, "W1508") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarDefault), @@ -316,7 +318,6 @@ 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, "R0904") => (RuleGroup::Preview, rules::pylint::rules::TooManyPublicMethods), (Pylint, "W2901") => (RuleGroup::Stable, rules::pylint::rules::RedefinedLoopName), #[allow(deprecated)] (Pylint, "W3201") => (RuleGroup::Nursery, rules::pylint::rules::BadDunderMethodName), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 86d3d2e897e37..867e6dfc9596b 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -190,6 +190,10 @@ mod tests { Path::new("useless_exception_statement.py") )] #[test_case(Rule::NanComparison, Path::new("nan_comparison.py"))] + #[test_case( + Rule::BadStaticmethodArgument, + Path::new("bad_staticmethod_argument.py") + )] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs new file mode 100644 index 0000000000000..c99cd52c8cc1b --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs @@ -0,0 +1,103 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_ast::ParameterWithDefault; +use ruff_python_semantic::analyze::function_type; +use ruff_python_semantic::Scope; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for static methods that use `self` or `cls` as their first argument. +/// +/// ## Why is this bad? +/// [PEP 8] recommends the use of `self` and `cls` as the first arguments for +/// instance methods and class methods, respectively. Naming the first argument +/// of a static method as `self` or `cls` can be misleading, as static methods +/// do not receive an instance or class reference as their first argument. +/// +/// ## Example +/// ```python +/// class Wolf: +/// @staticmethod +/// def eat(self): +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// class Wolf: +/// @staticmethod +/// def eat(sheep): +/// pass +/// ``` +/// +/// [PEP 8]: https://peps.python.org/pep-0008/#function-and-method-arguments +#[violation] +pub struct BadStaticmethodArgument { + argument_name: String, +} + +impl Violation for BadStaticmethodArgument { + #[derive_message_formats] + fn message(&self) -> String { + let Self { argument_name } = self; + format!("First argument of a static method should not be named `{argument_name}`") + } +} + +/// PLW0211 +pub(crate) fn bad_staticmethod_argument( + checker: &Checker, + scope: &Scope, + diagnostics: &mut Vec, +) { + let Some(func) = scope.kind.as_function() else { + return; + }; + + let ast::StmtFunctionDef { + name, + decorator_list, + parameters, + .. + } = func; + + let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + return; + }; + + let type_ = function_type::classify( + name, + decorator_list, + parent, + checker.semantic(), + &checker.settings.pep8_naming.classmethod_decorators, + &checker.settings.pep8_naming.staticmethod_decorators, + ); + if !matches!(type_, function_type::FunctionType::StaticMethod) { + return; + } + + let Some(ParameterWithDefault { + parameter: self_or_cls, + .. + }) = parameters + .posonlyargs + .first() + .or_else(|| parameters.args.first()) + else { + return; + }; + + if matches!(self_or_cls.name.as_str(), "self" | "cls") { + let diagnostic = Diagnostic::new( + BadStaticmethodArgument { + argument_name: self_or_cls.name.to_string(), + }, + self_or_cls.range(), + ); + diagnostics.push(diagnostic); + } +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 99bbb5063efdb..93f89e1f6cec0 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -3,6 +3,7 @@ pub(crate) use assert_on_string_literal::*; pub(crate) use await_outside_async::*; pub(crate) use bad_dunder_method_name::*; pub(crate) use bad_open_mode::*; +pub(crate) use bad_staticmethod_argument::*; pub(crate) use bad_str_strip_call::*; pub(crate) use bad_string_format_character::BadStringFormatCharacter; pub(crate) use bad_string_format_type::*; @@ -97,6 +98,7 @@ mod assert_on_string_literal; mod await_outside_async; mod bad_dunder_method_name; mod bad_open_mode; +mod bad_staticmethod_argument; mod bad_str_strip_call; pub(crate) mod bad_string_format_character; mod bad_string_format_type; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0211_bad_staticmethod_argument.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0211_bad_staticmethod_argument.py.snap new file mode 100644 index 0000000000000..add63e311b094 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0211_bad_staticmethod_argument.py.snap @@ -0,0 +1,28 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +bad_staticmethod_argument.py:3:13: PLW0211 First argument of a static method should not be named `self` + | +1 | class Wolf: +2 | @staticmethod +3 | def eat(self): # [bad-staticmethod-argument] + | ^^^^ PLW0211 +4 | pass + | + +bad_staticmethod_argument.py:15:13: PLW0211 First argument of a static method should not be named `cls` + | +13 | class Sheep: +14 | @staticmethod +15 | def eat(cls, x, y, z): # [bad-staticmethod-argument] + | ^^^ PLW0211 +16 | pass + | + +bad_staticmethod_argument.py:19:15: PLW0211 First argument of a static method should not be named `self` + | +18 | @staticmethod +19 | def sleep(self, x, y, z): # [bad-staticmethod-argument] + | ^^^^ PLW0211 +20 | pass + | diff --git a/ruff.schema.json b/ruff.schema.json index f6d36e1cfc51a..4aa04519ad805 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3389,6 +3389,8 @@ "PLW0131", "PLW0133", "PLW02", + "PLW021", + "PLW0211", "PLW024", "PLW0245", "PLW04",