[flake8-pyi] Implement PYI050 (#4884)

crates/ruff/resources/test/fixtures/flake8_pyi/PYI050.py

@@ -0,0 +1,32 @@
+from typing import NoReturn, Never
+import typing_extensions
+
+
+def foo(arg):
+    ...
+
+
+def foo_int(arg: int):
+    ...
+
+
+def foo_no_return(arg: NoReturn):
+    ...
+
+
+def foo_no_return_typing_extensions(
+    arg: typing_extensions.NoReturn,
+):
+    ...
+
+
+def foo_no_return_kwarg(arg: int, *, arg2: NoReturn):
+    ...
+
+
+def foo_no_return_pos_only(arg: int, /, arg2: NoReturn):
+    ...
+
+
+def foo_never(arg: Never):
+    ...

crates/ruff/resources/test/fixtures/flake8_pyi/PYI050.pyi

@@ -0,0 +1,12 @@
+from typing import NoReturn, Never
+import typing_extensions
+
+def foo(arg): ...
+def foo_int(arg: int): ...
+def foo_no_return(arg: NoReturn): ...  # Error: PYI050
+def foo_no_return_typing_extensions(
+    arg: typing_extensions.NoReturn,
+): ...  # Error: PYI050
+def foo_no_return_kwarg(arg: int, *, arg2: NoReturn): ...  # Error: PYI050
+def foo_no_return_pos_only(arg: int, /, arg2: NoReturn): ...  # Error: PYI050
+def foo_never(arg: Never): ...

crates/ruff/src/checkers/ast/mod.rs

@@ -463,6 +463,9 @@ where
                     if self.enabled(Rule::StrOrReprDefinedInStub) {
                         flake8_pyi::rules::str_or_repr_defined_in_stub(self, stmt);
                     }
+                    if self.enabled(Rule::NoReturnArgumentAnnotationInStub) {
+                        flake8_pyi::rules::no_return_argument_annotation(self, args);
+                    }
                 }
 
                 if self.enabled(Rule::DunderFunctionName) {

crates/ruff/src/codes.rs

@@ -614,6 +614,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Pyi, "043") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::TSuffixedTypeAlias),
         (Flake8Pyi, "045") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::IterMethodReturnIterable),
         (Flake8Pyi, "048") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StubBodyMultipleStatements),
+        (Flake8Pyi, "050") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NoReturnArgumentAnnotationInStub),
         (Flake8Pyi, "052") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnannotatedAssignmentInStub),
         (Flake8Pyi, "054") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NumericLiteralTooLong),
         (Flake8Pyi, "053") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StringOrBytesTooLong),

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

@@ -32,6 +32,8 @@ mod tests {
     #[test_case(Rule::NonSelfReturnType, Path::new("PYI034.pyi"))]
     #[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.py"))]
     #[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.pyi"))]
+    #[test_case(Rule::NoReturnArgumentAnnotationInStub, Path::new("PYI050.py"))]
+    #[test_case(Rule::NoReturnArgumentAnnotationInStub, Path::new("PYI050.pyi"))]
     #[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.py"))]
     #[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.pyi"))]
     #[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.py"))]

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

@@ -11,6 +11,9 @@ pub(crate) use ellipsis_in_non_empty_class_body::{
 pub(crate) use iter_method_return_iterable::{
     iter_method_return_iterable, IterMethodReturnIterable,
 };
+pub(crate) use no_return_argument_annotation::{
+    no_return_argument_annotation, NoReturnArgumentAnnotationInStub,
+};
 pub(crate) use non_empty_stub_body::{non_empty_stub_body, NonEmptyStubBody};
 pub(crate) use non_self_return_type::{non_self_return_type, NonSelfReturnType};
 pub(crate) use numeric_literal_too_long::{numeric_literal_too_long, NumericLiteralTooLong};
@@ -47,6 +50,7 @@ mod docstring_in_stubs;
 mod duplicate_union_member;
 mod ellipsis_in_non_empty_class_body;
 mod iter_method_return_iterable;
+mod no_return_argument_annotation;
 mod non_empty_stub_body;
 mod non_self_return_type;
 mod numeric_literal_too_long;

crates/ruff/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs

@@ -0,0 +1,90 @@
+use std::fmt;
+
+use itertools::chain;
+use rustpython_parser::ast::Ranged;
+
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::prelude::Arguments;
+
+use crate::checkers::ast::Checker;
+use crate::settings::types::PythonVersion::Py311;
+
+#[violation]
+pub struct NoReturnArgumentAnnotationInStub {
+    module: TypingModule,
+}
+
+/// ## What it does
+/// Checks for uses of `typing.NoReturn` (and `typing_extensions.NoReturn`) in
+/// stubs.
+///
+/// ## Why is this bad?
+/// Prefer `typing.Never` (or `typing_extensions.Never`) over `typing.NoReturn`,
+/// as the former is more explicit about the intent of the annotation. This is
+/// a purely stylistic choice, as the two are semantically equivalent.
+///
+/// ## Example
+/// ```python
+/// from typing import NoReturn
+///
+///
+/// def foo(x: NoReturn): ...
+/// ```
+///
+/// Use instead:
+/// ```python
+/// from typing import Never
+///
+///
+/// def foo(x: Never): ...
+/// ```
+impl Violation for NoReturnArgumentAnnotationInStub {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let NoReturnArgumentAnnotationInStub { module } = self;
+        format!("Prefer `{module}.Never` over `NoReturn` for argument annotations")
+    }
+}
+
+/// PYI050
+pub(crate) fn no_return_argument_annotation(checker: &mut Checker, args: &Arguments) {
+    for annotation in chain!(
+        args.args.iter(),
+        args.posonlyargs.iter(),
+        args.kwonlyargs.iter()
+    )
+    .filter_map(|arg| arg.annotation.as_ref())
+    {
+        if checker
+            .semantic_model()
+            .match_typing_expr(annotation, "NoReturn")
+        {
+            checker.diagnostics.push(Diagnostic::new(
+                NoReturnArgumentAnnotationInStub {
+                    module: if checker.settings.target_version >= Py311 {
+                        TypingModule::Typing
+                    } else {
+                        TypingModule::TypingExtensions
+                    },
+                },
+                annotation.range(),
+            ));
+        }
+    }
+}
+
+#[derive(Debug, PartialEq, Eq)]
+enum TypingModule {
+    Typing,
+    TypingExtensions,
+}
+
+impl fmt::Display for TypingModule {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            TypingModule::Typing => fmt.write_str("typing"),
+            TypingModule::TypingExtensions => fmt.write_str("typing_extensions"),
+        }
+    }
+}

crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI050_PYI050.py.snap

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

crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI050_PYI050.pyi.snap

@@ -0,0 +1,33 @@
+---
+source: crates/ruff/src/rules/flake8_pyi/mod.rs
+---
+PYI050.pyi:6:24: PYI050 Prefer `typing_extensions.Never` over `NoReturn` for argument annotations
+   |
+ 6 | def foo(arg): ...
+ 7 | def foo_int(arg: int): ...
+ 8 | def foo_no_return(arg: NoReturn): ...  # Error: PYI050
+   |                        ^^^^^^^^ PYI050
+ 9 | def foo_no_return_typing_extensions(
+10 |     arg: typing_extensions.NoReturn,
+   |
+
+PYI050.pyi:10:44: PYI050 Prefer `typing_extensions.Never` over `NoReturn` for argument annotations
+   |
+10 |     arg: typing_extensions.NoReturn,
+11 | ): ...  # Error: PYI050
+12 | def foo_no_return_kwarg(arg: int, *, arg2: NoReturn): ...  # Error: PYI050
+   |                                            ^^^^^^^^ PYI050
+13 | def foo_no_return_pos_only(arg: int, /, arg2: NoReturn): ...  # Error: PYI050
+14 | def foo_never(arg: Never): ...
+   |
+
+PYI050.pyi:11:47: PYI050 Prefer `typing_extensions.Never` over `NoReturn` for argument annotations
+   |
+11 | ): ...  # Error: PYI050
+12 | def foo_no_return_kwarg(arg: int, *, arg2: NoReturn): ...  # Error: PYI050
+13 | def foo_no_return_pos_only(arg: int, /, arg2: NoReturn): ...  # Error: PYI050
+   |                                               ^^^^^^^^ PYI050
+14 | def foo_never(arg: Never): ...
+   |
+
+