Skip to content

Commit

Permalink
Auto merge of #16100 - Young-Flash:assoc_func_quickfix, r=Veykril
Browse files Browse the repository at this point in the history
feat: add assoc func quickfix for `unresolved_method` diagnostic

![demo](https://github.com/rust-lang/rust-analyzer/assets/71162630/1ea1d8b8-3436-4251-a512-e0f9de01a13c)

close #13247

EDIT: I think add a demo gif would be helpful to `@lnicola` when he make a release change log :)
  • Loading branch information
bors committed Jan 2, 2024
2 parents 7659109 + 91bd596 commit f7a29e4
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 9 deletions.
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

0 comments on commit f7a29e4

Please sign in to comment.