mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 14:21:24 +00:00
[pylint
/pep8-naming
] Check __new__
argument name in bad-staticmethod-argument
and not invalid-first-argument-name-for-class-method
(PLW0211
/N804
) (#16676)
## Summary This PR stabilizes the behavior changes introduced by https://github.com/astral-sh/ruff/pull/13305 that were gated behind preview. The change is that `__new__` methods are now no longer flagged by `invalid-first-argument-name-for-class-method` (`N804`) but instead by `bad-staticmethod-argument` (`PLW0211`) > __new__ methods are technically static methods, with cls as their first argument. However, Ruff currently classifies them as classmethod, which causes two issues: ## Test Plan There have been no new issues or PRs related to `N804` or `PLW0211` since the behavior change was released in Ruff 0.9.7 (about 3 weeks ago). This is a somewhat recent change but I don't think it's necessary to leave this in preview for another 2 months. The main reason why it was in preview is that it is breaking, not because it is a risky change.
This commit is contained in:
parent
348815d6d6
commit
91674718c4
7 changed files with 18 additions and 80 deletions
|
@ -14,13 +14,13 @@ mod tests {
|
||||||
use crate::registry::Rule;
|
use crate::registry::Rule;
|
||||||
use crate::rules::pep8_naming::settings::IgnoreNames;
|
use crate::rules::pep8_naming::settings::IgnoreNames;
|
||||||
use crate::rules::{flake8_import_conventions, pep8_naming};
|
use crate::rules::{flake8_import_conventions, pep8_naming};
|
||||||
use crate::settings::types::PreviewMode;
|
|
||||||
use crate::test::test_path;
|
use crate::test::test_path;
|
||||||
use crate::{assert_messages, settings};
|
use crate::{assert_messages, settings};
|
||||||
|
|
||||||
#[test_case(Rule::InvalidClassName, Path::new("N801.py"))]
|
#[test_case(Rule::InvalidClassName, Path::new("N801.py"))]
|
||||||
#[test_case(Rule::InvalidFunctionName, Path::new("N802.py"))]
|
#[test_case(Rule::InvalidFunctionName, Path::new("N802.py"))]
|
||||||
#[test_case(Rule::InvalidArgumentName, Path::new("N803.py"))]
|
#[test_case(Rule::InvalidArgumentName, Path::new("N803.py"))]
|
||||||
|
#[test_case(Rule::InvalidArgumentName, Path::new("N804.py"))]
|
||||||
#[test_case(Rule::InvalidFirstArgumentNameForClassMethod, Path::new("N804.py"))]
|
#[test_case(Rule::InvalidFirstArgumentNameForClassMethod, Path::new("N804.py"))]
|
||||||
#[test_case(Rule::InvalidFirstArgumentNameForMethod, Path::new("N805.py"))]
|
#[test_case(Rule::InvalidFirstArgumentNameForMethod, Path::new("N805.py"))]
|
||||||
#[test_case(Rule::NonLowercaseVariableInFunction, Path::new("N806.py"))]
|
#[test_case(Rule::NonLowercaseVariableInFunction, Path::new("N806.py"))]
|
||||||
|
@ -89,24 +89,6 @@ mod tests {
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test_case(Rule::InvalidArgumentName, Path::new("N804.py"))]
|
|
||||||
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
|
||||||
let snapshot = format!(
|
|
||||||
"preview__{}_{}",
|
|
||||||
rule_code.noqa_code(),
|
|
||||||
path.to_string_lossy()
|
|
||||||
);
|
|
||||||
let diagnostics = test_path(
|
|
||||||
Path::new("pep8_naming").join(path).as_path(),
|
|
||||||
&settings::LinterSettings {
|
|
||||||
preview: PreviewMode::Enabled,
|
|
||||||
..settings::LinterSettings::for_rule(rule_code)
|
|
||||||
},
|
|
||||||
)?;
|
|
||||||
assert_messages!(snapshot, diagnostics);
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn camelcase_imported_as_incorrect_convention() -> Result<()> {
|
fn camelcase_imported_as_incorrect_convention() -> Result<()> {
|
||||||
let diagnostics = test_path(
|
let diagnostics = test_path(
|
||||||
|
|
|
@ -82,8 +82,8 @@ impl Violation for InvalidFirstArgumentNameForMethod {
|
||||||
/// Checks for class methods that use a name other than `cls` for their
|
/// Checks for class methods that use a name other than `cls` for their
|
||||||
/// first argument.
|
/// first argument.
|
||||||
///
|
///
|
||||||
/// With [`preview`] enabled, the method `__new__` is exempted from this
|
/// The method `__new__` is exempted from this
|
||||||
/// check and the corresponding violation is then caught by
|
/// check and the corresponding violation is caught by
|
||||||
/// [`bad-staticmethod-argument`][PLW0211].
|
/// [`bad-staticmethod-argument`][PLW0211].
|
||||||
///
|
///
|
||||||
/// ## Why is this bad?
|
/// ## Why is this bad?
|
||||||
|
@ -164,8 +164,6 @@ enum FunctionType {
|
||||||
Method,
|
Method,
|
||||||
/// The function is a class method.
|
/// The function is a class method.
|
||||||
ClassMethod,
|
ClassMethod,
|
||||||
/// The function is the method `__new__`
|
|
||||||
NewMethod,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl FunctionType {
|
impl FunctionType {
|
||||||
|
@ -177,11 +175,6 @@ impl FunctionType {
|
||||||
is_new: false,
|
is_new: false,
|
||||||
}
|
}
|
||||||
.into(),
|
.into(),
|
||||||
Self::NewMethod => InvalidFirstArgumentNameForClassMethod {
|
|
||||||
argument_name,
|
|
||||||
is_new: true,
|
|
||||||
}
|
|
||||||
.into(),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -189,7 +182,6 @@ impl FunctionType {
|
||||||
match self {
|
match self {
|
||||||
Self::Method => "self",
|
Self::Method => "self",
|
||||||
Self::ClassMethod => "cls",
|
Self::ClassMethod => "cls",
|
||||||
Self::NewMethod => "cls",
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -197,7 +189,6 @@ impl FunctionType {
|
||||||
match self {
|
match self {
|
||||||
Self::Method => Rule::InvalidFirstArgumentNameForMethod,
|
Self::Method => Rule::InvalidFirstArgumentNameForMethod,
|
||||||
Self::ClassMethod => Rule::InvalidFirstArgumentNameForClassMethod,
|
Self::ClassMethod => Rule::InvalidFirstArgumentNameForClassMethod,
|
||||||
Self::NewMethod => Rule::InvalidFirstArgumentNameForClassMethod,
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -241,11 +232,10 @@ pub(crate) fn invalid_first_argument_name(checker: &Checker, scope: &Scope) {
|
||||||
IsMetaclass::Maybe => return,
|
IsMetaclass::Maybe => return,
|
||||||
},
|
},
|
||||||
function_type::FunctionType::ClassMethod => FunctionType::ClassMethod,
|
function_type::FunctionType::ClassMethod => FunctionType::ClassMethod,
|
||||||
// In preview, this violation is caught by `PLW0211` instead
|
// This violation is caught by `PLW0211` instead
|
||||||
function_type::FunctionType::NewMethod if checker.settings.preview.is_enabled() => {
|
function_type::FunctionType::NewMethod => {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
function_type::FunctionType::NewMethod => FunctionType::NewMethod,
|
|
||||||
};
|
};
|
||||||
if !checker.enabled(function_type.rule()) {
|
if !checker.enabled(function_type.rule()) {
|
||||||
return;
|
return;
|
||||||
|
|
|
@ -446,10 +446,6 @@ mod tests {
|
||||||
Path::new("repeated_equality_comparison.py")
|
Path::new("repeated_equality_comparison.py")
|
||||||
)]
|
)]
|
||||||
#[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"))]
|
#[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"))]
|
||||||
#[test_case(
|
|
||||||
Rule::BadStaticmethodArgument,
|
|
||||||
Path::new("bad_staticmethod_argument.py")
|
|
||||||
)]
|
|
||||||
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!(
|
let snapshot = format!(
|
||||||
"preview__{}_{}",
|
"preview__{}_{}",
|
||||||
|
|
|
@ -3,6 +3,7 @@ use ruff_macros::{derive_message_formats, ViolationMetadata};
|
||||||
use ruff_python_ast as ast;
|
use ruff_python_ast as ast;
|
||||||
use ruff_python_ast::ParameterWithDefault;
|
use ruff_python_ast::ParameterWithDefault;
|
||||||
use ruff_python_semantic::analyze::function_type;
|
use ruff_python_semantic::analyze::function_type;
|
||||||
|
use ruff_python_semantic::analyze::function_type::FunctionType;
|
||||||
use ruff_python_semantic::Scope;
|
use ruff_python_semantic::Scope;
|
||||||
use ruff_text_size::Ranged;
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
|
@ -10,9 +11,7 @@ use crate::checkers::ast::Checker;
|
||||||
|
|
||||||
/// ## What it does
|
/// ## What it does
|
||||||
/// Checks for static methods that use `self` or `cls` as their first argument.
|
/// Checks for static methods that use `self` or `cls` as their first argument.
|
||||||
///
|
/// This rule also applies to `__new__` methods, which are implicitly static.
|
||||||
/// If [`preview`] mode is enabled, this rule also applies to
|
|
||||||
/// `__new__` methods, which are implicitly static.
|
|
||||||
///
|
///
|
||||||
/// ## Why is this bad?
|
/// ## Why is this bad?
|
||||||
/// [PEP 8] recommends the use of `self` and `cls` as the first arguments for
|
/// [PEP 8] recommends the use of `self` and `cls` as the first arguments for
|
||||||
|
@ -77,9 +76,8 @@ pub(crate) fn bad_staticmethod_argument(checker: &Checker, scope: &Scope) {
|
||||||
);
|
);
|
||||||
|
|
||||||
match type_ {
|
match type_ {
|
||||||
function_type::FunctionType::StaticMethod => {}
|
FunctionType::StaticMethod | FunctionType::NewMethod => {}
|
||||||
function_type::FunctionType::NewMethod if checker.settings.preview.is_enabled() => {}
|
FunctionType::Function | FunctionType::Method | FunctionType::ClassMethod => {
|
||||||
_ => {
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
|
@ -26,3 +26,12 @@ bad_staticmethod_argument.py:19:15: PLW0211 First argument of a static method sh
|
||||||
| ^^^^ PLW0211
|
| ^^^^ PLW0211
|
||||||
20 | pass
|
20 | pass
|
||||||
|
|
|
|
||||||
|
|
||||||
|
bad_staticmethod_argument.py:55:17: PLW0211 First argument of a static method should not be named `self`
|
||||||
|
|
|
||||||
|
53 | # `self` but not with `cls` as first argument - see above).
|
||||||
|
54 | class Foo:
|
||||||
|
55 | def __new__(self, x, y, z): # [bad-staticmethod-argument]
|
||||||
|
| ^^^^ PLW0211
|
||||||
|
56 | pass
|
||||||
|
|
|
||||||
|
|
|
@ -1,37 +0,0 @@
|
||||||
---
|
|
||||||
source: crates/ruff_linter/src/rules/pylint/mod.rs
|
|
||||||
---
|
|
||||||
bad_staticmethod_argument.py:3:13: PLW0211 First argument of a static method should not be named `self`
|
|
||||||
|
|
|
||||||
1 | class Wolf:
|
|
||||||
2 | @staticmethod
|
|
||||||
3 | def eat(self): # [bad-staticmethod-argument]
|
|
||||||
| ^^^^ PLW0211
|
|
||||||
4 | pass
|
|
||||||
|
|
|
||||||
|
|
||||||
bad_staticmethod_argument.py:15:13: PLW0211 First argument of a static method should not be named `cls`
|
|
||||||
|
|
|
||||||
13 | class Sheep:
|
|
||||||
14 | @staticmethod
|
|
||||||
15 | def eat(cls, x, y, z): # [bad-staticmethod-argument]
|
|
||||||
| ^^^ PLW0211
|
|
||||||
16 | pass
|
|
||||||
|
|
|
||||||
|
|
||||||
bad_staticmethod_argument.py:19:15: PLW0211 First argument of a static method should not be named `self`
|
|
||||||
|
|
|
||||||
18 | @staticmethod
|
|
||||||
19 | def sleep(self, x, y, z): # [bad-staticmethod-argument]
|
|
||||||
| ^^^^ PLW0211
|
|
||||||
20 | pass
|
|
||||||
|
|
|
||||||
|
|
||||||
bad_staticmethod_argument.py:55:17: PLW0211 First argument of a static method should not be named `self`
|
|
||||||
|
|
|
||||||
53 | # `self` but not with `cls` as first argument - see above).
|
|
||||||
54 | class Foo:
|
|
||||||
55 | def __new__(self, x, y, z): # [bad-staticmethod-argument]
|
|
||||||
| ^^^^ PLW0211
|
|
||||||
56 | pass
|
|
||||||
|
|
|
Loading…
Add table
Add a link
Reference in a new issue