diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.py new file mode 100644 index 0000000000..2d226ebe97 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.py @@ -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: ... + diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.pyi new file mode 100644 index 0000000000..82cb899e3b --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.pyi @@ -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: ... + diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 6bca01c629..4bacc82bae 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -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) { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 83cf212572..8fc12850a2 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -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), diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 3fb6707562..53458771c4 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -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, diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index ad62a00f86..6cc38f9200 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -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"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs b/crates/ruff/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs new file mode 100644 index 0000000000..03235b31be --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs @@ -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); + } +} diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 0c6b55d634..cb14a41f2b 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -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, }; @@ -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; diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI032_PYI032.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI032_PYI032.py.snap new file mode 100644 index 0000000000..d1aa2e9116 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI032_PYI032.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__PYI032_PYI032.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI032_PYI032.pyi.snap new file mode 100644 index 0000000000..9f4ffac084 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI032_PYI032.pyi.snap @@ -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: + + diff --git a/ruff.schema.json b/ruff.schema.json index d3547d8c39..ffc81fb068 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2205,6 +2205,7 @@ "PYI020", "PYI021", "PYI03", + "PYI032", "PYI033", "PYI04", "PYI042",