Skip to content

hir-ty: emit diagnostic for unused #[must_use] values#22239

Merged
ChayimFriedman2 merged 1 commit into
rust-lang:masterfrom
MavenRain:feat-unused-must-use-diagnostic
May 2, 2026
Merged

hir-ty: emit diagnostic for unused #[must_use] values#22239
ChayimFriedman2 merged 1 commit into
rust-lang:masterfrom
MavenRain:feat-unused-must-use-diagnostic

Conversation

@MavenRain

Copy link
Copy Markdown
Contributor

Adds the unused_must_use lint that warns when the result of a call or method call is dropped despite the callee or its return type carrying #[must_use].

  • intern: new must_use symbol.
  • hir-def: new AttrFlags::IS_MUST_USE flag, recognised on both #[must_use] (PathMeta) and #[must_use = "reason"] (KeyValueMeta).
  • hir-ty: new BodyValidationDiagnostic::UnusedMustUse variant. ExprValidator now walks Statement::Expr-with-semi inside blocks and unsafe blocks, checking the callee FunctionId, the resolved method FunctionId, and the result type's AdtId against IS_MUST_USE.
  • hir + ide-diagnostics: cooked struct, conversion, handler with RustcLint("unused_must_use").
  • ide-diagnostics tests: function call, method call, ADT type positives; value used, no attribute, value assigned negatives.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 1, 2026

@ChayimFriedman2 ChayimFriedman2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool! Excited for this!

View changes since this review

"unstable" => attr_flags.insert(AttrFlags::IS_UNSTABLE),
"deprecated" => attr_flags.insert(AttrFlags::IS_DEPRECATED),
"macro_export" => attr_flags.insert(AttrFlags::IS_MACRO_EXPORT),
"must_use" => attr_flags.insert(AttrFlags::IS_MUST_USE),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A possible enhancement (not necessarily in this PR) is to also have a query to fetch the must_use reason, like other queries in this file.

Comment thread crates/hir-ty/src/diagnostics/expr.rs Outdated
Comment on lines +333 to +339
if self.validate_lints {
for stmt in &**statements {
if let &Statement::Expr { expr: stmt_expr, has_semi: true } = stmt {
self.check_unused_must_use(stmt_expr);
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of looping over the expressions twice, please combine them with the run below in this function. It might make sense to extract the existing handling to a function.

MavenRain added a commit to MavenRain/rust-analyzer that referenced this pull request May 1, 2026
  Address review feedback on rust-lang#22239: combine the must-use loop and the
  non-exhaustive-let loop into a single pass over `statements`, and lift
  the existing handling into a dedicated `check_non_exhaustive_let`
  method.

  `check_unused_must_use` and `check_non_exhaustive_let` now return
  `Option<BodyValidationDiagnostic>`; the caller pushes the result. This
  keeps `&self.infcx` (held by `MatchCheckCtx`) compatible with the
  mutable borrow on `self.diagnostics` via field-level split borrows.
@MavenRain MavenRain force-pushed the feat-unused-must-use-diagnostic branch from d91195b to 62c49de Compare May 1, 2026 13:32
@rustbot

This comment has been minimized.

@MavenRain

Copy link
Copy Markdown
Contributor Author

Addressed the expression looping . . . let me know if adequate

@ChayimFriedman2 ChayimFriedman2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix Clippy and the issue link.

View changes since this review

MavenRain added a commit to MavenRain/rust-analyzer that referenced this pull request May 1, 2026
  Address review feedback on rust-lang#22239: combine the must-use loop and the
  non-exhaustive-let loop into a single pass over `statements`, and lift
  the existing handling into a dedicated `check_non_exhaustive_let`
  method.

  `check_unused_must_use` and `check_non_exhaustive_let` now return
  `Option<BodyValidationDiagnostic>`; the caller pushes the result. This
  keeps `&self.infcx` (held by `MatchCheckCtx`) compatible with the
  mutable borrow on `self.diagnostics` via field-level split borrows.
@MavenRain MavenRain force-pushed the feat-unused-must-use-diagnostic branch from 62c49de to 27d5f30 Compare May 1, 2026 17:55
@rustbot

This comment has been minimized.

Address review feedback on rust-lang#22239: combine the
must-use loop and the non-exhaustive-let loop into a single pass over
`statements`, and lift the existing handling into a dedicated
`check_non_exhaustive_let` method.

`check_unused_must_use` and `check_non_exhaustive_let` now return
`Option<BodyValidationDiagnostic>`; the caller pushes the result. This
keeps `&self.infcx` (held by `MatchCheckCtx`) compatible with the
mutable borrow on `self.diagnostics` via field-level split borrows.
@MavenRain MavenRain force-pushed the feat-unused-must-use-diagnostic branch from 27d5f30 to 9f186ad Compare May 1, 2026 22:52

@ChayimFriedman2 ChayimFriedman2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue May 2, 2026
Merged via the queue into rust-lang:master with commit e7c87b4 May 2, 2026
18 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2026
lnicola pushed a commit to lnicola/rust that referenced this pull request May 4, 2026
Address review feedback on rust-lang/rust-analyzer#22239: combine the
must-use loop and the non-exhaustive-let loop into a single pass over
`statements`, and lift the existing handling into a dedicated
`check_non_exhaustive_let` method.

`check_unused_must_use` and `check_non_exhaustive_let` now return
`Option<BodyValidationDiagnostic>`; the caller pushes the result. This
keeps `&self.infcx` (held by `MatchCheckCtx`) compatible with the
mutable borrow on `self.diagnostics` via field-level split borrows.
@MavenRain MavenRain deleted the feat-unused-must-use-diagnostic branch May 15, 2026 09:02
MavenRain added a commit to MavenRain/rust-analyzer that referenced this pull request Jun 7, 2026
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#22239.

Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
lnicola pushed a commit to lnicola/rust that referenced this pull request Jul 13, 2026
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 <softwareengineerasaservant@isurvivable.cv>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants