Skip to content

Commit f6dd112

Browse files
committed
fix: create the suggestion "differentially"
1 parent 48ee470 commit f6dd112

File tree

4 files changed

+118
-39
lines changed

4 files changed

+118
-39
lines changed

clippy_lints/src/unit_types/let_unit_value.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::macros::{FormatArgsStorage, find_format_arg_expr, is_format_macro, root_macro_call_first_node};
3-
use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_context};
3+
use clippy_utils::source::{snippet_indent, walk_span_to_context};
44
use clippy_utils::visitors::{for_each_local_assignment, for_each_value_source};
55
use core::ops::ControlFlow;
66
use rustc_ast::{FormatArgs, FormatArgumentKind};
@@ -74,10 +74,10 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, format_args: &FormatArgsStorag
7474
"this let-binding has unit value",
7575
|diag| {
7676
let mut suggestions = Vec::new();
77+
let init_new_span = walk_span_to_context(init.span, local.span.ctxt()).unwrap();
7778

7879
// Suggest omitting the `let` binding
79-
let mut app = Applicability::MachineApplicable;
80-
let snip = snippet_with_context(cx, init.span, local.span.ctxt(), "()", &mut app).0;
80+
let app = Applicability::MachineApplicable;
8181

8282
// If this is a binding pattern, we need to add suggestions to remove any usages
8383
// of the variable
@@ -114,21 +114,23 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, format_args: &FormatArgsStorag
114114
// ```
115115
// TODO: find a less awkward way to do this
116116
suggestions.push((
117-
init.span,
118-
format!("();\n{}", reindent_multiline(&snip, false, indent_of(cx, local.span))),
117+
init_new_span.shrink_to_lo(),
118+
format!("();\n{}", snippet_indent(cx, local.span).as_deref().unwrap_or("")),
119119
));
120-
diag.multipart_suggestion("replace variable usages with `()`", suggestions, app);
120+
diag.multipart_suggestion_verbose("replace variable usages with `()`", suggestions, app);
121121
return;
122122
}
123123
}
124124

125-
suggestions.push((local.span, format!("{snip};")));
125+
// let local = returns_unit();
126+
// ^^^^^^^^^^^^ remove this
127+
suggestions.push((local.span.until(init_new_span), String::new()));
126128
let message = if suggestions.len() == 1 {
127129
"omit the `let` binding"
128130
} else {
129131
"omit the `let` binding and replace variable usages with `()`"
130132
};
131-
diag.multipart_suggestion(message, suggestions, app);
133+
diag.multipart_suggestion_verbose(message, suggestions, app);
132134
},
133135
);
134136
}

tests/ui/let_unit.fixed

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
#![warn(clippy::let_unit_value)]
2-
#![allow(clippy::no_effect, clippy::needless_late_init, path_statements)]
2+
#![allow(
3+
clippy::no_effect,
4+
clippy::needless_late_init,
5+
path_statements,
6+
clippy::match_single_binding
7+
)]
38

49
macro_rules! let_and_return {
510
($n:expr) => {{
@@ -197,12 +202,30 @@ pub fn issue12594() {
197202
}
198203
}
199204

200-
fn issue15061() {
201-
fn do_something(x: ()) {}
205+
fn takes_unit(x: ()) {}
202206

207+
fn issue15061() {
203208
let res = ();
204209
returns_unit();
205210
//~^ let_unit_value
206-
do_something(());
211+
takes_unit(());
212+
println!("{res:?}");
213+
}
214+
215+
fn issue15771() {
216+
match "Example String" {
217+
_ => returns_unit(),
218+
//~^ let_unit_value
219+
}
220+
221+
if true {}
222+
//~^ let_unit_value
223+
}
224+
225+
fn issue_15784() {
226+
let res = ();
227+
eprintln!("I return unit");
228+
//~^ let_unit_value
229+
takes_unit(());
207230
println!("{res:?}");
208231
}

tests/ui/let_unit.rs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
#![warn(clippy::let_unit_value)]
2-
#![allow(clippy::no_effect, clippy::needless_late_init, path_statements)]
2+
#![allow(
3+
clippy::no_effect,
4+
clippy::needless_late_init,
5+
path_statements,
6+
clippy::match_single_binding
7+
)]
38

49
macro_rules! let_and_return {
510
($n:expr) => {{
@@ -197,11 +202,28 @@ pub fn issue12594() {
197202
}
198203
}
199204

200-
fn issue15061() {
201-
fn do_something(x: ()) {}
205+
fn takes_unit(x: ()) {}
202206

207+
fn issue15061() {
203208
let res = returns_unit();
204209
//~^ let_unit_value
205-
do_something(res);
210+
takes_unit(res);
211+
println!("{res:?}");
212+
}
213+
214+
fn issue15771() {
215+
match "Example String" {
216+
_ => _ = returns_unit(),
217+
//~^ let_unit_value
218+
}
219+
220+
_ = if true {}
221+
//~^ let_unit_value
222+
}
223+
224+
fn issue_15784() {
225+
let res = eprintln!("I return unit");
226+
//~^ let_unit_value
227+
takes_unit(res);
206228
println!("{res:?}");
207229
}

tests/ui/let_unit.stderr

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
error: this let-binding has unit value
2-
--> tests/ui/let_unit.rs:11:5
2+
--> tests/ui/let_unit.rs:16:5
33
|
44
LL | let _x = println!("x");
5-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `println!("x");`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::let-unit-value` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::let_unit_value)]`
9+
help: omit the `let` binding
10+
|
11+
LL - let _x = println!("x");
12+
LL + println!("x");
13+
|
914

1015
error: this let-binding has unit value
11-
--> tests/ui/let_unit.rs:60:5
16+
--> tests/ui/let_unit.rs:65:5
1217
|
1318
LL | / let _ = v
1419
LL | |
@@ -21,18 +26,12 @@ LL | | .unwrap();
2126
|
2227
help: omit the `let` binding
2328
|
24-
LL ~ v
25-
LL +
26-
LL + .into_iter()
27-
LL + .map(|i| i * 2)
28-
LL + .filter(|i| i.is_multiple_of(2))
29-
LL + .map(|_| ())
30-
LL + .next()
31-
LL + .unwrap();
29+
LL - let _ = v
30+
LL + v
3231
|
3332

3433
error: this let-binding has unit value
35-
--> tests/ui/let_unit.rs:110:5
34+
--> tests/ui/let_unit.rs:115:5
3635
|
3736
LL | / let x = match Some(0) {
3837
LL | |
@@ -45,17 +44,12 @@ LL | | };
4544
|
4645
help: omit the `let` binding
4746
|
48-
LL ~ match Some(0) {
49-
LL +
50-
LL + None => f2(1),
51-
LL + Some(0) => f(),
52-
LL + Some(1) => f2(3),
53-
LL + Some(_) => (),
54-
LL + };
47+
LL - let x = match Some(0) {
48+
LL + match Some(0) {
5549
|
5650

5751
error: this let-binding has unit value
58-
--> tests/ui/let_unit.rs:190:9
52+
--> tests/ui/let_unit.rs:195:9
5953
|
6054
LL | let res = returns_unit();
6155
| ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -69,7 +63,7 @@ LL ~ returns_result(()).unwrap();
6963
|
7064

7165
error: this let-binding has unit value
72-
--> tests/ui/let_unit.rs:203:5
66+
--> tests/ui/let_unit.rs:208:5
7367
|
7468
LL | let res = returns_unit();
7569
| ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -79,8 +73,46 @@ help: replace variable usages with `()`
7973
LL ~ let res = ();
8074
LL ~ returns_unit();
8175
LL |
82-
LL ~ do_something(());
76+
LL ~ takes_unit(());
77+
|
78+
79+
error: this let-binding has unit value
80+
--> tests/ui/let_unit.rs:216:14
81+
|
82+
LL | _ => _ = returns_unit(),
83+
| ^^^^^^^^^^^^^^^^^^
84+
|
85+
help: omit the `let` binding
86+
|
87+
LL - _ => _ = returns_unit(),
88+
LL + _ => returns_unit(),
89+
|
90+
91+
error: this let-binding has unit value
92+
--> tests/ui/let_unit.rs:220:5
93+
|
94+
LL | _ = if true {}
95+
| ^^^^^^^^^^^^^^
96+
|
97+
help: omit the `let` binding
98+
|
99+
LL - _ = if true {}
100+
LL + if true {}
101+
|
102+
103+
error: this let-binding has unit value
104+
--> tests/ui/let_unit.rs:225:5
105+
|
106+
LL | let res = eprintln!("I return unit");
107+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
108+
|
109+
help: replace variable usages with `()`
110+
|
111+
LL ~ let res = ();
112+
LL ~ eprintln!("I return unit");
113+
LL |
114+
LL ~ takes_unit(());
83115
|
84116

85-
error: aborting due to 5 previous errors
117+
error: aborting due to 8 previous errors
86118

0 commit comments

Comments
 (0)