Skip to content

Commit 75637c1

Browse files
bugadanibugadani
committed
Catch function calls in argument lists, add tests that tuples don't get linted
1 parent d7220db commit 75637c1

File tree

4 files changed

+52
-37
lines changed

4 files changed

+52
-37
lines changed

‎clippy_lints/src/methods/mod.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2744,12 +2744,8 @@ fn lint_lazy_eval<'a, 'tcx>(
27442744
paths.iter().any(|candidate| match_qpath(path, candidate))
27452745
}
27462746

2747-
if let hir::ExprKind::Closure(_, _, eid, _, _) = args[1].kind {
2748-
let body = cx.tcx.hir().body(eid);
2749-
let ex = &body.value;
2750-
let params = &body.params;
2751-
2752-
let simplify = match ex.kind {
2747+
fn can_simplify(expr: &hir::Expr<'_>, params: &[hir::Param<'_>], variant_calls: bool) -> bool {
2748+
match expr.kind {
27532749
// Closures returning literals can be unconditionally simplified
27542750
hir::ExprKind::Lit(_) => true,
27552751

@@ -2767,15 +2763,16 @@ fn lint_lazy_eval<'a, 'tcx>(
27672763
hir::ExprKind::Field(ref object, _) => !expr_uses_argument(object, params),
27682764

27692765
// Paths can be simplified if the root is not the argument, this also covers None
2770-
hir::ExprKind::Path(_) => !expr_uses_argument(ex, params),
2766+
hir::ExprKind::Path(_) => !expr_uses_argument(expr, params),
27712767

27722768
// Calls to Some, Ok, Err can be considered literals if they don't derive an argument
27732769
hir::ExprKind::Call(ref func, ref args) => if_chain! {
2774-
if allow_variant_calls; // Disable lint when rules conflict with bind_instead_of_map
2770+
if variant_calls; // Disable lint when rules conflict with bind_instead_of_map
27752771
if let hir::ExprKind::Path(ref path) = func.kind;
27762772
if match_any_qpath(path, &[&["Some"], &["Ok"], &["Err"]]);
27772773
then {
2778-
!args.iter().any(|arg| expr_uses_argument(arg, params))
2774+
// Recursively check all arguments
2775+
args.iter().all(|arg| can_simplify(arg, params, variant_calls))
27792776
} else {
27802777
false
27812778
}
@@ -2784,9 +2781,15 @@ fn lint_lazy_eval<'a, 'tcx>(
27842781
// For anything more complex than the above, a closure is probably the right solution,
27852782
// or the case is handled by an other lint
27862783
_ => false,
2787-
};
2784+
}
2785+
}
2786+
2787+
if let hir::ExprKind::Closure(_, _, eid, _, _) = args[1].kind {
2788+
let body = cx.tcx.hir().body(eid);
2789+
let ex = &body.value;
2790+
let params = &body.params;
27882791

2789-
if simplify {
2792+
if can_simplify(ex, params, allow_variant_calls) {
27902793
let msg = if is_option {
27912794
"unnecessary closure used to substitute value for `Option::None`"
27922795
} else {

‎tests/ui/unnecessary_lazy_eval.fixed

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,12 @@ fn main() {
2525
let ext_arr: [usize; 1] = [2];
2626
let ext_str = SomeStruct { some_field: 10 };
2727

28-
// Should lint - Option
2928
let mut opt = Some(42);
3029
let ext_opt = Some(42);
30+
let nested_opt = Some(Some(42));
31+
let nested_tuple_opt = Some(Some((42, 43)));
32+
33+
// Should lint - Option
3134
let _ = opt.unwrap_or(2);
3235
let _ = opt.unwrap_or(astronomers_pi);
3336
let _ = opt.unwrap_or(ext_str.some_field);
@@ -56,6 +59,9 @@ fn main() {
5659

5760
// Should not lint - Option
5861
let _ = opt.unwrap_or_else(|| ext_str.return_some_field());
62+
let _ = nested_opt.unwrap_or_else(|| Some(some_call()));
63+
let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2)));
64+
let _ = nested_tuple_opt.unwrap_or_else(|| Some((some_call(), some_call())));
5965
let _ = opt.or_else(some_call);
6066
let _ = opt.or_else(|| some_call());
6167
let _: Result<usize, usize> = opt.ok_or_else(|| some_call());

‎tests/ui/unnecessary_lazy_eval.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,12 @@ fn main() {
2525
let ext_arr: [usize; 1] = [2];
2626
let ext_str = SomeStruct { some_field: 10 };
2727

28-
// Should lint - Option
2928
let mut opt = Some(42);
3029
let ext_opt = Some(42);
30+
let nested_opt = Some(Some(42));
31+
let nested_tuple_opt = Some(Some((42, 43)));
32+
33+
// Should lint - Option
3134
let _ = opt.unwrap_or_else(|| 2);
3235
let _ = opt.unwrap_or_else(|| astronomers_pi);
3336
let _ = opt.unwrap_or_else(|| ext_str.some_field);
@@ -56,6 +59,9 @@ fn main() {
5659

5760
// Should not lint - Option
5861
let _ = opt.unwrap_or_else(|| ext_str.return_some_field());
62+
let _ = nested_opt.unwrap_or_else(|| Some(some_call()));
63+
let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2)));
64+
let _ = nested_tuple_opt.unwrap_or_else(|| Some((some_call(), some_call())));
5965
let _ = opt.or_else(some_call);
6066
let _ = opt.or_else(|| some_call());
6167
let _: Result<usize, usize> = opt.ok_or_else(|| some_call());

‎tests/ui/unnecessary_lazy_eval.stderr

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,145 +1,145 @@
11
error: unnecessary closure used to substitute value for `Option::None`
2-
--> $DIR/unnecessary_lazy_eval.rs:31:13
2+
--> $DIR/unnecessary_lazy_eval.rs:34:13
33
|
44
LL | let _ = opt.unwrap_or_else(|| 2);
55
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `opt.unwrap_or(2)`
66
|
77
= note: `-D clippy::unnecessary-lazy-evaluation` implied by `-D warnings`
88

99
error: unnecessary closure used to substitute value for `Option::None`
10-
--> $DIR/unnecessary_lazy_eval.rs:32:13
10+
--> $DIR/unnecessary_lazy_eval.rs:35:13
1111
|
1212
LL | let _ = opt.unwrap_or_else(|| astronomers_pi);
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `opt.unwrap_or(astronomers_pi)`
1414

1515
error: unnecessary closure used to substitute value for `Option::None`
16-
--> $DIR/unnecessary_lazy_eval.rs:33:13
16+
--> $DIR/unnecessary_lazy_eval.rs:36:13
1717
|
1818
LL | let _ = opt.unwrap_or_else(|| ext_str.some_field);
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `opt.unwrap_or(ext_str.some_field)`
2020

2121
error: unnecessary closure used to substitute value for `Option::None`
22-
--> $DIR/unnecessary_lazy_eval.rs:34:13
22+
--> $DIR/unnecessary_lazy_eval.rs:37:13
2323
|
2424
LL | let _ = opt.unwrap_or_else(|| ext_arr[0]);
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `opt.unwrap_or(ext_arr[0])`
2626

2727
error: unnecessary closure used to substitute value for `Option::None`
28-
--> $DIR/unnecessary_lazy_eval.rs:35:13
28+
--> $DIR/unnecessary_lazy_eval.rs:38:13
2929
|
3030
LL | let _ = opt.and_then(|_| ext_opt);
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `and` instead: `opt.and(ext_opt)`
3232

3333
error: unnecessary closure used to substitute value for `Option::None`
34-
--> $DIR/unnecessary_lazy_eval.rs:36:13
34+
--> $DIR/unnecessary_lazy_eval.rs:39:13
3535
|
3636
LL | let _ = opt.or_else(|| ext_opt);
3737
| ^^^^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `opt.or(ext_opt)`
3838

3939
error: unnecessary closure used to substitute value for `Option::None`
40-
--> $DIR/unnecessary_lazy_eval.rs:37:13
40+
--> $DIR/unnecessary_lazy_eval.rs:40:13
4141
|
4242
LL | let _ = opt.or_else(|| None);
4343
| ^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `opt.or(None)`
4444

4545
error: unnecessary closure used to substitute value for `Option::None`
46-
--> $DIR/unnecessary_lazy_eval.rs:38:13
46+
--> $DIR/unnecessary_lazy_eval.rs:41:13
4747
|
4848
LL | let _ = opt.get_or_insert_with(|| 2);
4949
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `get_or_insert` instead: `opt.get_or_insert(2)`
5050

5151
error: unnecessary closure used to substitute value for `Option::None`
52-
--> $DIR/unnecessary_lazy_eval.rs:39:13
52+
--> $DIR/unnecessary_lazy_eval.rs:42:13
5353
|
5454
LL | let _ = opt.ok_or_else(|| 2);
5555
| ^^^^^^^^^^^^^^^^^^^^ help: Use `ok_or` instead: `opt.ok_or(2)`
5656

5757
error: unnecessary closure used to substitute value for `Option::None`
58-
--> $DIR/unnecessary_lazy_eval.rs:40:13
58+
--> $DIR/unnecessary_lazy_eval.rs:43:13
5959
|
6060
LL | let _ = opt.ok_or_else(|| ext_arr[0]);
6161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `ok_or` instead: `opt.ok_or(ext_arr[0])`
6262

6363
error: unnecessary closure used to substitute value for `Option::None`
64-
--> $DIR/unnecessary_lazy_eval.rs:43:13
64+
--> $DIR/unnecessary_lazy_eval.rs:46:13
6565
|
6666
LL | let _ = Some(10).unwrap_or_else(|| 2);
6767
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `Some(10).unwrap_or(2)`
6868

6969
error: unnecessary closure used to substitute value for `Option::None`
70-
--> $DIR/unnecessary_lazy_eval.rs:44:13
70+
--> $DIR/unnecessary_lazy_eval.rs:47:13
7171
|
7272
LL | let _ = Some(10).and_then(|_| ext_opt);
7373
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `and` instead: `Some(10).and(ext_opt)`
7474

7575
error: unnecessary closure used to substitute value for `Option::None`
76-
--> $DIR/unnecessary_lazy_eval.rs:45:28
76+
--> $DIR/unnecessary_lazy_eval.rs:48:28
7777
|
7878
LL | let _: Option<usize> = None.or_else(|| ext_opt);
7979
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `None.or(ext_opt)`
8080

8181
error: unnecessary closure used to substitute value for `Option::None`
82-
--> $DIR/unnecessary_lazy_eval.rs:46:13
82+
--> $DIR/unnecessary_lazy_eval.rs:49:13
8383
|
8484
LL | let _ = None.get_or_insert_with(|| 2);
8585
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `get_or_insert` instead: `None.get_or_insert(2)`
8686

8787
error: unnecessary closure used to substitute value for `Option::None`
88-
--> $DIR/unnecessary_lazy_eval.rs:47:35
88+
--> $DIR/unnecessary_lazy_eval.rs:50:35
8989
|
9090
LL | let _: Result<usize, usize> = None.ok_or_else(|| 2);
9191
| ^^^^^^^^^^^^^^^^^^^^^ help: Use `ok_or` instead: `None.ok_or(2)`
9292

9393
error: unnecessary closure used to substitute value for `Option::None`
94-
--> $DIR/unnecessary_lazy_eval.rs:48:28
94+
--> $DIR/unnecessary_lazy_eval.rs:51:28
9595
|
9696
LL | let _: Option<usize> = None.or_else(|| None);
9797
| ^^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `None.or(None)`
9898

9999
error: unnecessary closure used to substitute value for `Option::None`
100-
--> $DIR/unnecessary_lazy_eval.rs:51:13
100+
--> $DIR/unnecessary_lazy_eval.rs:54:13
101101
|
102102
LL | let _ = deep.0.unwrap_or_else(|| 2);
103103
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `deep.0.unwrap_or(2)`
104104

105105
error: unnecessary closure used to substitute value for `Option::None`
106-
--> $DIR/unnecessary_lazy_eval.rs:52:13
106+
--> $DIR/unnecessary_lazy_eval.rs:55:13
107107
|
108108
LL | let _ = deep.0.and_then(|_| ext_opt);
109109
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `and` instead: `deep.0.and(ext_opt)`
110110

111111
error: unnecessary closure used to substitute value for `Option::None`
112-
--> $DIR/unnecessary_lazy_eval.rs:53:13
112+
--> $DIR/unnecessary_lazy_eval.rs:56:13
113113
|
114114
LL | let _ = deep.0.or_else(|| None);
115115
| ^^^^^^^^^^^^^^^^^^^^^^^ help: Use `or` instead: `deep.0.or(None)`
116116

117117
error: unnecessary closure used to substitute value for `Option::None`
118-
--> $DIR/unnecessary_lazy_eval.rs:54:13
118+
--> $DIR/unnecessary_lazy_eval.rs:57:13
119119
|
120120
LL | let _ = deep.0.get_or_insert_with(|| 2);
121121
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `get_or_insert` instead: `deep.0.get_or_insert(2)`
122122

123123
error: unnecessary closure used to substitute value for `Option::None`
124-
--> $DIR/unnecessary_lazy_eval.rs:55:13
124+
--> $DIR/unnecessary_lazy_eval.rs:58:13
125125
|
126126
LL | let _ = deep.0.ok_or_else(|| 2);
127127
| ^^^^^^^^^^^^^^^^^^^^^^^ help: Use `ok_or` instead: `deep.0.ok_or(2)`
128128

129129
error: unnecessary closure used to substitute value for `Result::Err`
130-
--> $DIR/unnecessary_lazy_eval.rs:78:13
130+
--> $DIR/unnecessary_lazy_eval.rs:84:13
131131
|
132132
LL | let _ = res2.unwrap_or_else(|_| 2);
133133
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `res2.unwrap_or(2)`
134134

135135
error: unnecessary closure used to substitute value for `Result::Err`
136-
--> $DIR/unnecessary_lazy_eval.rs:79:13
136+
--> $DIR/unnecessary_lazy_eval.rs:85:13
137137
|
138138
LL | let _ = res2.unwrap_or_else(|_| astronomers_pi);
139139
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `res2.unwrap_or(astronomers_pi)`
140140

141141
error: unnecessary closure used to substitute value for `Result::Err`
142-
--> $DIR/unnecessary_lazy_eval.rs:80:13
142+
--> $DIR/unnecessary_lazy_eval.rs:86:13
143143
|
144144
LL | let _ = res2.unwrap_or_else(|_| ext_str.some_field);
145145
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Use `unwrap_or` instead: `res2.unwrap_or(ext_str.some_field)`

0 commit comments

Comments
 (0)