From 758a9fd0f9ac191f0ea99215e07816631f402b7e Mon Sep 17 00:00:00 2001 From: Aaron Kofsky Date: Thu, 2 Jun 2022 14:58:44 -0400 Subject: [PATCH] Add `let_underscore_must_use` lint. Similar to `let_underscore_drop`, this lint checks for statements similar to `let _ = foo`, where `foo` is an expression marked `must_use`. --- compiler/rustc_lint/src/let_underscore.rs | 106 ++++++++++++++++++++- compiler/rustc_lint/src/lib.rs | 7 +- src/test/ui/let_underscore_must_use.rs | 12 +++ src/test/ui/let_underscore_must_use.stderr | 21 ++++ 4 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/let_underscore_must_use.rs create mode 100644 src/test/ui/let_underscore_must_use.stderr diff --git a/compiler/rustc_lint/src/let_underscore.rs b/compiler/rustc_lint/src/let_underscore.rs index 81906a24d902..40e6d12abf91 100644 --- a/compiler/rustc_lint/src/let_underscore.rs +++ b/compiler/rustc_lint/src/let_underscore.rs @@ -1,6 +1,6 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_hir as hir; -use rustc_middle::ty::{self, subst::GenericArgKind}; +use rustc_middle::ty::{self, subst::GenericArgKind, Ty}; use rustc_span::Symbol; declare_lint! { @@ -83,7 +83,32 @@ declare_lint! { "non-binding let on a synchronization lock" } -declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK]); +declare_lint! { + /// The `let_underscore_must_use` lint checks for statements which don't bind + /// a `must_use` expression to anything, causing the lock to be released + /// immediately instead of at end of scope, which is typically incorrect. + /// + /// ### Example + /// ```rust + /// #[must_use] + /// struct SomeStruct; + /// + /// fn main() { + /// // SomeStuct is dropped immediately instead of at end of scope. + /// let _ = SomeStruct; + /// } + /// ``` + /// ### Explanation + /// + /// Statements which assign an expression to an underscore causes the + /// expression to immediately drop. Usually, it's better to explicitly handle + /// the `must_use` expression. + pub LET_UNDERSCORE_MUST_USE, + Warn, + "non-binding let on a expression marked `must_use`" +} + +declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK, LET_UNDERSCORE_MUST_USE]); const SYNC_GUARD_PATHS: [&[&str]; 5] = [ &["std", "sync", "mutex", "MutexGuard"], @@ -114,6 +139,8 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore { GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false, }); + let is_must_use_ty = is_must_use_ty(cx, cx.typeck_results().expr_ty(init)); + let is_must_use_func_call = is_must_use_func_call(cx, init); if is_sync_lock { cx.struct_span_lint(LET_UNDERSCORE_LOCK, local.span, |lint| { lint.build("non-binding let on a synchronization lock") @@ -121,6 +148,13 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore { .help("consider explicitly droping with `std::mem::drop`") .emit(); }) + } else if is_must_use_ty || is_must_use_func_call { + cx.struct_span_lint(LET_UNDERSCORE_MUST_USE, local.span, |lint| { + lint.build("non-binding let on a expression marked `must_use`") + .help("consider binding to an unused variable") + .help("consider explicitly droping with `std::mem::drop`") + .emit(); + }) } else if needs_drop { cx.struct_span_lint(LET_UNDERSCORE_DROP, local.span, |lint| { lint.build("non-binding let on a type that implements `Drop`") @@ -130,5 +164,73 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore { }) } } + + fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + match ty.kind() { + ty::Adt(adt, _) => has_must_use_attr(cx, adt.did()), + ty::Foreign(ref did) => has_must_use_attr(cx, *did), + ty::Slice(ty) + | ty::Array(ty, _) + | ty::RawPtr(ty::TypeAndMut { ty, .. }) + | ty::Ref(_, ty, _) => { + // for the Array case we don't need to care for the len == 0 case + // because we don't want to lint functions returning empty arrays + is_must_use_ty(cx, *ty) + } + ty::Tuple(substs) => substs.iter().any(|ty| is_must_use_ty(cx, ty)), + ty::Opaque(ref def_id, _) => { + for (predicate, _) in cx.tcx.explicit_item_bounds(*def_id) { + if let ty::PredicateKind::Trait(trait_predicate) = + predicate.kind().skip_binder() + { + if has_must_use_attr(cx, trait_predicate.trait_ref.def_id) { + return true; + } + } + } + false + } + ty::Dynamic(binder, _) => { + for predicate in binder.iter() { + if let ty::ExistentialPredicate::Trait(ref trait_ref) = + predicate.skip_binder() + { + if has_must_use_attr(cx, trait_ref.def_id) { + return true; + } + } + } + false + } + _ => false, + } + } + + // check if expr is calling method or function with #[must_use] attribute + fn is_must_use_func_call(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { + let did = match expr.kind { + hir::ExprKind::Call(path, _) if let hir::ExprKind::Path(ref qpath) = path.kind => { + if let hir::def::Res::Def(_, did) = cx.qpath_res(qpath, path.hir_id) { + Some(did) + } else { + None + } + }, + hir::ExprKind::MethodCall(..) => { + cx.typeck_results().type_dependent_def_id(expr.hir_id) + } + _ => None, + }; + + did.map_or(false, |did| has_must_use_attr(cx, did)) + } + + // returns true if DefId contains a `#[must_use]` attribute + fn has_must_use_attr(cx: &LateContext<'_>, did: hir::def_id::DefId) -> bool { + cx.tcx + .get_attrs(did, rustc_span::sym::must_use) + .find(|a| a.has_name(rustc_span::sym::must_use)) + .is_some() + } } } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 79661c0fefe8..4359a54b698d 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -317,7 +317,12 @@ 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!( + "let_underscore", + LET_UNDERSCORE_DROP, + LET_UNDERSCORE_LOCK, + LET_UNDERSCORE_MUST_USE + ); add_lint_group!( "rust_2018_idioms", diff --git a/src/test/ui/let_underscore_must_use.rs b/src/test/ui/let_underscore_must_use.rs new file mode 100644 index 000000000000..6a78e3fc4b40 --- /dev/null +++ b/src/test/ui/let_underscore_must_use.rs @@ -0,0 +1,12 @@ +// run-pass + +#[must_use] +struct MustUseType; + +#[must_use] +fn must_use_function() -> () {} + +fn main() { + let _ = MustUseType; //~WARNING non-binding let on a expression marked `must_use` + let _ = must_use_function(); //~WARNING non-binding let on a expression marked `must_use` +} diff --git a/src/test/ui/let_underscore_must_use.stderr b/src/test/ui/let_underscore_must_use.stderr new file mode 100644 index 000000000000..0b840385e5df --- /dev/null +++ b/src/test/ui/let_underscore_must_use.stderr @@ -0,0 +1,21 @@ +warning: non-binding let on a expression marked `must_use` + --> $DIR/let_underscore_must_use.rs:10:5 + | +LL | let _ = MustUseType; + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(let_underscore_must_use)]` on by default + = help: consider binding to an unused variable + = help: consider explicitly droping with `std::mem::drop` + +warning: non-binding let on a expression marked `must_use` + --> $DIR/let_underscore_must_use.rs:11:5 + | +LL | let _ = must_use_function(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider binding to an unused variable + = help: consider explicitly droping with `std::mem::drop` + +warning: 2 warnings emitted +