Skip to content

Commit

Permalink
[flake8-pyi] Teach various rules that annotations might be stringized
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexWaygood committed Aug 17, 2024
1 parent 96802d6 commit 5bf9327
Show file tree
Hide file tree
Showing 15 changed files with 202 additions and 52 deletions.
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"): ...
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_text_size::Ranged;

use super::helpers::match_maybe_stringized_annotation;
use crate::checkers::ast::Checker;

/// ## What it does
Expand Down Expand Up @@ -78,23 +79,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 !match_maybe_stringized_annotation(annotation, checker.locator(), |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);
}
31 changes: 31 additions & 0 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/helpers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use ruff_python_ast as ast;
use ruff_python_parser::typing::parse_type_annotation;
use ruff_source_file::Locator;

/// 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(super) fn match_maybe_stringized_annotation(
expr: &ast::Expr,
locator: &Locator,
match_fn: impl FnOnce(&ast::Expr) -> bool,
) -> bool {
if let ast::Expr::StringLiteral(string_annotation) = expr {
let Ok((parsed_annotation, _)) =
parse_type_annotation(string_annotation, locator.contents())
else {
return false;
};
if !parsed_annotation.errors().is_empty() {
return false;
}
let ast::ModExpression {
body: annotation, ..
} = parsed_annotation.into_syntax();
match_fn(&annotation)
} else {
match_fn(expr)
}
}
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ mod ellipsis_in_non_empty_class_body;
mod exit_annotations;
mod future_annotations_in_stub;
mod generic_not_last_base_class;
mod helpers;
mod iter_method_return_iterable;
mod no_return_argument_annotation;
mod non_empty_stub_body;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ 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_python_semantic::SemanticModel;
use ruff_source_file::Locator;
use ruff_text_size::Ranged;

use super::helpers::match_maybe_stringized_annotation;
use crate::checkers::ast::Checker;
use crate::settings::types::PythonVersion::Py311;

Expand Down Expand Up @@ -54,14 +57,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.semantic(), checker.locator()) {
checker.diagnostics.push(Diagnostic::new(
NoReturnArgumentAnnotationInStub {
module: if checker.settings.target_version >= Py311 {
Expand All @@ -76,6 +82,12 @@ pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: &
}
}

fn is_no_return(expr: &ast::Expr, semantic: &SemanticModel, locator: &Locator) -> bool {
match_maybe_stringized_annotation(expr, locator, |expr| {
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 @@ -4,10 +4,12 @@ use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::map_subscript;
use ruff_python_ast::identifier::Identifier;
use ruff_source_file::Locator;
use ruff_python_semantic::analyze;
use ruff_python_semantic::analyze::visibility::{is_abstract, is_final, is_overload};
use ruff_python_semantic::{ScopeKind, SemanticModel};

use super::helpers::match_maybe_stringized_annotation;
use crate::checkers::ast::Checker;

/// ## What it does
Expand Down Expand Up @@ -156,7 +158,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, semantic, checker.locator()) {
checker.diagnostics.push(Diagnostic::new(
NonSelfReturnType {
class_name: class_def.name.to_string(),
Expand Down Expand Up @@ -241,9 +243,10 @@ fn is_name(expr: &Expr, name: &str) -> bool {
id.as_str() == name
}

/// 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, semantic: &SemanticModel, locator: &Locator) -> bool {
match_maybe_stringized_annotation(expr, locator, |expr| {
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
Loading

0 comments on commit 5bf9327

Please sign in to comment.