From 6a3dbe4798e3966b80b16373fda75f5e4dd91384 Mon Sep 17 00:00:00 2001 From: dswij Date: Tue, 15 Mar 2022 21:29:17 +0800 Subject: [PATCH] `unnecessary_lazy_eval` show suggestions on multiline lint --- .../src/methods/unnecessary_lazy_eval.rs | 26 ++-- tests/ui/unnecessary_lazy_eval.fixed | 8 + tests/ui/unnecessary_lazy_eval.rs | 8 + tests/ui/unnecessary_lazy_eval.stderr | 145 ++++++++++++++---- .../ui/unnecessary_lazy_eval_unfixable.stderr | 12 +- 5 files changed, 148 insertions(+), 51 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_lazy_eval.rs b/clippy_lints/src/methods/unnecessary_lazy_eval.rs index 1e2765263c8..0dd41a4dacf 100644 --- a/clippy_lints/src/methods/unnecessary_lazy_eval.rs +++ b/clippy_lints/src/methods/unnecessary_lazy_eval.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{eager_or_lazy, usage}; @@ -48,20 +48,16 @@ pub(super) fn check<'tcx>( Applicability::MaybeIncorrect }; - span_lint_and_sugg( - cx, - UNNECESSARY_LAZY_EVALUATIONS, - expr.span, - msg, - &format!("use `{}` instead", simplify_using), - format!( - "{0}.{1}({2})", - snippet(cx, recv.span, ".."), - simplify_using, - snippet(cx, body_expr.span, ".."), - ), - applicability, - ); + if let hir::ExprKind::MethodCall(_, _, span) = expr.kind { + span_lint_and_then(cx, UNNECESSARY_LAZY_EVALUATIONS, expr.span, msg, |diag| { + diag.span_suggestion( + span, + &format!("use `{}(..)` instead", simplify_using), + format!("{}({})", simplify_using, snippet(cx, body_expr.span, "..")), + applicability, + ); + }); + } } } } diff --git a/tests/ui/unnecessary_lazy_eval.fixed b/tests/ui/unnecessary_lazy_eval.fixed index 4ba2a0a5dbc..65fcdc43061 100644 --- a/tests/ui/unnecessary_lazy_eval.fixed +++ b/tests/ui/unnecessary_lazy_eval.fixed @@ -115,6 +115,14 @@ fn main() { let _: Result = res.or(Ok(2)); let _: Result = res.or(Ok(astronomers_pi)); let _: Result = res.or(Ok(ext_str.some_field)); + let _: Result = res. + // some lines + // some lines + // some lines + // some lines + // some lines + // some lines + or(Ok(ext_str.some_field)); // neither bind_instead_of_map nor unnecessary_lazy_eval applies here let _: Result = res.and_then(|x| Err(x)); diff --git a/tests/ui/unnecessary_lazy_eval.rs b/tests/ui/unnecessary_lazy_eval.rs index 466915217e4..206080ed69a 100644 --- a/tests/ui/unnecessary_lazy_eval.rs +++ b/tests/ui/unnecessary_lazy_eval.rs @@ -115,6 +115,14 @@ fn main() { let _: Result = res.or_else(|_| Ok(2)); let _: Result = res.or_else(|_| Ok(astronomers_pi)); let _: Result = res.or_else(|_| Ok(ext_str.some_field)); + let _: Result = res. + // some lines + // some lines + // some lines + // some lines + // some lines + // some lines + or_else(|_| Ok(ext_str.some_field)); // neither bind_instead_of_map nor unnecessary_lazy_eval applies here let _: Result = res.and_then(|x| Err(x)); diff --git a/tests/ui/unnecessary_lazy_eval.stderr b/tests/ui/unnecessary_lazy_eval.stderr index cc94bd5cd9e..7e4dd7730e7 100644 --- a/tests/ui/unnecessary_lazy_eval.stderr +++ b/tests/ui/unnecessary_lazy_eval.stderr @@ -2,7 +2,9 @@ error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:35:13 | LL | let _ = opt.unwrap_or_else(|| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `opt.unwrap_or(2)` + | ^^^^-------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(2)` | = note: `-D clippy::unnecessary-lazy-evaluations` implied by `-D warnings` @@ -10,187 +12,264 @@ error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:36:13 | LL | let _ = opt.unwrap_or_else(|| astronomers_pi); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `opt.unwrap_or(astronomers_pi)` + | ^^^^--------------------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(astronomers_pi)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:37:13 | LL | let _ = opt.unwrap_or_else(|| ext_str.some_field); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `opt.unwrap_or(ext_str.some_field)` + | ^^^^------------------------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(ext_str.some_field)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:39:13 | LL | let _ = opt.and_then(|_| ext_opt); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `and` instead: `opt.and(ext_opt)` + | ^^^^--------------------- + | | + | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:40:13 | LL | let _ = opt.or_else(|| ext_opt); - | ^^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `opt.or(ext_opt)` + | ^^^^------------------- + | | + | help: use `or(..)` instead: `or(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:41:13 | LL | let _ = opt.or_else(|| None); - | ^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `opt.or(None)` + | ^^^^---------------- + | | + | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:42:13 | LL | let _ = opt.get_or_insert_with(|| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `get_or_insert` instead: `opt.get_or_insert(2)` + | ^^^^------------------------ + | | + | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:43:13 | LL | let _ = opt.ok_or_else(|| 2); - | ^^^^^^^^^^^^^^^^^^^^ help: use `ok_or` instead: `opt.ok_or(2)` + | ^^^^---------------- + | | + | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:44:13 | LL | let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2))); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `nested_tuple_opt.unwrap_or(Some((1, 2)))` + | ^^^^^^^^^^^^^^^^^------------------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(Some((1, 2)))` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:47:13 | LL | let _ = Some(10).unwrap_or_else(|| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `Some(10).unwrap_or(2)` + | ^^^^^^^^^-------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:48:13 | LL | let _ = Some(10).and_then(|_| ext_opt); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `and` instead: `Some(10).and(ext_opt)` + | ^^^^^^^^^--------------------- + | | + | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:49:28 | LL | let _: Option = None.or_else(|| ext_opt); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `None.or(ext_opt)` + | ^^^^^------------------- + | | + | help: use `or(..)` instead: `or(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:50:13 | LL | let _ = None.get_or_insert_with(|| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `get_or_insert` instead: `None.get_or_insert(2)` + | ^^^^^------------------------ + | | + | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:51:35 | LL | let _: Result = None.ok_or_else(|| 2); - | ^^^^^^^^^^^^^^^^^^^^^ help: use `ok_or` instead: `None.ok_or(2)` + | ^^^^^---------------- + | | + | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:52:28 | LL | let _: Option = None.or_else(|| None); - | ^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `None.or(None)` + | ^^^^^---------------- + | | + | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:55:13 | LL | let _ = deep.0.unwrap_or_else(|| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `deep.0.unwrap_or(2)` + | ^^^^^^^-------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:56:13 | LL | let _ = deep.0.and_then(|_| ext_opt); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `and` instead: `deep.0.and(ext_opt)` + | ^^^^^^^--------------------- + | | + | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:57:13 | LL | let _ = deep.0.or_else(|| None); - | ^^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `deep.0.or(None)` + | ^^^^^^^---------------- + | | + | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:58:13 | LL | let _ = deep.0.get_or_insert_with(|| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `get_or_insert` instead: `deep.0.get_or_insert(2)` + | ^^^^^^^------------------------ + | | + | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:59:13 | LL | let _ = deep.0.ok_or_else(|| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^ help: use `ok_or` instead: `deep.0.ok_or(2)` + | ^^^^^^^---------------- + | | + | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:79:28 | LL | let _: Option = None.or_else(|| Some(3)); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `None.or(Some(3))` + | ^^^^^------------------- + | | + | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:80:13 | LL | let _ = deep.0.or_else(|| Some(3)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `deep.0.or(Some(3))` + | ^^^^^^^------------------- + | | + | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Option::None` --> $DIR/unnecessary_lazy_eval.rs:81:13 | LL | let _ = opt.or_else(|| Some(3)); - | ^^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `opt.or(Some(3))` + | ^^^^------------------- + | | + | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:87:13 | LL | let _ = res2.unwrap_or_else(|_| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `res2.unwrap_or(2)` + | ^^^^^--------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:88:13 | LL | let _ = res2.unwrap_or_else(|_| astronomers_pi); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `res2.unwrap_or(astronomers_pi)` + | ^^^^^---------------------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(astronomers_pi)` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:89:13 | LL | let _ = res2.unwrap_or_else(|_| ext_str.some_field); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `res2.unwrap_or(ext_str.some_field)` + | ^^^^^-------------------------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(ext_str.some_field)` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:111:35 | LL | let _: Result = res.and_then(|_| Err(2)); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use `and` instead: `res.and(Err(2))` + | ^^^^-------------------- + | | + | help: use `and(..)` instead: `and(Err(2))` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:112:35 | LL | let _: Result = res.and_then(|_| Err(astronomers_pi)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `and` instead: `res.and(Err(astronomers_pi))` + | ^^^^--------------------------------- + | | + | help: use `and(..)` instead: `and(Err(astronomers_pi))` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:113:35 | LL | let _: Result = res.and_then(|_| Err(ext_str.some_field)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `and` instead: `res.and(Err(ext_str.some_field))` + | ^^^^------------------------------------- + | | + | help: use `and(..)` instead: `and(Err(ext_str.some_field))` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:115:35 | LL | let _: Result = res.or_else(|_| Ok(2)); - | ^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `res.or(Ok(2))` + | ^^^^------------------ + | | + | help: use `or(..)` instead: `or(Ok(2))` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:116:35 | LL | let _: Result = res.or_else(|_| Ok(astronomers_pi)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `res.or(Ok(astronomers_pi))` + | ^^^^------------------------------- + | | + | help: use `or(..)` instead: `or(Ok(astronomers_pi))` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval.rs:117:35 | LL | let _: Result = res.or_else(|_| Ok(ext_str.some_field)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `or` instead: `res.or(Ok(ext_str.some_field))` + | ^^^^----------------------------------- + | | + | help: use `or(..)` instead: `or(Ok(ext_str.some_field))` -error: aborting due to 32 previous errors +error: unnecessary closure used to substitute value for `Result::Err` + --> $DIR/unnecessary_lazy_eval.rs:118:35 + | +LL | let _: Result = res. + | ___________________________________^ +LL | | // some lines +LL | | // some lines +LL | | // some lines +... | +LL | | // some lines +LL | | or_else(|_| Ok(ext_str.some_field)); + | |_________----------------------------------^ + | | + | help: use `or(..)` instead: `or(Ok(ext_str.some_field))` + +error: aborting due to 33 previous errors diff --git a/tests/ui/unnecessary_lazy_eval_unfixable.stderr b/tests/ui/unnecessary_lazy_eval_unfixable.stderr index 75674b0a9d2..20acab6e844 100644 --- a/tests/ui/unnecessary_lazy_eval_unfixable.stderr +++ b/tests/ui/unnecessary_lazy_eval_unfixable.stderr @@ -2,7 +2,9 @@ error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval_unfixable.rs:12:13 | LL | let _ = Ok(1).unwrap_or_else(|()| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `Ok(1).unwrap_or(2)` + | ^^^^^^---------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(2)` | = note: `-D clippy::unnecessary-lazy-evaluations` implied by `-D warnings` @@ -10,13 +12,17 @@ error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval_unfixable.rs:16:13 | LL | let _ = Ok(1).unwrap_or_else(|e::E| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `Ok(1).unwrap_or(2)` + | ^^^^^^------------------------ + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Result::Err` --> $DIR/unnecessary_lazy_eval_unfixable.rs:17:13 | LL | let _ = Ok(1).unwrap_or_else(|SomeStruct { .. }| 2); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `unwrap_or` instead: `Ok(1).unwrap_or(2)` + | ^^^^^^------------------------------------- + | | + | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: aborting due to 3 previous errors