Skip to content

Commit

Permalink
Only rename references to the argument binding
Browse files Browse the repository at this point in the history
  • Loading branch information
@GtrMo
GtrMo committed Mar 3, 2024
1 parent 5b3708b commit 5010761
Show file tree
Hide file tree
Showing 7 changed files with 574 additions and 70 deletions.
85 changes: 56 additions & 29 deletions crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use ruff_python_ast as ast;
use ruff_python_ast::ParameterWithDefault;

use ruff_diagnostics::{Diagnostic, Edit, Violation};
use ruff_diagnostics::{DiagnosticKind, Fix};
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::analyze::function_type;
use ruff_python_semantic::{Scope, ScopeKind};
use ruff_python_semantic::{Scope, ScopeKind, SemanticModel};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -148,7 +147,7 @@ impl Argumentable {
}
}

fn valid_argument_name(self) -> &'static str {
const fn valid_argument_name(self) -> &'static str {
match self {
Self::Method => "self",
Self::ClassMethod => "cls",
Expand All @@ -172,7 +171,6 @@ pub(crate) fn invalid_first_argument_name(
let ScopeKind::Function(ast::StmtFunctionDef {
name,
parameters,
// body,
decorator_list,
..
}) = &scope.kind
Expand Down Expand Up @@ -202,44 +200,73 @@ pub(crate) fn invalid_first_argument_name(
return;
}

let Some(ParameterWithDefault { parameter, .. }) = parameters
let Some(ParameterWithDefault {
parameter: first_parameter,
..
}) = parameters
.posonlyargs
.first()
.or_else(|| parameters.args.first())
else {
return;
};

if &parameter.name == argumentable.valid_argument_name() {
if &first_parameter.name == argumentable.valid_argument_name() {
return;
}
if checker.settings.pep8_naming.ignore_names.matches(name) {
return;
}

let fix = if let Some(bid) = scope.get(&parameter.name) {
let binding = checker.semantic().binding(bid);
let replacement = argumentable.valid_argument_name();
let fix = Fix::unsafe_edits(
Edit::range_replacement(replacement.to_string(), binding.range()),
binding
.references()
.map(|rid| checker.semantic().reference(rid))
.map(|reference| {
Edit::range_replacement(replacement.to_string(), reference.range())
}),
);
Some(fix)
} else {
None
};

let mut diagnostic = Diagnostic::new(
argumentable.check_for(parameter.name.to_string()),
parameter.range(),
argumentable.check_for(first_parameter.name.to_string()),
first_parameter.range(),
);
if let Some(fix) = fix {
diagnostic.set_fix(fix);
}
diagnostic.try_set_optional_fix(|| {
Ok(try_fix(
scope,
first_parameter,
parameters,
checker.semantic(),
argumentable,
))
});
diagnostics.push(diagnostic);
}

fn try_fix(
scope: &Scope<'_>,
first_parameter: &ast::Parameter,
parameters: &ast::Parameters,
semantic: &SemanticModel<'_>,
argumentable: Argumentable,
) -> Option<Fix> {
// Don't fix if another parameter has the valid name.
if let Some(_) = parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.skip(1)
.map(|parameter_with_default| &parameter_with_default.parameter)
.chain(parameters.vararg.as_deref().into_iter())
.chain(parameters.kwarg.as_deref().into_iter())
.find(|p| &p.name == argumentable.valid_argument_name())
{
return None;
}

let binding = scope
.get_all(&first_parameter.name)
.map(|id| semantic.binding(id))
.find(|b| b.kind.is_argument())?;
let replacement = argumentable.valid_argument_name();
let fix = Fix::unsafe_edits(
Edit::range_replacement(replacement.to_string(), binding.range()),
binding
.references()
.map(|rid| semantic.reference(rid))
.map(|reference| Edit::range_replacement(replacement.to_string(), reference.range())),
);
Some(fix)
}
74 changes: 69 additions & 5 deletions ...src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap
View file
Original file line number Diff line number Diff line change
@@ -1,30 +1,63 @@
---
source: crates/ruff_linter/src/rules/pep8_naming/mod.rs
---
N804.py:30:27: N804 First argument of a class method should be named `cls`
N804.py:30:27: N804 [*] First argument of a class method should be named `cls`
|
28 | ...
29 |
30 | def __init_subclass__(self, default_name, **kwargs):
| ^^^^ N804
31 | ...
|
= help: Rename `self` to `cls`

N804.py:38:56: N804 First argument of a class method should be named `cls`
ℹ Unsafe fix
27 27 | def __new__(cls, *args, **kwargs):
28 28 | ...
29 29 |
30 |- def __init_subclass__(self, default_name, **kwargs):
30 |+ def __init_subclass__(cls, default_name, **kwargs):
31 31 | ...
32 32 |
33 33 | @classmethod

N804.py:38:56: N804 [*] First argument of a class method should be named `cls`
|
37 | @classmethod
38 | def bad_class_method_with_positional_only_argument(self, x, /, other):
| ^^^^ N804
39 | ...
|
= help: Rename `self` to `cls`

ℹ Unsafe fix
35 35 | ...
36 36 |
37 37 | @classmethod
38 |- def bad_class_method_with_positional_only_argument(self, x, /, other):
38 |+ def bad_class_method_with_positional_only_argument(cls, x, /, other):
39 39 | ...
40 40 |
41 41 |

N804.py:43:20: N804 First argument of a class method should be named `cls`
N804.py:43:20: N804 [*] First argument of a class method should be named `cls`
|
42 | class MetaClass(ABCMeta):
43 | def bad_method(self):
| ^^^^ N804
44 | pass
|
= help: Rename `self` to `cls`

ℹ Unsafe fix
40 40 |
41 41 |
42 42 | class MetaClass(ABCMeta):
43 |- def bad_method(self):
43 |+ def bad_method(cls):
44 44 | pass
45 45 |
46 46 | def good_method(cls):

N804.py:54:25: N804 First argument of a class method should be named `cls`
|
Expand All @@ -33,6 +66,7 @@ N804.py:54:25: N804 First argument of a class method should be named `cls`
| ^^^^ N804
55 | pass
|
= help: Rename `this` to `cls`

N804.py:57:34: N804 First argument of a class method should be named `cls`
|
Expand All @@ -42,6 +76,7 @@ N804.py:57:34: N804 First argument of a class method should be named `cls`
| ^^^^ N804
58 | pass
|
= help: Rename `this` to `cls`

N804.py:60:33: N804 First argument of a class method should be named `cls`
|
Expand All @@ -51,6 +86,7 @@ N804.py:60:33: N804 First argument of a class method should be named `cls`
| ^^^^ N804
61 | pass
|
= help: Rename `this` to `cls`

N804.py:63:23: N804 First argument of a class method should be named `cls`
|
Expand All @@ -60,6 +96,7 @@ N804.py:63:23: N804 First argument of a class method should be named `cls`
| ^^^^ N804
64 | pass
|
= help: Rename `this` to `cls`

N804.py:66:23: N804 First argument of a class method should be named `cls`
|
Expand All @@ -69,21 +106,48 @@ N804.py:66:23: N804 First argument of a class method should be named `cls`
| ^^^^ N804
67 | pass
|
= help: Rename `this` to `cls`

N804.py:70:20: N804 First argument of a class method should be named `cls`
N804.py:70:20: N804 [*] First argument of a class method should be named `cls`
|
69 | class RenamingInMethodBodyClass(ABCMeta):
70 | def bad_method(this):
| ^^^^ N804
71 | this = this
72 | this
|
= help: Rename `this` to `cls`

N804.py:74:20: N804 First argument of a class method should be named `cls`
ℹ Unsafe fix
67 67 | pass
68 68 |
69 69 | class RenamingInMethodBodyClass(ABCMeta):
70 |- def bad_method(this):
71 |- this = this
70 |+ def bad_method(cls):
71 |+ this = cls
72 72 | this
73 73 |
74 74 | def bad_method(this):

N804.py:74:20: N804 [*] First argument of a class method should be named `cls`
|
72 | this
73 |
74 | def bad_method(this):
| ^^^^ N804
75 | self = this
|
= help: Rename `this` to `cls`

ℹ Unsafe fix
71 71 | this = this
72 72 | this
73 73 |
74 |- def bad_method(this):
75 |- self = this
74 |+ def bad_method(cls):
75 |+ self = cls
76 76 |
77 77 | def func(x):
78 78 | return x
Loading

0 comments on commit 5010761

Please sign in to comment.