diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 654674de06a..7faf1911775 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -25,7 +25,7 @@ use crate::utils::paths; use crate::utils::{ clip, comparisons, differing_macro_contexts, higher, in_constant, in_macro_or_desugar, int_bits, last_path_segment, match_def_path, match_path, multispan_sugg, same_tys, sext, snippet, snippet_opt, snippet_with_applicability, - span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext, + snippet_with_macro_callsite, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext, }; declare_clippy_lint! { @@ -467,15 +467,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnitValue { if higher::is_from_for_desugar(local) { return; } - span_lint( - cx, - LET_UNIT_VALUE, - stmt.span, - &format!( - "this let-binding has unit value. Consider omitting `let {} =`", - snippet(cx, local.pat.span, "..") - ), - ); + span_lint_and_then(cx, LET_UNIT_VALUE, stmt.span, "this let-binding has unit value", |db| { + if let Some(expr) = &local.init { + let snip = snippet_with_macro_callsite(cx, expr.span, "()"); + db.span_suggestion( + stmt.span, + "omit the `let` binding", + format!("{};", snip), + Applicability::MachineApplicable, // snippet + ); + } + }); } } } diff --git a/tests/ui/let_unit.fixed b/tests/ui/let_unit.fixed new file mode 100644 index 00000000000..f398edc23cb --- /dev/null +++ b/tests/ui/let_unit.fixed @@ -0,0 +1,63 @@ +// run-rustfix + +#![warn(clippy::let_unit_value)] +#![allow(clippy::no_effect)] +#![allow(unused_variables)] + +macro_rules! let_and_return { + ($n:expr) => {{ + let ret = $n; + }}; +} + +fn main() { + println!("x"); + let _y = 1; // this is fine + let _z = ((), 1); // this as well + if true { + (); + } + + consume_units_with_for_loop(); // should be fine as well + + multiline_sugg(); + + let_and_return!(()) // should be fine +} + +// Related to issue #1964 +fn consume_units_with_for_loop() { + // `for_let_unit` lint should not be triggered by consuming them using for loop. + let v = vec![(), (), ()]; + let mut count = 0; + for _ in v { + count += 1; + } + assert_eq!(count, 3); + + // Same for consuming from some other Iterator. + let (tx, rx) = ::std::sync::mpsc::channel(); + tx.send(()).unwrap(); + drop(tx); + + count = 0; + for _ in rx.iter() { + count += 1; + } + assert_eq!(count, 1); +} + +fn multiline_sugg() { + let v: Vec = vec![2]; + + v + .into_iter() + .map(|i| i * 2) + .filter(|i| i % 2 == 0) + .map(|_| ()) + .next() + .unwrap(); +} + +#[derive(Copy, Clone)] +pub struct ContainsUnit(()); // should be fine diff --git a/tests/ui/let_unit.rs b/tests/ui/let_unit.rs index dc17c98f6a8..af5b1fb2ac7 100644 --- a/tests/ui/let_unit.rs +++ b/tests/ui/let_unit.rs @@ -1,4 +1,7 @@ +// run-rustfix + #![warn(clippy::let_unit_value)] +#![allow(clippy::no_effect)] #![allow(unused_variables)] macro_rules! let_and_return { @@ -17,6 +20,8 @@ fn main() { consume_units_with_for_loop(); // should be fine as well + multiline_sugg(); + let_and_return!(()) // should be fine } @@ -42,5 +47,17 @@ fn consume_units_with_for_loop() { assert_eq!(count, 1); } +fn multiline_sugg() { + let v: Vec = vec![2]; + + let _ = v + .into_iter() + .map(|i| i * 2) + .filter(|i| i % 2 == 0) + .map(|_| ()) + .next() + .unwrap(); +} + #[derive(Copy, Clone)] pub struct ContainsUnit(()); // should be fine diff --git a/tests/ui/let_unit.stderr b/tests/ui/let_unit.stderr index e1773a40225..7130fcd870e 100644 --- a/tests/ui/let_unit.stderr +++ b/tests/ui/let_unit.stderr @@ -1,16 +1,37 @@ -error: this let-binding has unit value. Consider omitting `let _x =` - --> $DIR/let_unit.rs:11:5 +error: this let-binding has unit value + --> $DIR/let_unit.rs:14:5 | LL | let _x = println!("x"); - | ^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `println!("x");` | = note: `-D clippy::let-unit-value` implied by `-D warnings` -error: this let-binding has unit value. Consider omitting `let _a =` - --> $DIR/let_unit.rs:15:9 +error: this let-binding has unit value + --> $DIR/let_unit.rs:18:9 | LL | let _a = (); - | ^^^^^^^^^^^^ + | ^^^^^^^^^^^^ help: omit the `let` binding: `();` -error: aborting due to 2 previous errors +error: this let-binding has unit value + --> $DIR/let_unit.rs:53:5 + | +LL | / let _ = v +LL | | .into_iter() +LL | | .map(|i| i * 2) +LL | | .filter(|i| i % 2 == 0) +LL | | .map(|_| ()) +LL | | .next() +LL | | .unwrap(); + | |__________________^ +help: omit the `let` binding + | +LL | v +LL | .into_iter() +LL | .map(|i| i * 2) +LL | .filter(|i| i % 2 == 0) +LL | .map(|_| ()) +LL | .next() + ... + +error: aborting due to 3 previous errors