Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-pyi] Implement PYI050 #4884

Merged
merged 5 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI050.py
Original file line number Diff line number Diff line change
@@ -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):
...
12 changes: 12 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI050.pyi
Original file line number Diff line number Diff line change
@@ -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): ...
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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"),
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---

Original file line number Diff line number Diff line change
@@ -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): ...
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading