Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add assoc func quickfix for unresolved_method diagnostic #16100

Merged
merged 3 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/hir-ty/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ pub enum InferenceDiagnostic {
name: Name,
/// Contains the type the field resolves to
field_with_same_name: Option<Ty>,
assoc_func_with_same_name: Option<AssocItemId>,
},
// FIXME: This should be emitted in body lowering
BreakOutsideOfLoop {
Expand Down
19 changes: 19 additions & 0 deletions crates/hir-ty/src/infer/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion crates/hir/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -215,6 +215,7 @@ pub struct UnresolvedMethodCall {
pub receiver: Type,
pub name: Name,
pub field_with_same_name: Option<Type>,
pub assoc_func_with_same_name: Option<AssocItemId>,
}

#[derive(Debug)]
Expand Down
2 changes: 2 additions & 0 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1680,6 +1680,7 @@ impl DefWithBody {
receiver,
name,
field_with_same_name,
assoc_func_with_same_name,
} => {
let expr = expr_syntax(*expr);

Expand All @@ -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(),
)
Expand Down
204 changes: 196 additions & 8 deletions crates/ide-diagnostics/src/handlers/unresolved_method.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use hir::{db::ExpandDatabase, HirDisplay};
use hir::{db::ExpandDatabase, AssocItem, HirDisplay, InFile};
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};
Expand All @@ -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)
),
Expand All @@ -46,19 +51,35 @@ pub(crate) fn unresolved_method(
}

fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -> Option<Vec<Assist>> {
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)
}
}

fn field_fix(
ctx: &DiagnosticsContext<'_>,
d: &hir::UnresolvedMethodCall,
ty: &hir::Type,
) -> Option<Vec<Assist>> {
) -> Option<Assist> {
if !ty.impls_fnonce(ctx.sema.db) {
return None;
}
Expand All @@ -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,
Expand All @@ -88,13 +109,180 @@ 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<Assist> {
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 = 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;

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<T, A>`, containing unspecified generics.
// However, type of `receiver` is specified, it could be `Box<i32, Global>` 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<T, A>` or `Box<i32, Global>` 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<SmolStr> = 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)]
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<T, U> {
a: T,
b: U
}

impl<T, U> A<T, U> {
fn foo() {}
}
fn main() {
let a = A {a: 0, b: ""};
a.foo()$0;
}
"#,
r#"
struct A<T, U> {
a: T,
b: U
}

impl<T, U> A<T, U> {
fn foo() {}
}
fn main() {
let a = A {a: 0, b: ""};
A::<i32, &str>::foo();
}
"#,
);
}

#[test]
fn smoke_test() {
check_diagnostics(
Expand Down