From 5ca3f043da3b2d44fbca60e1eaf3094b6ee231bc Mon Sep 17 00:00:00 2001 From: Tuomas Siipola Date: Sun, 17 Dec 2023 22:05:51 +0200 Subject: [PATCH 1/3] Implement `reimplemented_operator` (FURB118) --- .../resources/test/fixtures/refurb/FURB118.py | 56 ++ .../checkers/ast/analyze/deferred_lambdas.rs | 5 +- .../src/checkers/ast/analyze/statement.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/refurb/mod.rs | 1 + .../ruff_linter/src/rules/refurb/rules/mod.rs | 2 + .../refurb/rules/reimplemented_operator.rs | 227 ++++++ ...es__refurb__tests__FURB118_FURB118.py.snap | 711 ++++++++++++++++++ ruff.schema.json | 1 + 9 files changed, 1006 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py create mode 100644 crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs create mode 100644 crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py new file mode 100644 index 0000000000000..9cabba5f683c0 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py @@ -0,0 +1,56 @@ +# Errors. +op_bitnot = lambda x: ~x +op_not = lambda x: not x +op_pos = lambda x: +x +op_neg = lambda x: -x + +op_add = lambda x, y: x + y +op_sub = lambda x, y: x - y +op_mult = lambda x, y: x * y +op_matmutl = lambda x, y: x @ y +op_truediv = lambda x, y: x / y +op_mod = lambda x, y: x % y +op_pow = lambda x, y: x ** y +op_lshift = lambda x, y: x << y +op_rshift = lambda x, y: x >> y +op_bitor = lambda x, y: x | y +op_xor = lambda x, y: x ^ y +op_bitand = lambda x, y: x & y +op_floordiv = lambda x, y: x // y + +op_eq = lambda x, y: x == y +op_ne = lambda x, y: x != y +op_lt = lambda x, y: x < y +op_lte = lambda x, y: x <= y +op_gt = lambda x, y: x > y +op_gte = lambda x, y: x >= y +op_is = lambda x, y: x is y +op_isnot = lambda x, y: x is not y +op_in = lambda x, y: x in y + +def op_not2(x): + return not x + +def op_add2(x, y): + return x + y + +class Adder: + def add(x, y): + return x + y + +# Ok. +op_add3 = lambda x, y = 1: x + y +op_neg2 = lambda x, y: y - x +op_notin = lambda x, y: y not in x +op_and = lambda x, y: y and x +op_or = lambda x, y: y or x + +def op_neg3(x, y): + return y - x + +def op_add4(x, y = 1): + return x + y + +def op_add5(x, y): + print("op_add5") + return x + y diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs index b3aa849f8feb0..f331ef113efd1 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs @@ -2,7 +2,7 @@ use ruff_python_ast::Expr; use crate::checkers::ast::Checker; use crate::codes::Rule; -use crate::rules::{flake8_pie, pylint}; +use crate::rules::{flake8_pie, pylint, refurb}; /// Run lint rules over all deferred lambdas in the [`SemanticModel`]. pub(crate) fn deferred_lambdas(checker: &mut Checker) { @@ -21,6 +21,9 @@ pub(crate) fn deferred_lambdas(checker: &mut Checker) { if checker.enabled(Rule::ReimplementedContainerBuiltin) { flake8_pie::rules::reimplemented_container_builtin(checker, lambda); } + if checker.enabled(Rule::ReimplementedOperator) { + refurb::rules::reimplemented_operator(checker, &lambda.into()); + } } } } diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 714ae3e4e3943..c803562bd5bf5 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -368,6 +368,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { .diagnostics .extend(ruff::rules::unreachable::in_function(name, body)); } + if checker.enabled(Rule::ReimplementedOperator) { + refurb::rules::reimplemented_operator(checker, &function_def.into()); + } } Stmt::Return(_) => { if checker.enabled(Rule::ReturnOutsideFunction) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index a1475760f4519..30a8af89b18b2 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -952,6 +952,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Refurb, "105") => (RuleGroup::Preview, rules::refurb::rules::PrintEmptyString), #[allow(deprecated)] (Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend), + (Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator), #[allow(deprecated)] (Refurb, "131") => (RuleGroup::Nursery, rules::refurb::rules::DeleteFullSlice), #[allow(deprecated)] diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index d27985762cef4..5ce76cd3420bd 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -16,6 +16,7 @@ mod tests { #[test_case(Rule::ReadWholeFile, Path::new("FURB101.py"))] #[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))] + #[test_case(Rule::ReimplementedOperator, Path::new("FURB118.py"))] #[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))] #[test_case(Rule::CheckAndRemoveFromSet, Path::new("FURB132.py"))] #[test_case(Rule::IfExprMinMax, Path::new("FURB136.py"))] diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index 4f165af3b36e5..c5536a029b7b2 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -8,6 +8,7 @@ pub(crate) use math_constant::*; pub(crate) use print_empty_string::*; pub(crate) use read_whole_file::*; pub(crate) use redundant_log_base::*; +pub(crate) use reimplemented_operator::*; pub(crate) use reimplemented_starmap::*; pub(crate) use repeated_append::*; pub(crate) use single_item_membership_test::*; @@ -25,6 +26,7 @@ mod math_constant; mod print_empty_string; mod read_whole_file; mod redundant_log_base; +mod reimplemented_operator; mod reimplemented_starmap; mod repeated_append; mod single_item_membership_test; diff --git a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs new file mode 100644 index 0000000000000..dc0c131870035 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs @@ -0,0 +1,227 @@ +use anyhow::{bail, Result}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Expr, Stmt}; +use ruff_text_size::{Ranged, TextRange}; + +use crate::checkers::ast::Checker; +use crate::importer::ImportRequest; + +/// ## What it does +/// Checks for lambda expressions and function definitions that can be replaced with a function +/// from `operator` module. +/// +/// ## Why is this bad? +/// Using functions from `operator` module is more concise and readable. +/// +/// ## Example +/// ```python +/// import functools +/// nums = [1, 2, 3] +/// sum = functools.reduce(lambda x, y: x + y, nums) +/// ``` +/// +/// Use instead: +/// ```python +/// import functools +/// import operator +/// nums = [1, 2, 3] +/// sum = functools.reduce(operator.add, nums) +/// ``` +/// +/// ## References +#[violation] +pub struct ReimplementedOperator { + target: &'static str, + operator: &'static str, +} + +impl Violation for ReimplementedOperator { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + let ReimplementedOperator { operator, target } = self; + format!("Use `operator.{operator}` instead of defining a {target}") + } + + fn fix_title(&self) -> Option { + let ReimplementedOperator { operator, .. } = self; + Some(format!("Replace with `operator.{operator}`")) + } +} + +/// FURB118 +pub(crate) fn reimplemented_operator(checker: &mut Checker, target: &LambdaLike) { + let Some(params) = target.parameters() else { + return; + }; + let Some(body) = target.body() else { return }; + let Some(operator) = get_operator(body, params) else { + return; + }; + let mut diagnostic = Diagnostic::new( + ReimplementedOperator { + operator, + target: target.kind(), + }, + target.range(), + ); + diagnostic.try_set_fix(|| target.try_fix(checker, operator)); + checker.diagnostics.push(diagnostic); +} + +/// Candidate for lambda expression or function definition consisting of a return statement. +pub(crate) enum LambdaLike<'a> { + Lambda(&'a ast::ExprLambda), + Function(&'a ast::StmtFunctionDef), +} + +impl<'a> From<&'a ast::ExprLambda> for LambdaLike<'a> { + fn from(lambda: &'a ast::ExprLambda) -> Self { + Self::Lambda(lambda) + } +} + +impl<'a> From<&'a ast::StmtFunctionDef> for LambdaLike<'a> { + fn from(function: &'a ast::StmtFunctionDef) -> Self { + Self::Function(function) + } +} + +impl Ranged for LambdaLike<'_> { + fn range(&self) -> TextRange { + match self { + Self::Lambda(expr) => expr.range(), + Self::Function(stmt) => stmt.range(), + } + } +} + +impl LambdaLike<'_> { + fn parameters(&self) -> Option<&ast::Parameters> { + match self { + Self::Lambda(expr) => expr.parameters.as_deref(), + Self::Function(stmt) => Some(&stmt.parameters), + } + } + + fn body(&self) -> Option<&Expr> { + match self { + Self::Lambda(expr) => Some(&expr.body), + Self::Function(stmt) => match stmt.body.as_slice() { + [Stmt::Return(ast::StmtReturn { value, .. })] => value.as_deref(), + _ => None, + }, + } + } + + fn try_fix(&self, checker: &Checker, operator: &'static str) -> Result { + match self { + Self::Lambda(_) => { + let (edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("operator", operator), + self.start(), + checker.semantic(), + )?; + Ok(Fix::safe_edits( + Edit::range_replacement(binding, self.range()), + [edit], + )) + } + Self::Function(_) => bail!("No fix available"), + } + } + + fn kind(&self) -> &'static str { + match self { + Self::Lambda(_) => "lambda", + Self::Function(_) => "function", + } + } +} + +fn get_operator(expr: &Expr, params: &ast::Parameters) -> Option<&'static str> { + match expr { + Expr::UnaryOp(expr) => unary_op(expr, params), + Expr::BinOp(expr) => bin_op(expr, params), + Expr::Compare(expr) => cmp_op(expr, params), + _ => None, + } +} + +fn unary_op(expr: &ast::ExprUnaryOp, params: &ast::Parameters) -> Option<&'static str> { + let [arg] = params.args.as_slice() else { + return None; + }; + if !is_same(arg, &expr.operand) { + return None; + } + Some(match expr.op { + ast::UnaryOp::Invert => "invert", + ast::UnaryOp::Not => "not_", + ast::UnaryOp::UAdd => "pos", + ast::UnaryOp::USub => "neg", + }) +} + +fn bin_op(expr: &ast::ExprBinOp, params: &ast::Parameters) -> Option<&'static str> { + let [arg1, arg2] = params.args.as_slice() else { + return None; + }; + if !is_same(arg1, &expr.left) || !is_same(arg2, &expr.right) { + return None; + } + Some(match expr.op { + ast::Operator::Add => "add", + ast::Operator::Sub => "sub", + ast::Operator::Mult => "mul", + ast::Operator::MatMult => "matmul", + ast::Operator::Div => "truediv", + ast::Operator::Mod => "mod", + ast::Operator::Pow => "pow", + ast::Operator::LShift => "lshift", + ast::Operator::RShift => "rshift", + ast::Operator::BitOr => "or_", + ast::Operator::BitXor => "xor", + ast::Operator::BitAnd => "and_", + ast::Operator::FloorDiv => "floordiv", + }) +} + +fn cmp_op(expr: &ast::ExprCompare, params: &ast::Parameters) -> Option<&'static str> { + let [arg1, arg2] = params.args.as_slice() else { + return None; + }; + let [op] = expr.ops.as_slice() else { + return None; + }; + let [right] = expr.comparators.as_slice() else { + return None; + }; + if !is_same(arg1, &expr.left) || !is_same(arg2, right) { + return None; + } + match op { + ast::CmpOp::Eq => Some("eq"), + ast::CmpOp::NotEq => Some("ne"), + ast::CmpOp::Lt => Some("lt"), + ast::CmpOp::LtE => Some("le"), + ast::CmpOp::Gt => Some("gt"), + ast::CmpOp::GtE => Some("ge"), + ast::CmpOp::Is => Some("is_"), + ast::CmpOp::IsNot => Some("is_not"), + ast::CmpOp::In => Some("contains"), + ast::CmpOp::NotIn => None, + } +} + +fn is_same(arg: &ast::ParameterWithDefault, expr: &Expr) -> bool { + if arg.default.is_some() { + false + } else if let Expr::Name(name) = expr { + name.id == arg.parameter.name.as_str() + } else { + false + } +} diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap new file mode 100644 index 0000000000000..67e2f228c9216 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap @@ -0,0 +1,711 @@ +--- +source: crates/ruff_linter/src/rules/refurb/mod.rs +--- +FURB118.py:2:13: FURB118 [*] Use `operator.invert` instead of defining a lambda + | +1 | # Errors. +2 | op_bitnot = lambda x: ~x + | ^^^^^^^^^^^^ FURB118 +3 | op_not = lambda x: not x +4 | op_pos = lambda x: +x + | + = help: Replace with `operator.invert` + +ℹ Safe fix +1 1 | # Errors. +2 |-op_bitnot = lambda x: ~x + 2 |+import operator + 3 |+op_bitnot = operator.invert +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +5 6 | op_neg = lambda x: -x + +FURB118.py:3:10: FURB118 [*] Use `operator.not_` instead of defining a lambda + | +1 | # Errors. +2 | op_bitnot = lambda x: ~x +3 | op_not = lambda x: not x + | ^^^^^^^^^^^^^^^ FURB118 +4 | op_pos = lambda x: +x +5 | op_neg = lambda x: -x + | + = help: Replace with `operator.not_` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 |-op_not = lambda x: not x + 4 |+op_not = operator.not_ +4 5 | op_pos = lambda x: +x +5 6 | op_neg = lambda x: -x +6 7 | + +FURB118.py:4:10: FURB118 [*] Use `operator.pos` instead of defining a lambda + | +2 | op_bitnot = lambda x: ~x +3 | op_not = lambda x: not x +4 | op_pos = lambda x: +x + | ^^^^^^^^^^^^ FURB118 +5 | op_neg = lambda x: -x + | + = help: Replace with `operator.pos` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 |-op_pos = lambda x: +x + 5 |+op_pos = operator.pos +5 6 | op_neg = lambda x: -x +6 7 | +7 8 | op_add = lambda x, y: x + y + +FURB118.py:5:10: FURB118 [*] Use `operator.neg` instead of defining a lambda + | +3 | op_not = lambda x: not x +4 | op_pos = lambda x: +x +5 | op_neg = lambda x: -x + | ^^^^^^^^^^^^ FURB118 +6 | +7 | op_add = lambda x, y: x + y + | + = help: Replace with `operator.neg` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +5 |-op_neg = lambda x: -x + 6 |+op_neg = operator.neg +6 7 | +7 8 | op_add = lambda x, y: x + y +8 9 | op_sub = lambda x, y: x - y + +FURB118.py:7:10: FURB118 [*] Use `operator.add` instead of defining a lambda + | +5 | op_neg = lambda x: -x +6 | +7 | op_add = lambda x, y: x + y + | ^^^^^^^^^^^^^^^^^^ FURB118 +8 | op_sub = lambda x, y: x - y +9 | op_mult = lambda x, y: x * y + | + = help: Replace with `operator.add` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +5 6 | op_neg = lambda x: -x +6 7 | +7 |-op_add = lambda x, y: x + y + 8 |+op_add = operator.add +8 9 | op_sub = lambda x, y: x - y +9 10 | op_mult = lambda x, y: x * y +10 11 | op_matmutl = lambda x, y: x @ y + +FURB118.py:8:10: FURB118 [*] Use `operator.sub` instead of defining a lambda + | + 7 | op_add = lambda x, y: x + y + 8 | op_sub = lambda x, y: x - y + | ^^^^^^^^^^^^^^^^^^ FURB118 + 9 | op_mult = lambda x, y: x * y +10 | op_matmutl = lambda x, y: x @ y + | + = help: Replace with `operator.sub` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +5 6 | op_neg = lambda x: -x +6 7 | +7 8 | op_add = lambda x, y: x + y +8 |-op_sub = lambda x, y: x - y + 9 |+op_sub = operator.sub +9 10 | op_mult = lambda x, y: x * y +10 11 | op_matmutl = lambda x, y: x @ y +11 12 | op_truediv = lambda x, y: x / y + +FURB118.py:9:11: FURB118 [*] Use `operator.mul` instead of defining a lambda + | + 7 | op_add = lambda x, y: x + y + 8 | op_sub = lambda x, y: x - y + 9 | op_mult = lambda x, y: x * y + | ^^^^^^^^^^^^^^^^^^ FURB118 +10 | op_matmutl = lambda x, y: x @ y +11 | op_truediv = lambda x, y: x / y + | + = help: Replace with `operator.mul` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +6 7 | +7 8 | op_add = lambda x, y: x + y +8 9 | op_sub = lambda x, y: x - y +9 |-op_mult = lambda x, y: x * y + 10 |+op_mult = operator.mul +10 11 | op_matmutl = lambda x, y: x @ y +11 12 | op_truediv = lambda x, y: x / y +12 13 | op_mod = lambda x, y: x % y + +FURB118.py:10:14: FURB118 [*] Use `operator.matmul` instead of defining a lambda + | + 8 | op_sub = lambda x, y: x - y + 9 | op_mult = lambda x, y: x * y +10 | op_matmutl = lambda x, y: x @ y + | ^^^^^^^^^^^^^^^^^^ FURB118 +11 | op_truediv = lambda x, y: x / y +12 | op_mod = lambda x, y: x % y + | + = help: Replace with `operator.matmul` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +7 8 | op_add = lambda x, y: x + y +8 9 | op_sub = lambda x, y: x - y +9 10 | op_mult = lambda x, y: x * y +10 |-op_matmutl = lambda x, y: x @ y + 11 |+op_matmutl = operator.matmul +11 12 | op_truediv = lambda x, y: x / y +12 13 | op_mod = lambda x, y: x % y +13 14 | op_pow = lambda x, y: x ** y + +FURB118.py:11:14: FURB118 [*] Use `operator.truediv` instead of defining a lambda + | + 9 | op_mult = lambda x, y: x * y +10 | op_matmutl = lambda x, y: x @ y +11 | op_truediv = lambda x, y: x / y + | ^^^^^^^^^^^^^^^^^^ FURB118 +12 | op_mod = lambda x, y: x % y +13 | op_pow = lambda x, y: x ** y + | + = help: Replace with `operator.truediv` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +8 9 | op_sub = lambda x, y: x - y +9 10 | op_mult = lambda x, y: x * y +10 11 | op_matmutl = lambda x, y: x @ y +11 |-op_truediv = lambda x, y: x / y + 12 |+op_truediv = operator.truediv +12 13 | op_mod = lambda x, y: x % y +13 14 | op_pow = lambda x, y: x ** y +14 15 | op_lshift = lambda x, y: x << y + +FURB118.py:12:10: FURB118 [*] Use `operator.mod` instead of defining a lambda + | +10 | op_matmutl = lambda x, y: x @ y +11 | op_truediv = lambda x, y: x / y +12 | op_mod = lambda x, y: x % y + | ^^^^^^^^^^^^^^^^^^ FURB118 +13 | op_pow = lambda x, y: x ** y +14 | op_lshift = lambda x, y: x << y + | + = help: Replace with `operator.mod` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +9 10 | op_mult = lambda x, y: x * y +10 11 | op_matmutl = lambda x, y: x @ y +11 12 | op_truediv = lambda x, y: x / y +12 |-op_mod = lambda x, y: x % y + 13 |+op_mod = operator.mod +13 14 | op_pow = lambda x, y: x ** y +14 15 | op_lshift = lambda x, y: x << y +15 16 | op_rshift = lambda x, y: x >> y + +FURB118.py:13:10: FURB118 [*] Use `operator.pow` instead of defining a lambda + | +11 | op_truediv = lambda x, y: x / y +12 | op_mod = lambda x, y: x % y +13 | op_pow = lambda x, y: x ** y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +14 | op_lshift = lambda x, y: x << y +15 | op_rshift = lambda x, y: x >> y + | + = help: Replace with `operator.pow` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +10 11 | op_matmutl = lambda x, y: x @ y +11 12 | op_truediv = lambda x, y: x / y +12 13 | op_mod = lambda x, y: x % y +13 |-op_pow = lambda x, y: x ** y + 14 |+op_pow = operator.pow +14 15 | op_lshift = lambda x, y: x << y +15 16 | op_rshift = lambda x, y: x >> y +16 17 | op_bitor = lambda x, y: x | y + +FURB118.py:14:13: FURB118 [*] Use `operator.lshift` instead of defining a lambda + | +12 | op_mod = lambda x, y: x % y +13 | op_pow = lambda x, y: x ** y +14 | op_lshift = lambda x, y: x << y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +15 | op_rshift = lambda x, y: x >> y +16 | op_bitor = lambda x, y: x | y + | + = help: Replace with `operator.lshift` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +11 12 | op_truediv = lambda x, y: x / y +12 13 | op_mod = lambda x, y: x % y +13 14 | op_pow = lambda x, y: x ** y +14 |-op_lshift = lambda x, y: x << y + 15 |+op_lshift = operator.lshift +15 16 | op_rshift = lambda x, y: x >> y +16 17 | op_bitor = lambda x, y: x | y +17 18 | op_xor = lambda x, y: x ^ y + +FURB118.py:15:13: FURB118 [*] Use `operator.rshift` instead of defining a lambda + | +13 | op_pow = lambda x, y: x ** y +14 | op_lshift = lambda x, y: x << y +15 | op_rshift = lambda x, y: x >> y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +16 | op_bitor = lambda x, y: x | y +17 | op_xor = lambda x, y: x ^ y + | + = help: Replace with `operator.rshift` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +12 13 | op_mod = lambda x, y: x % y +13 14 | op_pow = lambda x, y: x ** y +14 15 | op_lshift = lambda x, y: x << y +15 |-op_rshift = lambda x, y: x >> y + 16 |+op_rshift = operator.rshift +16 17 | op_bitor = lambda x, y: x | y +17 18 | op_xor = lambda x, y: x ^ y +18 19 | op_bitand = lambda x, y: x & y + +FURB118.py:16:12: FURB118 [*] Use `operator.or_` instead of defining a lambda + | +14 | op_lshift = lambda x, y: x << y +15 | op_rshift = lambda x, y: x >> y +16 | op_bitor = lambda x, y: x | y + | ^^^^^^^^^^^^^^^^^^ FURB118 +17 | op_xor = lambda x, y: x ^ y +18 | op_bitand = lambda x, y: x & y + | + = help: Replace with `operator.or_` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +13 14 | op_pow = lambda x, y: x ** y +14 15 | op_lshift = lambda x, y: x << y +15 16 | op_rshift = lambda x, y: x >> y +16 |-op_bitor = lambda x, y: x | y + 17 |+op_bitor = operator.or_ +17 18 | op_xor = lambda x, y: x ^ y +18 19 | op_bitand = lambda x, y: x & y +19 20 | op_floordiv = lambda x, y: x // y + +FURB118.py:17:10: FURB118 [*] Use `operator.xor` instead of defining a lambda + | +15 | op_rshift = lambda x, y: x >> y +16 | op_bitor = lambda x, y: x | y +17 | op_xor = lambda x, y: x ^ y + | ^^^^^^^^^^^^^^^^^^ FURB118 +18 | op_bitand = lambda x, y: x & y +19 | op_floordiv = lambda x, y: x // y + | + = help: Replace with `operator.xor` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +14 15 | op_lshift = lambda x, y: x << y +15 16 | op_rshift = lambda x, y: x >> y +16 17 | op_bitor = lambda x, y: x | y +17 |-op_xor = lambda x, y: x ^ y + 18 |+op_xor = operator.xor +18 19 | op_bitand = lambda x, y: x & y +19 20 | op_floordiv = lambda x, y: x // y +20 21 | + +FURB118.py:18:13: FURB118 [*] Use `operator.and_` instead of defining a lambda + | +16 | op_bitor = lambda x, y: x | y +17 | op_xor = lambda x, y: x ^ y +18 | op_bitand = lambda x, y: x & y + | ^^^^^^^^^^^^^^^^^^ FURB118 +19 | op_floordiv = lambda x, y: x // y + | + = help: Replace with `operator.and_` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +15 16 | op_rshift = lambda x, y: x >> y +16 17 | op_bitor = lambda x, y: x | y +17 18 | op_xor = lambda x, y: x ^ y +18 |-op_bitand = lambda x, y: x & y + 19 |+op_bitand = operator.and_ +19 20 | op_floordiv = lambda x, y: x // y +20 21 | +21 22 | op_eq = lambda x, y: x == y + +FURB118.py:19:15: FURB118 [*] Use `operator.floordiv` instead of defining a lambda + | +17 | op_xor = lambda x, y: x ^ y +18 | op_bitand = lambda x, y: x & y +19 | op_floordiv = lambda x, y: x // y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +20 | +21 | op_eq = lambda x, y: x == y + | + = help: Replace with `operator.floordiv` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +16 17 | op_bitor = lambda x, y: x | y +17 18 | op_xor = lambda x, y: x ^ y +18 19 | op_bitand = lambda x, y: x & y +19 |-op_floordiv = lambda x, y: x // y + 20 |+op_floordiv = operator.floordiv +20 21 | +21 22 | op_eq = lambda x, y: x == y +22 23 | op_ne = lambda x, y: x != y + +FURB118.py:21:9: FURB118 [*] Use `operator.eq` instead of defining a lambda + | +19 | op_floordiv = lambda x, y: x // y +20 | +21 | op_eq = lambda x, y: x == y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +22 | op_ne = lambda x, y: x != y +23 | op_lt = lambda x, y: x < y + | + = help: Replace with `operator.eq` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +18 19 | op_bitand = lambda x, y: x & y +19 20 | op_floordiv = lambda x, y: x // y +20 21 | +21 |-op_eq = lambda x, y: x == y + 22 |+op_eq = operator.eq +22 23 | op_ne = lambda x, y: x != y +23 24 | op_lt = lambda x, y: x < y +24 25 | op_lte = lambda x, y: x <= y + +FURB118.py:22:9: FURB118 [*] Use `operator.ne` instead of defining a lambda + | +21 | op_eq = lambda x, y: x == y +22 | op_ne = lambda x, y: x != y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +23 | op_lt = lambda x, y: x < y +24 | op_lte = lambda x, y: x <= y + | + = help: Replace with `operator.ne` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +19 20 | op_floordiv = lambda x, y: x // y +20 21 | +21 22 | op_eq = lambda x, y: x == y +22 |-op_ne = lambda x, y: x != y + 23 |+op_ne = operator.ne +23 24 | op_lt = lambda x, y: x < y +24 25 | op_lte = lambda x, y: x <= y +25 26 | op_gt = lambda x, y: x > y + +FURB118.py:23:9: FURB118 [*] Use `operator.lt` instead of defining a lambda + | +21 | op_eq = lambda x, y: x == y +22 | op_ne = lambda x, y: x != y +23 | op_lt = lambda x, y: x < y + | ^^^^^^^^^^^^^^^^^^ FURB118 +24 | op_lte = lambda x, y: x <= y +25 | op_gt = lambda x, y: x > y + | + = help: Replace with `operator.lt` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +20 21 | +21 22 | op_eq = lambda x, y: x == y +22 23 | op_ne = lambda x, y: x != y +23 |-op_lt = lambda x, y: x < y + 24 |+op_lt = operator.lt +24 25 | op_lte = lambda x, y: x <= y +25 26 | op_gt = lambda x, y: x > y +26 27 | op_gte = lambda x, y: x >= y + +FURB118.py:24:10: FURB118 [*] Use `operator.le` instead of defining a lambda + | +22 | op_ne = lambda x, y: x != y +23 | op_lt = lambda x, y: x < y +24 | op_lte = lambda x, y: x <= y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +25 | op_gt = lambda x, y: x > y +26 | op_gte = lambda x, y: x >= y + | + = help: Replace with `operator.le` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +21 22 | op_eq = lambda x, y: x == y +22 23 | op_ne = lambda x, y: x != y +23 24 | op_lt = lambda x, y: x < y +24 |-op_lte = lambda x, y: x <= y + 25 |+op_lte = operator.le +25 26 | op_gt = lambda x, y: x > y +26 27 | op_gte = lambda x, y: x >= y +27 28 | op_is = lambda x, y: x is y + +FURB118.py:25:9: FURB118 [*] Use `operator.gt` instead of defining a lambda + | +23 | op_lt = lambda x, y: x < y +24 | op_lte = lambda x, y: x <= y +25 | op_gt = lambda x, y: x > y + | ^^^^^^^^^^^^^^^^^^ FURB118 +26 | op_gte = lambda x, y: x >= y +27 | op_is = lambda x, y: x is y + | + = help: Replace with `operator.gt` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +22 23 | op_ne = lambda x, y: x != y +23 24 | op_lt = lambda x, y: x < y +24 25 | op_lte = lambda x, y: x <= y +25 |-op_gt = lambda x, y: x > y + 26 |+op_gt = operator.gt +26 27 | op_gte = lambda x, y: x >= y +27 28 | op_is = lambda x, y: x is y +28 29 | op_isnot = lambda x, y: x is not y + +FURB118.py:26:10: FURB118 [*] Use `operator.ge` instead of defining a lambda + | +24 | op_lte = lambda x, y: x <= y +25 | op_gt = lambda x, y: x > y +26 | op_gte = lambda x, y: x >= y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +27 | op_is = lambda x, y: x is y +28 | op_isnot = lambda x, y: x is not y + | + = help: Replace with `operator.ge` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +23 24 | op_lt = lambda x, y: x < y +24 25 | op_lte = lambda x, y: x <= y +25 26 | op_gt = lambda x, y: x > y +26 |-op_gte = lambda x, y: x >= y + 27 |+op_gte = operator.ge +27 28 | op_is = lambda x, y: x is y +28 29 | op_isnot = lambda x, y: x is not y +29 30 | op_in = lambda x, y: x in y + +FURB118.py:27:9: FURB118 [*] Use `operator.is_` instead of defining a lambda + | +25 | op_gt = lambda x, y: x > y +26 | op_gte = lambda x, y: x >= y +27 | op_is = lambda x, y: x is y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +28 | op_isnot = lambda x, y: x is not y +29 | op_in = lambda x, y: x in y + | + = help: Replace with `operator.is_` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +24 25 | op_lte = lambda x, y: x <= y +25 26 | op_gt = lambda x, y: x > y +26 27 | op_gte = lambda x, y: x >= y +27 |-op_is = lambda x, y: x is y + 28 |+op_is = operator.is_ +28 29 | op_isnot = lambda x, y: x is not y +29 30 | op_in = lambda x, y: x in y +30 31 | + +FURB118.py:28:12: FURB118 [*] Use `operator.is_not` instead of defining a lambda + | +26 | op_gte = lambda x, y: x >= y +27 | op_is = lambda x, y: x is y +28 | op_isnot = lambda x, y: x is not y + | ^^^^^^^^^^^^^^^^^^^^^^^ FURB118 +29 | op_in = lambda x, y: x in y + | + = help: Replace with `operator.is_not` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +25 26 | op_gt = lambda x, y: x > y +26 27 | op_gte = lambda x, y: x >= y +27 28 | op_is = lambda x, y: x is y +28 |-op_isnot = lambda x, y: x is not y + 29 |+op_isnot = operator.is_not +29 30 | op_in = lambda x, y: x in y +30 31 | +31 32 | def op_not2(x): + +FURB118.py:29:9: FURB118 [*] Use `operator.contains` instead of defining a lambda + | +27 | op_is = lambda x, y: x is y +28 | op_isnot = lambda x, y: x is not y +29 | op_in = lambda x, y: x in y + | ^^^^^^^^^^^^^^^^^^^ FURB118 +30 | +31 | def op_not2(x): + | + = help: Replace with `operator.contains` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +26 27 | op_gte = lambda x, y: x >= y +27 28 | op_is = lambda x, y: x is y +28 29 | op_isnot = lambda x, y: x is not y +29 |-op_in = lambda x, y: x in y + 30 |+op_in = operator.contains +30 31 | +31 32 | def op_not2(x): +32 33 | return not x + +FURB118.py:31:1: FURB118 Use `operator.not_` instead of defining a function + | +29 | op_in = lambda x, y: x in y +30 | +31 | / def op_not2(x): +32 | | return not x + | |________________^ FURB118 +33 | +34 | def op_add2(x, y): + | + = help: Replace with `operator.not_` + +FURB118.py:34:1: FURB118 Use `operator.add` instead of defining a function + | +32 | return not x +33 | +34 | / def op_add2(x, y): +35 | | return x + y + | |________________^ FURB118 +36 | +37 | class Adder: + | + = help: Replace with `operator.add` + +FURB118.py:38:5: FURB118 Use `operator.add` instead of defining a function + | +37 | class Adder: +38 | def add(x, y): + | _____^ +39 | | return x + y + | |____________________^ FURB118 +40 | +41 | # Ok. + | + = help: Replace with `operator.add` + + diff --git a/ruff.schema.json b/ruff.schema.json index c7f0f8566d49e..92d360d20122c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2903,6 +2903,7 @@ "FURB105", "FURB11", "FURB113", + "FURB118", "FURB13", "FURB131", "FURB132", From a5d623137d7b9dabb5bdc83100d147c091282b84 Mon Sep 17 00:00:00 2001 From: Tuomas Siipola Date: Sun, 17 Dec 2023 22:36:41 +0200 Subject: [PATCH 2/3] Fix example formatting --- .../src/rules/refurb/rules/reimplemented_operator.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs index dc0c131870035..d2100465c94b4 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs @@ -17,6 +17,7 @@ use crate::importer::ImportRequest; /// ## Example /// ```python /// import functools +/// /// nums = [1, 2, 3] /// sum = functools.reduce(lambda x, y: x + y, nums) /// ``` @@ -25,6 +26,7 @@ use crate::importer::ImportRequest; /// ```python /// import functools /// import operator +/// /// nums = [1, 2, 3] /// sum = functools.reduce(operator.add, nums) /// ``` From 5dc47c324e3a6bac1f78a93ba68ed861d714396a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 18 Dec 2023 09:53:54 -0500 Subject: [PATCH 3/3] Tweak docs; add some docstrings --- .../refurb/rules/reimplemented_operator.rs | 75 ++++++++++++------- 1 file changed, 50 insertions(+), 25 deletions(-) diff --git a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs index d2100465c94b4..1c3d81c834545 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs @@ -2,17 +2,21 @@ use anyhow::{bail, Result}; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr, Stmt}; +use ruff_python_semantic::SemanticModel; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; -use crate::importer::ImportRequest; +use crate::importer::{ImportRequest, Importer}; /// ## What it does -/// Checks for lambda expressions and function definitions that can be replaced with a function -/// from `operator` module. +/// Checks for lambda expressions and function definitions that can be replaced +/// with a function from the `operator` module. /// /// ## Why is this bad? -/// Using functions from `operator` module is more concise and readable. +/// The `operator` module provides functions that implement the same functionality +/// as the corresponding operators. For example, `operator.add` is equivalent to +/// `lambda x, y: x + y`. Using the functions from the `operator` module is more +/// concise and communicates the intent of the code more clearly. /// /// ## Example /// ```python @@ -54,7 +58,7 @@ impl Violation for ReimplementedOperator { } /// FURB118 -pub(crate) fn reimplemented_operator(checker: &mut Checker, target: &LambdaLike) { +pub(crate) fn reimplemented_operator(checker: &mut Checker, target: &FunctionLike) { let Some(params) = target.parameters() else { return; }; @@ -69,29 +73,30 @@ pub(crate) fn reimplemented_operator(checker: &mut Checker, target: &LambdaLike) }, target.range(), ); - diagnostic.try_set_fix(|| target.try_fix(checker, operator)); + diagnostic.try_set_fix(|| target.try_fix(operator, checker.importer(), checker.semantic())); checker.diagnostics.push(diagnostic); } /// Candidate for lambda expression or function definition consisting of a return statement. -pub(crate) enum LambdaLike<'a> { +#[derive(Debug)] +pub(crate) enum FunctionLike<'a> { Lambda(&'a ast::ExprLambda), Function(&'a ast::StmtFunctionDef), } -impl<'a> From<&'a ast::ExprLambda> for LambdaLike<'a> { +impl<'a> From<&'a ast::ExprLambda> for FunctionLike<'a> { fn from(lambda: &'a ast::ExprLambda) -> Self { Self::Lambda(lambda) } } -impl<'a> From<&'a ast::StmtFunctionDef> for LambdaLike<'a> { +impl<'a> From<&'a ast::StmtFunctionDef> for FunctionLike<'a> { fn from(function: &'a ast::StmtFunctionDef) -> Self { Self::Function(function) } } -impl Ranged for LambdaLike<'_> { +impl Ranged for FunctionLike<'_> { fn range(&self) -> TextRange { match self { Self::Lambda(expr) => expr.range(), @@ -100,7 +105,8 @@ impl Ranged for LambdaLike<'_> { } } -impl LambdaLike<'_> { +impl FunctionLike<'_> { + /// Return the [`ast::Parameters`] of the function-like node. fn parameters(&self) -> Option<&ast::Parameters> { match self { Self::Lambda(expr) => expr.parameters.as_deref(), @@ -108,6 +114,10 @@ impl LambdaLike<'_> { } } + /// Return the body of the function-like node. + /// + /// If the node is a function definition that consists of more than a single return statement, + /// returns `None`. fn body(&self) -> Option<&Expr> { match self { Self::Lambda(expr) => Some(&expr.body), @@ -118,13 +128,28 @@ impl LambdaLike<'_> { } } - fn try_fix(&self, checker: &Checker, operator: &'static str) -> Result { + /// Return the display kind of the function-like node. + fn kind(&self) -> &'static str { + match self { + Self::Lambda(_) => "lambda", + Self::Function(_) => "function", + } + } + + /// Attempt to fix the function-like node by replacing it with a call to the corresponding + /// function from `operator` module. + fn try_fix( + &self, + operator: &'static str, + importer: &Importer, + semantic: &SemanticModel, + ) -> Result { match self { Self::Lambda(_) => { - let (edit, binding) = checker.importer().get_or_import_symbol( + let (edit, binding) = importer.get_or_import_symbol( &ImportRequest::import("operator", operator), self.start(), - checker.semantic(), + semantic, )?; Ok(Fix::safe_edits( Edit::range_replacement(binding, self.range()), @@ -134,15 +159,9 @@ impl LambdaLike<'_> { Self::Function(_) => bail!("No fix available"), } } - - fn kind(&self) -> &'static str { - match self { - Self::Lambda(_) => "lambda", - Self::Function(_) => "function", - } - } } +/// Return the name of the `operator` implemented by the given expression. fn get_operator(expr: &Expr, params: &ast::Parameters) -> Option<&'static str> { match expr { Expr::UnaryOp(expr) => unary_op(expr, params), @@ -152,11 +171,12 @@ fn get_operator(expr: &Expr, params: &ast::Parameters) -> Option<&'static str> { } } +/// Return the name of the `operator` implemented by the given unary expression. fn unary_op(expr: &ast::ExprUnaryOp, params: &ast::Parameters) -> Option<&'static str> { let [arg] = params.args.as_slice() else { return None; }; - if !is_same(arg, &expr.operand) { + if !is_same_expression(arg, &expr.operand) { return None; } Some(match expr.op { @@ -167,11 +187,12 @@ fn unary_op(expr: &ast::ExprUnaryOp, params: &ast::Parameters) -> Option<&'stati }) } +/// Return the name of the `operator` implemented by the given binary expression. fn bin_op(expr: &ast::ExprBinOp, params: &ast::Parameters) -> Option<&'static str> { let [arg1, arg2] = params.args.as_slice() else { return None; }; - if !is_same(arg1, &expr.left) || !is_same(arg2, &expr.right) { + if !is_same_expression(arg1, &expr.left) || !is_same_expression(arg2, &expr.right) { return None; } Some(match expr.op { @@ -191,6 +212,7 @@ fn bin_op(expr: &ast::ExprBinOp, params: &ast::Parameters) -> Option<&'static st }) } +/// Return the name of the `operator` implemented by the given comparison expression. fn cmp_op(expr: &ast::ExprCompare, params: &ast::Parameters) -> Option<&'static str> { let [arg1, arg2] = params.args.as_slice() else { return None; @@ -201,7 +223,7 @@ fn cmp_op(expr: &ast::ExprCompare, params: &ast::Parameters) -> Option<&'static let [right] = expr.comparators.as_slice() else { return None; }; - if !is_same(arg1, &expr.left) || !is_same(arg2, right) { + if !is_same_expression(arg1, &expr.left) || !is_same_expression(arg2, right) { return None; } match op { @@ -218,7 +240,10 @@ fn cmp_op(expr: &ast::ExprCompare, params: &ast::Parameters) -> Option<&'static } } -fn is_same(arg: &ast::ParameterWithDefault, expr: &Expr) -> bool { +/// Returns `true` if the given argument is the "same" as the given expression. For example, if +/// the argument has a default, it is not considered the same as any expression; if both match the +/// same name, they are considered the same. +fn is_same_expression(arg: &ast::ParameterWithDefault, expr: &Expr) -> bool { if arg.default.is_some() { false } else if let Expr::Name(name) = expr {