Skip to content

Commit 73bc121

Browse files
committed
Auto merge of #112380 - jieyouxu:useless-bindings-lint, r=WaffleLapkin
Add allow-by-default lint for unit bindings ### Example ```rust #![warn(unit_bindings)] macro_rules! owo { () => { let whats_this = (); } } fn main() { // No warning if user explicitly wrote `()` on either side. let expr = (); let () = expr; let _ = (); let _ = expr; //~ WARN binding has unit type let pat = expr; //~ WARN binding has unit type let _pat = expr; //~ WARN binding has unit type // No warning for let bindings with unit type in macro expansions. owo!(); // No warning if user explicitly annotates the unit type on the binding. let pat: () = expr; } ``` outputs ``` warning: binding has unit type `()` --> $DIR/unit-bindings.rs:17:5 | LL | let _ = expr; | ^^^^-^^^^^^^^ | | | this pattern is inferred to be the unit type `()` | note: the lint level is defined here --> $DIR/unit-bindings.rs:3:9 | LL | #![warn(unit_bindings)] | ^^^^^^^^^^^^^ warning: binding has unit type `()` --> $DIR/unit-bindings.rs:18:5 | LL | let pat = expr; | ^^^^---^^^^^^^^ | | | this pattern is inferred to be the unit type `()` warning: binding has unit type `()` --> $DIR/unit-bindings.rs:19:5 | LL | let _pat = expr; | ^^^^----^^^^^^^^ | | | this pattern is inferred to be the unit type `()` warning: 3 warnings emitted ``` This lint is not triggered if any of the following conditions are met: - The user explicitly annotates the binding with the `()` type. - The binding is from a macro expansion. - The user explicitly wrote `let () = init;` - The user explicitly wrote `let pat = ();`. This is allowed for local lifetimes. ### Known Issue It is known that this lint can trigger on some proc-macro generated code whose span returns false for `Span::from_expansion` because e.g. the proc-macro simply forwards user code spans, and otherwise don't have distinguishing syntax context compared to non-macro-generated code. For those kind of proc-macros, I believe the correct way to fix them is to instead emit identifers with span like `Span::mixed_site().located_at(user_span)`. Closes #71432.
2 parents a6b8ae5 + 8da09ae commit 73bc121

File tree

6 files changed

+88
-2
lines changed

6 files changed

+88
-2
lines changed

compiler/rustc_lint/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,9 @@ lint_undropped_manually_drops = calls to `std::mem::drop` with `std::mem::Manual
517517
lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op
518518
.label = this function will not propagate the caller location
519519
520+
lint_unit_bindings = binding has unit type `()`
521+
.label = this pattern is inferred to be the unit type `()`
522+
520523
lint_unknown_gated_lint =
521524
unknown lint: `{$name}`
522525
.note = the `{$name}` lint is unstable

compiler/rustc_lint/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ mod redundant_semicolon;
8585
mod reference_casting;
8686
mod traits;
8787
mod types;
88+
mod unit_bindings;
8889
mod unused;
8990

9091
pub use array_into_iter::ARRAY_INTO_ITER;
@@ -123,6 +124,7 @@ use redundant_semicolon::*;
123124
use reference_casting::*;
124125
use traits::*;
125126
use types::*;
127+
use unit_bindings::*;
126128
use unused::*;
127129

128130
/// Useful for other parts of the compiler / Clippy.
@@ -202,6 +204,7 @@ late_lint_methods!(
202204
InvalidReferenceCasting: InvalidReferenceCasting,
203205
// Depends on referenced function signatures in expressions
204206
UnusedResults: UnusedResults,
207+
UnitBindings: UnitBindings,
205208
NonUpperCaseGlobals: NonUpperCaseGlobals,
206209
NonShorthandFieldPatterns: NonShorthandFieldPatterns,
207210
UnusedAllocation: UnusedAllocation,

compiler/rustc_lint/src/lints.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1830,3 +1830,10 @@ impl<'a> DecorateLint<'a, ()> for AsyncFnInTraitDiag {
18301830
fluent::lint_async_fn_in_trait
18311831
}
18321832
}
1833+
1834+
#[derive(LintDiagnostic)]
1835+
#[diag(lint_unit_bindings)]
1836+
pub struct UnitBindingsDiag {
1837+
#[label]
1838+
pub label: Span,
1839+
}
+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
use crate::lints::UnitBindingsDiag;
2+
use crate::{LateLintPass, LintContext};
3+
use rustc_hir as hir;
4+
use rustc_middle::ty::Ty;
5+
6+
declare_lint! {
7+
/// The `unit_bindings` lint detects cases where bindings are useless because they have
8+
/// the unit type `()` as their inferred type. The lint is suppressed if the user explicitly
9+
/// annotates the let binding with the unit type `()`, or if the let binding uses an underscore
10+
/// wildcard pattern, i.e. `let _ = expr`, or if the binding is produced from macro expansions.
11+
///
12+
/// ### Example
13+
///
14+
/// ```rust,compile_fail
15+
/// #![deny(unit_bindings)]
16+
///
17+
/// fn foo() {
18+
/// println!("do work");
19+
/// }
20+
///
21+
/// pub fn main() {
22+
/// let x = foo(); // useless binding
23+
/// }
24+
/// ```
25+
///
26+
/// {{produces}}
27+
///
28+
/// ### Explanation
29+
///
30+
/// Creating a local binding with the unit type `()` does not do much and can be a sign of a
31+
/// user error, such as in this example:
32+
///
33+
/// ```rust,no_run
34+
/// fn main() {
35+
/// let mut x = [1, 2, 3];
36+
/// x[0] = 5;
37+
/// let y = x.sort(); // useless binding as `sort` returns `()` and not the sorted array.
38+
/// println!("{:?}", y); // prints "()"
39+
/// }
40+
/// ```
41+
pub UNIT_BINDINGS,
42+
Allow,
43+
"binding is useless because it has the unit `()` type"
44+
}
45+
46+
declare_lint_pass!(UnitBindings => [UNIT_BINDINGS]);
47+
48+
impl<'tcx> LateLintPass<'tcx> for UnitBindings {
49+
fn check_local(&mut self, cx: &crate::LateContext<'tcx>, local: &'tcx hir::Local<'tcx>) {
50+
// Suppress warning if user:
51+
// - explicitly ascribes a type to the pattern
52+
// - explicitly wrote `let pat = ();`
53+
// - explicitly wrote `let () = init;`.
54+
if !local.span.from_expansion()
55+
&& let Some(tyck_results) = cx.maybe_typeck_results()
56+
&& let Some(init) = local.init
57+
&& let init_ty = tyck_results.expr_ty(init)
58+
&& let local_ty = tyck_results.node_type(local.hir_id)
59+
&& init_ty == Ty::new_unit(cx.tcx)
60+
&& local_ty == Ty::new_unit(cx.tcx)
61+
&& local.ty.is_none()
62+
&& !matches!(init.kind, hir::ExprKind::Tup([]))
63+
&& !matches!(local.pat.kind, hir::PatKind::Tuple([], ..))
64+
{
65+
cx.emit_spanned_lint(
66+
UNIT_BINDINGS,
67+
local.span,
68+
UnitBindingsDiag { label: local.pat.span },
69+
);
70+
}
71+
}
72+
}

tests/ui/closures/issue-868.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// run-pass
22
#![allow(unused_parens)]
3+
#![allow(unit_bindings)]
34
// pretty-expanded FIXME #23616
45

56
fn f<T, F>(g: F) -> T where F: FnOnce() -> T { g() }

tests/ui/never_type/diverging-fallback-unconstrained-return.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Variant of diverging-falllback-control-flow that tests
1+
// Variant of diverging-fallback-control-flow that tests
22
// the specific case of a free function with an unconstrained
33
// return type. This captures the pattern we saw in the wild
44
// in the objc crate, where changing the fallback from `!` to `()`
@@ -9,7 +9,7 @@
99
// revisions: nofallback fallback
1010

1111
#![cfg_attr(fallback, feature(never_type, never_type_fallback))]
12-
12+
#![allow(unit_bindings)]
1313

1414
fn make_unit() {}
1515

0 commit comments

Comments
 (0)