From 69410bb48857ecfebc6056d25b11ad6cd6f0cef4 Mon Sep 17 00:00:00 2001 From: Young-Flash <871946895@qq.com> Date: Tue, 12 Dec 2023 20:24:33 +0800 Subject: [PATCH 1/3] feat: add assoc func quickfix for `unresolved_method` diagnostic --- crates/hir-ty/src/infer.rs | 1 + crates/hir-ty/src/infer/expr.rs | 19 +++ crates/hir/src/diagnostics.rs | 3 +- crates/hir/src/lib.rs | 2 + .../src/handlers/unresolved_method.rs | 123 ++++++++++++++++-- 5 files changed, 139 insertions(+), 9 deletions(-) diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index e295dd8d4e10a..428cedbb49be2 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -217,6 +217,7 @@ pub enum InferenceDiagnostic { name: Name, /// Contains the type the field resolves to field_with_same_name: Option, + assoc_func_with_same_name: Option, }, // FIXME: This should be emitted in body lowering BreakOutsideOfLoop { diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index a5e77a12d8c50..84954ca7e9043 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -1575,11 +1575,30 @@ impl InferenceContext<'_> { } None => None, }; + + let assoc_func_with_same_name = method_resolution::iterate_method_candidates( + &canonicalized_receiver.value, + self.db, + self.table.trait_env.clone(), + self.get_traits_in_scope().as_ref().left_or_else(|&it| it), + VisibleFromModule::Filter(self.resolver.module()), + Some(method_name), + method_resolution::LookupMode::Path, + |_ty, item, visible| { + if visible { + Some(item) + } else { + None + } + }, + ); + self.result.diagnostics.push(InferenceDiagnostic::UnresolvedMethodCall { expr: tgt_expr, receiver: receiver_ty.clone(), name: method_name.clone(), field_with_same_name: field_with_same_name_exists, + assoc_func_with_same_name, }); ( receiver_ty, diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 1cb36f9b021fe..ba591e18921de 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -8,7 +8,7 @@ pub use hir_ty::diagnostics::{CaseType, IncorrectCase}; use base_db::CrateId; use cfg::{CfgExpr, CfgOptions}; use either::Either; -use hir_def::path::ModPath; +use hir_def::{path::ModPath, AssocItemId}; use hir_expand::{name::Name, HirFileId, InFile}; use syntax::{ast, AstPtr, SyntaxError, SyntaxNodePtr, TextRange}; @@ -215,6 +215,7 @@ pub struct UnresolvedMethodCall { pub receiver: Type, pub name: Name, pub field_with_same_name: Option, + pub assoc_func_with_same_name: Option, } #[derive(Debug)] diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 85762603ed158..60da8a185f836 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1680,6 +1680,7 @@ impl DefWithBody { receiver, name, field_with_same_name, + assoc_func_with_same_name, } => { let expr = expr_syntax(*expr); @@ -1691,6 +1692,7 @@ impl DefWithBody { field_with_same_name: field_with_same_name .clone() .map(|ty| Type::new(db, DefWithBodyId::from(self), ty)), + assoc_func_with_same_name: assoc_func_with_same_name.clone(), } .into(), ) diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs index 464b0a710ea7b..4adb7000dda48 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_method.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_method.rs @@ -1,11 +1,14 @@ -use hir::{db::ExpandDatabase, HirDisplay}; +use hir::{db::ExpandDatabase, AssocItem, HirDisplay}; use ide_db::{ assists::{Assist, AssistId, AssistKind}, base_db::FileRange, label::Label, source_change::SourceChange, }; -use syntax::{ast, AstNode, TextRange}; +use syntax::{ + ast::{self, make, HasArgList}, + AstNode, SmolStr, TextRange, +}; use text_edit::TextEdit; use crate::{adjusted_display_range_new, Diagnostic, DiagnosticCode, DiagnosticsContext}; @@ -17,15 +20,17 @@ pub(crate) fn unresolved_method( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall, ) -> Diagnostic { - let field_suffix = if d.field_with_same_name.is_some() { + let suffix = if d.field_with_same_name.is_some() { ", but a field with a similar name exists" + } else if d.assoc_func_with_same_name.is_some() { + ", but an associated function with a similar name exists" } else { "" }; Diagnostic::new( DiagnosticCode::RustcHardError("E0599"), format!( - "no method `{}` on type `{}`{field_suffix}", + "no method `{}` on type `{}`{suffix}", d.name.display(ctx.sema.db), d.receiver.display(ctx.sema.db) ), @@ -46,11 +51,27 @@ pub(crate) fn unresolved_method( } fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -> Option> { - if let Some(ty) = &d.field_with_same_name { + let field_fix = if let Some(ty) = &d.field_with_same_name { field_fix(ctx, d, ty) } else { // FIXME: add quickfix None + }; + + let assoc_func_fix = assoc_func_fix(ctx, d); + + let mut fixes = vec![]; + if let Some(field_fix) = field_fix { + fixes.push(field_fix); + } + if let Some(assoc_func_fix) = assoc_func_fix { + fixes.push(assoc_func_fix); + } + + if fixes.is_empty() { + None + } else { + Some(fixes) } } @@ -58,7 +79,7 @@ fn field_fix( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall, ty: &hir::Type, -) -> Option> { +) -> Option { if !ty.impls_fnonce(ctx.sema.db) { return None; } @@ -78,7 +99,7 @@ fn field_fix( } _ => return None, }; - Some(vec![Assist { + Some(Assist { id: AssistId("expected-method-found-field-fix", AssistKind::QuickFix), label: Label::new("Use parentheses to call the value of the field".to_string()), group: None, @@ -88,7 +109,93 @@ fn field_fix( (file_id, TextEdit::insert(range.end(), ")".to_owned())), ])), trigger_signature_help: false, - }]) + }) +} + +fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -> Option { + if let Some(assoc_item_id) = d.assoc_func_with_same_name { + let db = ctx.sema.db; + + let expr_ptr = &d.expr; + let root = db.parse_or_expand(expr_ptr.file_id); + let expr: ast::Expr = expr_ptr.value.to_node(&root); + + let call = ast::MethodCallExpr::cast(expr.syntax().clone())?; + let range = call.syntax().text_range(); + + let receiver = call.receiver()?; + let receiver_type = &ctx.sema.type_of_expr(&receiver)?.original; + + let need_to_take_receiver_as_first_arg = match hir::AssocItem::from(assoc_item_id) { + AssocItem::Function(f) => { + let assoc_fn_params = f.assoc_fn_params(db); + if assoc_fn_params.is_empty() { + false + } else { + assoc_fn_params + .first() + .map(|first_arg| { + // For generic type, say `Box`, take `Box::into_raw(b: Self)` as example, + // type of `b` is `Self`, which is `Box`, containing unspecified generics. + // However, type of `receiver` is specified, it could be `Box` or something like that, + // so `first_arg.ty() == receiver_type` evaluate to `false` here. + // Here add `first_arg.ty().as_adt() == receiver_type.as_adt()` as guard, + // apply `.as_adt()` over `Box` or `Box` gets `Box`, so we get `true` here. + + // FIXME: it fails when type of `b` is `Box` with other generic param different from `receiver` + first_arg.ty() == receiver_type + || first_arg.ty().as_adt() == receiver_type.as_adt() + }) + .unwrap_or(false) + } + } + _ => false, + }; + + let mut receiver_type_adt_name = receiver_type.as_adt()?.name(db).to_smol_str().to_string(); + + let generic_parameters: Vec = receiver_type.generic_parameters(db).collect(); + // if receiver should be pass as first arg in the assoc func, + // we could omit generic parameters cause compiler can deduce it automatically + if !need_to_take_receiver_as_first_arg && !generic_parameters.is_empty() { + let generic_parameters = generic_parameters.join(", ").to_string(); + receiver_type_adt_name = + format!("{}::<{}>", receiver_type_adt_name, generic_parameters); + } + + let method_name = call.name_ref()?; + let assoc_func_call = format!("{}::{}()", receiver_type_adt_name, method_name); + + let assoc_func_call = make::expr_path(make::path_from_text(&assoc_func_call)); + + let args: Vec<_> = if need_to_take_receiver_as_first_arg { + std::iter::once(receiver).chain(call.arg_list()?.args()).collect() + } else { + call.arg_list()?.args().collect() + }; + let args = make::arg_list(args); + + let assoc_func_call_expr_string = make::expr_call(assoc_func_call, args).to_string(); + + let file_id = ctx.sema.original_range_opt(call.receiver()?.syntax())?.file_id; + + Some(Assist { + id: AssistId("method_call_to_assoc_func_call_fix", AssistKind::QuickFix), + label: Label::new(format!( + "Use associated func call instead: `{}`", + assoc_func_call_expr_string + )), + group: None, + target: range, + source_change: Some(SourceChange::from_text_edit( + file_id, + TextEdit::replace(range, assoc_func_call_expr_string), + )), + trigger_signature_help: false, + }) + } else { + None + } } #[cfg(test)] From 481fab1591a32f5886043263611bd87b1acc6719 Mon Sep 17 00:00:00 2001 From: Young-Flash <871946895@qq.com> Date: Tue, 12 Dec 2023 20:32:13 +0800 Subject: [PATCH 2/3] chore: add test case for assoc func fix in `unresolved_method` diagnostic --- .../src/handlers/unresolved_method.rs | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs index 4adb7000dda48..6fd89b33a177e 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_method.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_method.rs @@ -202,6 +202,85 @@ fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) - mod tests { use crate::tests::{check_diagnostics, check_fix}; + #[test] + fn test_assoc_func_fix() { + check_fix( + r#" +struct A {} + +impl A { + fn hello() {} +} +fn main() { + let a = A{}; + a.hello$0(); +} +"#, + r#" +struct A {} + +impl A { + fn hello() {} +} +fn main() { + let a = A{}; + A::hello(); +} +"#, + ); + } + + #[test] + fn test_assoc_func_diagnostic() { + check_diagnostics( + r#" +struct A {} +impl A { + fn hello() {} +} +fn main() { + let a = A{}; + a.hello(); + // ^^^^^^^^^ 💡 error: no method `hello` on type `A`, but an associated function with a similar name exists +} +"#, + ); + } + + #[test] + fn test_assoc_func_fix_with_generic() { + check_fix( + r#" +struct A { + a: T, + b: U +} + +impl A { + fn foo() {} +} +fn main() { + let a = A {a: 0, b: ""}; + a.foo()$0; +} +"#, + r#" +struct A { + a: T, + b: U +} + +impl A { + fn foo() {} +} +fn main() { + let a = A {a: 0, b: ""}; + A::::foo(); +} +"#, + ); + } + #[test] fn smoke_test() { check_diagnostics( From 91bd59682a8975e7f3699b132103548c7409ed1c Mon Sep 17 00:00:00 2001 From: Young-Flash Date: Tue, 2 Jan 2024 21:30:13 +0800 Subject: [PATCH 3/3] fix: make editing range accommodate for macros --- crates/ide-diagnostics/src/handlers/unresolved_method.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs index 6fd89b33a177e..60a45a05a4a12 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_method.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_method.rs @@ -1,4 +1,4 @@ -use hir::{db::ExpandDatabase, AssocItem, HirDisplay}; +use hir::{db::ExpandDatabase, AssocItem, HirDisplay, InFile}; use ide_db::{ assists::{Assist, AssistId, AssistKind}, base_db::FileRange, @@ -121,7 +121,9 @@ fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) - let expr: ast::Expr = expr_ptr.value.to_node(&root); let call = ast::MethodCallExpr::cast(expr.syntax().clone())?; - let range = call.syntax().text_range(); + let range = InFile::new(expr_ptr.file_id, call.syntax().text_range()) + .original_node_file_range_rooted(db) + .range; let receiver = call.receiver()?; let receiver_type = &ctx.sema.type_of_expr(&receiver)?.original; @@ -241,7 +243,7 @@ impl A { fn main() { let a = A{}; a.hello(); - // ^^^^^^^^^ 💡 error: no method `hello` on type `A`, but an associated function with a similar name exists + // ^^^^^ 💡 error: no method `hello` on type `A`, but an associated function with a similar name exists } "#, );