diff --git a/compiler/rustc_lint/src/let_underscore.rs b/compiler/rustc_lint/src/let_underscore.rs new file mode 100644 index 0000000000000..7e885e6c51aad --- /dev/null +++ b/compiler/rustc_lint/src/let_underscore.rs @@ -0,0 +1,175 @@ +use crate::{LateContext, LateLintPass, LintContext}; +use rustc_errors::{Applicability, LintDiagnosticBuilder, MultiSpan}; +use rustc_hir as hir; +use rustc_middle::ty; +use rustc_span::Symbol; + +declare_lint! { + /// The `let_underscore_drop` lint checks for statements which don't bind + /// an expression which has a non-trivial Drop implementation to anything, + /// causing the expression to be dropped immediately instead of at end of + /// scope. + /// + /// ### Example + /// ``` + /// struct SomeStruct; + /// impl Drop for SomeStruct { + /// fn drop(&mut self) { + /// println!("Dropping SomeStruct"); + /// } + /// } + /// + /// fn main() { + /// #[warn(let_underscore_drop)] + /// // SomeStuct is dropped immediately instead of at end of scope, + /// // so "Dropping SomeStruct" is printed before "end of main". + /// // The order of prints would be reversed if SomeStruct was bound to + /// // a name (such as "_foo"). + /// let _ = SomeStruct; + /// println!("end of main"); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Statements which assign an expression to an underscore causes the + /// expression to immediately drop instead of extending the expression's + /// lifetime to the end of the scope. This is usually unintended, + /// especially for types like `MutexGuard`, which are typically used to + /// lock a mutex for the duration of an entire scope. + /// + /// If you want to extend the expression's lifetime to the end of the scope, + /// assign an underscore-prefixed name (such as `_foo`) to the expression. + /// If you do actually want to drop the expression immediately, then + /// calling `std::mem::drop` on the expression is clearer and helps convey + /// intent. + pub LET_UNDERSCORE_DROP, + Allow, + "non-binding let on a type that implements `Drop`" +} + +declare_lint! { + /// The `let_underscore_lock` lint checks for statements which don't bind + /// a mutex to anything, causing the lock to be released immediately instead + /// of at end of scope, which is typically incorrect. + /// + /// ### Example + /// ```compile_fail + /// use std::sync::{Arc, Mutex}; + /// use std::thread; + /// let data = Arc::new(Mutex::new(0)); + /// + /// thread::spawn(move || { + /// // The lock is immediately released instead of at the end of the + /// // scope, which is probably not intended. + /// let _ = data.lock().unwrap(); + /// println!("doing some work"); + /// let mut lock = data.lock().unwrap(); + /// *lock += 1; + /// }); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Statements which assign an expression to an underscore causes the + /// expression to immediately drop instead of extending the expression's + /// lifetime to the end of the scope. This is usually unintended, + /// especially for types like `MutexGuard`, which are typically used to + /// lock a mutex for the duration of an entire scope. + /// + /// If you want to extend the expression's lifetime to the end of the scope, + /// assign an underscore-prefixed name (such as `_foo`) to the expression. + /// If you do actually want to drop the expression immediately, then + /// calling `std::mem::drop` on the expression is clearer and helps convey + /// intent. + pub LET_UNDERSCORE_LOCK, + Deny, + "non-binding let on a synchronization lock" +} + +declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK]); + +const SYNC_GUARD_SYMBOLS: [Symbol; 3] = [ + rustc_span::sym::MutexGuard, + rustc_span::sym::RwLockReadGuard, + rustc_span::sym::RwLockWriteGuard, +]; + +impl<'tcx> LateLintPass<'tcx> for LetUnderscore { + fn check_local(&mut self, cx: &LateContext<'_>, local: &hir::Local<'_>) { + if !matches!(local.pat.kind, hir::PatKind::Wild) { + return; + } + if let Some(init) = local.init { + let init_ty = cx.typeck_results().expr_ty(init); + // If the type has a trivial Drop implementation, then it doesn't + // matter that we drop the value immediately. + if !init_ty.needs_drop(cx.tcx, cx.param_env) { + return; + } + let is_sync_lock = match init_ty.kind() { + ty::Adt(adt, _) => SYNC_GUARD_SYMBOLS + .iter() + .any(|guard_symbol| cx.tcx.is_diagnostic_item(*guard_symbol, adt.did())), + _ => false, + }; + + if is_sync_lock { + let mut span = MultiSpan::from_spans(vec![local.pat.span, init.span]); + span.push_span_label( + local.pat.span, + "this lock is not assigned to a binding and is immediately dropped".to_string(), + ); + span.push_span_label( + init.span, + "this binding will immediately drop the value assigned to it".to_string(), + ); + cx.struct_span_lint(LET_UNDERSCORE_LOCK, span, |lint| { + build_and_emit_lint( + lint, + local, + init.span, + "non-binding let on a synchronization lock", + ) + }) + } else { + cx.struct_span_lint(LET_UNDERSCORE_DROP, local.span, |lint| { + build_and_emit_lint( + lint, + local, + init.span, + "non-binding let on a type that implements `Drop`", + ); + }) + } + } + } +} + +fn build_and_emit_lint( + lint: LintDiagnosticBuilder<'_, ()>, + local: &hir::Local<'_>, + init_span: rustc_span::Span, + msg: &str, +) { + lint.build(msg) + .span_suggestion_verbose( + local.pat.span, + "consider binding to an unused variable to avoid immediately dropping the value", + "_unused", + Applicability::MachineApplicable, + ) + .multipart_suggestion( + "consider immediately dropping the value", + vec![ + (local.span.until(init_span), "drop(".to_string()), + (init_span.shrink_to_hi(), ")".to_string()), + ], + Applicability::MachineApplicable, + ) + .emit(); +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 0a0f292fe7a4d..79661c0fefe8d 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -54,6 +54,7 @@ mod expect; pub mod hidden_unicode_codepoints; mod internal; mod late; +mod let_underscore; mod levels; mod methods; mod non_ascii_idents; @@ -85,6 +86,7 @@ use builtin::*; use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; use hidden_unicode_codepoints::*; use internal::*; +use let_underscore::*; use methods::*; use non_ascii_idents::*; use non_fmt_panic::NonPanicFmt; @@ -199,6 +201,7 @@ macro_rules! late_lint_mod_passes { VariantSizeDifferences: VariantSizeDifferences, BoxPointers: BoxPointers, PathStatements: PathStatements, + LetUnderscore: LetUnderscore, // Depends on referenced function signatures in expressions UnusedResults: UnusedResults, NonUpperCaseGlobals: NonUpperCaseGlobals, @@ -314,6 +317,8 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { REDUNDANT_SEMICOLONS ); + add_lint_group!("let_underscore", LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK); + add_lint_group!( "rust_2018_idioms", BARE_TRAIT_OBJECTS, diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 5c9c16350e469..bc9f5c910df54 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -213,6 +213,7 @@ symbols! { LinkedList, LintPass, Mutex, + MutexGuard, N, None, Ok, @@ -250,6 +251,8 @@ symbols! { Right, RustcDecodable, RustcEncodable, + RwLockReadGuard, + RwLockWriteGuard, Send, SeqCst, SliceIndex, diff --git a/library/std/src/sync/mutex.rs b/library/std/src/sync/mutex.rs index 3d8281fe59389..d976725c2761a 100644 --- a/library/std/src/sync/mutex.rs +++ b/library/std/src/sync/mutex.rs @@ -193,6 +193,7 @@ unsafe impl Sync for Mutex {} and cause Futures to not implement `Send`"] #[stable(feature = "rust1", since = "1.0.0")] #[clippy::has_significant_drop] +#[cfg_attr(not(test), rustc_diagnostic_item = "MutexGuard")] pub struct MutexGuard<'a, T: ?Sized + 'a> { lock: &'a Mutex, poison: poison::Guard, diff --git a/library/std/src/sync/rwlock.rs b/library/std/src/sync/rwlock.rs index 4f1b4bedaab25..e956f00a12fd3 100644 --- a/library/std/src/sync/rwlock.rs +++ b/library/std/src/sync/rwlock.rs @@ -100,6 +100,7 @@ unsafe impl Sync for RwLock {} and cause Futures to not implement `Send`"] #[stable(feature = "rust1", since = "1.0.0")] #[clippy::has_significant_drop] +#[cfg_attr(not(test), rustc_diagnostic_item = "RwLockReadGuard")] pub struct RwLockReadGuard<'a, T: ?Sized + 'a> { lock: &'a RwLock, } @@ -124,6 +125,7 @@ unsafe impl Sync for RwLockReadGuard<'_, T> {} and cause Future's to not implement `Send`"] #[stable(feature = "rust1", since = "1.0.0")] #[clippy::has_significant_drop] +#[cfg_attr(not(test), rustc_diagnostic_item = "RwLockWriteGuard")] pub struct RwLockWriteGuard<'a, T: ?Sized + 'a> { lock: &'a RwLock, poison: poison::Guard, diff --git a/src/test/ui/lint/let_underscore/let_underscore_drop.rs b/src/test/ui/lint/let_underscore/let_underscore_drop.rs new file mode 100644 index 0000000000000..f298871f122d3 --- /dev/null +++ b/src/test/ui/lint/let_underscore/let_underscore_drop.rs @@ -0,0 +1,14 @@ +// check-pass +#![warn(let_underscore_drop)] + +struct NontrivialDrop; + +impl Drop for NontrivialDrop { + fn drop(&mut self) { + println!("Dropping!"); + } +} + +fn main() { + let _ = NontrivialDrop; //~WARNING non-binding let on a type that implements `Drop` +} diff --git a/src/test/ui/lint/let_underscore/let_underscore_drop.stderr b/src/test/ui/lint/let_underscore/let_underscore_drop.stderr new file mode 100644 index 0000000000000..7b7de202e4626 --- /dev/null +++ b/src/test/ui/lint/let_underscore/let_underscore_drop.stderr @@ -0,0 +1,22 @@ +warning: non-binding let on a type that implements `Drop` + --> $DIR/let_underscore_drop.rs:13:5 + | +LL | let _ = NontrivialDrop; + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/let_underscore_drop.rs:2:9 + | +LL | #![warn(let_underscore_drop)] + | ^^^^^^^^^^^^^^^^^^^ +help: consider binding to an unused variable to avoid immediately dropping the value + | +LL | let _unused = NontrivialDrop; + | ~~~~~~~ +help: consider immediately dropping the value + | +LL | drop(NontrivialDrop); + | ~~~~~ + + +warning: 1 warning emitted + diff --git a/src/test/ui/lint/let_underscore/let_underscore_lock.rs b/src/test/ui/lint/let_underscore/let_underscore_lock.rs new file mode 100644 index 0000000000000..7423862cdf05d --- /dev/null +++ b/src/test/ui/lint/let_underscore/let_underscore_lock.rs @@ -0,0 +1,7 @@ +// check-fail +use std::sync::{Arc, Mutex}; + +fn main() { + let data = Arc::new(Mutex::new(0)); + let _ = data.lock().unwrap(); //~ERROR non-binding let on a synchronization lock +} diff --git a/src/test/ui/lint/let_underscore/let_underscore_lock.stderr b/src/test/ui/lint/let_underscore/let_underscore_lock.stderr new file mode 100644 index 0000000000000..fb58af0a42f81 --- /dev/null +++ b/src/test/ui/lint/let_underscore/let_underscore_lock.stderr @@ -0,0 +1,20 @@ +error: non-binding let on a synchronization lock + --> $DIR/let_underscore_lock.rs:6:9 + | +LL | let _ = data.lock().unwrap(); + | ^ ^^^^^^^^^^^^^^^^^^^^ this binding will immediately drop the value assigned to it + | | + | this lock is not assigned to a binding and is immediately dropped + | + = note: `#[deny(let_underscore_lock)]` on by default +help: consider binding to an unused variable to avoid immediately dropping the value + | +LL | let _unused = data.lock().unwrap(); + | ~~~~~~~ +help: consider immediately dropping the value + | +LL | drop(data.lock().unwrap()); + | ~~~~~ + + +error: aborting due to previous error + diff --git a/src/tools/lint-docs/src/groups.rs b/src/tools/lint-docs/src/groups.rs index 9696e35b7963f..2a923a61b0a70 100644 --- a/src/tools/lint-docs/src/groups.rs +++ b/src/tools/lint-docs/src/groups.rs @@ -8,6 +8,7 @@ use std::process::Command; /// Descriptions of rustc lint groups. static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[ ("unused", "Lints that detect things being declared but not used, or excess syntax"), + ("let-underscore", "Lints that detect wildcard let bindings that are likely to be invalid"), ("rustdoc", "Rustdoc-specific lints"), ("rust-2018-idioms", "Lints to nudge you toward idiomatic features of Rust 2018"), ("nonstandard-style", "Violation of standard naming conventions"),