diff options
| -rw-r--r-- | compiler/rustc_lint/src/let_underscore.rs | 175 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/lib.rs | 5 | ||||
| -rw-r--r-- | compiler/rustc_span/src/symbol.rs | 3 | ||||
| -rw-r--r-- | library/std/src/sync/mutex.rs | 1 | ||||
| -rw-r--r-- | library/std/src/sync/rwlock.rs | 2 | ||||
| -rw-r--r-- | src/test/ui/lint/let_underscore/let_underscore_drop.rs | 14 | ||||
| -rw-r--r-- | src/test/ui/lint/let_underscore/let_underscore_drop.stderr | 22 | ||||
| -rw-r--r-- | src/test/ui/lint/let_underscore/let_underscore_lock.rs | 7 | ||||
| -rw-r--r-- | src/test/ui/lint/let_underscore/let_underscore_lock.stderr | 20 | ||||
| -rw-r--r-- | src/tools/lint-docs/src/groups.rs | 1 |
10 files changed, 250 insertions, 0 deletions
diff --git a/compiler/rustc_lint/src/let_underscore.rs b/compiler/rustc_lint/src/let_underscore.rs new file mode 100644 index 00000000000..7e885e6c51a --- /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 3478be1ed5d..8cbfc82c0f0 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -55,6 +55,7 @@ mod expect; pub mod hidden_unicode_codepoints; mod internal; mod late; +mod let_underscore; mod levels; mod methods; mod non_ascii_idents; @@ -86,6 +87,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; @@ -189,6 +191,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, @@ -315,6 +318,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 6eca7dc52b2..7d4827520e1 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -223,6 +223,7 @@ symbols! { LinkedList, LintPass, Mutex, + MutexGuard, N, NonZeroI128, NonZeroI16, @@ -271,6 +272,8 @@ symbols! { Rust, RustcDecodable, RustcEncodable, + RwLockReadGuard, + RwLockWriteGuard, Send, SeqCst, SessionDiagnostic, diff --git a/library/std/src/sync/mutex.rs b/library/std/src/sync/mutex.rs index e0d13cd648c..de851c8fbbe 100644 --- a/library/std/src/sync/mutex.rs +++ b/library/std/src/sync/mutex.rs @@ -192,6 +192,7 @@ unsafe impl<T: ?Sized + Send> Sync for Mutex<T> {} 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<T>, poison: poison::Guard, diff --git a/library/std/src/sync/rwlock.rs b/library/std/src/sync/rwlock.rs index 6e4a2cfc82a..9ab781561e9 100644 --- a/library/std/src/sync/rwlock.rs +++ b/library/std/src/sync/rwlock.rs @@ -101,6 +101,7 @@ unsafe impl<T: ?Sized + Send + Sync> Sync for RwLock<T> {} 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> { // NB: we use a pointer instead of `&'a T` to avoid `noalias` violations, because a // `Ref` argument doesn't hold immutability for its whole scope, only until it drops. @@ -130,6 +131,7 @@ unsafe impl<T: ?Sized + Sync> 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<T>, 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 00000000000..f298871f122 --- /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 00000000000..7b7de202e46 --- /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 00000000000..7423862cdf0 --- /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 00000000000..fb58af0a42f --- /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 9696e35b796..2a923a61b0a 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"), |