Skip to content

Commit a59c129

Browse files
pommicketpommicket
committed
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.
1 parent 7f6d507 commit a59c129

File tree

5 files changed

+130
-38
lines changed

5 files changed

+130
-38
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6154,6 +6154,7 @@ Released 2018-09-13
61546154
[`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment
61556155
[`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors
61566156
[`self_named_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_module_files
6157+
[`self_only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_only_used_in_recursion
61576158
[`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
61586159
[`semicolon_inside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_inside_block
61596160
[`semicolon_outside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_outside_block

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
566566
crate::nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES_INFO,
567567
crate::octal_escapes::OCTAL_ESCAPES_INFO,
568568
crate::only_used_in_recursion::ONLY_USED_IN_RECURSION_INFO,
569+
crate::only_used_in_recursion::SELF_ONLY_USED_IN_RECURSION_INFO,
569570
crate::operators::ABSURD_EXTREME_COMPARISONS_INFO,
570571
crate::operators::ARITHMETIC_SIDE_EFFECTS_INFO,
571572
crate::operators::ASSIGN_OP_PATTERN_INFO,

‎clippy_lints/src/only_used_in_recursion.rs‎

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,85 @@ declare_clippy_lint! {
8383
complexity,
8484
"arguments that is only used in recursion can be removed"
8585
}
86-
impl_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]);
86+
87+
declare_clippy_lint! {
88+
/// ### What it does
89+
/// Checks for `self` receiver that is only used in recursion with no side-effects.
90+
///
91+
/// ### Why is this bad?
92+
///
93+
/// It may be possible to remove the `self` argument, allowing the function to be
94+
/// used without an object of type `Self`.
95+
///
96+
/// ### Known problems
97+
/// Too many code paths in the linting code are currently untested and prone to produce false
98+
/// positives or are prone to have performance implications.
99+
///
100+
/// In some cases, this would not catch all useless arguments.
101+
///
102+
/// ```no_run
103+
/// struct Foo;
104+
/// impl Foo {
105+
/// fn foo(&self, a: usize) -> usize {
106+
/// let f = |x| x;
107+
///
108+
/// if a == 0 {
109+
/// 1
110+
/// } else {
111+
/// f(self).foo(a)
112+
/// }
113+
/// }
114+
/// }
115+
/// ```
116+
///
117+
/// For example, here `self` is only used in recursion, but the lint would not catch it.
118+
///
119+
/// List of some examples that can not be caught:
120+
/// - binary operation of non-primitive types
121+
/// - closure usage
122+
/// - some `break` relative operations
123+
/// - struct pattern binding
124+
///
125+
/// Also, when you recurse the function name with path segments, it is not possible to detect.
126+
///
127+
/// ### Example
128+
/// ```no_run
129+
/// struct Foo;
130+
/// impl Foo {
131+
/// fn f(&self, n: u32) -> u32 {
132+
/// if n == 0 {
133+
/// 1
134+
/// } else {
135+
/// n * self.f(n - 1)
136+
/// }
137+
/// }
138+
/// }
139+
/// # fn main() {
140+
/// # print!("{}", Foo.f(10));
141+
/// # }
142+
/// ```
143+
/// Use instead:
144+
/// ```no_run
145+
/// struct Foo;
146+
/// impl Foo {
147+
/// fn f(n: u32) -> u32 {
148+
/// if n == 0 {
149+
/// 1
150+
/// } else {
151+
/// n * Self::f(n - 1)
152+
/// }
153+
/// }
154+
/// }
155+
/// # fn main() {
156+
/// # print!("{}", Foo::f(10));
157+
/// # }
158+
/// ```
159+
#[clippy::version = "1.89.0"]
160+
pub SELF_ONLY_USED_IN_RECURSION,
161+
pedantic,
162+
"self receiver only used to recursively call method can be removed"
163+
}
164+
impl_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION, SELF_ONLY_USED_IN_RECURSION]);
87165

88166
#[derive(Clone, Copy)]
89167
enum FnKind {
@@ -355,13 +433,22 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
355433
self.params.flag_for_linting();
356434
for param in &self.params.params {
357435
if param.apply_lint.get() {
436+
let is_self = param.ident.name == kw::SelfLower;
358437
span_lint_and_then(
359438
cx,
360-
ONLY_USED_IN_RECURSION,
439+
if is_self {
440+
SELF_ONLY_USED_IN_RECURSION
441+
} else {
442+
ONLY_USED_IN_RECURSION
443+
},
361444
param.ident.span,
362-
"parameter is only used in recursion",
445+
if is_self {
446+
"`self` is only used in recursion"
447+
} else {
448+
"parameter is only used in recursion"
449+
},
363450
|diag| {
364-
if param.ident.name != kw::SelfLower {
451+
if !is_self {
365452
diag.span_suggestion(
366453
param.ident.span,
367454
"if this is intentional, prefix it with an underscore",

‎tests/ui/only_used_in_recursion.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::only_used_in_recursion)]
2+
#![warn(clippy::self_only_used_in_recursion)]
23
//@no-rustfix
34
fn _simple(x: u32) -> u32 {
45
x
@@ -74,7 +75,7 @@ impl A {
7475
}
7576

7677
fn _method_self(&self, flag: usize, a: usize) -> usize {
77-
//~^ only_used_in_recursion
78+
//~^ self_only_used_in_recursion
7879
//~| only_used_in_recursion
7980

8081
if flag == 0 { 0 } else { self._method_self(flag - 1, a) }

‎tests/ui/only_used_in_recursion.stderr‎

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,193 +1,195 @@
11
error: parameter is only used in recursion
2-
--> tests/ui/only_used_in_recursion.rs:11:27
2+
--> tests/ui/only_used_in_recursion.rs:12:27
33
|
44
LL | fn _one_unused(flag: u32, a: usize) -> usize {
55
| ^ help: if this is intentional, prefix it with an underscore: `_a`
66
|
77
note: parameter used here
8-
--> tests/ui/only_used_in_recursion.rs:14:53
8+
--> tests/ui/only_used_in_recursion.rs:15:53
99
|
1010
LL | if flag == 0 { 0 } else { _one_unused(flag - 1, a) }
1111
| ^
1212
= note: `-D clippy::only-used-in-recursion` implied by `-D warnings`
1313
= help: to override `-D warnings` add `#[allow(clippy::only_used_in_recursion)]`
1414

1515
error: parameter is only used in recursion
16-
--> tests/ui/only_used_in_recursion.rs:17:27
16+
--> tests/ui/only_used_in_recursion.rs:18:27
1717
|
1818
LL | fn _two_unused(flag: u32, a: u32, b: i32) -> usize {
1919
| ^ help: if this is intentional, prefix it with an underscore: `_a`
2020
|
2121
note: parameter used here
22-
--> tests/ui/only_used_in_recursion.rs:21:53
22+
--> tests/ui/only_used_in_recursion.rs:22:53
2323
|
2424
LL | if flag == 0 { 0 } else { _two_unused(flag - 1, a, b) }
2525
| ^
2626

2727
error: parameter is only used in recursion
28-
--> tests/ui/only_used_in_recursion.rs:17:35
28+
--> tests/ui/only_used_in_recursion.rs:18:35
2929
|
3030
LL | fn _two_unused(flag: u32, a: u32, b: i32) -> usize {
3131
| ^ help: if this is intentional, prefix it with an underscore: `_b`
3232
|
3333
note: parameter used here
34-
--> tests/ui/only_used_in_recursion.rs:21:56
34+
--> tests/ui/only_used_in_recursion.rs:22:56
3535
|
3636
LL | if flag == 0 { 0 } else { _two_unused(flag - 1, a, b) }
3737
| ^
3838

3939
error: parameter is only used in recursion
40-
--> tests/ui/only_used_in_recursion.rs:24:26
40+
--> tests/ui/only_used_in_recursion.rs:25:26
4141
|
4242
LL | fn _with_calc(flag: u32, a: i64) -> usize {
4343
| ^ help: if this is intentional, prefix it with an underscore: `_a`
4444
|
4545
note: parameter used here
46-
--> tests/ui/only_used_in_recursion.rs:30:32
46+
--> tests/ui/only_used_in_recursion.rs:31:32
4747
|
4848
LL | _with_calc(flag - 1, (-a + 10) * 5)
4949
| ^
5050

5151
error: parameter is only used in recursion
52-
--> tests/ui/only_used_in_recursion.rs:39:33
52+
--> tests/ui/only_used_in_recursion.rs:40:33
5353
|
5454
LL | fn _used_with_unused(flag: u32, a: i32, b: i32) -> usize {
5555
| ^ help: if this is intentional, prefix it with an underscore: `_a`
5656
|
5757
note: parameter used here
58-
--> tests/ui/only_used_in_recursion.rs:46:38
58+
--> tests/ui/only_used_in_recursion.rs:47:38
5959
|
6060
LL | _used_with_unused(flag - 1, -a, a + b)
6161
| ^ ^
6262

6363
error: parameter is only used in recursion
64-
--> tests/ui/only_used_in_recursion.rs:39:41
64+
--> tests/ui/only_used_in_recursion.rs:40:41
6565
|
6666
LL | fn _used_with_unused(flag: u32, a: i32, b: i32) -> usize {
6767
| ^ help: if this is intentional, prefix it with an underscore: `_b`
6868
|
6969
note: parameter used here
70-
--> tests/ui/only_used_in_recursion.rs:46:45
70+
--> tests/ui/only_used_in_recursion.rs:47:45
7171
|
7272
LL | _used_with_unused(flag - 1, -a, a + b)
7373
| ^
7474

7575
error: parameter is only used in recursion
76-
--> tests/ui/only_used_in_recursion.rs:50:35
76+
--> tests/ui/only_used_in_recursion.rs:51:35
7777
|
7878
LL | fn _codependent_unused(flag: u32, a: i32, b: i32) -> usize {
7979
| ^ help: if this is intentional, prefix it with an underscore: `_a`
8080
|
8181
note: parameter used here
82-
--> tests/ui/only_used_in_recursion.rs:57:39
82+
--> tests/ui/only_used_in_recursion.rs:58:39
8383
|
8484
LL | _codependent_unused(flag - 1, a * b, a + b)
8585
| ^ ^
8686

8787
error: parameter is only used in recursion
88-
--> tests/ui/only_used_in_recursion.rs:50:43
88+
--> tests/ui/only_used_in_recursion.rs:51:43
8989
|
9090
LL | fn _codependent_unused(flag: u32, a: i32, b: i32) -> usize {
9191
| ^ help: if this is intentional, prefix it with an underscore: `_b`
9292
|
9393
note: parameter used here
94-
--> tests/ui/only_used_in_recursion.rs:57:43
94+
--> tests/ui/only_used_in_recursion.rs:58:43
9595
|
9696
LL | _codependent_unused(flag - 1, a * b, a + b)
9797
| ^ ^
9898

9999
error: parameter is only used in recursion
100-
--> tests/ui/only_used_in_recursion.rs:61:30
100+
--> tests/ui/only_used_in_recursion.rs:62:30
101101
|
102102
LL | fn _not_primitive(flag: u32, b: String) -> usize {
103103
| ^ help: if this is intentional, prefix it with an underscore: `_b`
104104
|
105105
note: parameter used here
106-
--> tests/ui/only_used_in_recursion.rs:64:56
106+
--> tests/ui/only_used_in_recursion.rs:65:56
107107
|
108108
LL | if flag == 0 { 0 } else { _not_primitive(flag - 1, b) }
109109
| ^
110110

111111
error: parameter is only used in recursion
112-
--> tests/ui/only_used_in_recursion.rs:70:29
112+
--> tests/ui/only_used_in_recursion.rs:71:29
113113
|
114114
LL | fn _method(flag: usize, a: usize) -> usize {
115115
| ^ help: if this is intentional, prefix it with an underscore: `_a`
116116
|
117117
note: parameter used here
118-
--> tests/ui/only_used_in_recursion.rs:73:59
118+
--> tests/ui/only_used_in_recursion.rs:74:59
119119
|
120120
LL | if flag == 0 { 0 } else { Self::_method(flag - 1, a) }
121121
| ^
122122

123-
error: parameter is only used in recursion
124-
--> tests/ui/only_used_in_recursion.rs:76:22
123+
error: self is only used in recursion
124+
--> tests/ui/only_used_in_recursion.rs:77:22
125125
|
126126
LL | fn _method_self(&self, flag: usize, a: usize) -> usize {
127127
| ^^^^
128128
|
129129
note: parameter used here
130-
--> tests/ui/only_used_in_recursion.rs:80:35
130+
--> tests/ui/only_used_in_recursion.rs:81:35
131131
|
132132
LL | if flag == 0 { 0 } else { self._method_self(flag - 1, a) }
133133
| ^^^^
134+
= note: `-D clippy::self-only-used-in-recursion` implied by `-D warnings`
135+
= help: to override `-D warnings` add `#[allow(clippy::self_only_used_in_recursion)]`
134136

135137
error: parameter is only used in recursion
136-
--> tests/ui/only_used_in_recursion.rs:76:41
138+
--> tests/ui/only_used_in_recursion.rs:77:41
137139
|
138140
LL | fn _method_self(&self, flag: usize, a: usize) -> usize {
139141
| ^ help: if this is intentional, prefix it with an underscore: `_a`
140142
|
141143
note: parameter used here
142-
--> tests/ui/only_used_in_recursion.rs:80:63
144+
--> tests/ui/only_used_in_recursion.rs:81:63
143145
|
144146
LL | if flag == 0 { 0 } else { self._method_self(flag - 1, a) }
145147
| ^
146148

147149
error: parameter is only used in recursion
148-
--> tests/ui/only_used_in_recursion.rs:90:26
150+
--> tests/ui/only_used_in_recursion.rs:91:26
149151
|
150152
LL | fn method(flag: u32, a: usize) -> usize {
151153
| ^ help: if this is intentional, prefix it with an underscore: `_a`
152154
|
153155
note: parameter used here
154-
--> tests/ui/only_used_in_recursion.rs:93:58
156+
--> tests/ui/only_used_in_recursion.rs:94:58
155157
|
156158
LL | if flag == 0 { 0 } else { Self::method(flag - 1, a) }
157159
| ^
158160

159161
error: parameter is only used in recursion
160-
--> tests/ui/only_used_in_recursion.rs:96:38
162+
--> tests/ui/only_used_in_recursion.rs:97:38
161163
|
162164
LL | fn method_self(&self, flag: u32, a: usize) -> usize {
163165
| ^ help: if this is intentional, prefix it with an underscore: `_a`
164166
|
165167
note: parameter used here
166-
--> tests/ui/only_used_in_recursion.rs:99:62
168+
--> tests/ui/only_used_in_recursion.rs:100:62
167169
|
168170
LL | if flag == 0 { 0 } else { self.method_self(flag - 1, a) }
169171
| ^
170172

171173
error: parameter is only used in recursion
172-
--> tests/ui/only_used_in_recursion.rs:124:26
174+
--> tests/ui/only_used_in_recursion.rs:125:26
173175
|
174176
LL | fn method(flag: u32, a: usize) -> usize {
175177
| ^ help: if this is intentional, prefix it with an underscore: `_a`
176178
|
177179
note: parameter used here
178-
--> tests/ui/only_used_in_recursion.rs:127:58
180+
--> tests/ui/only_used_in_recursion.rs:128:58
179181
|
180182
LL | if flag == 0 { 0 } else { Self::method(flag - 1, a) }
181183
| ^
182184

183185
error: parameter is only used in recursion
184-
--> tests/ui/only_used_in_recursion.rs:130:38
186+
--> tests/ui/only_used_in_recursion.rs:131:38
185187
|
186188
LL | fn method_self(&self, flag: u32, a: usize) -> usize {
187189
| ^ help: if this is intentional, prefix it with an underscore: `_a`
188190
|
189191
note: parameter used here
190-
--> tests/ui/only_used_in_recursion.rs:133:62
192+
--> tests/ui/only_used_in_recursion.rs:134:62
191193
|
192194
LL | if flag == 0 { 0 } else { self.method_self(flag - 1, a) }
193195
| ^

0 commit comments

Comments
 (0)