feat: Add the ability to jump from `into` to `from` definitions #18934

1hakusai1 · 2025-01-14T08:31:43Z

close #18316
close #15315
(maybe related to #4558)

This PR add the ability to jump from into to from definitions.

from into to from
from try_into to try_from
from parse to from_str

Test

I can't write unit tests for TryFrom and FromStr because minicore doesn't contain them.

use std::str::FromStr;

struct SourceType;

struct TargetType;

impl Into<i64> for SourceType {
    // E
    fn into(self) -> i64 {
        todo!()
    }
}

impl TryInto<String> for SourceType {
    type Error = String;

    // F
    fn try_into(self) -> Result<String, Self::Error> {
        todo!()
    }
}

impl From<SourceType> for TargetType {
    // A
    fn from(value: SourceType) -> Self {
        todo!()
    }
}

impl FromStr for TargetType {
    type Err = String;

    // D
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        todo!()
    }
}

impl From<i64> for TargetType {
    // B
    fn from(value: i64) -> Self {
        todo!()
    }
}

impl TryFrom<String> for TargetType {
    type Error = String;

    // C
    fn try_from(value: String) -> Result<Self, Self::Error> {
        todo!()
    }
}

fn main() {
    // into to from
    let source = SourceType;
    let target: TargetType = source.into(); // goto A
    let target: TargetType = 12.into(); // goto B

    // try_into to try_from
    let maybe_target: Result<TargetType, _> = String::from("aaa").try_into(); // goto C

    // parse to from_str
    let maybe_target: Result<TargetType, String> = "aaaaa".parse(); // goto D

    // regular cases
    let source = SourceType;
    let integer: i64 = source.into(); // goto E
    let source = SourceType;
    let string: Result<String, _> = source.try_into(); // goto F
}

crates/ide/src/goto_definition.rs

1hakusai1 · 2025-01-14T10:15:26Z

crates/ide/src/goto_definition.rs

+        // 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()))


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.

ChayimFriedman2 · 2025-01-14T16:25:59Z

I can't write unit tests for TryFrom and FromStr because minicore doesn't contain them.

Feel free to add them to minicore (behind flags).

ChayimFriedman2 · 2025-01-14T16:28:22Z

crates/ide/src/goto_definition.rs

+    // - 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();


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

Veykril · 2025-01-15T12:19:28Z

crates/ide/src/goto_definition.rs

@@ -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(


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(

Veykril · 2025-01-15T12:19:44Z

crates/ide/src/goto_definition.rs

+fn find_from_definition(
+    file_id: FileId,
+    original_token: &SyntaxToken,
+    sema: &Semantics<'_, RootDatabase>,


nit, semantics is usually the first argument to a function

Veykril · 2025-01-15T12:23:07Z

crates/ide/src/goto_definition.rs

+    let method_call = ast::MethodCallExpr::cast(original_token.parent()?.parent()?)?;
+    let receiver_type = sema.type_of_expr(&method_call.receiver()?)?.adjusted();
+    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,
+    };


We should use Semantics::resolve_method_call_as_callable here, that gives us a bunch of the things here already like receiver param/type, function id and return type.

Veykril · 2025-01-15T12:27:55Z

crates/ide/src/goto_definition.rs

+    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()))
+    })?;


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

Add the ability to jump from into to from definitions

ea87eab

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 14, 2025

1hakusai1 commented Jan 14, 2025

View reviewed changes

crates/ide/src/goto_definition.rs Outdated Show resolved Hide resolved

fix_typo

c14cdcf

1hakusai1 changed the title ~~Add the ability to jump from into to from definitions~~ feat: Add the ability to jump from into to from definitions Jan 14, 2025

1hakusai1 added 2 commits January 14, 2025 17:54

Remove trailing spaces

a0be4b1

refactor: get(0) -> first()

96ed889

1hakusai1 commented Jan 14, 2025

View reviewed changes

ChayimFriedman2 reviewed Jan 14, 2025

View reviewed changes

1hakusai1 added 3 commits January 15, 2025 20:43

Add test cases

dc075fd

Use adjusted type

65342f4

Fix wrong fixture

b6d6361

Veykril reviewed Jan 15, 2025

View reviewed changes

1hakusai1 added 3 commits January 16, 2025 09:33

refactor: rename to descriptive name

c6317a3

refactor: Change order of arguments

afde04b

Use resolve_method_call_as_callable to handle function types

e8a63e4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add the ability to jump from `into` to `from` definitions #18934

feat: Add the ability to jump from `into` to `from` definitions #18934

1hakusai1 commented Jan 14, 2025 •

edited

Loading

1hakusai1 Jan 14, 2025

ChayimFriedman2 commented Jan 14, 2025

ChayimFriedman2 Jan 14, 2025

Veykril Jan 15, 2025

Veykril Jan 15, 2025

Veykril Jan 15, 2025

Veykril Jan 15, 2025

	fn find_from_definition(
	fn find_definition_for_known_blanket_dual_impls(

feat: Add the ability to jump from into to from definitions #18934

Are you sure you want to change the base?

feat: Add the ability to jump from into to from definitions #18934

Conversation

1hakusai1 commented Jan 14, 2025 • edited Loading

Test

1hakusai1 Jan 14, 2025

Choose a reason for hiding this comment

ChayimFriedman2 commented Jan 14, 2025

ChayimFriedman2 Jan 14, 2025

Choose a reason for hiding this comment

Veykril Jan 15, 2025

Choose a reason for hiding this comment

Veykril Jan 15, 2025

Choose a reason for hiding this comment

Veykril Jan 15, 2025

Choose a reason for hiding this comment

Veykril Jan 15, 2025

Choose a reason for hiding this comment

feat: Add the ability to jump from `into` to `from` definitions #18934

feat: Add the ability to jump from `into` to `from` definitions #18934

1hakusai1 commented Jan 14, 2025 •

edited

Loading