From 8da09aed94e0c407f3c82450fa8268e6bff0f71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Wed, 14 Jun 2023 00:19:09 +0800 Subject: [PATCH] Add allow-by-default lint for unit bindings 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. --- compiler/rustc_lint/messages.ftl | 3 + compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 7 ++ compiler/rustc_lint/src/unit_bindings.rs | 72 +++++++++++++++++++ tests/ui/closures/issue-868.rs | 1 + ...diverging-fallback-unconstrained-return.rs | 4 +- 6 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 compiler/rustc_lint/src/unit_bindings.rs diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 068f1372c0ecd..b44aa77f2d4b8 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -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 diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 606e18866165c..8e5b3690f8ef2 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -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; @@ -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. @@ -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, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 756899e50a8cd..2b74123fed46b 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -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, +} diff --git a/compiler/rustc_lint/src/unit_bindings.rs b/compiler/rustc_lint/src/unit_bindings.rs new file mode 100644 index 0000000000000..c80889f3ae752 --- /dev/null +++ b/compiler/rustc_lint/src/unit_bindings.rs @@ -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 }, + ); + } + } +} diff --git a/tests/ui/closures/issue-868.rs b/tests/ui/closures/issue-868.rs index ce0a3c7ca526c..df03b191a99e9 100644 --- a/tests/ui/closures/issue-868.rs +++ b/tests/ui/closures/issue-868.rs @@ -1,5 +1,6 @@ // run-pass #![allow(unused_parens)] +#![allow(unit_bindings)] // pretty-expanded FIXME #23616 fn f(g: F) -> T where F: FnOnce() -> T { g() } diff --git a/tests/ui/never_type/diverging-fallback-unconstrained-return.rs b/tests/ui/never_type/diverging-fallback-unconstrained-return.rs index 7ea97126f89c9..26c8175be638f 100644 --- a/tests/ui/never_type/diverging-fallback-unconstrained-return.rs +++ b/tests/ui/never_type/diverging-fallback-unconstrained-return.rs @@ -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 `()` @@ -9,7 +9,7 @@ // revisions: nofallback fallback #![cfg_attr(fallback, feature(never_type, never_type_fallback))] - +#![allow(unit_bindings)] fn make_unit() {}