From e2406353be230d0c09a056d09e38bdbce427659a Mon Sep 17 00:00:00 2001 From: pommicket Date: Mon, 12 May 2025 10:32:21 -0400 Subject: [PATCH] Add clippy::self_only_used_in_recursion lint and use it instead of clippy::only_used_in_recursion when the parameter in question is self. --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/only_used_in_recursion.rs | 95 +++++++++++++++++++++- tests/ui/only_used_in_recursion.rs | 3 +- tests/ui/only_used_in_recursion.stderr | 68 ++++++++-------- 5 files changed, 130 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc60b1c57f7d..9c17da1d3c71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6447,6 +6447,7 @@ Released 2018-09-13 [`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment [`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors [`self_named_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_module_files +[`self_only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_only_used_in_recursion [`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned [`semicolon_inside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_inside_block [`semicolon_outside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_outside_block diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index e1cb08e361ca..7faa656b78b3 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -575,6 +575,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES_INFO, crate::octal_escapes::OCTAL_ESCAPES_INFO, crate::only_used_in_recursion::ONLY_USED_IN_RECURSION_INFO, + crate::only_used_in_recursion::SELF_ONLY_USED_IN_RECURSION_INFO, crate::operators::ABSURD_EXTREME_COMPARISONS_INFO, crate::operators::ARITHMETIC_SIDE_EFFECTS_INFO, crate::operators::ASSIGN_OP_PATTERN_INFO, diff --git a/clippy_lints/src/only_used_in_recursion.rs b/clippy_lints/src/only_used_in_recursion.rs index ba8f6354d976..6a65c91f25e2 100644 --- a/clippy_lints/src/only_used_in_recursion.rs +++ b/clippy_lints/src/only_used_in_recursion.rs @@ -83,7 +83,85 @@ declare_clippy_lint! { complexity, "arguments that is only used in recursion can be removed" } -impl_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]); + +declare_clippy_lint! { + /// ### What it does + /// Checks for `self` receiver that is only used in recursion with no side-effects. + /// + /// ### Why is this bad? + /// + /// It may be possible to remove the `self` argument, allowing the function to be + /// used without an object of type `Self`. + /// + /// ### Known problems + /// Too many code paths in the linting code are currently untested and prone to produce false + /// positives or are prone to have performance implications. + /// + /// In some cases, this would not catch all useless arguments. + /// + /// ```no_run + /// struct Foo; + /// impl Foo { + /// fn foo(&self, a: usize) -> usize { + /// let f = |x| x; + /// + /// if a == 0 { + /// 1 + /// } else { + /// f(self).foo(a) + /// } + /// } + /// } + /// ``` + /// + /// For example, here `self` is only used in recursion, but the lint would not catch it. + /// + /// List of some examples that can not be caught: + /// - binary operation of non-primitive types + /// - closure usage + /// - some `break` relative operations + /// - struct pattern binding + /// + /// Also, when you recurse the function name with path segments, it is not possible to detect. + /// + /// ### Example + /// ```no_run + /// struct Foo; + /// impl Foo { + /// fn f(&self, n: u32) -> u32 { + /// if n == 0 { + /// 1 + /// } else { + /// n * self.f(n - 1) + /// } + /// } + /// } + /// # fn main() { + /// # print!("{}", Foo.f(10)); + /// # } + /// ``` + /// Use instead: + /// ```no_run + /// struct Foo; + /// impl Foo { + /// fn f(n: u32) -> u32 { + /// if n == 0 { + /// 1 + /// } else { + /// n * Self::f(n - 1) + /// } + /// } + /// } + /// # fn main() { + /// # print!("{}", Foo::f(10)); + /// # } + /// ``` + #[clippy::version = "1.91.0"] + pub SELF_ONLY_USED_IN_RECURSION, + pedantic, + "self receiver only used to recursively call method can be removed" +} +impl_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION, SELF_ONLY_USED_IN_RECURSION]); #[derive(Clone, Copy)] enum FnKind { @@ -355,13 +433,22 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion { self.params.flag_for_linting(); for param in &self.params.params { if param.apply_lint.get() { + let is_self = param.ident.name == kw::SelfLower; span_lint_and_then( cx, - ONLY_USED_IN_RECURSION, + if is_self { + SELF_ONLY_USED_IN_RECURSION + } else { + ONLY_USED_IN_RECURSION + }, param.ident.span, - "parameter is only used in recursion", + if is_self { + "`self` is only used in recursion" + } else { + "parameter is only used in recursion" + }, |diag| { - if param.ident.name != kw::SelfLower { + if !is_self { diag.span_suggestion( param.ident.span, "if this is intentional, prefix it with an underscore", diff --git a/tests/ui/only_used_in_recursion.rs b/tests/ui/only_used_in_recursion.rs index 7d6075ba9ea4..d58635969238 100644 --- a/tests/ui/only_used_in_recursion.rs +++ b/tests/ui/only_used_in_recursion.rs @@ -1,4 +1,5 @@ #![warn(clippy::only_used_in_recursion)] +#![warn(clippy::self_only_used_in_recursion)] //@no-rustfix fn _simple(x: u32) -> u32 { x @@ -74,7 +75,7 @@ impl A { } fn _method_self(&self, flag: usize, a: usize) -> usize { - //~^ only_used_in_recursion + //~^ self_only_used_in_recursion //~| only_used_in_recursion if flag == 0 { 0 } else { self._method_self(flag - 1, a) } diff --git a/tests/ui/only_used_in_recursion.stderr b/tests/ui/only_used_in_recursion.stderr index ca08319e1120..c3da3a0e3068 100644 --- a/tests/ui/only_used_in_recursion.stderr +++ b/tests/ui/only_used_in_recursion.stderr @@ -1,11 +1,11 @@ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:11:27 + --> tests/ui/only_used_in_recursion.rs:12:27 | LL | fn _one_unused(flag: u32, a: usize) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:14:53 + --> tests/ui/only_used_in_recursion.rs:15:53 | LL | if flag == 0 { 0 } else { _one_unused(flag - 1, a) } | ^ @@ -13,181 +13,183 @@ LL | if flag == 0 { 0 } else { _one_unused(flag - 1, a) } = help: to override `-D warnings` add `#[allow(clippy::only_used_in_recursion)]` error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:17:27 + --> tests/ui/only_used_in_recursion.rs:18:27 | LL | fn _two_unused(flag: u32, a: u32, b: i32) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:21:53 + --> tests/ui/only_used_in_recursion.rs:22:53 | LL | if flag == 0 { 0 } else { _two_unused(flag - 1, a, b) } | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:17:35 + --> tests/ui/only_used_in_recursion.rs:18:35 | LL | fn _two_unused(flag: u32, a: u32, b: i32) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_b` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:21:56 + --> tests/ui/only_used_in_recursion.rs:22:56 | LL | if flag == 0 { 0 } else { _two_unused(flag - 1, a, b) } | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:24:26 + --> tests/ui/only_used_in_recursion.rs:25:26 | LL | fn _with_calc(flag: u32, a: i64) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:30:32 + --> tests/ui/only_used_in_recursion.rs:31:32 | LL | _with_calc(flag - 1, (-a + 10) * 5) | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:39:33 + --> tests/ui/only_used_in_recursion.rs:40:33 | LL | fn _used_with_unused(flag: u32, a: i32, b: i32) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:46:38 + --> tests/ui/only_used_in_recursion.rs:47:38 | LL | _used_with_unused(flag - 1, -a, a + b) | ^ ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:39:41 + --> tests/ui/only_used_in_recursion.rs:40:41 | LL | fn _used_with_unused(flag: u32, a: i32, b: i32) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_b` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:46:45 + --> tests/ui/only_used_in_recursion.rs:47:45 | LL | _used_with_unused(flag - 1, -a, a + b) | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:50:35 + --> tests/ui/only_used_in_recursion.rs:51:35 | LL | fn _codependent_unused(flag: u32, a: i32, b: i32) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:57:39 + --> tests/ui/only_used_in_recursion.rs:58:39 | LL | _codependent_unused(flag - 1, a * b, a + b) | ^ ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:50:43 + --> tests/ui/only_used_in_recursion.rs:51:43 | LL | fn _codependent_unused(flag: u32, a: i32, b: i32) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_b` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:57:43 + --> tests/ui/only_used_in_recursion.rs:58:43 | LL | _codependent_unused(flag - 1, a * b, a + b) | ^ ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:61:30 + --> tests/ui/only_used_in_recursion.rs:62:30 | LL | fn _not_primitive(flag: u32, b: String) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_b` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:64:56 + --> tests/ui/only_used_in_recursion.rs:65:56 | LL | if flag == 0 { 0 } else { _not_primitive(flag - 1, b) } | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:70:29 + --> tests/ui/only_used_in_recursion.rs:71:29 | LL | fn _method(flag: usize, a: usize) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:73:59 + --> tests/ui/only_used_in_recursion.rs:74:59 | LL | if flag == 0 { 0 } else { Self::_method(flag - 1, a) } | ^ -error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:76:22 +error: `self` is only used in recursion + --> tests/ui/only_used_in_recursion.rs:77:22 | LL | fn _method_self(&self, flag: usize, a: usize) -> usize { | ^^^^ | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:80:35 + --> tests/ui/only_used_in_recursion.rs:81:35 | LL | if flag == 0 { 0 } else { self._method_self(flag - 1, a) } | ^^^^ + = note: `-D clippy::self-only-used-in-recursion` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::self_only_used_in_recursion)]` error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:76:41 + --> tests/ui/only_used_in_recursion.rs:77:41 | LL | fn _method_self(&self, flag: usize, a: usize) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:80:63 + --> tests/ui/only_used_in_recursion.rs:81:63 | LL | if flag == 0 { 0 } else { self._method_self(flag - 1, a) } | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:90:26 + --> tests/ui/only_used_in_recursion.rs:91:26 | LL | fn method(flag: u32, a: usize) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:93:58 + --> tests/ui/only_used_in_recursion.rs:94:58 | LL | if flag == 0 { 0 } else { Self::method(flag - 1, a) } | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:96:38 + --> tests/ui/only_used_in_recursion.rs:97:38 | LL | fn method_self(&self, flag: u32, a: usize) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:99:62 + --> tests/ui/only_used_in_recursion.rs:100:62 | LL | if flag == 0 { 0 } else { self.method_self(flag - 1, a) } | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:124:26 + --> tests/ui/only_used_in_recursion.rs:125:26 | LL | fn method(flag: u32, a: usize) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:127:58 + --> tests/ui/only_used_in_recursion.rs:128:58 | LL | if flag == 0 { 0 } else { Self::method(flag - 1, a) } | ^ error: parameter is only used in recursion - --> tests/ui/only_used_in_recursion.rs:130:38 + --> tests/ui/only_used_in_recursion.rs:131:38 | LL | fn method_self(&self, flag: u32, a: usize) -> usize { | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: parameter used here - --> tests/ui/only_used_in_recursion.rs:133:62 + --> tests/ui/only_used_in_recursion.rs:134:62 | LL | if flag == 0 { 0 } else { self.method_self(flag - 1, a) } | ^