Auto merge of rust-lang#112380 - jieyouxu:useless-bindings-lint, r=Wa…

…ffleLapkin

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 rust-lang#71432.

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

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,

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,
+}

compiler/rustc_lint/src/unit_bindings.rs

@@ -0,0 +1,66 @@
+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>) {
+        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() // suppress if user explicitly ascribes a type to the pattern
+            && !matches!(init.kind, hir::ExprKind::Tup([])) // suppress if user wrote `let x = ();`
+            && !matches!(local.pat.kind, hir::PatKind::Tuple([], ..)) // suppress if user wtote `let () = init;`
+        {
+            cx.emit_spanned_lint(UNIT_BINDINGS, local.span, UnitBindingsDiag {
+                label: local.pat.span,
+            });
+        }
+    }
+}

tests/ui/closures/issue-868.rs

@@ -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() }

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() {}