Skip to content

Commit 758a9fd

Browse files
committed
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`.
1 parent ad7587f commit 758a9fd

File tree

4 files changed

+143
-3
lines changed

4 files changed

+143
-3
lines changed

compiler/rustc_lint/src/let_underscore.rs

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{LateContext, LateLintPass, LintContext};
22
use rustc_hir as hir;
3-
use rustc_middle::ty::{self, subst::GenericArgKind};
3+
use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
44
use rustc_span::Symbol;
55

66
declare_lint! {
@@ -83,7 +83,32 @@ declare_lint! {
8383
"non-binding let on a synchronization lock"
8484
}
8585

86-
declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK]);
86+
declare_lint! {
87+
/// The `let_underscore_must_use` lint checks for statements which don't bind
88+
/// a `must_use` expression to anything, causing the lock to be released
89+
/// immediately instead of at end of scope, which is typically incorrect.
90+
///
91+
/// ### Example
92+
/// ```rust
93+
/// #[must_use]
94+
/// struct SomeStruct;
95+
///
96+
/// fn main() {
97+
/// // SomeStuct is dropped immediately instead of at end of scope.
98+
/// let _ = SomeStruct;
99+
/// }
100+
/// ```
101+
/// ### Explanation
102+
///
103+
/// Statements which assign an expression to an underscore causes the
104+
/// expression to immediately drop. Usually, it's better to explicitly handle
105+
/// the `must_use` expression.
106+
pub LET_UNDERSCORE_MUST_USE,
107+
Warn,
108+
"non-binding let on a expression marked `must_use`"
109+
}
110+
111+
declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK, LET_UNDERSCORE_MUST_USE]);
87112

88113
const SYNC_GUARD_PATHS: [&[&str]; 5] = [
89114
&["std", "sync", "mutex", "MutexGuard"],
@@ -114,13 +139,22 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
114139

115140
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
116141
});
142+
let is_must_use_ty = is_must_use_ty(cx, cx.typeck_results().expr_ty(init));
143+
let is_must_use_func_call = is_must_use_func_call(cx, init);
117144
if is_sync_lock {
118145
cx.struct_span_lint(LET_UNDERSCORE_LOCK, local.span, |lint| {
119146
lint.build("non-binding let on a synchronization lock")
120147
.help("consider binding to an unused variable")
121148
.help("consider explicitly droping with `std::mem::drop`")
122149
.emit();
123150
})
151+
} else if is_must_use_ty || is_must_use_func_call {
152+
cx.struct_span_lint(LET_UNDERSCORE_MUST_USE, local.span, |lint| {
153+
lint.build("non-binding let on a expression marked `must_use`")
154+
.help("consider binding to an unused variable")
155+
.help("consider explicitly droping with `std::mem::drop`")
156+
.emit();
157+
})
124158
} else if needs_drop {
125159
cx.struct_span_lint(LET_UNDERSCORE_DROP, local.span, |lint| {
126160
lint.build("non-binding let on a type that implements `Drop`")
@@ -130,5 +164,73 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
130164
})
131165
}
132166
}
167+
168+
fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
169+
match ty.kind() {
170+
ty::Adt(adt, _) => has_must_use_attr(cx, adt.did()),
171+
ty::Foreign(ref did) => has_must_use_attr(cx, *did),
172+
ty::Slice(ty)
173+
| ty::Array(ty, _)
174+
| ty::RawPtr(ty::TypeAndMut { ty, .. })
175+
| ty::Ref(_, ty, _) => {
176+
// for the Array case we don't need to care for the len == 0 case
177+
// because we don't want to lint functions returning empty arrays
178+
is_must_use_ty(cx, *ty)
179+
}
180+
ty::Tuple(substs) => substs.iter().any(|ty| is_must_use_ty(cx, ty)),
181+
ty::Opaque(ref def_id, _) => {
182+
for (predicate, _) in cx.tcx.explicit_item_bounds(*def_id) {
183+
if let ty::PredicateKind::Trait(trait_predicate) =
184+
predicate.kind().skip_binder()
185+
{
186+
if has_must_use_attr(cx, trait_predicate.trait_ref.def_id) {
187+
return true;
188+
}
189+
}
190+
}
191+
false
192+
}
193+
ty::Dynamic(binder, _) => {
194+
for predicate in binder.iter() {
195+
if let ty::ExistentialPredicate::Trait(ref trait_ref) =
196+
predicate.skip_binder()
197+
{
198+
if has_must_use_attr(cx, trait_ref.def_id) {
199+
return true;
200+
}
201+
}
202+
}
203+
false
204+
}
205+
_ => false,
206+
}
207+
}
208+
209+
// check if expr is calling method or function with #[must_use] attribute
210+
fn is_must_use_func_call(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
211+
let did = match expr.kind {
212+
hir::ExprKind::Call(path, _) if let hir::ExprKind::Path(ref qpath) = path.kind => {
213+
if let hir::def::Res::Def(_, did) = cx.qpath_res(qpath, path.hir_id) {
214+
Some(did)
215+
} else {
216+
None
217+
}
218+
},
219+
hir::ExprKind::MethodCall(..) => {
220+
cx.typeck_results().type_dependent_def_id(expr.hir_id)
221+
}
222+
_ => None,
223+
};
224+
225+
did.map_or(false, |did| has_must_use_attr(cx, did))
226+
}
227+
228+
// returns true if DefId contains a `#[must_use]` attribute
229+
fn has_must_use_attr(cx: &LateContext<'_>, did: hir::def_id::DefId) -> bool {
230+
cx.tcx
231+
.get_attrs(did, rustc_span::sym::must_use)
232+
.find(|a| a.has_name(rustc_span::sym::must_use))
233+
.is_some()
234+
}
133235
}
134236
}

compiler/rustc_lint/src/lib.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,12 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
317317
REDUNDANT_SEMICOLONS
318318
);
319319

320-
add_lint_group!("let_underscore", LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK);
320+
add_lint_group!(
321+
"let_underscore",
322+
LET_UNDERSCORE_DROP,
323+
LET_UNDERSCORE_LOCK,
324+
LET_UNDERSCORE_MUST_USE
325+
);
321326

322327
add_lint_group!(
323328
"rust_2018_idioms",
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// run-pass
2+
3+
#[must_use]
4+
struct MustUseType;
5+
6+
#[must_use]
7+
fn must_use_function() -> () {}
8+
9+
fn main() {
10+
let _ = MustUseType; //~WARNING non-binding let on a expression marked `must_use`
11+
let _ = must_use_function(); //~WARNING non-binding let on a expression marked `must_use`
12+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
warning: non-binding let on a expression marked `must_use`
2+
--> $DIR/let_underscore_must_use.rs:10:5
3+
|
4+
LL | let _ = MustUseType;
5+
| ^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `#[warn(let_underscore_must_use)]` on by default
8+
= help: consider binding to an unused variable
9+
= help: consider explicitly droping with `std::mem::drop`
10+
11+
warning: non-binding let on a expression marked `must_use`
12+
--> $DIR/let_underscore_must_use.rs:11:5
13+
|
14+
LL | let _ = must_use_function();
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
|
17+
= help: consider binding to an unused variable
18+
= help: consider explicitly droping with `std::mem::drop`
19+
20+
warning: 2 warnings emitted
21+

0 commit comments

Comments
 (0)