[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
#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.

Author: MichaReiser

crates/ruff_linter/src/rules/pep8_naming/mod.rs

@@ -14,13 +14,13 @@ mod tests {
     use crate::registry::Rule;
     use crate::rules::pep8_naming::settings::IgnoreNames;
     use crate::rules::{flake8_import_conventions, pep8_naming};
-    use crate::settings::types::PreviewMode;
     use crate::test::test_path;
     use crate::{assert_messages, settings};
 
     #[test_case(Rule::InvalidClassName, Path::new("N801.py"))]
     #[test_case(Rule::InvalidFunctionName, Path::new("N802.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::InvalidFirstArgumentNameForMethod, Path::new("N805.py"))]
     #[test_case(Rule::NonLowercaseVariableInFunction, Path::new("N806.py"))]
@@ -89,24 +89,6 @@ mod tests {
         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]
     fn camelcase_imported_as_incorrect_convention() -> Result<()> {
         let diagnostics = test_path(

crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs

@@ -82,8 +82,8 @@ impl Violation for InvalidFirstArgumentNameForMethod {
 /// Checks for class methods that use a name other than `cls` for their
 /// first argument.
 ///
-/// With [`preview`] enabled, the method `__new__` is exempted from this
-/// check and the corresponding violation is then caught by
+/// The method `__new__` is exempted from this
+/// check and the corresponding violation is caught by
 /// [`bad-staticmethod-argument`][PLW0211].
 ///
 /// ## Why is this bad?
@@ -164,8 +164,6 @@ enum FunctionType {
     Method,
     /// The function is a class method.
     ClassMethod,
-    /// The function is the method `__new__`
-    NewMethod,
 }
 
 impl FunctionType {
@@ -177,27 +175,20 @@ impl FunctionType {
                 is_new: false,
             }
             .into(),
-            Self::NewMethod => InvalidFirstArgumentNameForClassMethod {
-                argument_name,
-                is_new: true,
-            }
-            .into(),
         }
     }
 
     const fn valid_first_argument_name(self) -> &'static str {
         match self {
             Self::Method => "self",
             Self::ClassMethod => "cls",
-            Self::NewMethod => "cls",
         }
     }
 
     const fn rule(self) -> Rule {
         match self {
             Self::Method => Rule::InvalidFirstArgumentNameForMethod,
             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,
         },
         function_type::FunctionType::ClassMethod => FunctionType::ClassMethod,
-        // In preview, this violation is caught by `PLW0211` instead
-        function_type::FunctionType::NewMethod if checker.settings.preview.is_enabled() => {
+        // This violation is caught by `PLW0211` instead
+        function_type::FunctionType::NewMethod => {
             return;
         }
-        function_type::FunctionType::NewMethod => FunctionType::NewMethod,
     };
     if !checker.enabled(function_type.rule()) {
         return;

crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__preview__N803_N804.py.snap renamed to crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N803_N804.py.snap

@@ -446,10 +446,6 @@ mod tests {
         Path::new("repeated_equality_comparison.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<()> {
         let snapshot = format!(
             "preview__{}_{}",

crates/ruff_linter/src/rules/pylint/mod.rs

@@ -3,16 +3,15 @@ use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_ast as ast;
 use ruff_python_ast::ParameterWithDefault;
 use ruff_python_semantic::analyze::function_type;
+use ruff_python_semantic::analyze::function_type::FunctionType;
 use ruff_python_semantic::Scope;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 
 /// ## What it does
 /// Checks for static methods that use `self` or `cls` as their first argument.
-///
-/// If [`preview`] mode is enabled, this rule also applies to
-/// `__new__` methods, which are implicitly static.
+/// This rule also applies to `__new__` methods, which are implicitly static.
 ///
 /// ## Why is this bad?
 /// [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_ {
-        function_type::FunctionType::StaticMethod => {}
-        function_type::FunctionType::NewMethod if checker.settings.preview.is_enabled() => {}
-        _ => {
+        FunctionType::StaticMethod | FunctionType::NewMethod => {}
+        FunctionType::Function | FunctionType::Method | FunctionType::ClassMethod => {
             return;
         }
     };

crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs

@@ -26,3 +26,12 @@ bad_staticmethod_argument.py:19:15: PLW0211 First argument of a static method sh
    |               ^^^^ 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
+   |