Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] Implement bad-staticmethod-argument (PLW0211) #10781

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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
15 changes: 10 additions & 5 deletions crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
Expand Up @@ -15,35 +15,36 @@ 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,
Rule::TypingOnlyThirdPartyImport,
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;
}
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 7 additions & 6 deletions crates/ruff_linter/src/codes.rs
Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Expand Up @@ -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(
Expand Down
103 changes: 103 additions & 0 deletions 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<Diagnostic>,
) {
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);
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
@@ -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
|
2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.