From 894cd13ec1099ab1678aaa0274cc43dd9c2bb141 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 3 May 2024 16:03:12 -0400 Subject: [PATCH] [`refurb`] Ignore methods in `reimplemented-operator` (`FURB118`) (#11270) ## Summary This rule does more harm than good when applied to methods. Closes https://github.com/astral-sh/ruff/issues/10898. Closes https://github.com/astral-sh/ruff/issues/11045. --- .../resources/test/fixtures/refurb/FURB118.py | 7 +++++++ .../flake8_unused_arguments/rules/unused_arguments.rs | 2 +- .../pep8_naming/rules/invalid_first_argument_name.rs | 2 +- .../rules/pylint/rules/bad_staticmethod_argument.rs | 2 +- .../ruff_linter/src/rules/pylint/rules/no_self_use.rs | 2 +- .../src/rules/pylint/rules/singledispatch_method.rs | 2 +- .../pylint/rules/singledispatchmethod_function.rs | 2 +- .../src/rules/pylint/rules/super_without_brackets.rs | 2 +- .../src/rules/refurb/rules/reimplemented_operator.rs | 7 +++++++ ...nter__rules__refurb__tests__FURB118_FURB118.py.snap | 10 ---------- 10 files changed, 21 insertions(+), 17 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py index a680f3b12a1a9..ed515ab8145c3 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py @@ -73,3 +73,10 @@ def op_add4(x, y=1): def op_add5(x, y): print("op_add5") return x + y + + +# OK +class Class: + @staticmethod + def add(x, y): + return x + y diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 37d7ebffebc06..3403675ab06d4 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -322,7 +322,7 @@ pub(crate) fn unused_arguments( return; } - let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else { return; }; diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index 26f91091b154f..f503796e277af 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -190,7 +190,7 @@ pub(crate) fn invalid_first_argument_name( panic!("Expected ScopeKind::Function") }; - let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs index c99cd52c8cc1b..cc9ea5c36b1ea 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/bad_staticmethod_argument.rs @@ -64,7 +64,7 @@ pub(crate) fn bad_staticmethod_argument( .. } = func; - let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs index a642334774ef7..f6aea4e8b6d24 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs @@ -49,7 +49,7 @@ pub(crate) fn no_self_use( scope: &Scope, diagnostics: &mut Vec, ) { - let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/rules/singledispatch_method.rs b/crates/ruff_linter/src/rules/pylint/rules/singledispatch_method.rs index 17c24e1a1962f..8f1989217ff6f 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/singledispatch_method.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/singledispatch_method.rs @@ -74,7 +74,7 @@ pub(crate) fn singledispatch_method( .. } = func; - let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/rules/singledispatchmethod_function.rs b/crates/ruff_linter/src/rules/pylint/rules/singledispatchmethod_function.rs index 98a05582eb045..fa51711cb391c 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/singledispatchmethod_function.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/singledispatchmethod_function.rs @@ -72,7 +72,7 @@ pub(crate) fn singledispatchmethod_function( .. } = func; - let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/rules/super_without_brackets.rs b/crates/ruff_linter/src/rules/pylint/rules/super_without_brackets.rs index 7e9992a199bd2..81f9c0e44ba9f 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/super_without_brackets.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/super_without_brackets.rs @@ -83,7 +83,7 @@ pub(crate) fn super_without_brackets(checker: &mut Checker, func: &Expr) { return; }; - let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { + let Some(parent) = checker.semantic().first_non_type_parent_scope(scope) else { return; }; 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 eabce42e1dc6b..15dc329ae2cba 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs @@ -69,6 +69,13 @@ impl Violation for ReimplementedOperator { /// FURB118 pub(crate) fn reimplemented_operator(checker: &mut Checker, target: &FunctionLike) { + // Ignore methods. + if target.kind() == FunctionLikeKind::Function { + if checker.semantic().current_scope().kind.is_class() { + return; + } + } + let Some(params) = target.parameters() else { return; }; 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 index 122f66aded57f..937bf2ec46317 100644 --- 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 @@ -793,13 +793,3 @@ FURB118.py:40:1: FURB118 Use `operator.add` instead of defining a function | |________________^ FURB118 | = help: Replace with `operator.add` - -FURB118.py:45:5: FURB118 Use `operator.add` instead of defining a function - | -44 | class Adder: -45 | def add(x, y): - | _____^ -46 | | return x + y - | |____________________^ FURB118 - | - = help: Replace with `operator.add`