Skip to content

Commit

Permalink
implement bad-staticmethod-argument
Browse files Browse the repository at this point in the history
  • Loading branch information
augustelalande committed Apr 4, 2024
1 parent fd8da66 commit 7f1d1b2
Show file tree
Hide file tree
Showing 8 changed files with 196 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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::Stable, 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
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ 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
102 changes: 102 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
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, ScopeKind};
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 of
/// 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 ScopeKind::Function(ast::StmtFunctionDef {
name,
parameters,
decorator_list,
..
}) = &scope.kind
else {
panic!("Expected ScopeKind::Function")
};

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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 7f1d1b2

Please sign in to comment.