[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

@@ -32,7 +32,8 @@ pub(crate) fn definitions(checker: &mut Checker) {
         Rule::MissingTypeKwargs,
         Rule::MissingTypeSelf,
     ]);
-    let enforce_stubs = checker.source_type.is_stub() && checker.enabled(Rule::DocstringInStub);
+    let enforce_stubs = checker.source_type.is_stub()
+        && checker.any_enabled(&[Rule::DocstringInStub, Rule::OldStylePositionalOnlyArg]);
     let enforce_stubs_and_runtime = checker.enabled(Rule::IterMethodReturnIterable);
     let enforce_dunder_method = checker.enabled(Rule::BadDunderMethodName);
     let enforce_docstrings = checker.any_enabled(&[
@@ -149,23 +150,22 @@ pub(crate) fn definitions(checker: &mut Checker) {
             if checker.enabled(Rule::DocstringInStub) {
                 flake8_pyi::rules::docstring_in_stubs(checker, docstring);
             }
+            if checker.enabled(Rule::OldStylePositionalOnlyArg) {
+                flake8_pyi::rules::old_style_positional_only_arg(checker, definition);
+            }
         }
         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/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::OldStylePositionalOnlyArg),
         (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::OldStylePositionalOnlyArg, Path::new("PYI063.py"))]
+    #[test_case(Rule::OldStylePositionalOnlyArg, 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

@@ -18,6 +18,7 @@ pub(crate) use no_return_argument_annotation::*;
 pub(crate) use non_empty_stub_body::*;
 pub(crate) use non_self_return_type::*;
 pub(crate) use numeric_literal_too_long::*;
+pub(crate) use old_style_positional_only_arg::*;
 pub(crate) use pass_in_class_body::*;
 pub(crate) use pass_statement_stub_body::*;
 pub(crate) use prefix_type_params::*;
@@ -60,6 +61,7 @@ mod no_return_argument_annotation;
 mod non_empty_stub_body;
 mod non_self_return_type;
 mod numeric_literal_too_long;
+mod old_style_positional_only_arg;
 mod pass_in_class_body;
 mod pass_statement_stub_body;
 mod prefix_type_params;

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

@@ -0,0 +1,84 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::ParameterWithDefault;
+use ruff_python_semantic::{analyze::function_type, Definition};
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for the presence of old-style positional-only arguments in stub files.
+///
+/// ## Why is this bad?
+/// [PEP 570][1] defines new syntax for positional-only arguments, that should
+/// be preferred over the syntax defined in [PEP 484][2].
+///
+/// ## Example
+/// ```python
+/// def foo(__x: int) -> None:
+/// ```
+///
+/// Use instead:
+/// ```python
+/// def foo(x: int, /) -> None: ...
+/// ```
+///
+/// [1]: https://peps.python.org/pep-0570
+/// [2]: https://peps.python.org/pep-0484/#positional-only-arguments
+#[violation]
+pub struct OldStylePositionalOnlyArg;
+
+impl Violation for OldStylePositionalOnlyArg {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Prefer PEP 570 syntax for positional-only arguments in stubs")
+    }
+}
+
+/// PYI063
+pub(crate) fn old_style_positional_only_arg(checker: &mut Checker, definition: &Definition) {
+    let Some(function) = definition.as_function_def() else {
+        return;
+    };
+    if !function.parameters.posonlyargs.is_empty() {
+        return;
+    }
+
+    let mut args = function.parameters.args.iter();
+    let Some(first_arg) = args.next() else {
+        return;
+    };
+
+    let semantic = checker.semantic();
+    // TODO: this scope is wrong.
+    let scope = semantic.current_scope();
+    let function_type = function_type::classify(
+        &function.name,
+        &function.decorator_list,
+        scope,
+        semantic,
+        &checker.settings.pep8_naming.classmethod_decorators,
+        &checker.settings.pep8_naming.staticmethod_decorators,
+    );
+    if is_old_style_positional_only_arg(first_arg) {
+        checker.diagnostics.push(Diagnostic::new(
+            OldStylePositionalOnlyArg,
+            first_arg.range(),
+        ));
+    }
+    if matches!(function_type, function_type::FunctionType::Method) {
+        if let Some(second_arg) = args.next() {
+            if is_old_style_positional_only_arg(second_arg) {
+                checker.diagnostics.push(Diagnostic::new(
+                    OldStylePositionalOnlyArg,
+                    second_arg.range(),
+                ));
+            }
+        }
+    }
+}
+
+fn is_old_style_positional_only_arg(arg: &ParameterWithDefault) -> bool {
+    let arg_name = &arg.parameter.name;
+    arg_name.starts_with("__") && !arg_name.ends_with("__")
+}

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI063_PYI063.py.snap

@@ -0,0 +1,4 @@
+---
+source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
+---
+

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI063_PYI063.pyi.snap

@@ -0,0 +1,69 @@
+---
+source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
+---
+PYI063.pyi:6:9: PYI063 Prefer PEP 570 syntax for positional-only arguments in stubs
+  |
+4 | from typing import Self
+5 | 
+6 | def bad(__x: int) -> None: ...  # PYI063
+  |         ^^^^^^^^ PYI063
+7 | def also_bad(__x: int, __y: str) -> None: ...  # PYI063
+8 | def still_bad(__x_: int) -> None: ...  # PYI063
+  |
+
+PYI063.pyi:7:14: PYI063 Prefer PEP 570 syntax for positional-only arguments in stubs
+  |
+6 | def bad(__x: int) -> None: ...  # PYI063
+7 | def also_bad(__x: int, __y: str) -> None: ...  # PYI063
+  |              ^^^^^^^^ PYI063
+8 | def still_bad(__x_: int) -> None: ...  # PYI063
+  |
+
+PYI063.pyi:8:15: PYI063 Prefer PEP 570 syntax for positional-only arguments in stubs
+   |
+ 6 | def bad(__x: int) -> None: ...  # PYI063
+ 7 | def also_bad(__x: int, __y: str) -> None: ...  # PYI063
+ 8 | def still_bad(__x_: int) -> None: ...  # PYI063
+   |               ^^^^^^^^^ PYI063
+ 9 | 
+10 | def no_args() -> None: ...
+   |
+
+PYI063.pyi:22:13: PYI063 Prefer PEP 570 syntax for positional-only arguments in stubs
+   |
+21 | class Foo:
+22 |     def bad(__self) -> None: ...  # PYI063
+   |             ^^^^^^ PYI063
+23 |     @staticmethod
+24 |     def bad2(__self) -> None: ...  # PYI063
+   |
+
+PYI063.pyi:24:14: PYI063 Prefer PEP 570 syntax for positional-only arguments in stubs
+   |
+22 |     def bad(__self) -> None: ...  # PYI063
+23 |     @staticmethod
+24 |     def bad2(__self) -> None: ...  # PYI063
+   |              ^^^^^^ PYI063
+25 |     def bad3(__self, __x: int) -> None: ...  # PYI063
+26 |     def still_bad(self, __x_: int) -> None: ...  # PYI063  # TODO: doesn't get raised here
+   |
+
+PYI063.pyi:25:14: PYI063 Prefer PEP 570 syntax for positional-only arguments in stubs
+   |
+23 |     @staticmethod
+24 |     def bad2(__self) -> None: ...  # PYI063
+25 |     def bad3(__self, __x: int) -> None: ...  # PYI063
+   |              ^^^^^^ PYI063
+26 |     def still_bad(self, __x_: int) -> None: ...  # PYI063  # TODO: doesn't get raised here
+27 |     @staticmethod
+   |
+
+PYI063.pyi:28:25: PYI063 Prefer PEP 570 syntax for positional-only arguments in stubs
+   |
+26 |     def still_bad(self, __x_: int) -> None: ...  # PYI063  # TODO: doesn't get raised here
+27 |     @staticmethod
+28 |     def this_is_bad_too(__x: int) -> None: ...  # PYI063
+   |                         ^^^^^^^^ PYI063
+29 |     @classmethod
+30 |     def not_good(cls, __foo: int) -> None: ...  # PYI063
+   |