Skip to content

Add clippy::self_only_used_in_recursion lint #14787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add clippy::self_only_used_in_recursion lint #14787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6154,6 +6154,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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ pub static LINTS: &[&crate::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,
Expand Down
95 changes: 91 additions & 4 deletions clippy_lints/src/only_used_in_recursion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.88.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 {
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/only_used_in_recursion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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) }
Expand Down
68 changes: 35 additions & 33 deletions tests/ui/only_used_in_recursion.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,193 +1,195 @@
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) }
| ^
= note: `-D clippy::only-used-in-recursion` implied by `-D warnings`
= 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) }
| ^
Expand Down
Loading