Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #22239 from MavenRain/feat-unused-must-use-diagnostic
hir-ty: emit diagnostic for unused #[must_use] values
| -rw-r--r-- | crates/hir-def/src/attrs.rs | 4 | ||||
| -rw-r--r-- | crates/hir-ty/src/diagnostics/expr.rs | 137 | ||||
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 11 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/unused_must_use.rs | 110 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 2 | ||||
| -rw-r--r-- | crates/intern/src/symbol/symbols.rs | 1 |
6 files changed, 224 insertions, 41 deletions
diff --git a/crates/hir-def/src/attrs.rs b/crates/hir-def/src/attrs.rs index 5dc410be27..7757d537e6 100644 --- a/crates/hir-def/src/attrs.rs +++ b/crates/hir-def/src/attrs.rs @@ -137,6 +137,7 @@ fn match_attr_flags(attr_flags: &mut AttrFlags, attr: ast::Meta) -> ControlFlow< "deprecated" => attr_flags.insert(AttrFlags::IS_DEPRECATED), "ignore" => attr_flags.insert(AttrFlags::IS_IGNORE), "lang" => attr_flags.insert(AttrFlags::LANG_ITEM), + "must_use" => attr_flags.insert(AttrFlags::IS_MUST_USE), "path" => attr_flags.insert(AttrFlags::HAS_PATH), "unstable" => attr_flags.insert(AttrFlags::IS_UNSTABLE), "export_name" => { @@ -227,6 +228,7 @@ fn match_attr_flags(attr_flags: &mut AttrFlags, attr: ast::Meta) -> ControlFlow< "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), "no_mangle" => attr_flags.insert(AttrFlags::NO_MANGLE), "pointee" => attr_flags.insert(AttrFlags::IS_POINTEE), "non_exhaustive" => attr_flags.insert(AttrFlags::NON_EXHAUSTIVE), @@ -335,6 +337,8 @@ bitflags::bitflags! { const MACRO_STYLE_PARENTHESES = 1 << 48; const PREFER_UNDERSCORE_IMPORT = 1 << 49; + + const IS_MUST_USE = 1 << 50; } } diff --git a/crates/hir-ty/src/diagnostics/expr.rs b/crates/hir-ty/src/diagnostics/expr.rs index 93772ca452..be29926a38 100644 --- a/crates/hir-ty/src/diagnostics/expr.rs +++ b/crates/hir-ty/src/diagnostics/expr.rs @@ -7,7 +7,9 @@ use std::fmt; use base_db::Crate; use either::Either; use hir_def::{ - AdtId, AssocItemId, DefWithBodyId, HasModule, ItemContainerId, Lookup, + AdtId, AssocItemId, AttrDefId, CallableDefId, DefWithBodyId, HasModule, ItemContainerId, + Lookup, + attrs::AttrFlags, lang_item::LangItems, resolver::{HasResolver, ValueNs}, }; @@ -33,7 +35,7 @@ use crate::{ }, display::{DisplayTarget, HirDisplay}, next_solver::{ - DbInterner, ParamEnv, Ty, TyKind, TypingMode, + CallableIdWrapper, DbInterner, ParamEnv, Ty, TyKind, TypingMode, infer::{DbInternerInferExt, InferCtxt}, }, }; @@ -67,6 +69,9 @@ pub enum BodyValidationDiagnostic { RemoveUnnecessaryElse { if_expr: ExprId, }, + UnusedMustUse { + expr: ExprId, + }, } impl BodyValidationDiagnostic { @@ -328,52 +333,71 @@ 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 &Statement::Let { pat, initializer, else_branch: None, .. } = stmt else { - continue; + let diag = match *stmt { + Statement::Expr { expr: stmt_expr, has_semi: true } if self.validate_lints => { + self.check_unused_must_use(stmt_expr) + } + Statement::Let { pat, initializer, else_branch: None, .. } => { + self.check_non_exhaustive_let(&cx, &pattern_arena, pat, initializer) + } + _ => None, }; - if self.infer.type_mismatch_for_pat(pat).is_some() { - continue; - } - let Some(initializer) = initializer else { continue }; - let Some(ty) = self.infer.type_of_expr_with_adjust(initializer) else { continue }; - if ty.references_non_lt_error() { - continue; + if let Some(diag) = diag { + self.diagnostics.push(diag); } + } + } - let mut have_errors = false; - let deconstructed_pat = self.lower_pattern(&cx, pat, &mut have_errors); + fn check_non_exhaustive_let<'a>( + &self, + cx: &MatchCheckCtx<'a, 'db>, + pattern_arena: &'a Arena<DeconstructedPat<'a, 'db>>, + pat: PatId, + initializer: Option<ExprId>, + ) -> Option<BodyValidationDiagnostic> { + if self.infer.type_mismatch_for_pat(pat).is_some() { + return None; + } + let initializer = initializer?; + let ty = self.infer.type_of_expr_with_adjust(initializer)?; + if ty.references_non_lt_error() { + return None; + } - // optimization, wildcard trivially hold - if have_errors || matches!(deconstructed_pat.ctor(), Constructor::Wildcard) { - continue; - } + let mut have_errors = false; + let deconstructed_pat = self.lower_pattern(cx, pat, &mut have_errors); - let match_arm = rustc_pattern_analysis::MatchArm { - pat: pattern_arena.alloc(deconstructed_pat), - has_guard: false, - arm_data: (), - }; - let report = match cx.compute_match_usefulness(&[match_arm], ty, None) { - Ok(v) => v, - Err(e) => { - debug!(?e, "match usefulness error"); - continue; - } - }; - let witnesses = report.non_exhaustiveness_witnesses; - if !witnesses.is_empty() { - self.diagnostics.push(BodyValidationDiagnostic::NonExhaustiveLet { - pat, - uncovered_patterns: missing_match_arms( - &cx, - ty, - witnesses, - false, - self.owner.krate(self.db()), - ), - }); + // optimization, wildcard trivially hold + if have_errors || matches!(deconstructed_pat.ctor(), Constructor::Wildcard) { + return None; + } + + let match_arm = rustc_pattern_analysis::MatchArm { + pat: pattern_arena.alloc(deconstructed_pat), + has_guard: false, + arm_data: (), + }; + let report = match cx.compute_match_usefulness(&[match_arm], ty, None) { + Ok(v) => v, + Err(e) => { + debug!(?e, "match usefulness error"); + return None; } + }; + let witnesses = report.non_exhaustiveness_witnesses; + if witnesses.is_empty() { + return None; } + Some(BodyValidationDiagnostic::NonExhaustiveLet { + pat, + uncovered_patterns: missing_match_arms( + cx, + ty, + witnesses, + false, + self.owner.krate(self.db()), + ), + }) } fn lower_pattern<'a>( @@ -391,6 +415,37 @@ impl<'db> ExprValidator<'db> { pattern } + fn check_unused_must_use(&self, expr: ExprId) -> Option<BodyValidationDiagnostic> { + let db = self.db(); + let must_use_fn = match &self.body[expr] { + Expr::Call { callee, .. } => { + let callee_ty = self.infer.expr_ty(*callee); + if let TyKind::FnDef(CallableIdWrapper(CallableDefId::FunctionId(func)), _) = + callee_ty.kind() + { + AttrFlags::query(db, AttrDefId::FunctionId(func)) + .contains(AttrFlags::IS_MUST_USE) + } else { + false + } + } + Expr::MethodCall { .. } => { + self.infer.method_resolution(expr).is_some_and(|(func, _)| { + AttrFlags::query(db, AttrDefId::FunctionId(func)) + .contains(AttrFlags::IS_MUST_USE) + }) + } + _ => return None, + }; + let must_use_ty = + self.infer.type_of_expr_with_adjust(expr).is_some_and(|ty| match ty.kind() { + TyKind::Adt(adt, _) => AttrFlags::query(db, AttrDefId::AdtId(adt.def_id())) + .contains(AttrFlags::IS_MUST_USE), + _ => false, + }); + (must_use_fn || must_use_ty).then_some(BodyValidationDiagnostic::UnusedMustUse { expr }) + } + fn check_for_trailing_return(&mut self, body_expr: ExprId, body: &Body) { if !self.validate_lints { return; diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 082c29c174..3259abb536 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -86,6 +86,7 @@ diagnostics![AnyDiagnostic<'db> -> PrivateField, RemoveTrailingReturn, RemoveUnnecessaryElse, + UnusedMustUse, ReplaceFilterMapNextWithFindMap, TraitImplIncorrectSafety, TraitImplMissingAssocItems, @@ -398,6 +399,11 @@ pub struct RemoveUnnecessaryElse { } #[derive(Debug)] +pub struct UnusedMustUse { + pub expr: InFile<ExprOrPatPtr>, +} + +#[derive(Debug)] pub struct CastToUnsized<'db> { pub expr: InFile<ExprOrPatPtr>, pub cast_ty: Type<'db>, @@ -628,6 +634,11 @@ impl<'db> AnyDiagnostic<'db> { ); } } + BodyValidationDiagnostic::UnusedMustUse { expr } => { + if let Ok(source_ptr) = source_map.expr_syntax(expr) { + return Some(UnusedMustUse { expr: source_ptr }.into()); + } + } } None } diff --git a/crates/ide-diagnostics/src/handlers/unused_must_use.rs b/crates/ide-diagnostics/src/handlers/unused_must_use.rs new file mode 100644 index 0000000000..4b9ecf2169 --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/unused_must_use.rs @@ -0,0 +1,110 @@ +use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext}; + +// Diagnostic: unused-must-use +// +// This diagnostic is triggered when a value with the `#[must_use]` attribute +// is dropped without being used. +pub(crate) fn unused_must_use( + ctx: &DiagnosticsContext<'_, '_>, + d: &hir::UnusedMustUse, +) -> Diagnostic { + Diagnostic::new_with_syntax_node_ptr( + ctx, + DiagnosticCode::RustcLint("unused_must_use"), + "unused return value that must be used", + d.expr.map(Into::into), + ) + .stable() +} + +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn unused_must_use_function_call() { + check_diagnostics( + r#" +#[must_use] +fn produces() -> i32 { 0 } +fn main() { + produces(); + //^^^^^^^^^^ warn: unused return value that must be used +} +"#, + ); + } + + #[test] + fn unused_must_use_method_call() { + check_diagnostics( + r#" +struct S; +impl S { + #[must_use] + fn produces(&self) -> i32 { 0 } +} +fn main() { + let s = S; + s.produces(); + //^^^^^^^^^^^^ warn: unused return value that must be used +} +"#, + ); + } + + #[test] + fn unused_must_use_type() { + check_diagnostics( + r#" +#[must_use] +struct Important; +fn produces() -> Important { Important } +fn main() { + produces(); + //^^^^^^^^^^ warn: unused return value that must be used +} +"#, + ); + } + + #[test] + fn no_warning_when_value_used() { + check_diagnostics( + r#" +#[must_use] +fn produces() -> i32 { 0 } +fn main() { + let _x = produces(); +} +"#, + ); + } + + #[test] + fn no_warning_when_no_must_use_attribute() { + check_diagnostics( + r#" +fn ordinary() -> i32 { 0 } +fn main() { + ordinary(); +} +"#, + ); + } + + #[test] + fn no_warning_when_value_assigned() { + check_diagnostics( + r#" +#[must_use] +fn produces() -> i32 { 0 } +fn main() { + let x; + x = produces(); + let _ = x; +} +"#, + ); + } +} diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index 618edc5c61..1d5811954c 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -82,6 +82,7 @@ mod handlers { pub(crate) mod unresolved_macro_call; pub(crate) mod unresolved_method; pub(crate) mod unresolved_module; + pub(crate) mod unused_must_use; pub(crate) mod unused_variables; // The handlers below are unusual, the implement the diagnostics as well. @@ -461,6 +462,7 @@ pub fn semantic_diagnostics( AnyDiagnostic::UnresolvedMacroCall(d) => handlers::unresolved_macro_call::unresolved_macro_call(&ctx, &d), AnyDiagnostic::UnresolvedMethodCall(d) => handlers::unresolved_method::unresolved_method(&ctx, &d), AnyDiagnostic::UnresolvedModule(d) => handlers::unresolved_module::unresolved_module(&ctx, &d), + AnyDiagnostic::UnusedMustUse(d) => handlers::unused_must_use::unused_must_use(&ctx, &d), AnyDiagnostic::UnusedMut(d) => match handlers::mutability_errors::unused_mut(&ctx, &d) { Some(it) => it, None => continue, diff --git a/crates/intern/src/symbol/symbols.rs b/crates/intern/src/symbol/symbols.rs index c0053a3f21..db3c3c5200 100644 --- a/crates/intern/src/symbol/symbols.rs +++ b/crates/intern/src/symbol/symbols.rs @@ -332,6 +332,7 @@ define_symbols! { module_path, mul_assign, mul, + must_use, naked_asm, ne, neg, |