Skip to content

Commit

Permalink
[pylint] Implement unnecessary-dunder-call (C2801) (#9166)
Browse files Browse the repository at this point in the history
## Summary

Implements
[`C2801`/`unnecessary-dunder-calls`](https://pylint.readthedocs.io/en/stable/user_guide/messages/convention/unnecessary-dunder-call.html)

There are more that this could cover, but the implementations get a
little less straightforward and ugly. Might come back to it in a future
PR, or someone else can!

See: #970 

## Test Plan

`cargo test`
  • Loading branch information
diceroll123 authored Jan 3, 2024
1 parent 0e20271 commit e3ad163
Show file tree
Hide file tree
Showing 10 changed files with 849 additions and 138 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
from typing import Any


print((3.0).__add__(4.0)) # PLC2801
print((3.0).__sub__(4.0)) # PLC2801
print((3.0).__mul__(4.0)) # PLC2801
print((3.0).__truediv__(4.0)) # PLC2801
print((3.0).__floordiv__(4.0)) # PLC2801
print((3.0).__mod__(4.0)) # PLC2801
print((3.0).__eq__(4.0)) # PLC2801
print((3.0).__ne__(4.0)) # PLC2801
print((3.0).__lt__(4.0)) # PLC2801
print((3.0).__le__(4.0)) # PLC2801
print((3.0).__gt__(4.0)) # PLC2801
print((3.0).__ge__(4.0)) # PLC2801
print((3.0).__str__()) # PLC2801
print((3.0).__repr__()) # PLC2801
print([1, 2, 3].__len__()) # PLC2801
print((1).__neg__()) # PLC2801


class Thing:
def __init__(self, stuff: Any) -> None:
super().__init__() # OK
super().__class__(stuff=(1, 2, 3)) # OK


blah = lambda: {"a": 1}.__delitem__("a") # OK

blah = dict[{"a": 1}.__delitem__("a")] # OK
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::TrioZeroSleepCall) {
flake8_trio::rules::zero_sleep_call(checker, call);
}
if checker.enabled(Rule::UnnecessaryDunderCall) {
pylint::rules::unnecessary_dunder_call(checker, call);
}
}
Expr::Dict(dict) => {
if checker.any_enabled(&[
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "C0415") => (RuleGroup::Preview, rules::pylint::rules::ImportOutsideTopLevel),
(Pylint, "C2401") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiName),
(Pylint, "C2403") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiImportName),
(Pylint, "C2801") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDunderCall),
#[allow(deprecated)]
(Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString),
(Pylint, "C3002") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryDirectLambdaCall),
Expand Down
138 changes: 138 additions & 0 deletions crates/ruff_linter/src/rules/pylint/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,141 @@ impl<'a> Visitor<'_> for SequenceIndexVisitor<'a> {
}
}
}

/// Returns `true` if a method is a known dunder method.
pub(super) fn is_known_dunder_method(method: &str) -> bool {
matches!(
method,
"__abs__"
| "__add__"
| "__aenter__"
| "__aexit__"
| "__aiter__"
| "__and__"
| "__anext__"
| "__attrs_init__"
| "__attrs_post_init__"
| "__attrs_pre_init__"
| "__await__"
| "__bool__"
| "__buffer__"
| "__bytes__"
| "__call__"
| "__ceil__"
| "__class__"
| "__class_getitem__"
| "__complex__"
| "__contains__"
| "__copy__"
| "__deepcopy__"
| "__del__"
| "__delattr__"
| "__delete__"
| "__delitem__"
| "__dict__"
| "__dir__"
| "__divmod__"
| "__doc__"
| "__enter__"
| "__eq__"
| "__exit__"
| "__float__"
| "__floor__"
| "__floordiv__"
| "__format__"
| "__fspath__"
| "__ge__"
| "__get__"
| "__getattr__"
| "__getattribute__"
| "__getitem__"
| "__getnewargs__"
| "__getnewargs_ex__"
| "__getstate__"
| "__gt__"
| "__hash__"
| "__html__"
| "__iadd__"
| "__iand__"
| "__ifloordiv__"
| "__ilshift__"
| "__imatmul__"
| "__imod__"
| "__imul__"
| "__index__"
| "__init__"
| "__init_subclass__"
| "__instancecheck__"
| "__int__"
| "__invert__"
| "__ior__"
| "__ipow__"
| "__irshift__"
| "__isub__"
| "__iter__"
| "__itruediv__"
| "__ixor__"
| "__le__"
| "__len__"
| "__length_hint__"
| "__lshift__"
| "__lt__"
| "__matmul__"
| "__missing__"
| "__mod__"
| "__module__"
| "__mul__"
| "__ne__"
| "__neg__"
| "__new__"
| "__next__"
| "__or__"
| "__pos__"
| "__post_init__"
| "__pow__"
| "__radd__"
| "__rand__"
| "__rdivmod__"
| "__reduce__"
| "__reduce_ex__"
| "__release_buffer__"
| "__repr__"
| "__reversed__"
| "__rfloordiv__"
| "__rlshift__"
| "__rmatmul__"
| "__rmod__"
| "__rmul__"
| "__ror__"
| "__round__"
| "__rpow__"
| "__rrshift__"
| "__rshift__"
| "__rsub__"
| "__rtruediv__"
| "__rxor__"
| "__set__"
| "__set_name__"
| "__setattr__"
| "__setitem__"
| "__setstate__"
| "__sizeof__"
| "__str__"
| "__sub__"
| "__subclasscheck__"
| "__subclasses__"
| "__subclasshook__"
| "__truediv__"
| "__trunc__"
| "__weakref__"
| "__xor__"
// Overridable sunder names from the `Enum` class.
// See: https://docs.python.org/3/library/enum.html#supported-sunder-names
| "_name_"
| "_value_"
| "_missing_"
| "_ignore_"
| "_order_"
| "_generate_next_value_"
)
}
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ mod tests {
Path::new("unnecessary_list_index_lookup.py")
)]
#[test_case(Rule::NoClassmethodDecorator, Path::new("no_method_decorator.py"))]
#[test_case(Rule::UnnecessaryDunderCall, Path::new("unnecessary_dunder_call.py"))]
#[test_case(Rule::NoStaticmethodDecorator, Path::new("no_method_decorator.py"))]
#[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))]
#[test_case(
Expand Down
139 changes: 1 addition & 138 deletions crates/ruff_linter/src/rules/pylint/rules/bad_dunder_method_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ruff_python_ast::Stmt;
use ruff_python_semantic::analyze::visibility;

use crate::checkers::ast::Checker;
use crate::rules::pylint::helpers::is_known_dunder_method;

/// ## What it does
/// Checks for misspelled and unknown dunder names in method definitions.
Expand Down Expand Up @@ -84,141 +85,3 @@ pub(crate) fn bad_dunder_method_name(checker: &mut Checker, class_body: &[Stmt])
));
}
}

/// Returns `true` if a method is a known dunder method.
fn is_known_dunder_method(method: &str) -> bool {
matches!(
method,
"__abs__"
| "__add__"
| "__aenter__"
| "__aexit__"
| "__aiter__"
| "__and__"
| "__anext__"
| "__attrs_init__"
| "__attrs_post_init__"
| "__attrs_pre_init__"
| "__await__"
| "__bool__"
| "__buffer__"
| "__bytes__"
| "__call__"
| "__ceil__"
| "__class__"
| "__class_getitem__"
| "__complex__"
| "__contains__"
| "__copy__"
| "__deepcopy__"
| "__del__"
| "__delattr__"
| "__delete__"
| "__delitem__"
| "__dict__"
| "__dir__"
| "__divmod__"
| "__doc__"
| "__enter__"
| "__eq__"
| "__exit__"
| "__float__"
| "__floor__"
| "__floordiv__"
| "__format__"
| "__fspath__"
| "__ge__"
| "__get__"
| "__getattr__"
| "__getattribute__"
| "__getitem__"
| "__getnewargs__"
| "__getnewargs_ex__"
| "__getstate__"
| "__gt__"
| "__hash__"
| "__html__"
| "__iadd__"
| "__iand__"
| "__ifloordiv__"
| "__ilshift__"
| "__imatmul__"
| "__imod__"
| "__imul__"
| "__index__"
| "__init__"
| "__init_subclass__"
| "__instancecheck__"
| "__int__"
| "__invert__"
| "__ior__"
| "__ipow__"
| "__irshift__"
| "__isub__"
| "__iter__"
| "__itruediv__"
| "__ixor__"
| "__le__"
| "__len__"
| "__length_hint__"
| "__lshift__"
| "__lt__"
| "__matmul__"
| "__missing__"
| "__mod__"
| "__module__"
| "__mul__"
| "__ne__"
| "__neg__"
| "__new__"
| "__next__"
| "__or__"
| "__pos__"
| "__post_init__"
| "__pow__"
| "__radd__"
| "__rand__"
| "__rdivmod__"
| "__reduce__"
| "__reduce_ex__"
| "__release_buffer__"
| "__repr__"
| "__reversed__"
| "__rfloordiv__"
| "__rlshift__"
| "__rmatmul__"
| "__rmod__"
| "__rmul__"
| "__ror__"
| "__round__"
| "__rpow__"
| "__rrshift__"
| "__rshift__"
| "__rsub__"
| "__rtruediv__"
| "__rxor__"
| "__set__"
| "__set_name__"
| "__setattr__"
| "__setitem__"
| "__setstate__"
| "__sizeof__"
| "__str__"
| "__sub__"
| "__subclasscheck__"
| "__subclasses__"
| "__subclasshook__"
| "__truediv__"
| "__trunc__"
| "__weakref__"
| "__xor__"
// Overridable sunder names from the `Enum` class.
// See: https://docs.python.org/3/library/enum.html#supported-sunder-names
| "_name_"
| "_value_"
| "_missing_"
| "_ignore_"
| "_order_"
| "_generate_next_value_"
)
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ pub(crate) use type_param_name_mismatch::*;
pub(crate) use unexpected_special_method_signature::*;
pub(crate) use unnecessary_dict_index_lookup::*;
pub(crate) use unnecessary_direct_lambda_call::*;
pub(crate) use unnecessary_dunder_call::*;
pub(crate) use unnecessary_lambda::*;
pub(crate) use unnecessary_list_index_lookup::*;
pub(crate) use unspecified_encoding::*;
Expand Down Expand Up @@ -148,6 +149,7 @@ mod type_param_name_mismatch;
mod unexpected_special_method_signature;
mod unnecessary_dict_index_lookup;
mod unnecessary_direct_lambda_call;
mod unnecessary_dunder_call;
mod unnecessary_lambda;
mod unnecessary_list_index_lookup;
mod unspecified_encoding;
Expand Down
Loading

0 comments on commit e3ad163

Please sign in to comment.