Skip to content

Commit c859808

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, unsafe-blocks, if/else arms, match arms, and const blocks. Refactors check_unused_must_use to walk 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. Follow-up to #22239. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
1 parent 1a68212 commit c859808

2 files changed

Lines changed: 180 additions & 10 deletions

File tree

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

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -333,17 +333,20 @@ 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+
let mut diags = Vec::new();
339+
self.check_unused_must_use(stmt_expr, &mut diags);
340+
self.diagnostics.extend(diags);
339341
}
340342
Statement::Let { pat, initializer, else_branch: None, .. } => {
341-
self.check_non_exhaustive_let(&cx, &pattern_arena, pat, initializer)
343+
if let Some(diag) =
344+
self.check_non_exhaustive_let(&cx, &pattern_arena, pat, initializer)
345+
{
346+
self.diagnostics.push(diag);
347+
}
342348
}
343-
_ => None,
344-
};
345-
if let Some(diag) = diag {
346-
self.diagnostics.push(diag);
349+
_ => {}
347350
}
348351
}
349352
}
@@ -415,7 +418,37 @@ impl<'db> ExprValidator<'db> {
415418
pattern
416419
}
417420

418-
fn check_unused_must_use(&self, expr: ExprId) -> Option<BodyValidationDiagnostic<'db>> {
421+
fn check_unused_must_use(
422+
&self,
423+
mut expr: ExprId,
424+
acc: &mut Vec<BodyValidationDiagnostic<'db>>,
425+
) {
426+
// Walk through container expressions so that the value-producing leaf is
427+
// checked even when wrapped in a block, `unsafe { .. }`, `if`/`match`, or
428+
// a `const { .. }` block. Single-tail chains are followed by reassigning
429+
// `expr`; branching containers (`if`/`match`) recurse on each arm.
430+
loop {
431+
match &self.body[expr] {
432+
Expr::Block { tail: Some(tail), .. }
433+
| Expr::Unsafe { tail: Some(tail), .. }
434+
| Expr::Const(tail) => expr = *tail,
435+
Expr::If { then_branch, else_branch, .. } => {
436+
self.check_unused_must_use(*then_branch, acc);
437+
if let Some(else_branch) = else_branch {
438+
self.check_unused_must_use(*else_branch, acc);
439+
}
440+
return;
441+
}
442+
Expr::Match { arms, .. } => {
443+
for arm in arms.iter() {
444+
self.check_unused_must_use(arm.expr, acc);
445+
}
446+
return;
447+
}
448+
_ => break,
449+
}
450+
}
451+
419452
let fn_def = match &self.body[expr] {
420453
Expr::Call { callee, .. } => {
421454
let callee_ty = self.infer.expr_ty(*callee);
@@ -430,7 +463,7 @@ impl<'db> ExprValidator<'db> {
430463
Expr::MethodCall { .. } => {
431464
self.infer.method_resolution(expr).map(|(func, _)| func.into())
432465
}
433-
_ => return None,
466+
_ => None,
434467
};
435468
let ty_def = self.infer.type_of_expr_with_adjust(expr).and_then(|ty| match ty.kind() {
436469
TyKind::Adt(adt, _) => Some(adt.def_id().into()),
@@ -440,7 +473,9 @@ impl<'db> ExprValidator<'db> {
440473
AttrFlags::must_use_message(self.db(), owner?)
441474
.map(|message| BodyValidationDiagnostic::UnusedMustUse { expr, message })
442475
};
443-
must_use_diag(fn_def).or_else(|| must_use_diag(ty_def))
476+
if let Some(diag) = must_use_diag(fn_def).or_else(|| must_use_diag(ty_def)) {
477+
acc.push(diag);
478+
}
444479
}
445480

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

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

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,141 @@ 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+
}
129264
"#,
130265
);
131266
}

0 commit comments

Comments
 (0)