Auto merge of rust-lang#10356 - JirkaVebr:let_underscore_untyped, r=llogiq

Add `let_underscore_untyped`

Fixes rust-lang#6842

This adds a new pedantic `let_underscore_untyped` lint which checks for `let _ = <expr>`, and suggests to either provide a type annotation, or to remove the `let` keyword. That way the author is forced to specify the type they intended to ignore, and thus get forced to re-visit the decision should the type of `<expr>` change. Alternatively, they can drop the `let` keyword to truly just ignore the value no matter what.

r? `@llogiq`

changelog: New lint: [let_underscore_untyped]

Author: bors

CHANGELOG.md

@@ -4494,6 +4494,7 @@ Released 2018-09-13
 [`let_underscore_future`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_future
 [`let_underscore_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_lock
 [`let_underscore_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_must_use
+[`let_underscore_untyped`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_untyped
 [`let_unit_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
 [`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist
 [`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug

clippy_dev/src/new_lint.rs

@@ -1,5 +1,6 @@
 use crate::clippy_project_root;
 use indoc::{formatdoc, writedoc};
+use std::fmt;
 use std::fmt::Write as _;
 use std::fs::{self, OpenOptions};
 use std::io::prelude::*;
@@ -256,7 +257,7 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
         )
     });
 
-    let _ = write!(result, "{}", get_lint_declaration(&name_upper, category));
+    let _: fmt::Result = write!(result, "{}", get_lint_declaration(&name_upper, category));
 
     result.push_str(&if enable_msrv {
         formatdoc!(
@@ -353,7 +354,7 @@ fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::R
     let mut lint_file_contents = String::new();
 
     if enable_msrv {
-        let _ = writedoc!(
+        let _: fmt::Result = writedoc!(
             lint_file_contents,
             r#"
                 use clippy_utils::msrvs::{{self, Msrv}};
@@ -373,7 +374,7 @@ fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::R
             name_upper = name_upper,
         );
     } else {
-        let _ = writedoc!(
+        let _: fmt::Result = writedoc!(
             lint_file_contents,
             r#"
                 use rustc_lint::{{{context_import}, LintContext}};
@@ -521,7 +522,7 @@ fn setup_mod_file(path: &Path, lint: &LintData<'_>) -> io::Result<&'static str>
         .chain(std::iter::once(&*lint_name_upper))
         .filter(|s| !s.is_empty())
     {
-        let _ = write!(new_arr_content, "\n    {ident},");
+        let _: fmt::Result = write!(new_arr_content, "\n    {ident},");
     }
     new_arr_content.push('\n');
 

clippy_dev/src/update_lints.rs

@@ -5,7 +5,7 @@ use itertools::Itertools;
 use rustc_lexer::{tokenize, unescape, LiteralKind, TokenKind};
 use std::collections::{HashMap, HashSet};
 use std::ffi::OsStr;
-use std::fmt::Write;
+use std::fmt::{self, Write};
 use std::fs::{self, OpenOptions};
 use std::io::{self, Read, Seek, SeekFrom, Write as _};
 use std::ops::Range;
@@ -691,7 +691,7 @@ fn gen_deprecated(lints: &[DeprecatedLint]) -> String {
     let mut output = GENERATED_FILE_COMMENT.to_string();
     output.push_str("{\n");
     for lint in lints {
-        let _ = write!(
+        let _: fmt::Result = write!(
             output,
             concat!(
                 "    store.register_removed(\n",
@@ -726,7 +726,7 @@ fn gen_declared_lints<'a>(
         if !is_public {
             output.push_str("    #[cfg(feature = \"internal\")]\n");
         }
-        let _ = writeln!(output, "    crate::{module_name}::{lint_name}_INFO,");
+        let _: fmt::Result = writeln!(output, "    crate::{module_name}::{lint_name}_INFO,");
     }
     output.push_str("];\n");
 

clippy_lints/src/declared_lints.rs

@@ -224,6 +224,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::let_underscore::LET_UNDERSCORE_FUTURE_INFO,
     crate::let_underscore::LET_UNDERSCORE_LOCK_INFO,
     crate::let_underscore::LET_UNDERSCORE_MUST_USE_INFO,
+    crate::let_underscore::LET_UNDERSCORE_UNTYPED_INFO,
     crate::lifetimes::EXTRA_UNUSED_LIFETIMES_INFO,
     crate::lifetimes::NEEDLESS_LIFETIMES_INFO,
     crate::literal_representation::DECIMAL_LITERAL_REPRESENTATION_INFO,

clippy_lints/src/entry.rs

@@ -6,7 +6,7 @@ use clippy_utils::{
     source::{reindent_multiline, snippet_indent, snippet_with_applicability, snippet_with_context},
     SpanlessEq,
 };
-use core::fmt::Write;
+use core::fmt::{self, Write};
 use rustc_errors::Applicability;
 use rustc_hir::{
     hir_id::HirIdSet,
@@ -536,7 +536,7 @@ impl<'tcx> InsertSearchResults<'tcx> {
             if is_expr_used_or_unified(cx.tcx, insertion.call) {
                 write_wrapped(&mut res, insertion, ctxt, app);
             } else {
-                let _ = write!(
+                let _: fmt::Result = write!(
                     res,
                     "e.insert({})",
                     snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0
@@ -552,7 +552,7 @@ impl<'tcx> InsertSearchResults<'tcx> {
         (
             self.snippet(cx, span, app, |res, insertion, ctxt, app| {
                 // Insertion into a map would return `Some(&mut value)`, but the entry returns `&mut value`
-                let _ = write!(
+                let _: fmt::Result = write!(
                     res,
                     "Some(e.insert({}))",
                     snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0
@@ -566,7 +566,7 @@ impl<'tcx> InsertSearchResults<'tcx> {
         (
             self.snippet(cx, span, app, |res, insertion, ctxt, app| {
                 // Insertion into a map would return `None`, but the entry returns a mutable reference.
-                let _ = if is_expr_final_block_expr(cx.tcx, insertion.call) {
+                let _: fmt::Result = if is_expr_final_block_expr(cx.tcx, insertion.call) {
                     write!(
                         res,
                         "e.insert({});\n{}None",

clippy_lints/src/inconsistent_struct_constructor.rs

@@ -7,7 +7,7 @@ use rustc_hir::{self as hir, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::symbol::Symbol;
-use std::fmt::Write as _;
+use std::fmt::{self, Write as _};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -90,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor {
                 let mut fields_snippet = String::new();
                 let (last_ident, idents) = ordered_fields.split_last().unwrap();
                 for ident in idents {
-                    let _ = write!(fields_snippet, "{ident}, ");
+                    let _: fmt::Result = write!(fields_snippet, "{ident}, ");
                 }
                 fields_snippet.push_str(&last_ident.to_string());
 

clippy_lints/src/let_underscore.rs

@@ -90,7 +90,45 @@ declare_clippy_lint! {
     "non-binding `let` on a future"
 }
 
-declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE, LET_UNDERSCORE_LOCK, LET_UNDERSCORE_FUTURE]);
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `let _ = <expr>` without a type annotation, and suggests to either provide one,
+    /// or remove the `let` keyword altogether.
+    ///
+    /// ### Why is this bad?
+    /// The `let _ = <expr>` expression ignores the value of `<expr>` but will remain doing so even
+    /// if the type were to change, thus potentially introducing subtle bugs. By supplying a type
+    /// annotation, one will be forced to re-visit the decision to ignore the value in such cases.
+    ///
+    /// ### Known problems
+    /// The `_ = <expr>` is not properly supported by some tools (e.g. IntelliJ) and may seem odd
+    /// to many developers. This lint also partially overlaps with the other `let_underscore_*`
+    /// lints.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn foo() -> Result<u32, ()> {
+    ///     Ok(123)
+    /// }
+    /// let _ = foo();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn foo() -> Result<u32, ()> {
+    ///     Ok(123)
+    /// }
+    /// // Either provide a type annotation:
+    /// let _: Result<u32, ()> = foo();
+    /// // …or drop the let keyword:
+    /// _ = foo();
+    /// ```
+    #[clippy::version = "1.69.0"]
+    pub LET_UNDERSCORE_UNTYPED,
+    pedantic,
+    "non-binding `let` without a type annotation"
+}
+
+declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE, LET_UNDERSCORE_LOCK, LET_UNDERSCORE_FUTURE, LET_UNDERSCORE_UNTYPED]);
 
 const SYNC_GUARD_PATHS: [&[&str]; 3] = [
     &paths::PARKING_LOT_MUTEX_GUARD,
@@ -148,6 +186,18 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
                     "consider explicitly using function result",
                 );
             }
+
+            if local.pat.default_binding_modes && local.ty.is_none() {
+                // When `default_binding_modes` is true, the `let` keyword is present.
+                span_lint_and_help(
+                    cx,
+                    LET_UNDERSCORE_UNTYPED,
+                    local.span,
+                    "non-binding `let` without a type annotation",
+                    None,
+                    "consider adding a type annotation or removing the `let` keyword",
+                );
+            }
         }
     }
 }

clippy_lints/src/literal_representation.rs

@@ -488,7 +488,7 @@ impl DecimalLiteralRepresentation {
             then {
                 let hex = format!("{val:#X}");
                 let num_lit = NumericLiteral::new(&hex, num_lit.suffix, false);
-                let _ = Self::do_lint(num_lit.integer).map_err(|warning_type| {
+                let _: Result<(), ()> = Self::do_lint(num_lit.integer).map_err(|warning_type| {
                     warning_type.display(num_lit.format(), cx, span);
                 });
             }

clippy_lints/src/module_style.rs

@@ -134,7 +134,7 @@ fn process_paths_for_mod_files<'a>(
     mod_folders: &mut FxHashSet<&'a OsStr>,
 ) {
     let mut comp = path.components().rev().peekable();
-    let _ = comp.next();
+    let _: Option<_> = comp.next();
     if path.ends_with("mod.rs") {
         mod_folders.insert(comp.peek().map(|c| c.as_os_str()).unwrap_or_default());
     }

clippy_utils/src/numeric_literal.rs

@@ -186,7 +186,7 @@ impl<'a> NumericLiteral<'a> {
         // The exponent may have a sign, output it early, otherwise it will be
         // treated as a digit
         if digits.clone().next() == Some('-') {
-            let _ = digits.next();
+            let _: Option<char> = digits.next();
             output.push('-');
         }
 

clippy_utils/src/sugg.rs

@@ -20,7 +20,7 @@ use rustc_middle::mir::{FakeReadCause, Mutability};
 use rustc_middle::ty;
 use rustc_span::source_map::{BytePos, CharPos, Pos, Span, SyntaxContext};
 use std::borrow::Cow;
-use std::fmt::{Display, Write as _};
+use std::fmt::{self, Display, Write as _};
 use std::ops::{Add, Neg, Not, Sub};
 
 /// A helper type to build suggestion correctly handling parentheses.
@@ -932,7 +932,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
             if cmt.place.projections.is_empty() {
                 // handle item without any projection, that needs an explicit borrowing
                 // i.e.: suggest `&x` instead of `x`
-                let _ = write!(self.suggestion_start, "{start_snip}&{ident_str}");
+                let _: fmt::Result = write!(self.suggestion_start, "{start_snip}&{ident_str}");
             } else {
                 // cases where a parent `Call` or `MethodCall` is using the item
                 // i.e.: suggest `.contains(&x)` for `.find(|x| [1, 2, 3].contains(x)).is_none()`
@@ -947,7 +947,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
                         // given expression is the self argument and will be handled completely by the compiler
                         // i.e.: `|x| x.is_something()`
                         ExprKind::MethodCall(_, self_expr, ..) if self_expr.hir_id == cmt.hir_id => {
-                            let _ = write!(self.suggestion_start, "{start_snip}{ident_str_with_proj}");
+                            let _: fmt::Result = write!(self.suggestion_start, "{start_snip}{ident_str_with_proj}");
                             self.next_pos = span.hi();
                             return;
                         },
@@ -1055,7 +1055,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
                     }
                 }
 
-                let _ = write!(self.suggestion_start, "{start_snip}{replacement_str}");
+                let _: fmt::Result = write!(self.suggestion_start, "{start_snip}{replacement_str}");
             }
             self.next_pos = span.hi();
         }

lintcheck/src/main.rs

@@ -17,9 +17,9 @@ use crate::recursive::LintcheckServer;
 use std::collections::{HashMap, HashSet};
 use std::env;
 use std::env::consts::EXE_SUFFIX;
-use std::fmt::Write as _;
+use std::fmt::{self, Write as _};
 use std::fs;
-use std::io::ErrorKind;
+use std::io::{self, ErrorKind};
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -145,8 +145,8 @@ impl ClippyWarning {
             }
 
             let mut output = String::from("| ");
-            let _ = write!(output, "[`{file_with_pos}`]({file}#L{})", self.line);
-            let _ = write!(output, r#" | `{:<50}` | "{}" |"#, self.lint_type, self.message);
+            let _: fmt::Result = write!(output, "[`{file_with_pos}`]({file}#L{})", self.line);
+            let _: fmt::Result = write!(output, r#" | `{:<50}` | "{}" |"#, self.lint_type, self.message);
             output.push('\n');
             output
         } else {
@@ -632,7 +632,7 @@ fn main() {
         .unwrap();
 
     let server = config.recursive.then(|| {
-        let _ = fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive");
+        let _: io::Result<()> = fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive");
 
         LintcheckServer::spawn(recursive_options)
     });
@@ -689,7 +689,7 @@ fn main() {
     write!(text, "{}", all_msgs.join("")).unwrap();
     text.push_str("\n\n### ICEs:\n");
     for (cratename, msg) in &ices {
-        let _ = write!(text, "{cratename}: '{msg}'");
+        let _: fmt::Result = write!(text, "{cratename}: '{msg}'");
     }
 
     println!("Writing logs to {}", config.lintcheck_results_path.display());

tests/ui/let_underscore_untyped.rs

@@ -0,0 +1,54 @@
+#![allow(unused)]
+#![warn(clippy::let_underscore_untyped)]
+
+use std::future::Future;
+use std::{boxed::Box, fmt::Display};
+
+fn a() -> u32 {
+    1
+}
+
+fn b<T>(x: T) -> T {
+    x
+}
+
+fn c() -> impl Display {
+    1
+}
+
+fn d(x: &u32) -> &u32 {
+    x
+}
+
+fn e() -> Result<u32, ()> {
+    Ok(1)
+}
+
+fn f() -> Box<dyn Display> {
+    Box::new(1)
+}
+
+fn main() {
+    let _ = a();
+    let _ = b(1);
+    let _ = c();
+    let _ = d(&1);
+    let _ = e();
+    let _ = f();
+
+    _ = a();
+    _ = b(1);
+    _ = c();
+    _ = d(&1);
+    _ = e();
+    _ = f();
+
+    let _: u32 = a();
+    let _: u32 = b(1);
+    let _: &u32 = d(&1);
+    let _: Result<_, _> = e();
+    let _: Box<_> = f();
+
+    #[allow(clippy::let_underscore_untyped)]
+    let _ = a();
+}