Skip to content

Commit

Permalink
Add allow-by-default lint for unit bindings
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jieyouxu committed Nov 20, 2023
1 parent 4f3da90 commit 8da09ae
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 2 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,9 @@ lint_undropped_manually_drops = calls to `std::mem::drop` with `std::mem::Manual
lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op
.label = this function will not propagate the caller location
lint_unit_bindings = binding has unit type `()`
.label = this pattern is inferred to be the unit type `()`
lint_unknown_gated_lint =
unknown lint: `{$name}`
.note = the `{$name}` lint is unstable
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ mod redundant_semicolon;
mod reference_casting;
mod traits;
mod types;
mod unit_bindings;
mod unused;

pub use array_into_iter::ARRAY_INTO_ITER;
Expand Down Expand Up @@ -123,6 +124,7 @@ use redundant_semicolon::*;
use reference_casting::*;
use traits::*;
use types::*;
use unit_bindings::*;
use unused::*;

/// Useful for other parts of the compiler / Clippy.
Expand Down Expand Up @@ -203,6 +205,7 @@ late_lint_methods!(
InvalidReferenceCasting: InvalidReferenceCasting,
// Depends on referenced function signatures in expressions
UnusedResults: UnusedResults,
UnitBindings: UnitBindings,
NonUpperCaseGlobals: NonUpperCaseGlobals,
NonShorthandFieldPatterns: NonShorthandFieldPatterns,
UnusedAllocation: UnusedAllocation,
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1845,3 +1845,10 @@ impl<'a> DecorateLint<'a, ()> for AsyncFnInTraitDiag {
fluent::lint_async_fn_in_trait
}
}

#[derive(LintDiagnostic)]
#[diag(lint_unit_bindings)]
pub struct UnitBindingsDiag {
#[label]
pub label: Span,
}
72 changes: 72 additions & 0 deletions compiler/rustc_lint/src/unit_bindings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
use crate::lints::UnitBindingsDiag;
use crate::{LateLintPass, LintContext};
use rustc_hir as hir;
use rustc_middle::ty::Ty;

declare_lint! {
/// The `unit_bindings` lint detects cases where bindings are useless because they have
/// the unit type `()` as their inferred type. The lint is suppressed if the user explicitly
/// annotates the let binding with the unit type `()`, or if the let binding uses an underscore
/// wildcard pattern, i.e. `let _ = expr`, or if the binding is produced from macro expansions.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(unit_bindings)]
///
/// fn foo() {
/// println!("do work");
/// }
///
/// pub fn main() {
/// let x = foo(); // useless binding
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Creating a local binding with the unit type `()` does not do much and can be a sign of a
/// user error, such as in this example:
///
/// ```rust,no_run
/// fn main() {
/// let mut x = [1, 2, 3];
/// x[0] = 5;
/// let y = x.sort(); // useless binding as `sort` returns `()` and not the sorted array.
/// println!("{:?}", y); // prints "()"
/// }
/// ```
pub UNIT_BINDINGS,
Allow,
"binding is useless because it has the unit `()` type"
}

declare_lint_pass!(UnitBindings => [UNIT_BINDINGS]);

impl<'tcx> LateLintPass<'tcx> for UnitBindings {
fn check_local(&mut self, cx: &crate::LateContext<'tcx>, local: &'tcx hir::Local<'tcx>) {
// Suppress warning if user:
// - explicitly ascribes a type to the pattern
// - explicitly wrote `let pat = ();`
// - explicitly wrote `let () = init;`.
if !local.span.from_expansion()
&& let Some(tyck_results) = cx.maybe_typeck_results()
&& let Some(init) = local.init
&& let init_ty = tyck_results.expr_ty(init)
&& let local_ty = tyck_results.node_type(local.hir_id)
&& init_ty == Ty::new_unit(cx.tcx)
&& local_ty == Ty::new_unit(cx.tcx)
&& local.ty.is_none()
&& !matches!(init.kind, hir::ExprKind::Tup([]))
&& !matches!(local.pat.kind, hir::PatKind::Tuple([], ..))
{
cx.emit_spanned_lint(
UNIT_BINDINGS,
local.span,
UnitBindingsDiag { label: local.pat.span },
);
}
}
}
1 change: 1 addition & 0 deletions tests/ui/closures/issue-868.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// run-pass
#![allow(unused_parens)]
#![allow(unit_bindings)]
// pretty-expanded FIXME #23616

fn f<T, F>(g: F) -> T where F: FnOnce() -> T { g() }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Variant of diverging-falllback-control-flow that tests
// Variant of diverging-fallback-control-flow that tests
// the specific case of a free function with an unconstrained
// return type. This captures the pattern we saw in the wild
// in the objc crate, where changing the fallback from `!` to `()`
Expand All @@ -9,7 +9,7 @@
// revisions: nofallback fallback

#![cfg_attr(fallback, feature(never_type, never_type_fallback))]

#![allow(unit_bindings)]

fn make_unit() {}

Expand Down

0 comments on commit 8da09ae

Please sign in to comment.