Skip to content

Commit

Permalink
[flake8-pyi] Add PYI032 rule with autofix (#4695)
Browse files Browse the repository at this point in the history
  • Loading branch information
qdegraaf authored May 28, 2023
1 parent 51f04ee commit 0911ce4
Show file tree
Hide file tree
Showing 11 changed files with 198 additions and 0 deletions.
24 changes: 24 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from typing import Any
import typing


class Bad:
def __eq__(self, other: Any) -> bool: ... # Fine because not a stub file
def __ne__(self, other: typing.Any) -> typing.Any: ... # Fine because not a stub file


class Good:
def __eq__(self, other: object) -> bool: ...

def __ne__(self, obj: object) -> int: ...


class WeirdButFine:
def __eq__(self, other: Any, strange_extra_arg: list[str]) -> Any: ...
def __ne__(self, *, kw_only_other: Any) -> bool: ...


class Unannotated:
def __eq__(self) -> Any: ...
def __ne__(self) -> bool: ...

24 changes: 24 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from typing import Any
import typing


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


class Good:
def __eq__(self, other: object) -> bool: ...

def __ne__(self, obj: object) -> int: ...


class WeirdButFine:
def __eq__(self, other: Any, strange_extra_arg: list[str]) -> Any: ...
def __ne__(self, *, kw_only_other: Any) -> bool: ...


class Unannotated:
def __eq__(self) -> Any: ...
def __ne__(self) -> bool: ...

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 @@ -429,6 +429,9 @@ where
if self.enabled(Rule::StubBodyMultipleStatements) {
flake8_pyi::rules::stub_body_multiple_statements(self, stmt, body);
}
if self.enabled(Rule::AnyEqNeAnnotation) {
flake8_pyi::rules::any_eq_ne_annotation(self, name, 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 @@ -590,6 +590,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Pyi, "016") => (RuleGroup::Unspecified, Rule::DuplicateUnionMember),
(Flake8Pyi, "020") => (RuleGroup::Unspecified, Rule::QuotedAnnotationInStub),
(Flake8Pyi, "021") => (RuleGroup::Unspecified, Rule::DocstringInStub),
(Flake8Pyi, "032") => (RuleGroup::Unspecified, Rule::AnyEqNeAnnotation),
(Flake8Pyi, "033") => (RuleGroup::Unspecified, Rule::TypeCommentInStub),
(Flake8Pyi, "042") => (RuleGroup::Unspecified, Rule::SnakeCaseTypeAlias),
(Flake8Pyi, "043") => (RuleGroup::Unspecified, Rule::TSuffixedTypeAlias),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ ruff_macros::register_rules!(
rules::flake8_errmsg::rules::FStringInException,
rules::flake8_errmsg::rules::DotFormatInException,
// flake8-pyi
rules::flake8_pyi::rules::AnyEqNeAnnotation,
rules::flake8_pyi::rules::ArgumentDefaultInStub,
rules::flake8_pyi::rules::AssignmentDefaultInStub,
rules::flake8_pyi::rules::BadVersionInfoComparison,
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 @@ -12,6 +12,8 @@ mod tests {
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Rule::AnyEqNeAnnotation, Path::new("PYI032.py"))]
#[test_case(Rule::AnyEqNeAnnotation, Path::new("PYI032.pyi"))]
#[test_case(Rule::ArgumentDefaultInStub, Path::new("PYI014.py"))]
#[test_case(Rule::ArgumentDefaultInStub, Path::new("PYI014.pyi"))]
#[test_case(Rule::AssignmentDefaultInStub, Path::new("PYI015.py"))]
Expand Down
94 changes: 94 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
use rustpython_parser::ast::{Arguments, Ranged};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;
use crate::registry::AsRule;

/// ## What it does
/// Checks for `__eq__` and `__ne__` implementations that use `typing.Any` as
/// the type annotation for the `obj` parameter.
///
/// ## Why is this bad?
/// The Python documentation recommends the use of `object` to "indicate that a
/// value could be any type in a typesafe manner", while `Any` should be used to
/// "indicate that a value is dynamically typed."
///
/// The semantics of `__eq__` and `__ne__` are such that the `obj` parameter
/// should be any type, as opposed to a dynamically typed value. Therefore, the
/// `object` type annotation is more appropriate.
///
/// ## Example
/// ```python
/// class Foo:
/// def __eq__(self, obj: typing.Any):
/// ...
/// ```
///
/// Use instead:
/// ```python
/// class Foo:
/// def __eq__(self, obj: object):
/// ...
/// ```
/// ## References
/// - [Python documentation](https://docs.python.org/3/library/typing.html#the-any-type)
/// - [Mypy documentation](https://mypy.readthedocs.io/en/latest/dynamic_typing.html#any-vs-object)
#[violation]
pub struct AnyEqNeAnnotation {
method_name: String,
}

impl AlwaysAutofixableViolation for AnyEqNeAnnotation {
#[derive_message_formats]
fn message(&self) -> String {
let AnyEqNeAnnotation { method_name } = self;
format!("Prefer `object` to `Any` for the second parameter to `{method_name}`")
}

fn autofix_title(&self) -> String {
format!("Replace with `object`")
}
}

/// PYI032
pub(crate) fn any_eq_ne_annotation(checker: &mut Checker, name: &str, args: &Arguments) {
if !matches!(name, "__eq__" | "__ne__") {
return;
}

if args.args.len() != 2 {
return;
}

let Some(annotation) = &args.args[1].annotation else {
return;
};

if !checker.semantic_model().scope().kind.is_class() {
return;
}

if checker
.semantic_model()
.match_typing_expr(annotation, "Any")
{
let mut diagnostic = Diagnostic::new(
AnyEqNeAnnotation {
method_name: name.to_string(),
},
annotation.range(),
);
if checker.patch(diagnostic.kind.rule()) {
// Ex) `def __eq__(self, obj: Any): ...`
if checker.semantic_model().is_builtin("object") {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
"object".to_string(),
annotation.range(),
)));
}
}
checker.diagnostics.push(diagnostic);
}
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub(crate) use any_eq_ne_annotation::{any_eq_ne_annotation, AnyEqNeAnnotation};
pub(crate) use bad_version_info_comparison::{
bad_version_info_comparison, BadVersionInfoComparison,
};
Expand Down Expand Up @@ -27,6 +28,7 @@ pub(crate) use unrecognized_platform::{
unrecognized_platform, UnrecognizedPlatformCheck, UnrecognizedPlatformName,
};

mod any_eq_ne_annotation;
mod bad_version_info_comparison;
mod docstring_in_stubs;
mod duplicate_union_member;
Expand Down
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,42 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI032.pyi:6:29: PYI032 [*] Prefer `object` to `Any` for the second parameter to `__eq__`
|
6 | class Bad:
7 | def __eq__(self, other: Any) -> bool: ... # Y032
| ^^^ PYI032
8 | def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032
|
= help: Replace with `object`

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
8 8 |
9 9 |

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

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
8 8 |
9 9 |
10 10 | class Good:


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.

0 comments on commit 0911ce4

Please sign in to comment.