From 46d09f582a8ad084c37dc101c23afe9d53ce7b0d Mon Sep 17 00:00:00 2001 From: Gautier Moin Date: Sun, 3 Mar 2024 14:46:51 +0100 Subject: [PATCH 1/8] Merge invalid first argument logic, move to defered scopes --- .../checkers/ast/analyze/deferred_scopes.rs | 12 +- .../src/checkers/ast/analyze/statement.rs | 24 -- .../rules/invalid_first_argument_name.rs | 206 ++++++++++++++++++ ...id_first_argument_name_for_class_method.rs | 104 --------- .../invalid_first_argument_name_for_method.rs | 96 -------- .../src/rules/pep8_naming/rules/mod.rs | 6 +- 6 files changed, 218 insertions(+), 230 deletions(-) create mode 100644 crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs delete mode 100644 crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name_for_class_method.rs delete mode 100644 crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name_for_method.rs 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 c2d2dc1492357..9437f698e5ff4 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -7,8 +7,8 @@ use crate::checkers::ast::Checker; use crate::codes::Rule; use crate::fix; use crate::rules::{ - flake8_builtins, flake8_pyi, flake8_type_checking, flake8_unused_arguments, pyflakes, pylint, - ruff, + flake8_builtins, flake8_pyi, flake8_type_checking, flake8_unused_arguments, pep8_naming, + pyflakes, pylint, ruff, }; /// Run lint rules over all deferred scopes in the [`SemanticModel`]. @@ -18,6 +18,8 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { Rule::GlobalVariableNotAssigned, Rule::ImportPrivateName, Rule::ImportShadowedByLoopVar, + Rule::InvalidFirstArgumentNameForMethod, + Rule::InvalidFirstArgumentNameForClassMethod, Rule::NoSelfUse, Rule::RedefinedArgumentFromLocal, Rule::RedefinedWhileUnused, @@ -394,6 +396,12 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { if checker.enabled(Rule::SingledispatchMethod) { pylint::rules::singledispatch_method(checker, scope, &mut diagnostics); } + if checker.any_enabled(&[ + Rule::InvalidFirstArgumentNameForClassMethod, + Rule::InvalidFirstArgumentNameForMethod, + ]) { + pep8_naming::rules::invalid_first_argument_name(checker, scope, &mut diagnostics) + } } } checker.diagnostics.extend(diagnostics); diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index f8867e68cf5c3..ea1335bbc0cd1 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -105,30 +105,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } - if checker.enabled(Rule::InvalidFirstArgumentNameForClassMethod) { - if let Some(diagnostic) = - pep8_naming::rules::invalid_first_argument_name_for_class_method( - checker, - checker.semantic.current_scope(), - name, - decorator_list, - parameters, - ) - { - checker.diagnostics.push(diagnostic); - } - } - if checker.enabled(Rule::InvalidFirstArgumentNameForMethod) { - if let Some(diagnostic) = pep8_naming::rules::invalid_first_argument_name_for_method( - checker, - checker.semantic.current_scope(), - name, - decorator_list, - parameters, - ) { - checker.diagnostics.push(diagnostic); - } - } if checker.source_type.is_stub() { if checker.enabled(Rule::PassStatementStubBody) { flake8_pyi::rules::pass_statement_stub_body(checker, body); diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs new file mode 100644 index 0000000000000..1319deb9add92 --- /dev/null +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -0,0 +1,206 @@ +use ruff_python_ast as ast; +use ruff_python_ast::ParameterWithDefault; + +use ruff_diagnostics::DiagnosticKind; +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_semantic::analyze::function_type; +use ruff_python_semantic::{Scope, ScopeKind}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::registry::Rule; + +/// ## What it does +/// Checks for instance methods that use a name other than `self` for their +/// first argument. +/// +/// ## Why is this bad? +/// [PEP 8] recommends the use of `self` as first argument for all instance +/// methods: +/// +/// > Always use self for the first argument to instance methods. +/// > +/// > If a function argument’s name clashes with a reserved keyword, it is generally better to +/// > append a single trailing underscore rather than use an abbreviation or spelling corruption. +/// > Thus `class_` is better than `clss`. (Perhaps better is to avoid such clashes by using a synonym.) +/// +/// Names can be excluded from this rule using the [`lint.pep8-naming.ignore-names`] +/// or [`lint.pep8-naming.extend-ignore-names`] configuration options. For example, +/// to allow the use of `this` as the first argument to instance methods, set +/// the [`lint.pep8-naming.extend-ignore-names`] option to `["this"]`. +/// +/// ## Example +/// ```python +/// class Example: +/// def function(cls, data): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// class Example: +/// def function(self, data): +/// ... +/// ``` +/// +/// ## Options +/// - `lint.pep8-naming.classmethod-decorators` +/// - `lint.pep8-naming.staticmethod-decorators` +/// - `lint.pep8-naming.ignore-names` +/// - `lint.pep8-naming.extend-ignore-names` +/// +/// [PEP 8]: https://peps.python.org/pep-0008/#function-and-method-arguments +#[violation] +pub struct InvalidFirstArgumentNameForMethod { + argument_name: String, +} + +impl Violation for InvalidFirstArgumentNameForMethod { + #[derive_message_formats] + fn message(&self) -> String { + format!("First argument of a method should be named `self`") + } +} + +/// ## What it does +/// Checks for class methods that use a name other than `cls` for their +/// first argument. +/// +/// ## Why is this bad? +/// [PEP 8] recommends the use of `cls` as the first argument for all class +/// methods: +/// +/// > Always use `cls` for the first argument to class methods. +/// > +/// > If a function argument’s name clashes with a reserved keyword, it is generally better to +/// > append a single trailing underscore rather than use an abbreviation or spelling corruption. +/// > Thus `class_` is better than `clss`. (Perhaps better is to avoid such clashes by using a synonym.) +/// +/// Names can be excluded from this rule using the [`lint.pep8-naming.ignore-names`] +/// or [`lint.pep8-naming.extend-ignore-names`] configuration options. For example, +/// to allow the use of `klass` as the first argument to class methods, set +/// the [`lint.pep8-naming.extend-ignore-names`] option to `["klass"]`. +/// +/// ## Example +/// ```python +/// class Example: +/// @classmethod +/// def function(self, data): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// class Example: +/// @classmethod +/// def function(cls, data): +/// ... +/// ``` +/// +/// ## Options +/// - `lint.pep8-naming.classmethod-decorators` +/// - `lint.pep8-naming.staticmethod-decorators` +/// - `lint.pep8-naming.ignore-names` +/// - `lint.pep8-naming.extend-ignore-names` +/// +/// [PEP 8]: https://peps.python.org/pep-0008/#function-and-method-arguments +#[violation] +pub struct InvalidFirstArgumentNameForClassMethod { + argument_name: String, +} + +impl Violation for InvalidFirstArgumentNameForClassMethod { + #[derive_message_formats] + fn message(&self) -> String { + format!("First argument of a class method should be named `cls`") + } +} + +/// An AST node that can contain arguments. +#[derive(Debug, Copy, Clone)] +enum Argumentable { + Method, + ClassMethod, +} + +impl Argumentable { + fn check_for(self, argument_name: String) -> DiagnosticKind { + match self { + Self::Method => InvalidFirstArgumentNameForMethod { argument_name }.into(), + Self::ClassMethod => InvalidFirstArgumentNameForClassMethod { argument_name }.into(), + } + } + + fn valid_argument_name(self) -> &'static str { + match self { + Self::Method => "self", + Self::ClassMethod => "cls", + } + } + + const fn rule_code(self) -> Rule { + match self { + Self::Method => Rule::InvalidFirstArgumentNameForMethod, + Self::ClassMethod => Rule::InvalidFirstArgumentNameForClassMethod, + } + } +} + +/// N804, N805 +pub(crate) fn invalid_first_argument_name( + checker: &Checker, + scope: &Scope, + diagnostics: &mut Vec, +) { + 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 argumentable = match function_type::classify( + name, + decorator_list, + parent, + checker.semantic(), + &checker.settings.pep8_naming.classmethod_decorators, + &checker.settings.pep8_naming.staticmethod_decorators, + ) { + function_type::FunctionType::Function | function_type::FunctionType::StaticMethod => { + return; + } + function_type::FunctionType::Method => Argumentable::Method, + function_type::FunctionType::ClassMethod => Argumentable::ClassMethod, + }; + if !checker.enabled(argumentable.rule_code()) { + return; + } + + let Some(ParameterWithDefault { parameter, .. }) = parameters + .posonlyargs + .first() + .or_else(|| parameters.args.first()) + else { + return; + }; + + if ¶meter.name == argumentable.valid_argument_name() { + return; + } + if checker.settings.pep8_naming.ignore_names.matches(name) { + return; + } + diagnostics.push(Diagnostic::new( + argumentable.check_for(parameter.name.to_string()), + parameter.range(), + )) +} diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name_for_class_method.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name_for_class_method.rs deleted file mode 100644 index ce6e75ea3c0f3..0000000000000 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name_for_class_method.rs +++ /dev/null @@ -1,104 +0,0 @@ -use ruff_python_ast::{Decorator, ParameterWithDefault, Parameters}; - -use ruff_diagnostics::{Diagnostic, Violation}; -use ruff_macros::{derive_message_formats, violation}; -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 class methods that use a name other than `cls` for their -/// first argument. -/// -/// ## Why is this bad? -/// [PEP 8] recommends the use of `cls` as the first argument for all class -/// methods: -/// -/// > Always use `cls` for the first argument to class methods. -/// > -/// > If a function argument’s name clashes with a reserved keyword, it is generally better to -/// > append a single trailing underscore rather than use an abbreviation or spelling corruption. -/// > Thus `class_` is better than `clss`. (Perhaps better is to avoid such clashes by using a synonym.) -/// -/// Names can be excluded from this rule using the [`lint.pep8-naming.ignore-names`] -/// or [`lint.pep8-naming.extend-ignore-names`] configuration options. For example, -/// to allow the use of `klass` as the first argument to class methods, set -/// the [`lint.pep8-naming.extend-ignore-names`] option to `["klass"]`. -/// -/// ## Example -/// ```python -/// class Example: -/// @classmethod -/// def function(self, data): -/// ... -/// ``` -/// -/// Use instead: -/// ```python -/// class Example: -/// @classmethod -/// def function(cls, data): -/// ... -/// ``` -/// -/// ## Options -/// - `lint.pep8-naming.classmethod-decorators` -/// - `lint.pep8-naming.staticmethod-decorators` -/// - `lint.pep8-naming.ignore-names` -/// - `lint.pep8-naming.extend-ignore-names` -/// -/// [PEP 8]: https://peps.python.org/pep-0008/#function-and-method-arguments -#[violation] -pub struct InvalidFirstArgumentNameForClassMethod; - -impl Violation for InvalidFirstArgumentNameForClassMethod { - #[derive_message_formats] - fn message(&self) -> String { - format!("First argument of a class method should be named `cls`") - } -} - -/// N804 -pub(crate) fn invalid_first_argument_name_for_class_method( - checker: &Checker, - scope: &Scope, - name: &str, - decorator_list: &[Decorator], - parameters: &Parameters, -) -> Option { - if !matches!( - function_type::classify( - name, - decorator_list, - scope, - checker.semantic(), - &checker.settings.pep8_naming.classmethod_decorators, - &checker.settings.pep8_naming.staticmethod_decorators, - ), - function_type::FunctionType::ClassMethod - ) { - return None; - } - if let Some(ParameterWithDefault { - parameter, - default: _, - range: _, - }) = parameters - .posonlyargs - .first() - .or_else(|| parameters.args.first()) - { - if ¶meter.name != "cls" { - if checker.settings.pep8_naming.ignore_names.matches(name) { - return None; - } - return Some(Diagnostic::new( - InvalidFirstArgumentNameForClassMethod, - parameter.range(), - )); - } - } - None -} diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name_for_method.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name_for_method.rs deleted file mode 100644 index 1407ef0842790..0000000000000 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name_for_method.rs +++ /dev/null @@ -1,96 +0,0 @@ -use ruff_python_ast::{Decorator, Parameters}; - -use ruff_diagnostics::{Diagnostic, Violation}; -use ruff_macros::{derive_message_formats, violation}; -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 instance methods that use a name other than `self` for their -/// first argument. -/// -/// ## Why is this bad? -/// [PEP 8] recommends the use of `self` as first argument for all instance -/// methods: -/// -/// > Always use self for the first argument to instance methods. -/// > -/// > If a function argument’s name clashes with a reserved keyword, it is generally better to -/// > append a single trailing underscore rather than use an abbreviation or spelling corruption. -/// > Thus `class_` is better than `clss`. (Perhaps better is to avoid such clashes by using a synonym.) -/// -/// Names can be excluded from this rule using the [`lint.pep8-naming.ignore-names`] -/// or [`lint.pep8-naming.extend-ignore-names`] configuration options. For example, -/// to allow the use of `this` as the first argument to instance methods, set -/// the [`lint.pep8-naming.extend-ignore-names`] option to `["this"]`. -/// -/// ## Example -/// ```python -/// class Example: -/// def function(cls, data): -/// ... -/// ``` -/// -/// Use instead: -/// ```python -/// class Example: -/// def function(self, data): -/// ... -/// ``` -/// -/// ## Options -/// - `lint.pep8-naming.classmethod-decorators` -/// - `lint.pep8-naming.staticmethod-decorators` -/// - `lint.pep8-naming.ignore-names` -/// - `lint.pep8-naming.extend-ignore-names` -/// -/// [PEP 8]: https://peps.python.org/pep-0008/#function-and-method-arguments -#[violation] -pub struct InvalidFirstArgumentNameForMethod; - -impl Violation for InvalidFirstArgumentNameForMethod { - #[derive_message_formats] - fn message(&self) -> String { - format!("First argument of a method should be named `self`") - } -} - -/// N805 -pub(crate) fn invalid_first_argument_name_for_method( - checker: &Checker, - scope: &Scope, - name: &str, - decorator_list: &[Decorator], - parameters: &Parameters, -) -> Option { - if !matches!( - function_type::classify( - name, - decorator_list, - scope, - checker.semantic(), - &checker.settings.pep8_naming.classmethod_decorators, - &checker.settings.pep8_naming.staticmethod_decorators, - ), - function_type::FunctionType::Method - ) { - return None; - } - let arg = parameters - .posonlyargs - .first() - .or_else(|| parameters.args.first())?; - if &arg.parameter.name == "self" { - return None; - } - if checker.settings.pep8_naming.ignore_names.matches(name) { - return None; - } - Some(Diagnostic::new( - InvalidFirstArgumentNameForMethod, - arg.parameter.range(), - )) -} diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/mod.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/mod.rs index 636ae11477c0f..36d10ddd8f44b 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/mod.rs @@ -6,8 +6,7 @@ pub(crate) use dunder_function_name::*; pub(crate) use error_suffix_on_exception_name::*; pub(crate) use invalid_argument_name::*; pub(crate) use invalid_class_name::*; -pub(crate) use invalid_first_argument_name_for_class_method::*; -pub(crate) use invalid_first_argument_name_for_method::*; +pub(crate) use invalid_first_argument_name::*; pub(crate) use invalid_function_name::*; pub(crate) use invalid_module_name::*; pub(crate) use lowercase_imported_as_non_lowercase::*; @@ -23,8 +22,7 @@ mod dunder_function_name; mod error_suffix_on_exception_name; mod invalid_argument_name; mod invalid_class_name; -mod invalid_first_argument_name_for_class_method; -mod invalid_first_argument_name_for_method; +mod invalid_first_argument_name; mod invalid_function_name; mod invalid_module_name; mod lowercase_imported_as_non_lowercase; From d367c12a6f76597fba698de810734965b1e33ebb Mon Sep 17 00:00:00 2001 From: Gautier Moin Date: Sun, 3 Mar 2024 16:02:59 +0100 Subject: [PATCH 2/8] Add fix --- .../rules/invalid_first_argument_name.rs | 49 +++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index 1319deb9add92..ee19199e59d3c 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -1,8 +1,8 @@ use ruff_python_ast as ast; use ruff_python_ast::ParameterWithDefault; -use ruff_diagnostics::DiagnosticKind; -use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_diagnostics::{Diagnostic, Edit, Violation}; +use ruff_diagnostics::{DiagnosticKind, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_semantic::analyze::function_type; use ruff_python_semantic::{Scope, ScopeKind}; @@ -57,10 +57,19 @@ pub struct InvalidFirstArgumentNameForMethod { } impl Violation for InvalidFirstArgumentNameForMethod { + const FIX_AVAILABILITY: ruff_diagnostics::FixAvailability = + ruff_diagnostics::FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { format!("First argument of a method should be named `self`") } + + fn fix_title(&self) -> Option { + let Self { argument_name } = self; + Some(format!("Rename `{argument_name}` to `self`")); + None + } } /// ## What it does @@ -111,10 +120,19 @@ pub struct InvalidFirstArgumentNameForClassMethod { } impl Violation for InvalidFirstArgumentNameForClassMethod { + const FIX_AVAILABILITY: ruff_diagnostics::FixAvailability = + ruff_diagnostics::FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { format!("First argument of a class method should be named `cls`") } + + fn fix_title(&self) -> Option { + let Self { argument_name } = self; + Some(format!("Rename `{argument_name}` to `cls`")); + None + } } /// An AST node that can contain arguments. @@ -156,6 +174,7 @@ pub(crate) fn invalid_first_argument_name( let ScopeKind::Function(ast::StmtFunctionDef { name, parameters, + // body, decorator_list, .. }) = &scope.kind @@ -199,8 +218,30 @@ pub(crate) fn invalid_first_argument_name( if checker.settings.pep8_naming.ignore_names.matches(name) { return; } - diagnostics.push(Diagnostic::new( + + let fix = if let Some(bid) = scope.get(¶meter.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(), - )) + ); + if let Some(fix) = fix { + diagnostic.set_fix(fix); + } + diagnostics.push(diagnostic); } From 5b3708b0d1c87348f4f315fc3bd3ad8c2fce218d Mon Sep 17 00:00:00 2001 From: Gautier Moin Date: Sun, 3 Mar 2024 16:03:21 +0100 Subject: [PATCH 3/8] Update test cases for renaming --- .../test/fixtures/pep8_naming/N804.py | 23 +++++++ .../test/fixtures/pep8_naming/N805.py | 26 +++++++- .../rules/invalid_first_argument_name.rs | 6 +- ...les__pep8_naming__tests__N804_N804.py.snap | 60 ++++++++++++++++++ ...les__pep8_naming__tests__N805_N805.py.snap | 62 ++++++++++++++++++- ...naming__tests__classmethod_decorators.snap | 62 ++++++++++++++++++- ...ing__tests__ignore_names_N804_N804.py.snap | 2 - ...ing__tests__ignore_names_N805_N805.py.snap | 2 - ...aming__tests__staticmethod_decorators.snap | 62 ++++++++++++++++++- 9 files changed, 293 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N804.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N804.py index 6ca9a649e1ce5..17d4d5187adc5 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N804.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N804.py @@ -50,6 +50,29 @@ def good_method(cls): def static_method(not_cls) -> bool: return False +class ClsInArgsClass(ABCMeta): + def cls_as_argument(this, cls): + pass + + def cls_as_pos_only_argument(this, cls, /): + pass + + def cls_as_kw_only_argument(this, *, cls): + pass + + def cls_as_varags(this, *cls): + pass + + def cls_as_kwargs(this, **cls): + pass + +class RenamingInMethodBodyClass(ABCMeta): + def bad_method(this): + this = this + this + + def bad_method(this): + self = this def func(x): return x diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N805.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N805.py index 395da9c81a8c0..ade0a7c78bfbd 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N805.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N805.py @@ -61,7 +61,7 @@ class PosOnlyClass: def good_method_pos_only(self, blah, /, something: str): pass - def bad_method_pos_only(this, blah, /, self, something: str): + def bad_method_pos_only(this, blah, /, something: str): pass @@ -93,3 +93,27 @@ def good(self): @foobar.thisisstatic def badstatic(foo): pass + +class SelfInArgsClass: + def self_as_argument(this, self): + pass + + def self_as_pos_only_argument(this, self, /): + pass + + def self_as_kw_only_argument(this, *, self): + pass + + def self_as_varags(this, *self): + pass + + def self_as_kwargs(this, **self): + pass + +class RenamingInMethodBodyClass: + def bad_method(this): + this = this + this + + def bad_method(this): + self = this diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index ee19199e59d3c..ab3a4e39f2f8b 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -67,8 +67,7 @@ impl Violation for InvalidFirstArgumentNameForMethod { fn fix_title(&self) -> Option { let Self { argument_name } = self; - Some(format!("Rename `{argument_name}` to `self`")); - None + Some(format!("Rename `{argument_name}` to `self`")) } } @@ -130,8 +129,7 @@ impl Violation for InvalidFirstArgumentNameForClassMethod { fn fix_title(&self) -> Option { let Self { argument_name } = self; - Some(format!("Rename `{argument_name}` to `cls`")); - None + Some(format!("Rename `{argument_name}` to `cls`")) } } diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap index 0546db9dc8cfc..abfa4b25e92ab 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap @@ -26,4 +26,64 @@ N804.py:43:20: N804 First argument of a class method should be named `cls` 44 | pass | +N804.py:54:25: N804 First argument of a class method should be named `cls` + | +53 | class ClsInArgsClass(ABCMeta): +54 | def cls_as_argument(this, cls): + | ^^^^ N804 +55 | pass + | + +N804.py:57:34: N804 First argument of a class method should be named `cls` + | +55 | pass +56 | +57 | def cls_as_pos_only_argument(this, cls, /): + | ^^^^ N804 +58 | pass + | + +N804.py:60:33: N804 First argument of a class method should be named `cls` + | +58 | pass +59 | +60 | def cls_as_kw_only_argument(this, *, cls): + | ^^^^ N804 +61 | pass + | + +N804.py:63:23: N804 First argument of a class method should be named `cls` + | +61 | pass +62 | +63 | def cls_as_varags(this, *cls): + | ^^^^ N804 +64 | pass + | +N804.py:66:23: N804 First argument of a class method should be named `cls` + | +64 | pass +65 | +66 | def cls_as_kwargs(this, **cls): + | ^^^^ N804 +67 | pass + | + +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 + | + +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 + | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap index 3a50cb65fb51d..26bde9147b5a7 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap @@ -38,7 +38,7 @@ N805.py:64:29: N805 First argument of a method should be named `self` | 62 | pass 63 | -64 | def bad_method_pos_only(this, blah, /, self, something: str): +64 | def bad_method_pos_only(this, blah, /, something: str): | ^^^^ N805 65 | pass | @@ -76,4 +76,64 @@ N805.py:94:19: N805 First argument of a method should be named `self` 95 | pass | +N805.py:98:26: N805 First argument of a method should be named `self` + | +97 | class SelfInArgsClass: +98 | def self_as_argument(this, self): + | ^^^^ N805 +99 | pass + | + +N805.py:101:35: N805 First argument of a method should be named `self` + | + 99 | pass +100 | +101 | def self_as_pos_only_argument(this, self, /): + | ^^^^ N805 +102 | pass + | + +N805.py:104:34: N805 First argument of a method should be named `self` + | +102 | pass +103 | +104 | def self_as_kw_only_argument(this, *, self): + | ^^^^ N805 +105 | pass + | + +N805.py:107:24: N805 First argument of a method should be named `self` + | +105 | pass +106 | +107 | def self_as_varags(this, *self): + | ^^^^ N805 +108 | pass + | + +N805.py:110:24: N805 First argument of a method should be named `self` + | +108 | pass +109 | +110 | def self_as_kwargs(this, **self): + | ^^^^ N805 +111 | pass + | + +N805.py:114:20: N805 First argument of a method should be named `self` + | +113 | class RenamingInMethodBodyClass: +114 | def bad_method(this): + | ^^^^ N805 +115 | this = this +116 | this + | +N805.py:118:20: N805 First argument of a method should be named `self` + | +116 | this +117 | +118 | def bad_method(this): + | ^^^^ N805 +119 | self = this + | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap index 59e0688efa8ff..65b238eab1393 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap @@ -22,7 +22,7 @@ N805.py:64:29: N805 First argument of a method should be named `self` | 62 | pass 63 | -64 | def bad_method_pos_only(this, blah, /, self, something: str): +64 | def bad_method_pos_only(this, blah, /, something: str): | ^^^^ N805 65 | pass | @@ -52,4 +52,64 @@ N805.py:94:19: N805 First argument of a method should be named `self` 95 | pass | +N805.py:98:26: N805 First argument of a method should be named `self` + | +97 | class SelfInArgsClass: +98 | def self_as_argument(this, self): + | ^^^^ N805 +99 | pass + | + +N805.py:101:35: N805 First argument of a method should be named `self` + | + 99 | pass +100 | +101 | def self_as_pos_only_argument(this, self, /): + | ^^^^ N805 +102 | pass + | + +N805.py:104:34: N805 First argument of a method should be named `self` + | +102 | pass +103 | +104 | def self_as_kw_only_argument(this, *, self): + | ^^^^ N805 +105 | pass + | + +N805.py:107:24: N805 First argument of a method should be named `self` + | +105 | pass +106 | +107 | def self_as_varags(this, *self): + | ^^^^ N805 +108 | pass + | + +N805.py:110:24: N805 First argument of a method should be named `self` + | +108 | pass +109 | +110 | def self_as_kwargs(this, **self): + | ^^^^ N805 +111 | pass + | + +N805.py:114:20: N805 First argument of a method should be named `self` + | +113 | class RenamingInMethodBodyClass: +114 | def bad_method(this): + | ^^^^ N805 +115 | this = this +116 | this + | +N805.py:118:20: N805 First argument of a method should be named `self` + | +116 | this +117 | +118 | def bad_method(this): + | ^^^^ N805 +119 | self = this + | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N804_N804.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N804_N804.py.snap index cc74a934cea20..8ed4a5125173f 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N804_N804.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N804_N804.py.snap @@ -25,5 +25,3 @@ N804.py:21:18: N804 First argument of a class method should be named `cls` | ^^^^ N804 22 | pass | - - diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N805_N805.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N805_N805.py.snap index e4844d4fe3cbe..62fc53a502a2e 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N805_N805.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N805_N805.py.snap @@ -43,5 +43,3 @@ N805.py:58:18: N805 First argument of a method should be named `self` | ^^^^ N805 59 | pass | - - diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap index 8e72fe683ac0b..151022068d304 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap @@ -38,7 +38,7 @@ N805.py:64:29: N805 First argument of a method should be named `self` | 62 | pass 63 | -64 | def bad_method_pos_only(this, blah, /, self, something: str): +64 | def bad_method_pos_only(this, blah, /, something: str): | ^^^^ N805 65 | pass | @@ -68,4 +68,64 @@ N805.py:86:14: N805 First argument of a method should be named `self` 87 | pass | +N805.py:98:26: N805 First argument of a method should be named `self` + | +97 | class SelfInArgsClass: +98 | def self_as_argument(this, self): + | ^^^^ N805 +99 | pass + | + +N805.py:101:35: N805 First argument of a method should be named `self` + | + 99 | pass +100 | +101 | def self_as_pos_only_argument(this, self, /): + | ^^^^ N805 +102 | pass + | + +N805.py:104:34: N805 First argument of a method should be named `self` + | +102 | pass +103 | +104 | def self_as_kw_only_argument(this, *, self): + | ^^^^ N805 +105 | pass + | + +N805.py:107:24: N805 First argument of a method should be named `self` + | +105 | pass +106 | +107 | def self_as_varags(this, *self): + | ^^^^ N805 +108 | pass + | + +N805.py:110:24: N805 First argument of a method should be named `self` + | +108 | pass +109 | +110 | def self_as_kwargs(this, **self): + | ^^^^ N805 +111 | pass + | + +N805.py:114:20: N805 First argument of a method should be named `self` + | +113 | class RenamingInMethodBodyClass: +114 | def bad_method(this): + | ^^^^ N805 +115 | this = this +116 | this + | +N805.py:118:20: N805 First argument of a method should be named `self` + | +116 | this +117 | +118 | def bad_method(this): + | ^^^^ N805 +119 | self = this + | From 5010761fd8d6eb6a927c444d1ef03c106a3299fc Mon Sep 17 00:00:00 2001 From: Gautier Moin Date: Sun, 3 Mar 2024 18:52:40 +0100 Subject: [PATCH 4/8] Only rename references to the argument binding --- .../rules/invalid_first_argument_name.rs | 85 ++++++---- ...les__pep8_naming__tests__N804_N804.py.snap | 74 ++++++++- ...les__pep8_naming__tests__N805_N805.py.snap | 149 ++++++++++++++++-- ...naming__tests__classmethod_decorators.snap | 110 ++++++++++++- ...ing__tests__ignore_names_N804_N804.py.snap | 37 ++++- ...ing__tests__ignore_names_N805_N805.py.snap | 53 ++++++- ...aming__tests__staticmethod_decorators.snap | 136 ++++++++++++++-- 7 files changed, 574 insertions(+), 70 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index ab3a4e39f2f8b..dc873df30f0d9 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -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; @@ -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", @@ -172,7 +171,6 @@ pub(crate) fn invalid_first_argument_name( let ScopeKind::Function(ast::StmtFunctionDef { name, parameters, - // body, decorator_list, .. }) = &scope.kind @@ -202,7 +200,10 @@ 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()) @@ -210,36 +211,62 @@ pub(crate) fn invalid_first_argument_name( return; }; - if ¶meter.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(¶meter.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 { + // Don't fix if another parameter has the valid name. + if let Some(_) = parameters + .posonlyargs + .iter() + .chain(¶meters.args) + .chain(¶meters.kwonlyargs) + .skip(1) + .map(|parameter_with_default| ¶meter_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) +} diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap index abfa4b25e92ab..3fee4f17ea8af 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap @@ -1,7 +1,7 @@ --- 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 | @@ -9,22 +9,55 @@ N804.py:30:27: N804 First argument of a class method should be named `cls` | ^^^^ 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` | @@ -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` | @@ -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` | @@ -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` | @@ -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` | @@ -69,8 +106,9 @@ 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): @@ -78,8 +116,21 @@ N804.py:70:20: N804 First argument of a class method should be named `cls` 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 | @@ -87,3 +138,16 @@ N804.py:74:20: N804 First argument of a class method should be named `cls` | ^^^^ 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 diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap index 26bde9147b5a7..cb012249dad23 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap @@ -1,15 +1,26 @@ --- source: crates/ruff_linter/src/rules/pep8_naming/mod.rs --- -N805.py:7:20: N805 First argument of a method should be named `self` +N805.py:7:20: N805 [*] First argument of a method should be named `self` | 6 | class Class: 7 | def bad_method(this): | ^^^^ N805 8 | pass | + = help: Rename `this` to `self` -N805.py:12:30: N805 First argument of a method should be named `self` +ℹ Unsafe fix +4 4 | +5 5 | +6 6 | class Class: +7 |- def bad_method(this): + 7 |+ def bad_method(self): +8 8 | pass +9 9 | +10 10 | if False: + +N805.py:12:30: N805 [*] First argument of a method should be named `self` | 10 | if False: 11 | @@ -17,24 +28,57 @@ N805.py:12:30: N805 First argument of a method should be named `self` | ^^^^ N805 13 | pass | + = help: Rename `this` to `self` + +ℹ Unsafe fix +9 9 | +10 10 | if False: +11 11 | +12 |- def extra_bad_method(this): + 12 |+ def extra_bad_method(self): +13 13 | pass +14 14 | +15 15 | def good_method(self): -N805.py:31:15: N805 First argument of a method should be named `self` +N805.py:31:15: N805 [*] First argument of a method should be named `self` | 30 | @pydantic.validator 31 | def lower(cls, my_field: str) -> str: | ^^^ N805 32 | pass | + = help: Rename `cls` to `self` + +ℹ Unsafe fix +28 28 | return x +29 29 | +30 30 | @pydantic.validator +31 |- def lower(cls, my_field: str) -> str: + 31 |+ def lower(self, my_field: str) -> str: +32 32 | pass +33 33 | +34 34 | @pydantic.validator("my_field") -N805.py:35:15: N805 First argument of a method should be named `self` +N805.py:35:15: N805 [*] First argument of a method should be named `self` | 34 | @pydantic.validator("my_field") 35 | def lower(cls, my_field: str) -> str: | ^^^ N805 36 | pass | + = help: Rename `cls` to `self` -N805.py:64:29: N805 First argument of a method should be named `self` +ℹ Unsafe fix +32 32 | pass +33 33 | +34 34 | @pydantic.validator("my_field") +35 |- def lower(cls, my_field: str) -> str: + 35 |+ def lower(self, my_field: str) -> str: +36 36 | pass +37 37 | +38 38 | def __init__(self): + +N805.py:64:29: N805 [*] First argument of a method should be named `self` | 62 | pass 63 | @@ -42,8 +86,19 @@ N805.py:64:29: N805 First argument of a method should be named `self` | ^^^^ N805 65 | pass | + = help: Rename `this` to `self` + +ℹ Unsafe fix +61 61 | def good_method_pos_only(self, blah, /, something: str): +62 62 | pass +63 63 | +64 |- def bad_method_pos_only(this, blah, /, something: str): + 64 |+ def bad_method_pos_only(self, blah, /, something: str): +65 65 | pass +66 66 | +67 67 | -N805.py:70:13: N805 First argument of a method should be named `self` +N805.py:70:13: N805 [*] First argument of a method should be named `self` | 68 | class ModelClass: 69 | @hybrid_property @@ -51,30 +106,74 @@ N805.py:70:13: N805 First argument of a method should be named `self` | ^^^ N805 71 | pass | + = help: Rename `cls` to `self` + +ℹ Unsafe fix +67 67 | +68 68 | class ModelClass: +69 69 | @hybrid_property +70 |- def bad(cls): + 70 |+ def bad(self): +71 71 | pass +72 72 | +73 73 | @bad.expression -N805.py:78:13: N805 First argument of a method should be named `self` +N805.py:78:13: N805 [*] First argument of a method should be named `self` | 77 | @bad.wtf 78 | def bad(cls): | ^^^ N805 79 | pass | + = help: Rename `cls` to `self` -N805.py:86:14: N805 First argument of a method should be named `self` +ℹ Unsafe fix +75 75 | pass +76 76 | +77 77 | @bad.wtf +78 |- def bad(cls): + 78 |+ def bad(self): +79 79 | pass +80 80 | +81 81 | @hybrid_property + +N805.py:86:14: N805 [*] First argument of a method should be named `self` | 85 | @good.expression 86 | def good(cls): | ^^^ N805 87 | pass | + = help: Rename `cls` to `self` + +ℹ Unsafe fix +83 83 | pass +84 84 | +85 85 | @good.expression +86 |- def good(cls): + 86 |+ def good(self): +87 87 | pass +88 88 | +89 89 | @good.wtf -N805.py:94:19: N805 First argument of a method should be named `self` +N805.py:94:19: N805 [*] First argument of a method should be named `self` | 93 | @foobar.thisisstatic 94 | def badstatic(foo): | ^^^ N805 95 | pass | + = help: Rename `foo` to `self` + +ℹ Unsafe fix +91 91 | pass +92 92 | +93 93 | @foobar.thisisstatic +94 |- def badstatic(foo): + 94 |+ def badstatic(self): +95 95 | pass +96 96 | +97 97 | class SelfInArgsClass: N805.py:98:26: N805 First argument of a method should be named `self` | @@ -83,6 +182,7 @@ N805.py:98:26: N805 First argument of a method should be named `self` | ^^^^ N805 99 | pass | + = help: Rename `this` to `self` N805.py:101:35: N805 First argument of a method should be named `self` | @@ -92,6 +192,7 @@ N805.py:101:35: N805 First argument of a method should be named `self` | ^^^^ N805 102 | pass | + = help: Rename `this` to `self` N805.py:104:34: N805 First argument of a method should be named `self` | @@ -101,6 +202,7 @@ N805.py:104:34: N805 First argument of a method should be named `self` | ^^^^ N805 105 | pass | + = help: Rename `this` to `self` N805.py:107:24: N805 First argument of a method should be named `self` | @@ -110,6 +212,7 @@ N805.py:107:24: N805 First argument of a method should be named `self` | ^^^^ N805 108 | pass | + = help: Rename `this` to `self` N805.py:110:24: N805 First argument of a method should be named `self` | @@ -119,8 +222,9 @@ N805.py:110:24: N805 First argument of a method should be named `self` | ^^^^ N805 111 | pass | + = help: Rename `this` to `self` -N805.py:114:20: N805 First argument of a method should be named `self` +N805.py:114:20: N805 [*] First argument of a method should be named `self` | 113 | class RenamingInMethodBodyClass: 114 | def bad_method(this): @@ -128,8 +232,21 @@ N805.py:114:20: N805 First argument of a method should be named `self` 115 | this = this 116 | this | + = help: Rename `this` to `self` -N805.py:118:20: N805 First argument of a method should be named `self` +ℹ Unsafe fix +111 111 | pass +112 112 | +113 113 | class RenamingInMethodBodyClass: +114 |- def bad_method(this): +115 |- this = this + 114 |+ def bad_method(self): + 115 |+ this = self +116 116 | this +117 117 | +118 118 | def bad_method(this): + +N805.py:118:20: N805 [*] First argument of a method should be named `self` | 116 | this 117 | @@ -137,3 +254,13 @@ N805.py:118:20: N805 First argument of a method should be named `self` | ^^^^ N805 119 | self = this | + = help: Rename `this` to `self` + +ℹ Unsafe fix +115 115 | this = this +116 116 | this +117 117 | +118 |- def bad_method(this): +119 |- self = this + 118 |+ def bad_method(self): + 119 |+ self = self diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap index 65b238eab1393..8e65dc3423ea9 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap @@ -1,15 +1,26 @@ --- source: crates/ruff_linter/src/rules/pep8_naming/mod.rs --- -N805.py:7:20: N805 First argument of a method should be named `self` +N805.py:7:20: N805 [*] First argument of a method should be named `self` | 6 | class Class: 7 | def bad_method(this): | ^^^^ N805 8 | pass | + = help: Rename `this` to `self` -N805.py:12:30: N805 First argument of a method should be named `self` +ℹ Unsafe fix +4 4 | +5 5 | +6 6 | class Class: +7 |- def bad_method(this): + 7 |+ def bad_method(self): +8 8 | pass +9 9 | +10 10 | if False: + +N805.py:12:30: N805 [*] First argument of a method should be named `self` | 10 | if False: 11 | @@ -17,8 +28,19 @@ N805.py:12:30: N805 First argument of a method should be named `self` | ^^^^ N805 13 | pass | + = help: Rename `this` to `self` + +ℹ Unsafe fix +9 9 | +10 10 | if False: +11 11 | +12 |- def extra_bad_method(this): + 12 |+ def extra_bad_method(self): +13 13 | pass +14 14 | +15 15 | def good_method(self): -N805.py:64:29: N805 First argument of a method should be named `self` +N805.py:64:29: N805 [*] First argument of a method should be named `self` | 62 | pass 63 | @@ -26,8 +48,19 @@ N805.py:64:29: N805 First argument of a method should be named `self` | ^^^^ N805 65 | pass | + = help: Rename `this` to `self` -N805.py:70:13: N805 First argument of a method should be named `self` +ℹ Unsafe fix +61 61 | def good_method_pos_only(self, blah, /, something: str): +62 62 | pass +63 63 | +64 |- def bad_method_pos_only(this, blah, /, something: str): + 64 |+ def bad_method_pos_only(self, blah, /, something: str): +65 65 | pass +66 66 | +67 67 | + +N805.py:70:13: N805 [*] First argument of a method should be named `self` | 68 | class ModelClass: 69 | @hybrid_property @@ -35,22 +68,55 @@ N805.py:70:13: N805 First argument of a method should be named `self` | ^^^ N805 71 | pass | + = help: Rename `cls` to `self` + +ℹ Unsafe fix +67 67 | +68 68 | class ModelClass: +69 69 | @hybrid_property +70 |- def bad(cls): + 70 |+ def bad(self): +71 71 | pass +72 72 | +73 73 | @bad.expression -N805.py:78:13: N805 First argument of a method should be named `self` +N805.py:78:13: N805 [*] First argument of a method should be named `self` | 77 | @bad.wtf 78 | def bad(cls): | ^^^ N805 79 | pass | + = help: Rename `cls` to `self` -N805.py:94:19: N805 First argument of a method should be named `self` +ℹ Unsafe fix +75 75 | pass +76 76 | +77 77 | @bad.wtf +78 |- def bad(cls): + 78 |+ def bad(self): +79 79 | pass +80 80 | +81 81 | @hybrid_property + +N805.py:94:19: N805 [*] First argument of a method should be named `self` | 93 | @foobar.thisisstatic 94 | def badstatic(foo): | ^^^ N805 95 | pass | + = help: Rename `foo` to `self` + +ℹ Unsafe fix +91 91 | pass +92 92 | +93 93 | @foobar.thisisstatic +94 |- def badstatic(foo): + 94 |+ def badstatic(self): +95 95 | pass +96 96 | +97 97 | class SelfInArgsClass: N805.py:98:26: N805 First argument of a method should be named `self` | @@ -59,6 +125,7 @@ N805.py:98:26: N805 First argument of a method should be named `self` | ^^^^ N805 99 | pass | + = help: Rename `this` to `self` N805.py:101:35: N805 First argument of a method should be named `self` | @@ -68,6 +135,7 @@ N805.py:101:35: N805 First argument of a method should be named `self` | ^^^^ N805 102 | pass | + = help: Rename `this` to `self` N805.py:104:34: N805 First argument of a method should be named `self` | @@ -77,6 +145,7 @@ N805.py:104:34: N805 First argument of a method should be named `self` | ^^^^ N805 105 | pass | + = help: Rename `this` to `self` N805.py:107:24: N805 First argument of a method should be named `self` | @@ -86,6 +155,7 @@ N805.py:107:24: N805 First argument of a method should be named `self` | ^^^^ N805 108 | pass | + = help: Rename `this` to `self` N805.py:110:24: N805 First argument of a method should be named `self` | @@ -95,8 +165,9 @@ N805.py:110:24: N805 First argument of a method should be named `self` | ^^^^ N805 111 | pass | + = help: Rename `this` to `self` -N805.py:114:20: N805 First argument of a method should be named `self` +N805.py:114:20: N805 [*] First argument of a method should be named `self` | 113 | class RenamingInMethodBodyClass: 114 | def bad_method(this): @@ -104,8 +175,21 @@ N805.py:114:20: N805 First argument of a method should be named `self` 115 | this = this 116 | this | + = help: Rename `this` to `self` -N805.py:118:20: N805 First argument of a method should be named `self` +ℹ Unsafe fix +111 111 | pass +112 112 | +113 113 | class RenamingInMethodBodyClass: +114 |- def bad_method(this): +115 |- this = this + 114 |+ def bad_method(self): + 115 |+ this = self +116 116 | this +117 117 | +118 118 | def bad_method(this): + +N805.py:118:20: N805 [*] First argument of a method should be named `self` | 116 | this 117 | @@ -113,3 +197,13 @@ N805.py:118:20: N805 First argument of a method should be named `self` | ^^^^ N805 119 | self = this | + = help: Rename `this` to `self` + +ℹ Unsafe fix +115 115 | this = this +116 116 | this +117 117 | +118 |- def bad_method(this): +119 |- self = this + 118 |+ def bad_method(self): + 119 |+ self = self diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N804_N804.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N804_N804.py.snap index 8ed4a5125173f..4434e4b603717 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N804_N804.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N804_N804.py.snap @@ -1,23 +1,45 @@ --- source: crates/ruff_linter/src/rules/pep8_naming/mod.rs --- -N804.py:5:27: N804 First argument of a class method should be named `cls` +N804.py:5:27: N804 [*] First argument of a class method should be named `cls` | 4 | class Class: 5 | def __init_subclass__(self, default_name, **kwargs): | ^^^^ N804 6 | ... | + = help: Rename `self` to `cls` -N804.py:13:18: N804 First argument of a class method should be named `cls` +ℹ Unsafe fix +2 2 | +3 3 | +4 4 | class Class: +5 |- def __init_subclass__(self, default_name, **kwargs): + 5 |+ def __init_subclass__(cls, default_name, **kwargs): +6 6 | ... +7 7 | +8 8 | @classmethod + +N804.py:13:18: N804 [*] First argument of a class method should be named `cls` | 12 | @classmethod 13 | def stillBad(self, x, /, other): | ^^^^ N804 14 | ... | + = help: Rename `self` to `cls` + +ℹ Unsafe fix +10 10 | ... +11 11 | +12 12 | @classmethod +13 |- def stillBad(self, x, /, other): + 13 |+ def stillBad(cls, x, /, other): +14 14 | ... +15 15 | +16 16 | -N804.py:21:18: N804 First argument of a class method should be named `cls` +N804.py:21:18: N804 [*] First argument of a class method should be named `cls` | 19 | pass 20 | @@ -25,3 +47,12 @@ N804.py:21:18: N804 First argument of a class method should be named `cls` | ^^^^ N804 22 | pass | + = help: Rename `self` to `cls` + +ℹ Unsafe fix +18 18 | def badAllowed(self): +19 19 | pass +20 20 | +21 |- def stillBad(self): + 21 |+ def stillBad(cls): +22 22 | pass diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N805_N805.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N805_N805.py.snap index 62fc53a502a2e..4596583f3fe0a 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N805_N805.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__ignore_names_N805_N805.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/pep8_naming/mod.rs --- -N805.py:10:18: N805 First argument of a method should be named `self` +N805.py:10:18: N805 [*] First argument of a method should be named `self` | 8 | pass 9 | @@ -9,8 +9,19 @@ N805.py:10:18: N805 First argument of a method should be named `self` | ^^^^ N805 11 | pass | + = help: Rename `this` to `self` -N805.py:18:22: N805 First argument of a method should be named `self` +ℹ Unsafe fix +7 7 | def badAllowed(this): +8 8 | pass +9 9 | +10 |- def stillBad(this): + 10 |+ def stillBad(self): +11 11 | pass +12 12 | +13 13 | if False: + +N805.py:18:22: N805 [*] First argument of a method should be named `self` | 16 | pass 17 | @@ -18,22 +29,55 @@ N805.py:18:22: N805 First argument of a method should be named `self` | ^^^^ N805 19 | pass | + = help: Rename `this` to `self` + +ℹ Unsafe fix +15 15 | def badAllowed(this): +16 16 | pass +17 17 | +18 |- def stillBad(this): + 18 |+ def stillBad(self): +19 19 | pass +20 20 | +21 21 | @pydantic.validator -N805.py:26:18: N805 First argument of a method should be named `self` +N805.py:26:18: N805 [*] First argument of a method should be named `self` | 25 | @pydantic.validator 26 | def stillBad(cls, my_field: str) -> str: | ^^^ N805 27 | pass | + = help: Rename `cls` to `self` -N805.py:34:18: N805 First argument of a method should be named `self` +ℹ Unsafe fix +23 23 | pass +24 24 | +25 25 | @pydantic.validator +26 |- def stillBad(cls, my_field: str) -> str: + 26 |+ def stillBad(self, my_field: str) -> str: +27 27 | pass +28 28 | +29 29 | @pydantic.validator("my_field") + +N805.py:34:18: N805 [*] First argument of a method should be named `self` | 33 | @pydantic.validator("my_field") 34 | def stillBad(cls, my_field: str) -> str: | ^^^ N805 35 | pass | + = help: Rename `cls` to `self` + +ℹ Unsafe fix +31 31 | pass +32 32 | +33 33 | @pydantic.validator("my_field") +34 |- def stillBad(cls, my_field: str) -> str: + 34 |+ def stillBad(self, my_field: str) -> str: +35 35 | pass +36 36 | +37 37 | @classmethod N805.py:58:18: N805 First argument of a method should be named `self` | @@ -43,3 +87,4 @@ N805.py:58:18: N805 First argument of a method should be named `self` | ^^^^ N805 59 | pass | + = help: Rename `this` to `self` diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap index 151022068d304..b3eaa29ece69a 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap @@ -1,15 +1,26 @@ --- source: crates/ruff_linter/src/rules/pep8_naming/mod.rs --- -N805.py:7:20: N805 First argument of a method should be named `self` +N805.py:7:20: N805 [*] First argument of a method should be named `self` | 6 | class Class: 7 | def bad_method(this): | ^^^^ N805 8 | pass | + = help: Rename `this` to `self` -N805.py:12:30: N805 First argument of a method should be named `self` +ℹ Unsafe fix +4 4 | +5 5 | +6 6 | class Class: +7 |- def bad_method(this): + 7 |+ def bad_method(self): +8 8 | pass +9 9 | +10 10 | if False: + +N805.py:12:30: N805 [*] First argument of a method should be named `self` | 10 | if False: 11 | @@ -17,24 +28,57 @@ N805.py:12:30: N805 First argument of a method should be named `self` | ^^^^ N805 13 | pass | + = help: Rename `this` to `self` + +ℹ Unsafe fix +9 9 | +10 10 | if False: +11 11 | +12 |- def extra_bad_method(this): + 12 |+ def extra_bad_method(self): +13 13 | pass +14 14 | +15 15 | def good_method(self): -N805.py:31:15: N805 First argument of a method should be named `self` +N805.py:31:15: N805 [*] First argument of a method should be named `self` | 30 | @pydantic.validator 31 | def lower(cls, my_field: str) -> str: | ^^^ N805 32 | pass | + = help: Rename `cls` to `self` + +ℹ Unsafe fix +28 28 | return x +29 29 | +30 30 | @pydantic.validator +31 |- def lower(cls, my_field: str) -> str: + 31 |+ def lower(self, my_field: str) -> str: +32 32 | pass +33 33 | +34 34 | @pydantic.validator("my_field") -N805.py:35:15: N805 First argument of a method should be named `self` +N805.py:35:15: N805 [*] First argument of a method should be named `self` | 34 | @pydantic.validator("my_field") 35 | def lower(cls, my_field: str) -> str: | ^^^ N805 36 | pass | + = help: Rename `cls` to `self` -N805.py:64:29: N805 First argument of a method should be named `self` +ℹ Unsafe fix +32 32 | pass +33 33 | +34 34 | @pydantic.validator("my_field") +35 |- def lower(cls, my_field: str) -> str: + 35 |+ def lower(self, my_field: str) -> str: +36 36 | pass +37 37 | +38 38 | def __init__(self): + +N805.py:64:29: N805 [*] First argument of a method should be named `self` | 62 | pass 63 | @@ -42,8 +86,19 @@ N805.py:64:29: N805 First argument of a method should be named `self` | ^^^^ N805 65 | pass | + = help: Rename `this` to `self` + +ℹ Unsafe fix +61 61 | def good_method_pos_only(self, blah, /, something: str): +62 62 | pass +63 63 | +64 |- def bad_method_pos_only(this, blah, /, something: str): + 64 |+ def bad_method_pos_only(self, blah, /, something: str): +65 65 | pass +66 66 | +67 67 | -N805.py:70:13: N805 First argument of a method should be named `self` +N805.py:70:13: N805 [*] First argument of a method should be named `self` | 68 | class ModelClass: 69 | @hybrid_property @@ -51,22 +106,55 @@ N805.py:70:13: N805 First argument of a method should be named `self` | ^^^ N805 71 | pass | + = help: Rename `cls` to `self` -N805.py:78:13: N805 First argument of a method should be named `self` +ℹ Unsafe fix +67 67 | +68 68 | class ModelClass: +69 69 | @hybrid_property +70 |- def bad(cls): + 70 |+ def bad(self): +71 71 | pass +72 72 | +73 73 | @bad.expression + +N805.py:78:13: N805 [*] First argument of a method should be named `self` | 77 | @bad.wtf 78 | def bad(cls): | ^^^ N805 79 | pass | + = help: Rename `cls` to `self` + +ℹ Unsafe fix +75 75 | pass +76 76 | +77 77 | @bad.wtf +78 |- def bad(cls): + 78 |+ def bad(self): +79 79 | pass +80 80 | +81 81 | @hybrid_property -N805.py:86:14: N805 First argument of a method should be named `self` +N805.py:86:14: N805 [*] First argument of a method should be named `self` | 85 | @good.expression 86 | def good(cls): | ^^^ N805 87 | pass | + = help: Rename `cls` to `self` + +ℹ Unsafe fix +83 83 | pass +84 84 | +85 85 | @good.expression +86 |- def good(cls): + 86 |+ def good(self): +87 87 | pass +88 88 | +89 89 | @good.wtf N805.py:98:26: N805 First argument of a method should be named `self` | @@ -75,6 +163,7 @@ N805.py:98:26: N805 First argument of a method should be named `self` | ^^^^ N805 99 | pass | + = help: Rename `this` to `self` N805.py:101:35: N805 First argument of a method should be named `self` | @@ -84,6 +173,7 @@ N805.py:101:35: N805 First argument of a method should be named `self` | ^^^^ N805 102 | pass | + = help: Rename `this` to `self` N805.py:104:34: N805 First argument of a method should be named `self` | @@ -93,6 +183,7 @@ N805.py:104:34: N805 First argument of a method should be named `self` | ^^^^ N805 105 | pass | + = help: Rename `this` to `self` N805.py:107:24: N805 First argument of a method should be named `self` | @@ -102,6 +193,7 @@ N805.py:107:24: N805 First argument of a method should be named `self` | ^^^^ N805 108 | pass | + = help: Rename `this` to `self` N805.py:110:24: N805 First argument of a method should be named `self` | @@ -111,8 +203,9 @@ N805.py:110:24: N805 First argument of a method should be named `self` | ^^^^ N805 111 | pass | + = help: Rename `this` to `self` -N805.py:114:20: N805 First argument of a method should be named `self` +N805.py:114:20: N805 [*] First argument of a method should be named `self` | 113 | class RenamingInMethodBodyClass: 114 | def bad_method(this): @@ -120,8 +213,21 @@ N805.py:114:20: N805 First argument of a method should be named `self` 115 | this = this 116 | this | + = help: Rename `this` to `self` -N805.py:118:20: N805 First argument of a method should be named `self` +ℹ Unsafe fix +111 111 | pass +112 112 | +113 113 | class RenamingInMethodBodyClass: +114 |- def bad_method(this): +115 |- this = this + 114 |+ def bad_method(self): + 115 |+ this = self +116 116 | this +117 117 | +118 118 | def bad_method(this): + +N805.py:118:20: N805 [*] First argument of a method should be named `self` | 116 | this 117 | @@ -129,3 +235,13 @@ N805.py:118:20: N805 First argument of a method should be named `self` | ^^^^ N805 119 | self = this | + = help: Rename `this` to `self` + +ℹ Unsafe fix +115 115 | this = this +116 116 | this +117 117 | +118 |- def bad_method(this): +119 |- self = this + 118 |+ def bad_method(self): + 119 |+ self = self From 6b6b273f2435fa23022827717dee35d6e926b8bf Mon Sep 17 00:00:00 2001 From: Gautier Moin Date: Sun, 3 Mar 2024 19:09:42 +0100 Subject: [PATCH 5/8] tidy --- .../rules/invalid_first_argument_name.rs | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index dc873df30f0d9..71c4cc6944f76 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -132,29 +132,28 @@ impl Violation for InvalidFirstArgumentNameForClassMethod { } } -/// An AST node that can contain arguments. #[derive(Debug, Copy, Clone)] -enum Argumentable { +enum ApplicableFunctionType { Method, ClassMethod, } -impl Argumentable { - fn check_for(self, argument_name: String) -> DiagnosticKind { +impl ApplicableFunctionType { + fn diagnostic_kind(self, argument_name: String) -> DiagnosticKind { match self { Self::Method => InvalidFirstArgumentNameForMethod { argument_name }.into(), Self::ClassMethod => InvalidFirstArgumentNameForClassMethod { argument_name }.into(), } } - const fn valid_argument_name(self) -> &'static str { + const fn valid_first_argument_name(self) -> &'static str { match self { Self::Method => "self", Self::ClassMethod => "cls", } } - const fn rule_code(self) -> Rule { + const fn rule(self) -> Rule { match self { Self::Method => Rule::InvalidFirstArgumentNameForMethod, Self::ClassMethod => Rule::InvalidFirstArgumentNameForClassMethod, @@ -182,7 +181,7 @@ pub(crate) fn invalid_first_argument_name( return; }; - let argumentable = match function_type::classify( + let function_type = match function_type::classify( name, decorator_list, parent, @@ -193,10 +192,12 @@ pub(crate) fn invalid_first_argument_name( function_type::FunctionType::Function | function_type::FunctionType::StaticMethod => { return; } - function_type::FunctionType::Method => Argumentable::Method, - function_type::FunctionType::ClassMethod => Argumentable::ClassMethod, + function_type::FunctionType::Method => ApplicableFunctionType::Method, + function_type::FunctionType::ClassMethod => ApplicableFunctionType::ClassMethod, }; - if !checker.enabled(argumentable.rule_code()) { + if !checker.enabled(function_type.rule()) + || checker.settings.pep8_naming.ignore_names.matches(name) + { return; } @@ -211,15 +212,12 @@ pub(crate) fn invalid_first_argument_name( return; }; - if &first_parameter.name == argumentable.valid_argument_name() { - return; - } - if checker.settings.pep8_naming.ignore_names.matches(name) { + if &first_parameter.name == function_type.valid_first_argument_name() { return; } let mut diagnostic = Diagnostic::new( - argumentable.check_for(first_parameter.name.to_string()), + function_type.diagnostic_kind(first_parameter.name.to_string()), first_parameter.range(), ); diagnostic.try_set_optional_fix(|| { @@ -228,7 +226,7 @@ pub(crate) fn invalid_first_argument_name( first_parameter, parameters, checker.semantic(), - argumentable, + function_type, )) }); diagnostics.push(diagnostic); @@ -239,7 +237,7 @@ fn try_fix( first_parameter: &ast::Parameter, parameters: &ast::Parameters, semantic: &SemanticModel<'_>, - argumentable: Argumentable, + function_type: ApplicableFunctionType, ) -> Option { // Don't fix if another parameter has the valid name. if let Some(_) = parameters @@ -251,7 +249,7 @@ fn try_fix( .map(|parameter_with_default| ¶meter_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()) + .find(|p| &p.name == function_type.valid_first_argument_name()) { return None; } @@ -260,13 +258,12 @@ fn try_fix( .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( + let replacement = function_type.valid_first_argument_name(); + Some(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) + )) } From 90651a281076d91ca06721c59cbc8b469b03ee02 Mon Sep 17 00:00:00 2001 From: Gautier Moin Date: Sun, 3 Mar 2024 20:24:41 +0100 Subject: [PATCH 6/8] clippy --- .../src/checkers/ast/analyze/deferred_scopes.rs | 2 +- .../pep8_naming/rules/invalid_first_argument_name.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) 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 9437f698e5ff4..2f82d246becb3 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -400,7 +400,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { Rule::InvalidFirstArgumentNameForClassMethod, Rule::InvalidFirstArgumentNameForMethod, ]) { - pep8_naming::rules::invalid_first_argument_name(checker, scope, &mut diagnostics) + pep8_naming::rules::invalid_first_argument_name(checker, scope, &mut diagnostics); } } } diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index 71c4cc6944f76..b345b9dda7b5b 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -240,16 +240,16 @@ fn try_fix( function_type: ApplicableFunctionType, ) -> Option { // Don't fix if another parameter has the valid name. - if let Some(_) = parameters + if parameters .posonlyargs .iter() .chain(¶meters.args) .chain(¶meters.kwonlyargs) .skip(1) .map(|parameter_with_default| ¶meter_with_default.parameter) - .chain(parameters.vararg.as_deref().into_iter()) - .chain(parameters.kwarg.as_deref().into_iter()) - .find(|p| &p.name == function_type.valid_first_argument_name()) + .chain(parameters.vararg.as_deref()) + .chain(parameters.kwarg.as_deref()) + .any(|p| &p.name == function_type.valid_first_argument_name()) { return None; } From c69b2b7957a33b912690cf4fff84105da6125a8f Mon Sep 17 00:00:00 2001 From: Gautier Moin Date: Mon, 4 Mar 2024 01:09:11 +0100 Subject: [PATCH 7/8] Use `Renamer::rename` --- .../rules/invalid_first_argument_name.rs | 35 +++++++++---------- ...les__pep8_naming__tests__N804_N804.py.snap | 6 ++-- ...les__pep8_naming__tests__N805_N805.py.snap | 6 ++-- ...naming__tests__classmethod_decorators.snap | 6 ++-- ...aming__tests__staticmethod_decorators.snap | 6 ++-- 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index b345b9dda7b5b..ea1459dfb7be7 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -1,14 +1,16 @@ -use ruff_python_ast as ast; -use ruff_python_ast::ParameterWithDefault; +use anyhow::Result; -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix, Violation}; +use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix, 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, SemanticModel}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::registry::Rule; +use crate::renamer::Renamer; /// ## What it does /// Checks for instance methods that use a name other than `self` for their @@ -221,13 +223,13 @@ pub(crate) fn invalid_first_argument_name( first_parameter.range(), ); diagnostic.try_set_optional_fix(|| { - Ok(try_fix( + try_fix( scope, first_parameter, parameters, checker.semantic(), function_type, - )) + ) }); diagnostics.push(diagnostic); } @@ -238,7 +240,7 @@ fn try_fix( parameters: &ast::Parameters, semantic: &SemanticModel<'_>, function_type: ApplicableFunctionType, -) -> Option { +) -> Result> { // Don't fix if another parameter has the valid name. if parameters .posonlyargs @@ -251,19 +253,14 @@ fn try_fix( .chain(parameters.kwarg.as_deref()) .any(|p| &p.name == function_type.valid_first_argument_name()) { - return None; + return Ok(None); } - let binding = scope - .get_all(&first_parameter.name) - .map(|id| semantic.binding(id)) - .find(|b| b.kind.is_argument())?; - let replacement = function_type.valid_first_argument_name(); - Some(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())), - )) + let (edit, rest) = Renamer::rename( + &first_parameter.name, + function_type.valid_first_argument_name(), + scope, + semantic, + )?; + Ok(Some(Fix::unsafe_edits(edit, rest))) } diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap index 3fee4f17ea8af..ae7c7a8bc9c07 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N804_N804.py.snap @@ -124,11 +124,13 @@ N804.py:70:20: N804 [*] First argument of a class method should be named `cls` 69 69 | class RenamingInMethodBodyClass(ABCMeta): 70 |- def bad_method(this): 71 |- this = this +72 |- this 70 |+ def bad_method(cls): - 71 |+ this = cls -72 72 | this + 71 |+ cls = cls + 72 |+ cls 73 73 | 74 74 | def bad_method(this): +75 75 | self = this N804.py:74:20: N804 [*] First argument of a class method should be named `cls` | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap index cb012249dad23..d0ee08834b9f9 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N805_N805.py.snap @@ -240,11 +240,13 @@ N805.py:114:20: N805 [*] First argument of a method should be named `self` 113 113 | class RenamingInMethodBodyClass: 114 |- def bad_method(this): 115 |- this = this +116 |- this 114 |+ def bad_method(self): - 115 |+ this = self -116 116 | this + 115 |+ self = self + 116 |+ self 117 117 | 118 118 | def bad_method(this): +119 119 | self = this N805.py:118:20: N805 [*] First argument of a method should be named `self` | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap index 8e65dc3423ea9..76d07c7ca9d1b 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__classmethod_decorators.snap @@ -183,11 +183,13 @@ N805.py:114:20: N805 [*] First argument of a method should be named `self` 113 113 | class RenamingInMethodBodyClass: 114 |- def bad_method(this): 115 |- this = this +116 |- this 114 |+ def bad_method(self): - 115 |+ this = self -116 116 | this + 115 |+ self = self + 116 |+ self 117 117 | 118 118 | def bad_method(this): +119 119 | self = this N805.py:118:20: N805 [*] First argument of a method should be named `self` | diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap index b3eaa29ece69a..b32a5581675ef 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__staticmethod_decorators.snap @@ -221,11 +221,13 @@ N805.py:114:20: N805 [*] First argument of a method should be named `self` 113 113 | class RenamingInMethodBodyClass: 114 |- def bad_method(this): 115 |- this = this +116 |- this 114 |+ def bad_method(self): - 115 |+ this = self -116 116 | this + 115 |+ self = self + 116 |+ self 117 117 | 118 118 | def bad_method(this): +119 119 | self = this N805.py:118:20: N805 [*] First argument of a method should be named `self` | From ffdd3c145f46bdeb88f352666c98b1a082965c4b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 3 Mar 2024 21:16:11 -0500 Subject: [PATCH 8/8] Rename some methods --- .../checkers/ast/analyze/deferred_scopes.rs | 1 + .../rules/invalid_first_argument_name.rs | 41 ++++++++++++------- 2 files changed, 27 insertions(+), 15 deletions(-) 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 2f82d246becb3..e1d662922384b 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -396,6 +396,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { if checker.enabled(Rule::SingledispatchMethod) { pylint::rules::singledispatch_method(checker, scope, &mut diagnostics); } + if checker.any_enabled(&[ Rule::InvalidFirstArgumentNameForClassMethod, Rule::InvalidFirstArgumentNameForMethod, diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index ea1459dfb7be7..a560fab4c1a8c 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -45,6 +45,10 @@ use crate::renamer::Renamer; /// ... /// ``` /// +/// ## Fix safety +/// This rule's fix is marked as unsafe, as renaming a method parameter +/// can change the behavior of the program. +/// /// ## Options /// - `lint.pep8-naming.classmethod-decorators` /// - `lint.pep8-naming.staticmethod-decorators` @@ -107,6 +111,10 @@ impl Violation for InvalidFirstArgumentNameForMethod { /// ... /// ``` /// +/// ## Fix safety +/// This rule's fix is marked as unsafe, as renaming a method parameter +/// can change the behavior of the program. +/// /// ## Options /// - `lint.pep8-naming.classmethod-decorators` /// - `lint.pep8-naming.staticmethod-decorators` @@ -135,12 +143,14 @@ impl Violation for InvalidFirstArgumentNameForClassMethod { } #[derive(Debug, Copy, Clone)] -enum ApplicableFunctionType { +enum FunctionType { + /// The function is an instance method. Method, + /// The function is a class method. ClassMethod, } -impl ApplicableFunctionType { +impl FunctionType { fn diagnostic_kind(self, argument_name: String) -> DiagnosticKind { match self { Self::Method => InvalidFirstArgumentNameForMethod { argument_name }.into(), @@ -194,8 +204,8 @@ pub(crate) fn invalid_first_argument_name( function_type::FunctionType::Function | function_type::FunctionType::StaticMethod => { return; } - function_type::FunctionType::Method => ApplicableFunctionType::Method, - function_type::FunctionType::ClassMethod => ApplicableFunctionType::ClassMethod, + function_type::FunctionType::Method => FunctionType::Method, + function_type::FunctionType::ClassMethod => FunctionType::ClassMethod, }; if !checker.enabled(function_type.rule()) || checker.settings.pep8_naming.ignore_names.matches(name) @@ -204,7 +214,7 @@ pub(crate) fn invalid_first_argument_name( } let Some(ParameterWithDefault { - parameter: first_parameter, + parameter: self_or_cls, .. }) = parameters .posonlyargs @@ -214,18 +224,18 @@ pub(crate) fn invalid_first_argument_name( return; }; - if &first_parameter.name == function_type.valid_first_argument_name() { + if &self_or_cls.name == function_type.valid_first_argument_name() { return; } let mut diagnostic = Diagnostic::new( - function_type.diagnostic_kind(first_parameter.name.to_string()), - first_parameter.range(), + function_type.diagnostic_kind(self_or_cls.name.to_string()), + self_or_cls.range(), ); diagnostic.try_set_optional_fix(|| { - try_fix( + rename_parameter( scope, - first_parameter, + self_or_cls, parameters, checker.semantic(), function_type, @@ -234,12 +244,13 @@ pub(crate) fn invalid_first_argument_name( diagnostics.push(diagnostic); } -fn try_fix( +/// Rename the first parameter to `self` or `cls`, if no other parameter has the target name. +fn rename_parameter( scope: &Scope<'_>, - first_parameter: &ast::Parameter, + self_or_cls: &ast::Parameter, parameters: &ast::Parameters, semantic: &SemanticModel<'_>, - function_type: ApplicableFunctionType, + function_type: FunctionType, ) -> Result> { // Don't fix if another parameter has the valid name. if parameters @@ -251,13 +262,13 @@ fn try_fix( .map(|parameter_with_default| ¶meter_with_default.parameter) .chain(parameters.vararg.as_deref()) .chain(parameters.kwarg.as_deref()) - .any(|p| &p.name == function_type.valid_first_argument_name()) + .any(|parameter| ¶meter.name == function_type.valid_first_argument_name()) { return Ok(None); } let (edit, rest) = Renamer::rename( - &first_parameter.name, + &self_or_cls.name, function_type.valid_first_argument_name(), scope, semantic,