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] Teach various rules that annotations might be stringized #12951

Merged
merged 1 commit into from
Sep 2, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@


class Bad:
def __eq__(self, other: Any) -> bool: ... # Y032
def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
def __eq__(self, other: Any) -> bool: ... # PYI032
def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032


class Good:
Expand All @@ -22,3 +22,7 @@ class Unannotated:
def __eq__(self) -> Any: ...
def __ne__(self) -> bool: ...


class BadStringized:
def __eq__(self, other: "Any") -> bool: ... # PYI032
def __ne__(self, other: "Any") -> bool: ... # PYI032
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import typing


class Bad:
def __eq__(self, other: Any) -> bool: ... # Y032
def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
def __eq__(self, other: Any) -> bool: ... # PYI032
def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032


class Good:
Expand All @@ -22,3 +22,6 @@ class Unannotated:
def __eq__(self) -> Any: ...
def __ne__(self) -> bool: ...

class BadStringized:
def __eq__(self, other: "Any") -> bool: ... # PYI032
def __ne__(self, other: "Any") -> bool: ... # PYI032
Original file line number Diff line number Diff line change
Expand Up @@ -317,3 +317,7 @@ def __ne__(self, other: Any) -> bool:

def __imul__(self, other: Any) -> list[str]:
...

class UsesStringizedAnnotations:
def __iadd__(self, other: "UsesStringizedAnnotations") -> "typing.Self":
return self
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,6 @@ def __str__(self) -> str: ...
def __eq__(self, other: Any) -> bool: ...
def __ne__(self, other: Any) -> bool: ...
def __imul__(self, other: Any) -> list[str]: ...

class UsesStringizedAnnotations:
def __iadd__(self, other: "UsesStringizedAnnotations") -> "typing.Self": ...
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,11 @@ def foo_no_return_pos_only(arg: int, /, arg2: NoReturn):

def foo_never(arg: Never):
...


def stringized(arg: "NoReturn"):
...


def stringized_good(arg: "Never"):
...
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@ def foo_int_kwargs_no_return(*args: NoReturn, **kwargs: int): ... # Error: PYI0
def foo_args_never(*args: Never): ...
def foo_kwargs_never(**kwargs: Never): ...
def foo_args_kwargs_never(*args: Never, **kwargs: Never): ...
def stringized(arg: "NoReturn"): ...
def stringized_good(arg: "Never"): ...
20 changes: 20 additions & 0 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,26 @@ impl<'a> Checker<'a> {
self.parsed_annotations_cache
.lookup_or_parse(annotation, self.locator.contents())
}

/// Apply a test to an annotation expression,
/// abstracting over the fact that the annotation expression might be "stringized".
///
/// A stringized annotation is one enclosed in string quotes:
/// `foo: "typing.Any"` means the same thing to a type checker as `foo: typing.Any`.
pub(crate) fn match_maybe_stringized_annotation(
&self,
expr: &ast::Expr,
match_fn: impl FnOnce(&ast::Expr) -> bool,
) -> bool {
if let ast::Expr::StringLiteral(string_annotation) = expr {
let Some(parsed_annotation) = self.parse_type_annotation(string_annotation) else {
return false;
};
match_fn(parsed_annotation.expression())
} else {
match_fn(expr)
}
}
}

impl<'a> Visitor<'a> for Checker<'a> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,23 +78,27 @@ pub(crate) fn any_eq_ne_annotation(checker: &mut Checker, name: &str, parameters
return;
}

if semantic.match_typing_expr(annotation, "Any") {
let mut diagnostic = Diagnostic::new(
AnyEqNeAnnotation {
method_name: name.to_string(),
},
annotation.range(),
);
// Ex) `def __eq__(self, obj: Any): ...`
diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol(
"object",
annotation.start(),
semantic,
)?;
let binding_edit = Edit::range_replacement(binding, annotation.range());
Ok(Fix::safe_edits(binding_edit, import_edit))
});
checker.diagnostics.push(diagnostic);
if !checker.match_maybe_stringized_annotation(annotation, |expr| {
semantic.match_typing_expr(expr, "Any")
}) {
return;
}

let mut diagnostic = Diagnostic::new(
AnyEqNeAnnotation {
method_name: name.to_string(),
},
annotation.range(),
);
// Ex) `def __eq__(self, obj: Any): ...`
diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol(
"object",
annotation.start(),
semantic,
)?;
let binding_edit = Edit::range_replacement(binding, annotation.range());
Ok(Fix::safe_edits(binding_edit, import_edit))
});
checker.diagnostics.push(diagnostic);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::fmt;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{AnyParameterRef, Parameters};
use ruff_python_ast as ast;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -54,14 +54,17 @@ impl Violation for NoReturnArgumentAnnotationInStub {
}

/// PYI050
pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: &Parameters) {
pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: &ast::Parameters) {
// Ex) def func(arg: NoReturn): ...
// Ex) def func(arg: NoReturn, /): ...
// Ex) def func(*, arg: NoReturn): ...
// Ex) def func(*args: NoReturn): ...
// Ex) def func(**kwargs: NoReturn): ...
for annotation in parameters.iter().filter_map(AnyParameterRef::annotation) {
if checker.semantic().match_typing_expr(annotation, "NoReturn") {
for annotation in parameters
.iter()
.filter_map(ast::AnyParameterRef::annotation)
{
if is_no_return(annotation, checker) {
checker.diagnostics.push(Diagnostic::new(
NoReturnArgumentAnnotationInStub {
module: if checker.settings.target_version >= Py311 {
Expand All @@ -76,6 +79,12 @@ pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: &
}
}

fn is_no_return(expr: &ast::Expr, checker: &Checker) -> bool {
checker.match_maybe_stringized_annotation(expr, |expr| {
checker.semantic().match_typing_expr(expr, "NoReturn")
})
}

#[derive(Debug, PartialEq, Eq)]
enum TypingModule {
Typing,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pub(crate) fn non_self_return_type(

// In-place methods that are expected to return `Self`.
if is_inplace_bin_op(name) {
if !is_self(returns, semantic) {
if !is_self(returns, checker) {
checker.diagnostics.push(Diagnostic::new(
NonSelfReturnType {
class_name: class_def.name.to_string(),
Expand Down Expand Up @@ -235,8 +235,10 @@ fn is_name(expr: &Expr, name: &str) -> bool {
}

/// Return `true` if the given expression resolves to `typing.Self`.
fn is_self(expr: &Expr, semantic: &SemanticModel) -> bool {
semantic.match_typing_expr(expr, "Self")
fn is_self(expr: &Expr, checker: &Checker) -> bool {
checker.match_maybe_stringized_annotation(expr, |expr| {
checker.semantic().match_typing_expr(expr, "Self")
})
}

/// Return `true` if the given class extends `collections.abc.Iterator`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,70 @@ source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
PYI032.py:6:29: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__eq__`
|
5 | class Bad:
6 | def __eq__(self, other: Any) -> bool: ... # Y032
6 | def __eq__(self, other: Any) -> bool: ... # PYI032
| ^^^ PYI032
7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032
|
= help: Replace with `object`

ℹ Safe fix
3 3 |
4 4 |
5 5 | class Bad:
6 |- def __eq__(self, other: Any) -> bool: ... # Y032
6 |+ def __eq__(self, other: object) -> bool: ... # Y032
7 7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
6 |- def __eq__(self, other: Any) -> bool: ... # PYI032
6 |+ def __eq__(self, other: object) -> bool: ... # PYI032
7 7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032
8 8 |
9 9 |

PYI032.py:7:29: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__ne__`
|
5 | class Bad:
6 | def __eq__(self, other: Any) -> bool: ... # Y032
7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
6 | def __eq__(self, other: Any) -> bool: ... # PYI032
7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032
| ^^^^^^^^^^ PYI032
|
= help: Replace with `object`

ℹ Safe fix
4 4 |
5 5 | class Bad:
6 6 | def __eq__(self, other: Any) -> bool: ... # Y032
7 |- def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
7 |+ def __ne__(self, other: object) -> typing.Any: ... # Y032
6 6 | def __eq__(self, other: Any) -> bool: ... # PYI032
7 |- def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032
7 |+ def __ne__(self, other: object) -> typing.Any: ... # PYI032
8 8 |
9 9 |
10 10 | class Good:

PYI032.py:27:28: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__eq__`
|
26 | class BadStringized:
27 | def __eq__(self, other: "Any") -> bool: ... # PYI032
| ^^^^^ PYI032
28 | def __ne__(self, other: "Any") -> bool: ... # PYI032
|
= help: Replace with `object`

ℹ Safe fix
24 24 |
25 25 |
26 26 | class BadStringized:
27 |- def __eq__(self, other: "Any") -> bool: ... # PYI032
27 |+ def __eq__(self, other: object) -> bool: ... # PYI032
28 28 | def __ne__(self, other: "Any") -> bool: ... # PYI032

PYI032.py:28:28: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__ne__`
|
26 | class BadStringized:
27 | def __eq__(self, other: "Any") -> bool: ... # PYI032
28 | def __ne__(self, other: "Any") -> bool: ... # PYI032
| ^^^^^ PYI032
|
= help: Replace with `object`

ℹ Safe fix
25 25 |
26 26 | class BadStringized:
27 27 | def __eq__(self, other: "Any") -> bool: ... # PYI032
28 |- def __ne__(self, other: "Any") -> bool: ... # PYI032
28 |+ def __ne__(self, other: object) -> bool: ... # PYI032
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,70 @@ source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
PYI032.pyi:6:29: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__eq__`
|
5 | class Bad:
6 | def __eq__(self, other: Any) -> bool: ... # Y032
6 | def __eq__(self, other: Any) -> bool: ... # PYI032
| ^^^ PYI032
7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032
|
= help: Replace with `object`

ℹ Safe fix
3 3 |
4 4 |
5 5 | class Bad:
6 |- def __eq__(self, other: Any) -> bool: ... # Y032
6 |+ def __eq__(self, other: object) -> bool: ... # Y032
7 7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
6 |- def __eq__(self, other: Any) -> bool: ... # PYI032
6 |+ def __eq__(self, other: object) -> bool: ... # PYI032
7 7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032
8 8 |
9 9 |

PYI032.pyi:7:29: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__ne__`
|
5 | class Bad:
6 | def __eq__(self, other: Any) -> bool: ... # Y032
7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
6 | def __eq__(self, other: Any) -> bool: ... # PYI032
7 | def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032
| ^^^^^^^^^^ PYI032
|
= help: Replace with `object`

ℹ Safe fix
4 4 |
5 5 | class Bad:
6 6 | def __eq__(self, other: Any) -> bool: ... # Y032
7 |- def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
7 |+ def __ne__(self, other: object) -> typing.Any: ... # Y032
6 6 | def __eq__(self, other: Any) -> bool: ... # PYI032
7 |- def __ne__(self, other: typing.Any) -> typing.Any: ... # PYI032
7 |+ def __ne__(self, other: object) -> typing.Any: ... # PYI032
8 8 |
9 9 |
10 10 | class Good:

PYI032.pyi:26:28: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__eq__`
|
25 | class BadStringized:
26 | def __eq__(self, other: "Any") -> bool: ... # PYI032
| ^^^^^ PYI032
27 | def __ne__(self, other: "Any") -> bool: ... # PYI032
|
= help: Replace with `object`

ℹ Safe fix
23 23 | def __ne__(self) -> bool: ...
24 24 |
25 25 | class BadStringized:
26 |- def __eq__(self, other: "Any") -> bool: ... # PYI032
26 |+ def __eq__(self, other: object) -> bool: ... # PYI032
27 27 | def __ne__(self, other: "Any") -> bool: ... # PYI032

PYI032.pyi:27:28: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__ne__`
|
25 | class BadStringized:
26 | def __eq__(self, other: "Any") -> bool: ... # PYI032
27 | def __ne__(self, other: "Any") -> bool: ... # PYI032
| ^^^^^ PYI032
|
= help: Replace with `object`

ℹ Safe fix
24 24 |
25 25 | class BadStringized:
26 26 | def __eq__(self, other: "Any") -> bool: ... # PYI032
27 |- def __ne__(self, other: "Any") -> bool: ... # PYI032
27 |+ def __ne__(self, other: object) -> bool: ... # PYI032
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,9 @@ PYI050.py:27:47: PYI050 Prefer `typing.Never` over `NoReturn` for argument annot
28 | ...
|


PYI050.py:35:21: PYI050 Prefer `typing.Never` over `NoReturn` for argument annotations
|
35 | def stringized(arg: "NoReturn"):
| ^^^^^^^^^^ PYI050
36 | ...
|
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,11 @@ PYI050.pyi:20:37: PYI050 Prefer `typing.Never` over `NoReturn` for argument anno
22 | def foo_kwargs_never(**kwargs: Never): ...
|


PYI050.pyi:24:21: PYI050 Prefer `typing.Never` over `NoReturn` for argument annotations
|
22 | def foo_kwargs_never(**kwargs: Never): ...
23 | def foo_args_kwargs_never(*args: Never, **kwargs: Never): ...
24 | def stringized(arg: "NoReturn"): ...
| ^^^^^^^^^^ PYI050
25 | def stringized_good(arg: "Never"): ...
|
Loading