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 the ability to jump from into to from definitions #18934

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
8 changes: 8 additions & 0 deletions crates/ide-db/src/famous_defs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ impl FamousDefs<'_, '_> {
self.find_trait("core:convert:From")
}

pub fn core_convert_TryFrom(&self) -> Option<Trait> {
self.find_trait("core:convert:TryFrom")
}

pub fn core_str_FromStr(&self) -> Option<Trait> {
self.find_trait("core:str:FromStr")
}

pub fn core_convert_Into(&self) -> Option<Trait> {
self.find_trait("core:convert:Into")
}
Expand Down
86 changes: 85 additions & 1 deletion crates/ide/src/goto_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ use crate::{
navigation_target::{self, ToNav},
FilePosition, NavigationTarget, RangeInfo, TryToNav, UpmappingResult,
};
use hir::{AsAssocItem, AssocItem, FileRange, InFile, MacroFileIdExt, ModuleDef, Semantics};
use hir::{AsAssocItem, AssocItem, FileRange, Impl, InFile, MacroFileIdExt, ModuleDef, Semantics};
use ide_db::{
base_db::{AnchoredPath, FileLoader, SourceDatabase},
defs::{Definition, IdentClass},
famous_defs::FamousDefs,
helpers::pick_best_token,
RootDatabase, SymbolKind,
};
Expand Down Expand Up @@ -81,6 +82,10 @@ pub(crate) fn goto_definition(
return Some(RangeInfo::new(original_token.text_range(), navs));
}

if let Some(navs) = find_from_definition(file_id, &original_token, sema) {
return Some(RangeInfo::new(original_token.text_range(), navs));
}

let navs = sema
.descend_into_macros_no_opaque(original_token.clone())
.into_iter()
Expand Down Expand Up @@ -125,6 +130,62 @@ pub(crate) fn goto_definition(
Some(RangeInfo::new(original_token.text_range(), navs))
}

// If the token is into(), try_into(), parse(), search the definition of From, TryFrom, FromStr.
fn find_from_definition(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is rather confusing / non-descriptive so I'd rather have a longer one like (especially given this handles more than just From impls

Suggested change
fn find_from_definition(
fn find_definition_for_known_blanket_dual_impls(

file_id: FileId,
original_token: &SyntaxToken,
sema: &Semantics<'_, RootDatabase>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, semantics is usually the first argument to a function

) -> Option<Vec<NavigationTarget>> {
let db = sema.db;
let krate = sema.file_to_module_def(file_id)?.krate();

// e.g. if the method call is let b = a.into(),
// - receiver_type is A (type of a)
// - return_type is B (type of b)
// We will find the definition of B::from(a: A).
let method_call = ast::MethodCallExpr::cast(original_token.parent()?.parent()?)?;
let receiver_type = sema.type_of_expr(&method_call.receiver()?)?.original();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we actually need the adjusted type for the receiver.

let return_type = sema.type_of_expr(&method_call.clone().into())?.original();

let (search_method, search_trait, return_type) = match method_call.name_ref()?.text().as_str() {
"into" => ("from", FamousDefs(sema, krate).core_convert_From()?, return_type),
// If the method is try_into() or parse(), return_type is Result<T, Error>.
// Get T from type arguments of Result<T, Error>.
"try_into" => (
"try_from",
FamousDefs(sema, krate).core_convert_TryFrom()?,
return_type.type_arguments().next()?,
),
"parse" => (
"from_str",
FamousDefs(sema, krate).core_str_FromStr()?,
return_type.type_arguments().next()?,
),
_ => return None,
};

let from_impls = Impl::all_for_type(db, return_type)
.into_iter()
.filter(|impl_| impl_.trait_(db).is_some_and(|trait_| trait_ == search_trait));
let from_methods = from_impls.flat_map(|impl_| impl_.items(db)).filter_map(|item| match item {
AssocItem::Function(function) if function.name(db).as_str() == search_method => {
Some(function)
}
_ => None,
});
let target_method = from_methods.into_iter().find(|method| {
let args = method.assoc_fn_params(db);

// FIXME: This condition does not work for complicated cases such as
// receiver_type: Vec<i64>
// arg.ty(): T: IntoIterator<Item = i64>
args.first().is_some_and(|arg| receiver_type.could_coerce_to(db, arg.ty()))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this works in many cases but does not cover all cases.

If someone knows how to check type compatibilities, please comment.
Maybe this feature is required to cover all cases.

})?;
Comment on lines +169 to +185
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be going through the (currently unexposed) SourceAnalyzer::resolve_impl_method_or_trait_def function


let def = Definition::from(target_method);
Some(def_to_nav(db, def))
}

fn try_lookup_include_path(
sema: &Semantics<'_, RootDatabase>,
token: ast::String,
Expand Down Expand Up @@ -3022,4 +3083,27 @@ fn foo() {
"#,
);
}
#[test]
fn into_call_to_from_definition() {
check(
r#"
//- minicore: from
struct A;

struct B;

impl From<A> for B {
fn from(value: A) -> Self {
//^^^^
B
}
}

fn f() {
let a = A;
let b: B = a.into$0();
}
"#,
);
}
}
Loading