Unnamed repository; edit this file 'description' to name the repository.
feat(diagnostics): add handler for E0040
Co-authored-by: WaterWhisperer <[email protected]>
| -rw-r--r-- | crates/hir-ty/src/infer.rs | 10 | ||||
| -rw-r--r-- | crates/hir-ty/src/infer/path.rs | 10 | ||||
| -rw-r--r-- | crates/hir-ty/src/lib.rs | 7 | ||||
| -rw-r--r-- | crates/hir-ty/src/method_resolution/confirm.rs | 6 | ||||
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 30 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/explicit_drop_method_use.rs | 426 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 2 |
7 files changed, 483 insertions, 8 deletions
diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 51e9435c80..86a4f613b3 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -478,6 +478,16 @@ pub enum InferenceDiagnostic { found: StoredTy, }, SolverDiagnostic(SolverDiagnostic), + ExplicitDropMethodUse { + #[type_visitable(ignore)] + kind: ExplicitDropMethodUseKind, + }, +} + +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum ExplicitDropMethodUseKind { + MethodCall(ExprId), + Path(ExprOrPatId), } /// Represents coercing a value to a different type of value. diff --git a/crates/hir-ty/src/infer/path.rs b/crates/hir-ty/src/infer/path.rs index 1c3d93ae6e..0ec72edc3d 100644 --- a/crates/hir-ty/src/infer/path.rs +++ b/crates/hir-ty/src/infer/path.rs @@ -11,7 +11,7 @@ use rustc_type_ir::inherent::{SliceLike, Ty as _}; use stdx::never; use crate::{ - InferenceDiagnostic, Span, ValueTyDefId, + ExplicitDropMethodUseKind, InferenceDiagnostic, Span, ValueTyDefId, infer::{ InferenceTyLoweringVarsCtx, diagnostics::InferenceTyLoweringContext as TyLoweringContext, }, @@ -33,6 +33,14 @@ impl<'db> InferenceContext<'_, 'db> { ) -> Option<(ValueNs, Ty<'db>)> { let (value, self_subst) = self.resolve_value_path_inner(path, id, false)?; + if let ValueNs::FunctionId(f) = value + && self.lang_items.Drop_drop.is_some_and(|drop_fn| drop_fn == f) + { + self.push_diagnostic(InferenceDiagnostic::ExplicitDropMethodUse { + kind: ExplicitDropMethodUseKind::Path(id), + }); + } + let (value_def, generic_def, substs) = match self.resolve_value_path(path, id, value, self_subst)? { ValuePathResolution::GenericDef(value_def, generic_def, substs) => { diff --git a/crates/hir-ty/src/lib.rs b/crates/hir-ty/src/lib.rs index 91e3b85aa1..1900a41f7f 100644 --- a/crates/hir-ty/src/lib.rs +++ b/crates/hir-ty/src/lib.rs @@ -104,9 +104,10 @@ use crate::{ pub use autoderef::autoderef; pub use infer::{ - Adjust, Adjustment, AutoBorrow, BindingMode, ByRef, InferenceDiagnostic, InferenceResult, - InferenceTyDiagnosticSource, OverloadedDeref, PointerCast, cast::CastError, could_coerce, - could_unify, could_unify_deeply, infer_query_with_inspect, + Adjust, Adjustment, AutoBorrow, BindingMode, ByRef, ExplicitDropMethodUseKind, + InferenceDiagnostic, InferenceResult, InferenceTyDiagnosticSource, OverloadedDeref, + PointerCast, cast::CastError, could_coerce, could_unify, could_unify_deeply, + infer_query_with_inspect, }; pub use lower::{ GenericDefaults, GenericDefaultsRef, GenericPredicates, ImplTraits, LifetimeElisionKind, diff --git a/crates/hir-ty/src/method_resolution/confirm.rs b/crates/hir-ty/src/method_resolution/confirm.rs index c425e69dc5..d960a6547d 100644 --- a/crates/hir-ty/src/method_resolution/confirm.rs +++ b/crates/hir-ty/src/method_resolution/confirm.rs @@ -18,7 +18,7 @@ use crate::{ Adjust, Adjustment, AutoBorrow, IncorrectGenericsLenKind, InferenceDiagnostic, LifetimeElisionKind, PointerCast, Span, db::HirDatabase, - infer::{AllowTwoPhase, AutoBorrowMutability, InferenceContext}, + infer::{AllowTwoPhase, AutoBorrowMutability, ExplicitDropMethodUseKind, InferenceContext}, lower::{ GenericPredicates, path::{GenericArgsLowerer, TypeLikeConst, substs_from_args_and_bindings}, @@ -582,7 +582,9 @@ impl<'a, 'b, 'db> ConfirmContext<'a, 'b, 'db> { fn check_for_illegal_method_calls(&self) { // Disallow calls to the method `drop` defined in the `Drop` trait. if self.ctx.lang_items.Drop_drop.is_some_and(|drop_fn| drop_fn == self.candidate) { - // FIXME: Report an error. + self.ctx.push_diagnostic(InferenceDiagnostic::ExplicitDropMethodUse { + kind: ExplicitDropMethodUseKind::MethodCall(self.call_expr), + }); } } diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index f4a29a4bcc..a399df8276 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -15,8 +15,9 @@ use hir_def::{ }; use hir_expand::{HirFileId, InFile, mod_path::ModPath, name::Name}; use hir_ty::{ - CastError, InferenceDiagnostic, InferenceTyDiagnosticSource, ParamEnvAndCrate, - PathGenericsSource, PathLoweringDiagnostic, TyLoweringDiagnostic, TyLoweringDiagnosticKind, + CastError, ExplicitDropMethodUseKind, InferenceDiagnostic, InferenceTyDiagnosticSource, + ParamEnvAndCrate, PathGenericsSource, PathLoweringDiagnostic, TyLoweringDiagnostic, + TyLoweringDiagnosticKind, db::HirDatabase, diagnostics::{BodyValidationDiagnostic, UnsafetyReason}, display::{DisplayTarget, HirDisplay}, @@ -106,6 +107,7 @@ diagnostics![AnyDiagnostic<'db> -> CastToUnsized<'db>, ExpectedArrayOrSlicePat<'db>, ExpectedFunction<'db>, + ExplicitDropMethodUse, FruInDestructuringAssignment, FunctionalRecordUpdateOnNonStruct, GenericDefaultRefersToSelf, @@ -326,6 +328,11 @@ pub struct CannotBeDereferenced<'db> { } #[derive(Debug)] +pub struct ExplicitDropMethodUse { + pub expr_or_path: Either<InFile<AstPtr<ast::MethodCallExpr>>, InFile<AstPtr<ast::Path>>>, +} + +#[derive(Debug)] pub struct FruInDestructuringAssignment { pub node: InFile<AstPtr<ast::Expr>>, } @@ -1044,6 +1051,25 @@ impl<'db> AnyDiagnostic<'db> { let span = span_syntax(d.span)?; Self::solver_diagnostic(db, &d.kind, span, env)? } + InferenceDiagnostic::ExplicitDropMethodUse { kind } => { + let expr_or_path = match kind { + ExplicitDropMethodUseKind::MethodCall(expr) => { + let expr = expr_syntax(*expr)?; + let expr = expr.with_value(expr.value.cast::<ast::MethodCallExpr>()?); + Either::Left(expr) + } + ExplicitDropMethodUseKind::Path(path_expr_id) => { + let syntax = expr_or_pat_syntax(*path_expr_id)?; + let file_id = syntax.file_id; + let syntax = + syntax.with_value(syntax.value.cast::<ast::PathExpr>()?).to_node(db); + let path = syntax.path()?; + let path = InFile::new(file_id, AstPtr::new(&path)); + Either::Right(path) + } + }; + ExplicitDropMethodUse { expr_or_path }.into() + } }) } diff --git a/crates/ide-diagnostics/src/handlers/explicit_drop_method_use.rs b/crates/ide-diagnostics/src/handlers/explicit_drop_method_use.rs new file mode 100644 index 0000000000..b02bccf5a5 --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/explicit_drop_method_use.rs @@ -0,0 +1,426 @@ +use either::Either; +use hir::InFile; +use ide_db::assists::Assist; +use ide_db::source_change::{SourceChange, SourceChangeBuilder}; +use ide_db::text_edit::TextEdit; +use itertools::Itertools; +use syntax::{ + AstNode, AstPtr, + ast::{self, HasArgList}, +}; + +use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, adjusted_display_range, fix}; + +// Diagnostic: explicit-drop-method-use +// +// This diagnostic is triggered when the `Drop::drop` method is called (or named) explicitly. +pub(crate) fn explicit_drop_method_use( + ctx: &DiagnosticsContext<'_, '_>, + d: &hir::ExplicitDropMethodUse, +) -> Diagnostic { + match d.expr_or_path { + Either::Left(expr) => { + let display_range = adjusted_display_range(ctx, expr, &|node| { + Some(node.name_ref()?.syntax().text_range()) + }); + Diagnostic::new( + DiagnosticCode::RustcHardError("E0040"), + "explicit use of destructor method", + display_range, + ) + .stable() + .with_main_node(expr.map(Into::into)) + .with_fixes(fix_method_call(ctx, expr)) + } + Either::Right(path) => Diagnostic::new_with_syntax_node_ptr( + ctx, + DiagnosticCode::RustcHardError("E0040"), + "explicit use of destructor method", + path.map(Into::into), + ) + .stable() + .with_fixes(fix_path(ctx, path)), + } +} + +fn fix_method_call( + ctx: &DiagnosticsContext<'_, '_>, + mcall_ptr: InFile<AstPtr<ast::MethodCallExpr>>, +) -> Option<Vec<Assist>> { + if mcall_ptr.file_id.is_macro() { + // TODO: handle macro calls. Rough plan: + // 1. upmap the range of the receiver and the range of the whole call + // 2. delete everything outside the receiver and replace it with `drop(...)`, using range edits only. + return None; + } + + let db = ctx.db(); + + let file_id = mcall_ptr.file_id; + let mcall = mcall_ptr.to_node(db); + let range = mcall.syntax().text_range(); + + // `mcall` is `foo.drop()` -- extract the receiver, and wrap it in `drop()` + // NOTE: it could theoretically be `(&mut foo).drop()` instead, in which case the fix + // below would be incorrect, as it'd result in `drop((&mut foo))` instead of `drop(foo)` + // -- but we don't bother to deal with that case. + let recv = mcall.receiver()?; + + let mut builder = SourceChangeBuilder::new(file_id.original_file(db).file_id(db)); + let editor = builder.make_editor(mcall.syntax()); + let make = editor.make(); + let new_call = + make.expr_call(make.expr_path(make.path_from_text("drop")), make.arg_list([recv])); + builder.replace_ast(ast::Expr::MethodCallExpr(mcall), ast::Expr::CallExpr(new_call)); + let source_change = builder.finish(); + Some(vec![fix("use-drop-function", "Use `drop` function", source_change, range)]) +} + +fn fix_path( + ctx: &DiagnosticsContext<'_, '_>, + path_ptr: InFile<AstPtr<ast::Path>>, +) -> Option<Vec<Assist>> { + let db = ctx.db(); + + let file_id = path_ptr.file_id; + let path = path_ptr.to_node(db); + + if let Some(call) = + path.syntax().parent().and_then(|it| it.parent()).and_then(ast::CallExpr::cast) + { + if file_id.is_macro() { + // TODO: make this work in macros? Might not be worth it, as this is a niche way to trigger this + // already niche error + return None; + } + + // `call` is `Drop::drop(&mut foo)` -- extract the arg, and wrap it in `drop()` + let arg_list = call.arg_list()?; + let ref_recv = arg_list.args().exactly_one().ok()?; + let ast::Expr::RefExpr(ref_recv) = ref_recv else { + return None; + }; + let recv = ref_recv.expr()?; + + let range = call.syntax().text_range(); + + let mut builder = SourceChangeBuilder::new(file_id.original_file(db).file_id(db)); + let editor = builder.make_editor(call.syntax()); + let make = editor.make(); + let new_call = + make.expr_call(make.expr_path(make.path_from_text("drop")), make.arg_list([recv])); + builder.replace_ast(call, new_call); + let source_change = builder.finish(); + Some(vec![fix("use-drop-function", "Use `drop` function", source_change, range)]) + } else { + // `path` could be the `Foo::drop` in `let d = Foo::drop;` + // -- replace the path with `drop` + + let range = InFile::new(file_id, path.syntax().text_range()) + .original_node_file_range_rooted_opt(db)?; + + let edit = TextEdit::replace(range.range, "drop".to_owned()); + let source_change = SourceChange::from_text_edit(range.file_id.file_id(db), edit); + Some(vec![fix("use-drop-function", "Use `drop` function", source_change, range.range)]) + } +} + +#[cfg(test)] +mod tests { + use crate::tests::{ + check_diagnostics, check_diagnostics_with_disabled, check_fix, check_fix_with_disabled, + }; + + #[test] + fn method_call_diagnostic() { + check_diagnostics( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + a.drop(); + // ^^^^ 💡 error: explicit use of destructor method +} +"#, + ); + } + + #[test] + fn method_call_fix() { + check_fix( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + a.drop$0(); +} +"#, + r#" +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + drop(a); +} +"#, + ); + } + + #[test] + fn qualified_call_1_diagnostic() { + check_diagnostics( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + A::drop(&mut a); + // ^^^^^^^ 💡 error: explicit use of destructor method +} +"#, + ); + } + + #[test] + fn qualified_call_1_fix() { + check_fix( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + A::drop(&mut a$0); +} +"#, + r#" +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + drop(a); +} +"#, + ) + } + + #[test] + fn qualified_call_2_diagnostic() { + check_diagnostics( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + Drop::drop(&mut a); + // ^^^^^^^^^^ 💡 error: explicit use of destructor method +} +"#, + ); + } + + #[test] + fn qualified_call_2_fix() { + check_fix( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + Drop::drop(&mut a$0); +} +"#, + r#" +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + drop(a); +} +"#, + ) + } + + #[test] + fn fully_qualified_call_diagnostic() { + check_diagnostics( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + <A as Drop>::drop(&mut a); + // ^^^^^^^^^^^^^^^^^ 💡 error: explicit use of destructor method +} +"#, + ); + } + + #[test] + fn fully_qualified_call_fix() { + check_fix( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + <A as Drop>::drop(&mut a$0); +} +"#, + r#" +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + drop(a); +} +"#, + ) + } + + #[test] + fn path_diagnostic() { + check_diagnostics_with_disabled( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + let d = A::drop; + // ^^^^^^^ 💡 error: explicit use of destructor method + d(&mut a); +} +"#, + // Because of the error, the code isn't analyzed further (?), and so `d` is warned on as unused. + // Arguably a bug in r-a (rustc doesn't emit a warning in this case) + // FIXME: remove this once r-a no longer warns + &["unused_variables"], + ); + } + + #[test] + // NOTE: Here, the fix is not completely correct, as it doesn't replace `d(&mut a)` with `d(a)`. + // Oh well, rustc doesn't either + fn path_fix() { + check_fix_with_disabled( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + let d = A::drop$0; + d(&mut a); +} +"#, + r#" +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + let d = drop; + d(&mut a); +} +"#, + // Because of the error, the code isn't analyzed further (?), and so `d` is warned on as unused. + // Arguably a bug in r-a (rustc doesn't emit a warning in this case) + // FIXME: remove this once r-a no longer warns + &["unused_variables"], + ); + } + + #[test] + // NOTE: Here, the fix is not completely correct, as it doesn't replace `d(&mut a)` with `d(a)`. + // Oh well, rustc doesn't either + fn path_fix_in_macro() { + check_fix( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +macro_rules! main { + ($e:expr) => { + fn main() { $e } + } +} + +main!{{ + let mut a = A; + let d = A::drop$0; + d(&mut a); +}}; +"#, + r#" +struct A; +impl Drop for A { fn drop(&mut self) {} } + +macro_rules! main { + ($e:expr) => { + fn main() { $e } + } +} + +main!{{ + let mut a = A; + let d = drop; + d(&mut a); +}}; +"#, + ); + } + + #[test] + fn std_mem_drop() { + check_diagnostics( + r#" +//- minicore: drop +struct A; +impl Drop for A { fn drop(&mut self) {} } + +fn main(a: A) { + drop(a); +} +"#, + ); + } + + #[test] + fn inherent_drop_method() { + check_diagnostics( + r#" +struct A; +impl A { fn drop(&mut self) {} } + +fn main(mut a: A) { + a.drop(); +} +"#, + ); + } + + #[test] + fn custom_trait_drop_method() { + check_diagnostics( + r#" +struct A; +trait MyDrop { fn drop(&mut self); } +impl MyDrop for A { fn drop(&mut self) {} } + +fn main(mut a: A) { + a.drop(); +} +"#, + ); + } +} diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index d38780ede2..c8972316f9 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -37,6 +37,7 @@ mod handlers { pub(crate) mod elided_lifetimes_in_path; pub(crate) mod expected_array_or_slice_pat; pub(crate) mod expected_function; + pub(crate) mod explicit_drop_method_use; pub(crate) mod fru_in_destructuring_assignment; pub(crate) mod functional_record_update_on_non_struct; pub(crate) mod generic_args_prohibited; @@ -535,6 +536,7 @@ pub fn semantic_diagnostics( AnyDiagnostic::UnionExprMustHaveExactlyOneField(d) => handlers::union_expr_must_have_exactly_one_field::union_expr_must_have_exactly_one_field(&ctx, &d), AnyDiagnostic::UnimplementedTrait(d) => handlers::unimplemented_trait::unimplemented_trait(&ctx, &d), AnyDiagnostic::FruInDestructuringAssignment(d) => handlers::fru_in_destructuring_assignment::fru_in_destructuring_assignment(&ctx, &d), + AnyDiagnostic::ExplicitDropMethodUse(d) => handlers::explicit_drop_method_use::explicit_drop_method_use(&ctx, &d), }; res.push(d) } |