Skip to content

Commit 68efe3e

Browse files
committed
hir-ty: walk container exprs for unused_must_use
ExprValidator::check_unused_must_use previously matched only Expr::Call and Expr::MethodCall directly on the expression of a Statement::Expr with semicolon. This left several stmt-with-semi cases unwarned: blocks whose tail is a #[must_use] call (`{ produces() };`), unsafe-blocks (`unsafe { produces() };`), if/else arms, match arms, and const blocks. Refactors check_unused_must_use into a recursive collector that walks into Expr::Block { tail, .. }, Expr::Unsafe { tail, .. }, Expr::If { then_branch, else_branch, .. }, Expr::Match { arms, .. }, and Expr::Const(inner) before applying the existing leaf check (callee or method #[must_use] attribute, or #[must_use] ADT return type). The diagnostic stays at the leaf call so the span points to the actual must_use producer rather than the wrapping block. Adds ide-diagnostics tests covering each new container case (block tail, unsafe block tail, nested blocks, if/else branches, match arms, const block, #[must_use] ADT through a block) and lock-in tests confirming that the existing RustcLint plumbing already honors #[allow(unused_must_use)], #[allow(unused)], and #[deny(unused_must_use)] attribute resolution. Follow-up to #22239. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
1 parent 1a68212 commit 68efe3e

2 files changed

Lines changed: 243 additions & 10 deletions

File tree

crates/hir-ty/src/diagnostics/expr.rs

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -333,17 +333,18 @@ impl<'db> ExprValidator<'db> {
333333
let pattern_arena = Arena::new();
334334
let cx = MatchCheckCtx::new(self.owner.module(self.db()), &self.infcx, self.env);
335335
for stmt in &**statements {
336-
let diag = match *stmt {
336+
match *stmt {
337337
Statement::Expr { expr: stmt_expr, has_semi: true } if self.validate_lints => {
338-
self.check_unused_must_use(stmt_expr)
338+
self.diagnostics.extend(self.check_unused_must_use(stmt_expr));
339339
}
340340
Statement::Let { pat, initializer, else_branch: None, .. } => {
341-
self.check_non_exhaustive_let(&cx, &pattern_arena, pat, initializer)
341+
if let Some(diag) =
342+
self.check_non_exhaustive_let(&cx, &pattern_arena, pat, initializer)
343+
{
344+
self.diagnostics.push(diag);
345+
}
342346
}
343-
_ => None,
344-
};
345-
if let Some(diag) = diag {
346-
self.diagnostics.push(diag);
347+
_ => {}
347348
}
348349
}
349350
}
@@ -415,7 +416,45 @@ impl<'db> ExprValidator<'db> {
415416
pattern
416417
}
417418

418-
fn check_unused_must_use(&self, expr: ExprId) -> Option<BodyValidationDiagnostic<'db>> {
419+
fn check_unused_must_use(&self, expr: ExprId) -> Vec<BodyValidationDiagnostic<'db>> {
420+
let mut diags = Vec::new();
421+
self.collect_unused_must_use(expr, &mut diags);
422+
diags
423+
}
424+
425+
fn collect_unused_must_use(
426+
&self,
427+
expr: ExprId,
428+
diags: &mut Vec<BodyValidationDiagnostic<'db>>,
429+
) {
430+
// Walk through container expressions so that the value-producing leaf is
431+
// checked even when wrapped in a block, `unsafe { .. }`, `if`/`match`, or
432+
// a `const { .. }` block.
433+
match &self.body[expr] {
434+
Expr::Block { tail: Some(tail), .. } | Expr::Unsafe { tail: Some(tail), .. } => {
435+
self.collect_unused_must_use(*tail, diags);
436+
return;
437+
}
438+
Expr::If { then_branch, else_branch, .. } => {
439+
self.collect_unused_must_use(*then_branch, diags);
440+
if let Some(else_branch) = else_branch {
441+
self.collect_unused_must_use(*else_branch, diags);
442+
}
443+
return;
444+
}
445+
Expr::Match { arms, .. } => {
446+
for arm in arms.iter() {
447+
self.collect_unused_must_use(arm.expr, diags);
448+
}
449+
return;
450+
}
451+
Expr::Const(inner) => {
452+
self.collect_unused_must_use(*inner, diags);
453+
return;
454+
}
455+
_ => {}
456+
}
457+
419458
let fn_def = match &self.body[expr] {
420459
Expr::Call { callee, .. } => {
421460
let callee_ty = self.infer.expr_ty(*callee);
@@ -430,7 +469,7 @@ impl<'db> ExprValidator<'db> {
430469
Expr::MethodCall { .. } => {
431470
self.infer.method_resolution(expr).map(|(func, _)| func.into())
432471
}
433-
_ => return None,
472+
_ => None,
434473
};
435474
let ty_def = self.infer.type_of_expr_with_adjust(expr).and_then(|ty| match ty.kind() {
436475
TyKind::Adt(adt, _) => Some(adt.def_id().into()),
@@ -440,7 +479,9 @@ impl<'db> ExprValidator<'db> {
440479
AttrFlags::must_use_message(self.db(), owner?)
441480
.map(|message| BodyValidationDiagnostic::UnusedMustUse { expr, message })
442481
};
443-
must_use_diag(fn_def).or_else(|| must_use_diag(ty_def))
482+
if let Some(diag) = must_use_diag(fn_def).or_else(|| must_use_diag(ty_def)) {
483+
diags.push(diag);
484+
}
444485
}
445486

446487
fn check_for_trailing_return(&mut self, body_expr: ExprId, body: &Body) {

crates/ide-diagnostics/src/handlers/unused_must_use.rs

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,198 @@ fn main() {
126126
x = produces();
127127
let _ = x;
128128
}
129+
"#,
130+
);
131+
}
132+
133+
#[test]
134+
fn block_tail_expression_in_stmt_position() {
135+
check_diagnostics(
136+
r#"
137+
#[must_use]
138+
fn produces() -> i32 { 0 }
139+
fn main() {
140+
{
141+
produces()
142+
//^^^^^^^^^^ warn: unused return value that must be used
143+
};
144+
}
145+
"#,
146+
);
147+
}
148+
149+
#[test]
150+
fn unsafe_block_tail_expression_in_stmt_position() {
151+
check_diagnostics(
152+
r#"
153+
#[must_use]
154+
unsafe fn produces() -> i32 { 0 }
155+
fn main() {
156+
unsafe {
157+
produces()
158+
//^^^^^^^^^^ warn: unused return value that must be used
159+
};
160+
}
161+
"#,
162+
);
163+
}
164+
165+
#[test]
166+
fn nested_block_tail_expression() {
167+
check_diagnostics(
168+
r#"
169+
#[must_use]
170+
fn produces() -> i32 { 0 }
171+
fn main() {
172+
{
173+
{
174+
produces()
175+
//^^^^^^^^^^ warn: unused return value that must be used
176+
}
177+
};
178+
}
179+
"#,
180+
);
181+
}
182+
183+
#[test]
184+
fn no_warning_when_block_tail_is_bound() {
185+
check_diagnostics(
186+
r#"
187+
#[must_use]
188+
fn produces() -> i32 { 0 }
189+
fn main() {
190+
let _x = {
191+
produces()
192+
};
193+
}
194+
"#,
195+
);
196+
}
197+
198+
#[test]
199+
fn if_branches_in_stmt_position() {
200+
check_diagnostics(
201+
r#"
202+
#[must_use]
203+
fn produces() -> i32 { 0 }
204+
fn main() {
205+
if true {
206+
produces()
207+
//^^^^^^^^^^ warn: unused return value that must be used
208+
} else {
209+
produces()
210+
//^^^^^^^^^^ warn: unused return value that must be used
211+
};
212+
}
213+
"#,
214+
);
215+
}
216+
217+
#[test]
218+
fn match_arms_in_stmt_position() {
219+
check_diagnostics(
220+
r#"
221+
#[must_use]
222+
fn produces() -> i32 { 0 }
223+
fn main() {
224+
match 0 {
225+
0 => produces(),
226+
//^^^^^^^^^^ warn: unused return value that must be used
227+
_ => produces(),
228+
//^^^^^^^^^^ warn: unused return value that must be used
229+
};
230+
}
231+
"#,
232+
);
233+
}
234+
235+
#[test]
236+
fn const_block_in_stmt_position() {
237+
check_diagnostics(
238+
r#"
239+
#[must_use]
240+
const fn produces() -> i32 { 0 }
241+
fn main() {
242+
const {
243+
produces()
244+
//^^^^^^^^^^ warn: unused return value that must be used
245+
};
246+
}
247+
"#,
248+
);
249+
}
250+
251+
#[test]
252+
fn must_use_type_through_block() {
253+
check_diagnostics(
254+
r#"
255+
#[must_use]
256+
struct Important;
257+
fn produces() -> Important { Important }
258+
fn main() {
259+
{
260+
produces()
261+
//^^^^^^^^^^ warn: unused return value that must be used
262+
};
263+
}
264+
"#,
265+
);
266+
}
267+
268+
#[test]
269+
fn allow_attribute_on_fn_silences_lint() {
270+
check_diagnostics(
271+
r#"
272+
#[must_use]
273+
fn produces() -> i32 { 0 }
274+
#[allow(unused_must_use)]
275+
fn main() {
276+
produces();
277+
}
278+
"#,
279+
);
280+
}
281+
282+
#[test]
283+
fn allow_attribute_on_stmt_silences_lint() {
284+
check_diagnostics(
285+
r#"
286+
#[must_use]
287+
fn produces() -> i32 { 0 }
288+
fn main() {
289+
#[allow(unused_must_use)]
290+
produces();
291+
}
292+
"#,
293+
);
294+
}
295+
296+
#[test]
297+
fn allow_unused_group_silences_lint() {
298+
check_diagnostics(
299+
r#"
300+
#[must_use]
301+
fn produces() -> i32 { 0 }
302+
#[allow(unused)]
303+
fn main() {
304+
produces();
305+
}
306+
"#,
307+
);
308+
}
309+
310+
#[test]
311+
fn deny_attribute_escalates_lint() {
312+
check_diagnostics(
313+
r#"
314+
#[must_use]
315+
fn produces() -> i32 { 0 }
316+
#[deny(unused_must_use)]
317+
fn main() {
318+
produces();
319+
//^^^^^^^^^^ error: unused return value that must be used
320+
}
129321
"#,
130322
);
131323
}

0 commit comments

Comments
 (0)