Unnamed repository; edit this file 'description' to name the repository.
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 rust-lang/rust-analyzer#22239.
Signed-off-by: Onyeka Obi <[email protected]>
| -rw-r--r-- | crates/hir-ty/src/diagnostics/expr.rs | 55 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/unused_must_use.rs | 135 |
2 files changed, 180 insertions, 10 deletions
diff --git a/crates/hir-ty/src/diagnostics/expr.rs b/crates/hir-ty/src/diagnostics/expr.rs index 760ebd27e0..ee75023c4a 100644 --- a/crates/hir-ty/src/diagnostics/expr.rs +++ b/crates/hir-ty/src/diagnostics/expr.rs @@ -333,17 +333,20 @@ impl<'db> ExprValidator<'db> { let pattern_arena = Arena::new(); let cx = MatchCheckCtx::new(self.owner.module(self.db()), &self.infcx, self.env); for stmt in &**statements { - let diag = match *stmt { + match *stmt { Statement::Expr { expr: stmt_expr, has_semi: true } if self.validate_lints => { - self.check_unused_must_use(stmt_expr) + let mut diags = Vec::new(); + self.check_unused_must_use(stmt_expr, &mut diags); + self.diagnostics.extend(diags); } Statement::Let { pat, initializer, else_branch: None, .. } => { - self.check_non_exhaustive_let(&cx, &pattern_arena, pat, initializer) + if let Some(diag) = + self.check_non_exhaustive_let(&cx, &pattern_arena, pat, initializer) + { + self.diagnostics.push(diag); + } } - _ => None, - }; - if let Some(diag) = diag { - self.diagnostics.push(diag); + _ => {} } } } @@ -415,7 +418,37 @@ impl<'db> ExprValidator<'db> { pattern } - fn check_unused_must_use(&self, expr: ExprId) -> Option<BodyValidationDiagnostic<'db>> { + fn check_unused_must_use( + &self, + mut expr: ExprId, + acc: &mut Vec<BodyValidationDiagnostic<'db>>, + ) { + // Walk through container expressions so that the value-producing leaf is + // checked even when wrapped in a block, `unsafe { .. }`, `if`/`match`, or + // a `const { .. }` block. Single-tail chains are followed by reassigning + // `expr`; branching containers (`if`/`match`) recurse on each arm. + loop { + match &self.body[expr] { + Expr::Block { tail: Some(tail), .. } + | Expr::Unsafe { tail: Some(tail), .. } + | Expr::Const(tail) => expr = *tail, + Expr::If { then_branch, else_branch, .. } => { + self.check_unused_must_use(*then_branch, acc); + if let Some(else_branch) = else_branch { + self.check_unused_must_use(*else_branch, acc); + } + return; + } + Expr::Match { arms, .. } => { + for arm in arms.iter() { + self.check_unused_must_use(arm.expr, acc); + } + return; + } + _ => break, + } + } + let fn_def = match &self.body[expr] { Expr::Call { callee, .. } => { let callee_ty = self.infer.expr_ty(*callee); @@ -430,7 +463,7 @@ impl<'db> ExprValidator<'db> { Expr::MethodCall { .. } => { self.infer.method_resolution(expr).map(|(func, _)| func.into()) } - _ => return None, + _ => None, }; let ty_def = self.infer.type_of_expr_with_adjust(expr).and_then(|ty| match ty.kind() { TyKind::Adt(adt, _) => Some(adt.def_id().into()), @@ -440,7 +473,9 @@ impl<'db> ExprValidator<'db> { AttrFlags::must_use_message(self.db(), owner?) .map(|message| BodyValidationDiagnostic::UnusedMustUse { expr, message }) }; - must_use_diag(fn_def).or_else(|| must_use_diag(ty_def)) + if let Some(diag) = must_use_diag(fn_def).or_else(|| must_use_diag(ty_def)) { + acc.push(diag); + } } fn check_for_trailing_return(&mut self, body_expr: ExprId, body: &Body) { diff --git a/crates/ide-diagnostics/src/handlers/unused_must_use.rs b/crates/ide-diagnostics/src/handlers/unused_must_use.rs index e8d0717c91..2173dc9c0a 100644 --- a/crates/ide-diagnostics/src/handlers/unused_must_use.rs +++ b/crates/ide-diagnostics/src/handlers/unused_must_use.rs @@ -129,4 +129,139 @@ fn main() { "#, ); } + + #[test] + fn block_tail_expression_in_stmt_position() { + check_diagnostics( + r#" +#[must_use] +fn produces() -> i32 { 0 } +fn main() { + { + produces() + //^^^^^^^^^^ warn: unused return value that must be used + }; +} +"#, + ); + } + + #[test] + fn unsafe_block_tail_expression_in_stmt_position() { + check_diagnostics( + r#" +#[must_use] +unsafe fn produces() -> i32 { 0 } +fn main() { + unsafe { + produces() + //^^^^^^^^^^ warn: unused return value that must be used + }; +} +"#, + ); + } + + #[test] + fn nested_block_tail_expression() { + check_diagnostics( + r#" +#[must_use] +fn produces() -> i32 { 0 } +fn main() { + { + { + produces() + //^^^^^^^^^^ warn: unused return value that must be used + } + }; +} +"#, + ); + } + + #[test] + fn no_warning_when_block_tail_is_bound() { + check_diagnostics( + r#" +#[must_use] +fn produces() -> i32 { 0 } +fn main() { + let _x = { + produces() + }; +} +"#, + ); + } + + #[test] + fn if_branches_in_stmt_position() { + check_diagnostics( + r#" +#[must_use] +fn produces() -> i32 { 0 } +fn main() { + if true { + produces() + //^^^^^^^^^^ warn: unused return value that must be used + } else { + produces() + //^^^^^^^^^^ warn: unused return value that must be used + }; +} +"#, + ); + } + + #[test] + fn match_arms_in_stmt_position() { + check_diagnostics( + r#" +#[must_use] +fn produces() -> i32 { 0 } +fn main() { + match 0 { + 0 => produces(), + //^^^^^^^^^^ warn: unused return value that must be used + _ => produces(), + //^^^^^^^^^^ warn: unused return value that must be used + }; +} +"#, + ); + } + + #[test] + fn const_block_in_stmt_position() { + check_diagnostics( + r#" +#[must_use] +const fn produces() -> i32 { 0 } +fn main() { + const { + produces() + //^^^^^^^^^^ warn: unused return value that must be used + }; +} +"#, + ); + } + + #[test] + fn must_use_type_through_block() { + check_diagnostics( + r#" +#[must_use] +struct Important; +fn produces() -> Important { Important } +fn main() { + { + produces() + //^^^^^^^^^^ warn: unused return value that must be used + }; +} +"#, + ); + } } |