Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't lint let_unit_value when () is explicit #10844

Merged
merged 1 commit into from
Jan 5, 2024
Merged

Don't lint let_unit_value when () is explicit #10844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion clippy_lints/src/unit_types/let_unit_value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,31 @@ use rustc_middle::ty;
use super::LET_UNIT_VALUE;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {
// skip `let () = { ... }`
if let PatKind::Tuple(fields, ..) = local.pat.kind
&& fields.is_empty()
{
return;
}

if let Some(init) = local.init
&& !local.pat.span.from_expansion()
&& !in_external_macro(cx.sess(), local.span)
&& !is_from_async_await(local.span)
&& cx.typeck_results().pat_ty(local.pat).is_unit()
{
// skip `let awa = ()`
if let ExprKind::Tup([]) = init.kind {
return;
}

// skip `let _: () = { ... }`
if let Some(ty) = local.ty
&& let TyKind::Tup([]) = ty.kind
{
return;
}

if (local.ty.map_or(false, |ty| !matches!(ty.kind, TyKind::Infer))
|| matches!(local.pat.kind, PatKind::Tuple([], ddpos) if ddpos.as_opt_usize().is_none()))
&& expr_needs_inferred_result(cx, init)
Expand All @@ -34,7 +53,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {
|diag| {
diag.span_suggestion(
local.pat.span,
"use a wild (`_`) binding",
"use a wildcard binding",
"_",
Applicability::MaybeIncorrect, // snippet
);
Expand Down
10 changes: 0 additions & 10 deletions tests/ui/crashes/ice-8821.fixed
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions tests/ui/crashes/ice-8821.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,4 @@ static FN: fn() = f;

fn main() {
let _: () = FN();
//~^ ERROR: this let-binding has unit value
//~| NOTE: `-D clippy::let-unit-value` implied by `-D warnings`
}
11 changes: 0 additions & 11 deletions tests/ui/crashes/ice-8821.stderr
View file
Open in desktop

This file was deleted.

50 changes: 24 additions & 26 deletions tests/ui/let_unit.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@ fn main() {
let _y = 1; // this is fine
let _z = ((), 1); // this as well
if true {
();
// do not lint this, since () is explicit
let _a = ();
let () = dummy();
let () = ();
() = dummy();
() = ();
let _a: () = ();
let _a: () = dummy();
}

consume_units_with_for_loop(); // should be fine as well
Expand All @@ -23,6 +30,8 @@ fn main() {
let_and_return!(()) // should be fine
}

fn dummy() {}

// Related to issue #1964
fn consume_units_with_for_loop() {
// `for_let_unit` lint should not be triggered by consuming them using for loop.
Expand Down Expand Up @@ -74,40 +83,29 @@ fn _returns_generic() {
x.then(|| T::default())
}

let _: () = f(); // Ok
let _: () = f(); // Lint.
let _: () = f();
let x: () = f();

let _: () = f2(0i32); // Ok
let _: () = f2(0i32); // Lint.
let _: () = f2(0i32);
let x: () = f2(0i32);

f3(()); // Lint
f3(()); // Lint
let _: () = f3(());
let x: () = f3(());

// Should lint:
// fn f4<T>(mut x: Vec<T>) -> T {
// x.pop().unwrap()
// }
// let _: () = f4(vec![()]);
// let x: () = f4(vec![()]);
fn f4<T>(mut x: Vec<T>) -> T {
x.pop().unwrap()
}
let _: () = f4(vec![()]);
let x: () = f4(vec![()]);

// Ok
let _: () = {
let x = 5;
f2(x)
};

let _: () = if true { f() } else { f2(0) }; // Ok
let _: () = if true { f() } else { f2(0) }; // Lint

// Ok
let _: () = match Some(0) {
None => f2(1),
Some(0) => f(),
Some(1) => f2(3),
Some(_) => f2('x'),
};
let _: () = if true { f() } else { f2(0) };
let x: () = if true { f() } else { f2(0) };

// Lint
match Some(0) {
None => f2(1),
Some(0) => f(),
Expand Down Expand Up @@ -155,7 +153,7 @@ fn _returns_generic() {
{
let _: () = x;
let _: () = y;
z;
let _: () = z;
let _: () = x1;
let _: () = x2;
let _: () = opt;
Expand Down
48 changes: 23 additions & 25 deletions tests/ui/let_unit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@ fn main() {
let _y = 1; // this is fine
let _z = ((), 1); // this as well
if true {
// do not lint this, since () is explicit
let _a = ();
let () = dummy();
let () = ();
() = dummy();
() = ();
let _a: () = ();
let _a: () = dummy();
}

consume_units_with_for_loop(); // should be fine as well
Expand All @@ -23,6 +30,8 @@ fn main() {
let_and_return!(()) // should be fine
}

fn dummy() {}

// Related to issue #1964
fn consume_units_with_for_loop() {
// `for_let_unit` lint should not be triggered by consuming them using for loop.
Expand Down Expand Up @@ -74,41 +83,30 @@ fn _returns_generic() {
x.then(|| T::default())
}

let _: () = f(); // Ok
let x: () = f(); // Lint.
let _: () = f();
let x: () = f();

let _: () = f2(0i32); // Ok
let x: () = f2(0i32); // Lint.
let _: () = f2(0i32);
let x: () = f2(0i32);

let _: () = f3(()); // Lint
let x: () = f3(()); // Lint
let _: () = f3(());
let x: () = f3(());

// Should lint:
// fn f4<T>(mut x: Vec<T>) -> T {
// x.pop().unwrap()
// }
// let _: () = f4(vec![()]);
// let x: () = f4(vec![()]);
fn f4<T>(mut x: Vec<T>) -> T {
x.pop().unwrap()
}
let _: () = f4(vec![()]);
let x: () = f4(vec![()]);

// Ok
let _: () = {
let x = 5;
f2(x)
};

let _: () = if true { f() } else { f2(0) }; // Ok
let x: () = if true { f() } else { f2(0) }; // Lint

// Ok
let _: () = match Some(0) {
None => f2(1),
Some(0) => f(),
Some(1) => f2(3),
Some(_) => f2('x'),
};
let _: () = if true { f() } else { f2(0) };
let x: () = if true { f() } else { f2(0) };

// Lint
let _: () = match Some(0) {
let x = match Some(0) {
None => f2(1),
Some(0) => f(),
Some(1) => f2(3),
Expand Down
56 changes: 4 additions & 52 deletions tests/ui/let_unit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,7 @@ LL | let _x = println!("x");
= help: to override `-D warnings` add `#[allow(clippy::let_unit_value)]`

error: this let-binding has unit value
--> $DIR/let_unit.rs:16:9
|
LL | let _a = ();
| ^^^^^^^^^^^^ help: omit the `let` binding: `();`

error: this let-binding has unit value
--> $DIR/let_unit.rs:51:5
--> $DIR/let_unit.rs:60:5
|
LL | / let _ = v
LL | | .into_iter()
Expand All @@ -37,45 +31,9 @@ LL + .unwrap();
|

error: this let-binding has unit value
--> $DIR/let_unit.rs:78:5
|
LL | let x: () = f(); // Lint.
| ^^^^-^^^^^^^^^^^
| |
| help: use a wild (`_`) binding: `_`

error: this let-binding has unit value
--> $DIR/let_unit.rs:81:5
--> $DIR/let_unit.rs:109:5
|
LL | let x: () = f2(0i32); // Lint.
| ^^^^-^^^^^^^^^^^^^^^^
| |
| help: use a wild (`_`) binding: `_`

error: this let-binding has unit value
--> $DIR/let_unit.rs:83:5
|
LL | let _: () = f3(()); // Lint
| ^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f3(());`

error: this let-binding has unit value
--> $DIR/let_unit.rs:84:5
|
LL | let x: () = f3(()); // Lint
| ^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f3(());`

error: this let-binding has unit value
--> $DIR/let_unit.rs:100:5
|
LL | let x: () = if true { f() } else { f2(0) }; // Lint
| ^^^^-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: use a wild (`_`) binding: `_`
Centri3 marked this conversation as resolved.
Show resolved Hide resolved

error: this let-binding has unit value
--> $DIR/let_unit.rs:111:5
|
LL | / let _: () = match Some(0) {
LL | / let x = match Some(0) {
LL | | None => f2(1),
LL | | Some(0) => f(),
LL | | Some(1) => f2(3),
Expand All @@ -93,11 +51,5 @@ LL + Some(_) => (),
LL + };
|

error: this let-binding has unit value
--> $DIR/let_unit.rs:158:13
|
LL | let _: () = z;
| ^^^^^^^^^^^^^^ help: omit the `let` binding: `z;`

error: aborting due to 10 previous errors
error: aborting due to 3 previous errors