[flake8-pyi] Implement PYI063

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI063.py

@@ -0,0 +1,64 @@
+# See https://peps.python.org/pep-0484/#positional-only-arguments
+# for the full details on which arguments using the older syntax should/shouldn't
+# be considered positional-only arguments by type checkers.
+from typing import Self
+
+def bad(__x: int) -> None: ...  # PYI063
+def also_bad(__x: int, __y: str) -> None: ...  # PYI063
+def still_bad(__x_: int) -> None: ...  # PYI063
+
+def no_args() -> None: ...
+def okay(__x__: int) -> None: ...
+# The first argument isn't positional-only, so logically the second can't be either:
+def also_okay(x: int, __y: str) -> None: ...
+def fine(x: bytes, /) -> None: ...
+def no_idea_why_youd_do_this(__x: int, /, __y: str) -> None: ...
+def cool(_x__: int) -> None: ...
+def also_cool(x__: int) -> None: ...
+def unclear_from_pep_484_if_this_is_positional_or_not(__: str) -> None: ...
+def _(_: int) -> None: ...
+
+class Foo:
+    def bad(__self) -> None: ...  # PYI063
+    @staticmethod
+    def bad2(__self) -> None: ...  # PYI063
+    def bad3(__self, __x: int) -> None: ...  # PYI063
+    def still_bad(self, __x_: int) -> None: ...  # PYI063
+    @staticmethod
+    def this_is_bad_too(__x: int) -> None: ...  # PYI063
+    @classmethod
+    def not_good(cls, __foo: int) -> None: ...  # PYI063
+
+    # The first non-self argument isn't positional-only, so logically the second can't be either:
+    def okay1(self, x: int, __y: int) -> None: ...
+    # Same here:
+    @staticmethod
+    def okay2(x: int, __y_: int) -> None: ...
+    @staticmethod
+    def no_args() -> int: ...
+    def okay3(__self__, __x__: int, __y: str) -> None: ...
+    def okay4(self, /) -> None: ...
+    def okay5(self, x: int, /) -> None: ...
+    def okay6(__self, /) -> None: ...
+    def cool(_self__: int) -> None: ...
+    def also_cool(self__: int) -> None: ...
+    def unclear_from_pep_484_if_this_is_positional_or_not(__: str) -> None: ...
+    def _(_: int) -> None: ...
+    @classmethod
+    def fine(cls, foo: int, /) -> None: ...
+
+class Metaclass(type):
+    @classmethod
+    def __new__(mcls, __name: str, __bases: tuple[type, ...], __namespace: dict, **kwds) -> Self: ...  # PYI063
+
+class Metaclass2(type):
+    @classmethod
+    def __new__(metacls, __name: str, __bases: tuple[type, ...], __namespace: dict, **kwds) -> Self: ...  # PYI063
+
+class GoodMetaclass(type):
+    @classmethod
+    def __new__(mcls, name: str, bases: tuple[type, ...], namespace: dict, /, **kwds) -> Self: ...
+
+class GoodMetaclass2(type):
+    @classmethod
+    def __new__(metacls, name: str, bases: tuple[type, ...], namespace: dict, /, **kwds) -> Self: ...

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI063.pyi

@@ -0,0 +1,64 @@
+# See https://peps.python.org/pep-0484/#positional-only-arguments
+# for the full details on which arguments using the older syntax should/shouldn't
+# be considered positional-only arguments by type checkers.
+from typing import Self
+
+def bad(__x: int) -> None: ...  # PYI063
+def also_bad(__x: int, __y: str) -> None: ...  # PYI063
+def still_bad(__x_: int) -> None: ...  # PYI063
+
+def no_args() -> None: ...
+def okay(__x__: int) -> None: ...
+# The first argument isn't positional-only, so logically the second can't be either:
+def also_okay(x: int, __y: str) -> None: ...
+def fine(x: bytes, /) -> None: ...
+def no_idea_why_youd_do_this(__x: int, /, __y: str) -> None: ...
+def cool(_x__: int) -> None: ...
+def also_cool(x__: int) -> None: ...
+def unclear_from_pep_484_if_this_is_positional_or_not(__: str) -> None: ...
+def _(_: int) -> None: ...
+
+class Foo:
+    def bad(__self) -> None: ...  # PYI063
+    @staticmethod
+    def bad2(__self) -> None: ...  # PYI063
+    def bad3(__self, __x: int) -> None: ...  # PYI063
+    def still_bad(self, __x_: int) -> None: ...  # PYI063  # TODO: doesn't get raised here
+    @staticmethod
+    def this_is_bad_too(__x: int) -> None: ...  # PYI063
+    @classmethod
+    def not_good(cls, __foo: int) -> None: ...  # PYI063
+
+    # The first non-self argument isn't positional-only, so logically the second can't be either:
+    def okay1(self, x: int, __y: int) -> None: ...
+    # Same here:
+    @staticmethod
+    def okay2(x: int, __y_: int) -> None: ...
+    @staticmethod
+    def no_args() -> int: ...
+    def okay3(__self__, __x__: int, __y: str) -> None: ...
+    def okay4(self, /) -> None: ...
+    def okay5(self, x: int, /) -> None: ...
+    def okay6(__self, /) -> None: ...
+    def cool(_self__: int) -> None: ...
+    def also_cool(self__: int) -> None: ...
+    def unclear_from_pep_484_if_this_is_positional_or_not(__: str) -> None: ...
+    def _(_: int) -> None: ...
+    @classmethod
+    def fine(cls, foo: int, /) -> None: ...
+
+class Metaclass(type):
+    @classmethod
+    def __new__(mcls, __name: str, __bases: tuple[type, ...], __namespace: dict, **kwds) -> Self: ...  # PYI063
+
+class Metaclass2(type):
+    @classmethod
+    def __new__(metacls, __name: str, __bases: tuple[type, ...], __namespace: dict, **kwds) -> Self: ...  # PYI063
+
+class GoodMetaclass(type):
+    @classmethod
+    def __new__(mcls, name: str, bases: tuple[type, ...], namespace: dict, /, **kwds) -> Self: ...
+
+class GoodMetaclass2(type):
+    @classmethod
+    def __new__(metacls, name: str, bases: tuple[type, ...], namespace: dict, /, **kwds) -> Self: ...

crates/ruff_linter/src/checkers/ast/analyze/definitions.rs

@@ -146,26 +146,20 @@ pub(crate) fn definitions(checker: &mut Checker) {
 
         // flake8-pyi
         if enforce_stubs {
-            if checker.enabled(Rule::DocstringInStub) {
-                flake8_pyi::rules::docstring_in_stubs(checker, docstring);
-            }
+            flake8_pyi::rules::docstring_in_stubs(checker, docstring);
         }
         if enforce_stubs_and_runtime {
-            if checker.enabled(Rule::IterMethodReturnIterable) {
-                flake8_pyi::rules::iter_method_return_iterable(checker, definition);
-            }
+            flake8_pyi::rules::iter_method_return_iterable(checker, definition);
         }
 
         // pylint
         if enforce_dunder_method {
-            if checker.enabled(Rule::BadDunderMethodName) {
-                if let Definition::Member(Member {
-                    kind: MemberKind::Method(method),
-                    ..
-                }) = definition
-                {
-                    pylint::rules::bad_dunder_method_name(checker, method);
-                }
+            if let Definition::Member(Member {
+                kind: MemberKind::Method(method),
+                ..
+            }) = definition
+            {
+                pylint::rules::bad_dunder_method_name(checker, method);
             }
         }
 

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

@@ -178,6 +178,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::RedundantNumericUnion) {
                 flake8_pyi::rules::redundant_numeric_union(checker, parameters);
             }
+            if checker.enabled(Rule::PrePep570PositionalArgument) {
+                flake8_pyi::rules::pre_pep570_positional_argument(checker, function_def);
+            }
             if checker.enabled(Rule::DunderFunctionName) {
                 if let Some(diagnostic) = pep8_naming::rules::dunder_function_name(
                     checker.semantic.current_scope(),

crates/ruff_linter/src/codes.rs

@@ -813,6 +813,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Pyi, "057") => (RuleGroup::Preview, rules::flake8_pyi::rules::ByteStringUsage),
         (Flake8Pyi, "059") => (RuleGroup::Preview, rules::flake8_pyi::rules::GenericNotLastBaseClass),
         (Flake8Pyi, "062") => (RuleGroup::Preview, rules::flake8_pyi::rules::DuplicateLiteralMember),
+        (Flake8Pyi, "063") => (RuleGroup::Preview, rules::flake8_pyi::rules::PrePep570PositionalArgument),
         (Flake8Pyi, "064") => (RuleGroup::Preview, rules::flake8_pyi::rules::RedundantFinalLiteral),
         (Flake8Pyi, "066") => (RuleGroup::Preview, rules::flake8_pyi::rules::BadVersionInfoOrder),
 

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

@@ -67,6 +67,8 @@ mod tests {
     #[test_case(Rule::PatchVersionComparison, Path::new("PYI004.pyi"))]
     #[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.py"))]
     #[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.pyi"))]
+    #[test_case(Rule::PrePep570PositionalArgument, Path::new("PYI063.py"))]
+    #[test_case(Rule::PrePep570PositionalArgument, Path::new("PYI063.pyi"))]
     #[test_case(Rule::RedundantFinalLiteral, Path::new("PYI064.py"))]
     #[test_case(Rule::RedundantFinalLiteral, Path::new("PYI064.pyi"))]
     #[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.py"))]

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

@@ -20,6 +20,7 @@ pub(crate) use non_self_return_type::*;
 pub(crate) use numeric_literal_too_long::*;
 pub(crate) use pass_in_class_body::*;
 pub(crate) use pass_statement_stub_body::*;
+pub(crate) use pre_pep570_positional_argument::*;
 pub(crate) use prefix_type_params::*;
 pub(crate) use quoted_annotation_in_stub::*;
 pub(crate) use redundant_final_literal::*;
@@ -62,6 +63,7 @@ mod non_self_return_type;
 mod numeric_literal_too_long;
 mod pass_in_class_body;
 mod pass_statement_stub_body;
+mod pre_pep570_positional_argument;
 mod prefix_type_params;
 mod quoted_annotation_in_stub;
 mod redundant_final_literal;

crates/ruff_linter/src/rules/flake8_pyi/rules/pre_pep570_positional_argument.rs

@@ -0,0 +1,98 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::identifier::Identifier;
+use ruff_python_ast::{self as ast, ParameterWithDefault};
+use ruff_python_semantic::analyze::function_type;
+
+use crate::checkers::ast::Checker;
+use crate::settings::types::PythonVersion;
+
+/// ## What it does
+/// Checks for the presence of [PEP 484]-style positional-only arguments.
+///
+/// ## Why is this bad?
+/// Historically, [PEP 484] recommended prefixing positional-only arguments
+/// with a double underscore (`__`). However, [PEP 570] introduced a dedicated
+/// syntax for positional-only arguments, which should be preferred.
+///
+/// ## Example
+///
+/// ```python
+/// def foo(__x: int) -> None:
+///     ...
+/// ```
+///
+/// Use instead:
+///
+/// ```python
+/// def foo(x: int, /) -> None:
+///     ...
+/// ```
+///
+/// [PEP 484]: https://peps.python.org/pep-0484/#positional-only-arguments
+/// [PEP 570]: https://peps.python.org/pep-0570
+#[violation]
+pub struct PrePep570PositionalArgument;
+
+impl Violation for PrePep570PositionalArgument {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Use PEP 570 syntax for positional-only arguments")
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Add `/` to function signature".to_string())
+    }
+}
+
+/// PYI063
+pub(crate) fn pre_pep570_positional_argument(
+    checker: &mut Checker,
+    function_def: &ast::StmtFunctionDef,
+) {
+    // PEP 570 was introduced in Python 3.8.
+    if checker.settings.target_version < PythonVersion::Py38 {
+        return;
+    }
+
+    if !function_def.parameters.posonlyargs.is_empty() {
+        return;
+    }
+
+    if function_def.parameters.args.is_empty() {
+        return;
+    }
+
+    let semantic = checker.semantic();
+    let scope = semantic.current_scope();
+    let function_type = function_type::classify(
+        &function_def.name,
+        &function_def.decorator_list,
+        scope,
+        semantic,
+        &checker.settings.pep8_naming.classmethod_decorators,
+        &checker.settings.pep8_naming.staticmethod_decorators,
+    );
+
+    // If the method has a `self` or `cls` argument, skip it.
+    let skip = usize::from(matches!(
+        function_type,
+        function_type::FunctionType::Method | function_type::FunctionType::ClassMethod
+    ));
+
+    if let Some(arg) = function_def.parameters.args.get(skip) {
+        if is_pre_pep570_positional_only(arg) {
+            checker.diagnostics.push(Diagnostic::new(
+                PrePep570PositionalArgument,
+                arg.identifier(),
+            ));
+        }
+    }
+}
+
+/// Returns `true` if the [`ParameterWithDefault`] is an old-style positional-only argument (i.e.,
+/// its name starts with `__` and does not end with `__`).
+fn is_pre_pep570_positional_only(arg: &ParameterWithDefault) -> bool {
+    let arg_name = &arg.parameter.name;
+    arg_name.starts_with("__") && !arg_name.ends_with("__")
+}