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

Add `let_underscore_untyped`

Fixes #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]
This commit is contained in:
bors 2023-02-16 12:58:43 +00:00
commit 99d4ea4f39
19 changed files with 199 additions and 38 deletions

View file

@ -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

View file

@ -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');

View file

@ -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");

View file

@ -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,

View file

@ -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",

View file

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

View file

@ -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",
);
}
}
}
}

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -0,0 +1,51 @@
error: non-binding `let` without a type annotation
--> $DIR/let_underscore_untyped.rs:32:5
|
LL | let _ = a();
| ^^^^^^^^^^^^
|
= help: consider adding a type annotation or removing the `let` keyword
= note: `-D clippy::let-underscore-untyped` implied by `-D warnings`
error: non-binding `let` without a type annotation
--> $DIR/let_underscore_untyped.rs:33:5
|
LL | let _ = b(1);
| ^^^^^^^^^^^^^
|
= help: consider adding a type annotation or removing the `let` keyword
error: non-binding `let` without a type annotation
--> $DIR/let_underscore_untyped.rs:34:5
|
LL | let _ = c();
| ^^^^^^^^^^^^
|
= help: consider adding a type annotation or removing the `let` keyword
error: non-binding `let` without a type annotation
--> $DIR/let_underscore_untyped.rs:35:5
|
LL | let _ = d(&1);
| ^^^^^^^^^^^^^^
|
= help: consider adding a type annotation or removing the `let` keyword
error: non-binding `let` without a type annotation
--> $DIR/let_underscore_untyped.rs:36:5
|
LL | let _ = e();
| ^^^^^^^^^^^^
|
= help: consider adding a type annotation or removing the `let` keyword
error: non-binding `let` without a type annotation
--> $DIR/let_underscore_untyped.rs:37:5
|
LL | let _ = f();
| ^^^^^^^^^^^^
|
= help: consider adding a type annotation or removing the `let` keyword
error: aborting due to 6 previous errors

View file

@ -1,6 +1,7 @@
// run-rustfix
#![warn(clippy::all, clippy::pedantic)]
#![allow(clippy::let_underscore_untyped)]
#![allow(clippy::missing_docs_in_private_items)]
#![allow(clippy::map_identity)]
#![allow(clippy::redundant_closure)]

View file

@ -1,6 +1,7 @@
// run-rustfix
#![warn(clippy::all, clippy::pedantic)]
#![allow(clippy::let_underscore_untyped)]
#![allow(clippy::missing_docs_in_private_items)]
#![allow(clippy::map_identity)]
#![allow(clippy::redundant_closure)]

View file

@ -1,5 +1,5 @@
error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:17:47
--> $DIR/map_flatten_fixable.rs:18:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id)`
@ -7,43 +7,43 @@ LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().coll
= note: `-D clippy::map-flatten` implied by `-D warnings`
error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:18:47
--> $DIR/map_flatten_fixable.rs:19:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_ref)`
error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:19:47
--> $DIR/map_flatten_fixable.rs:20:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_closure)`
error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:20:47
--> $DIR/map_flatten_fixable.rs:21:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(|x| x.checked_add(1))`
error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:23:47
--> $DIR/map_flatten_fixable.rs:24:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `flat_map` and remove the `.flatten()`: `flat_map(|x| 0..x)`
error: called `map(..).flatten()` on `Option`
--> $DIR/map_flatten_fixable.rs:26:40
--> $DIR/map_flatten_fixable.rs:27:40
|
LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
| ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)`
error: called `map(..).flatten()` on `Result`
--> $DIR/map_flatten_fixable.rs:29:42
--> $DIR/map_flatten_fixable.rs:30:42
|
LL | let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten();
| ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)`
error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:38:10
--> $DIR/map_flatten_fixable.rs:39:10
|
LL | .map(|n| match n {
| __________^
@ -72,7 +72,7 @@ LL ~ });
|
error: called `map(..).flatten()` on `Option`
--> $DIR/map_flatten_fixable.rs:58:10
--> $DIR/map_flatten_fixable.rs:59:10
|
LL | .map(|_| {
| __________^

View file

@ -4,6 +4,7 @@
#![allow(
clippy::disallowed_names,
clippy::default_trait_access,
clippy::let_underscore_untyped,
clippy::missing_docs_in_private_items,
clippy::missing_safety_doc,
clippy::non_ascii_literal,

View file

@ -1,5 +1,5 @@
error: methods called `new` usually return `Self`
--> $DIR/methods.rs:104:5
--> $DIR/methods.rs:105:5
|
LL | / fn new() -> i32 {
LL | | 0
@ -9,7 +9,7 @@ LL | | }
= note: `-D clippy::new-ret-no-self` implied by `-D warnings`
error: called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(..)` instead
--> $DIR/methods.rs:125:13
--> $DIR/methods.rs:126:13
|
LL | let _ = v.iter().filter(|&x| {
| _____________^