diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI050.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI050.py new file mode 100644 index 0000000000..042fe887ec --- /dev/null +++ b/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): + ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI050.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI050.pyi new file mode 100644 index 0000000000..4720ee7756 --- /dev/null +++ b/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): ... diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 03165631dc..e2892a3d49 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/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) { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index d0eb6f83f5..641755699e 100644 --- a/crates/ruff/src/codes.rs +++ b/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), diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index 7e27c598a5..783018fba8 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/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"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 8b897e7ad5..a754a518d2 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/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; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs b/crates/ruff/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs new file mode 100644 index 0000000000..8fcb45d403 --- /dev/null +++ b/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"), + } + } +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI050_PYI050.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI050_PYI050.py.snap new file mode 100644 index 0000000000..d1aa2e9116 --- /dev/null +++ b/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 +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI050_PYI050.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI050_PYI050.pyi.snap new file mode 100644 index 0000000000..f4f7de2503 --- /dev/null +++ b/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): ... + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 29b6d3f32e..737f0fdc4a 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2254,6 +2254,7 @@ "PYI045", "PYI048", "PYI05", + "PYI050", "PYI052", "PYI053", "PYI054",