Skip to content

Commit

Permalink
PLR0203: Delete entire statement, including semicolons (#10074)
Browse files Browse the repository at this point in the history
Co-authored-by: Micha Reiser <[email protected]>
  • Loading branch information
sanxiyn and MichaReiser authored Feb 22, 2024
1 parent ecd5a70 commit 7d9ce50
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,14 @@ def pick_one_color(): # [no-staticmethod-decorator]
return choice(Fruit.COLORS)

pick_one_color = staticmethod(pick_one_color)

class Class:
def class_method(cls):
pass

class_method = classmethod(class_method);another_statement

def static_method():
pass

static_method = staticmethod(static_method);
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,10 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
},
) => {
if checker.enabled(Rule::NoClassmethodDecorator) {
pylint::rules::no_classmethod_decorator(checker, class_def);
pylint::rules::no_classmethod_decorator(checker, stmt);
}
if checker.enabled(Rule::NoStaticmethodDecorator) {
pylint::rules::no_staticmethod_decorator(checker, class_def);
pylint::rules::no_staticmethod_decorator(checker, stmt);
}
if checker.enabled(Rule::DjangoNullableModelStringField) {
flake8_django::rules::nullable_model_string_field(checker, body);
Expand Down
39 changes: 19 additions & 20 deletions crates/ruff_linter/src/rules/pylint/rules/no_method_decorator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, DiagnosticKind, Edit,
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 ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
use crate::fix;

/// ## What it does
/// Checks for the use of a classmethod being made without the decorator.
Expand Down Expand Up @@ -86,21 +87,21 @@ enum MethodType {
}

/// PLR0202
pub(crate) fn no_classmethod_decorator(checker: &mut Checker, class_def: &ast::StmtClassDef) {
get_undecorated_methods(checker, class_def, &MethodType::Classmethod);
pub(crate) fn no_classmethod_decorator(checker: &mut Checker, stmt: &Stmt) {
get_undecorated_methods(checker, stmt, &MethodType::Classmethod);
}

/// PLR0203
pub(crate) fn no_staticmethod_decorator(checker: &mut Checker, class_def: &ast::StmtClassDef) {
get_undecorated_methods(checker, class_def, &MethodType::Staticmethod);
pub(crate) fn no_staticmethod_decorator(checker: &mut Checker, stmt: &Stmt) {
get_undecorated_methods(checker, stmt, &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();
fn get_undecorated_methods(checker: &mut Checker, class_stmt: &Stmt, method_type: &MethodType) {
let Stmt::ClassDef(class_def) = class_stmt else {
return;
};

let mut explicit_decorator_calls: HashMap<String, &Stmt> = HashMap::default();

let (method_name, diagnostic_type): (&str, DiagnosticKind) = match method_type {
MethodType::Classmethod => ("classmethod", NoClassmethodDecorator.into()),
Expand Down Expand Up @@ -131,7 +132,7 @@ fn get_undecorated_methods(

if let Expr::Name(ast::ExprName { id, .. }) = &arguments.args[0] {
if target_name == *id {
explicit_decorator_calls.insert(id.clone(), stmt.range());
explicit_decorator_calls.insert(id.clone(), stmt);
}
};
}
Expand All @@ -151,7 +152,7 @@ fn get_undecorated_methods(
..
}) = stmt
{
if !explicit_decorator_calls.contains_key(name.as_str()) {
let Some(decorator_call_statement) = explicit_decorator_calls.get(name.as_str()) else {
continue;
};

Expand All @@ -177,18 +178,16 @@ fn get_undecorated_methods(

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(),
[fix::edits::delete_stmt(
decorator_call_statement,
Some(class_stmt),
checker.locator(),
checker.indexer(),
)],
));
checker.diagnostics.push(diagnostic);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,28 @@ no_method_decorator.py:9:5: PLR0202 [*] Class method defined without decorator
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)
15 15 | def pick_one_color(): # [no-staticmethod-decorator]
16 16 | """staticmethod to pick one fruit color"""

no_method_decorator.py:22:5: PLR0202 [*] Class method defined without decorator
|
21 | class Class:
22 | def class_method(cls):
| PLR0202
23 | pass
|
= help: Add @classmethod decorator

Safe fix
19 19 | pick_one_color = staticmethod(pick_one_color)
20 20 |
21 21 | class Class:
22 |+ @classmethod
22 23 | def class_method(cls):
23 24 | pass
24 25 |
25 |- class_method = classmethod(class_method);another_statement
26 |+ another_statement
26 27 |
27 28 | def static_method():
28 29 | pass
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,27 @@ no_method_decorator.py:15:5: PLR0203 [*] Static method defined without decorator
17 18 | return choice(Fruit.COLORS)
18 19 |
19 |- pick_one_color = staticmethod(pick_one_color)
20 |+
20 20 |
21 21 | class Class:
22 22 | def class_method(cls):

no_method_decorator.py:27:5: PLR0203 [*] Static method defined without decorator
|
25 | class_method = classmethod(class_method);another_statement
26 |
27 | def static_method():
| PLR0203
28 | pass
|
= help: Add @staticmethod decorator

Safe fix
24 24 |
25 25 | class_method = classmethod(class_method);another_statement
26 26 |
27 |+ @staticmethod
27 28 | def static_method():
28 29 | pass
29 30 |
30 |- static_method = staticmethod(static_method);
31 |+

0 comments on commit 7d9ce50

Please sign in to comment.