diff --git a/src/lang/defs.rs b/src/lang/defs.rs index 7a504c4f..c377bb3d 100644 --- a/src/lang/defs.rs +++ b/src/lang/defs.rs @@ -2,8 +2,8 @@ use std::iter; use cairo_lang_defs::db::DefsGroup; use cairo_lang_defs::ids::{ - LanguageElementId, LookupItemId, MemberId, ModuleId, ModuleItemId, NamedLanguageElementId, - SubmoduleLongId, TopLevelLanguageElementId, TraitItemId, VarId, + LanguageElementId, LocalVarLongId, LookupItemId, MemberId, ModuleId, ModuleItemId, + NamedLanguageElementId, SubmoduleLongId, TopLevelLanguageElementId, TraitItemId, VarId, }; use cairo_lang_diagnostics::ToOption; use cairo_lang_doc::db::DocGroup; @@ -444,7 +444,7 @@ fn find_definition( return Some((ResolvedItem::Member(member_id), member_id.untyped_stable_ptr(db))); } - for lookup_item_id in lookup_items.iter().copied() { + for &lookup_item_id in lookup_items { if let Some(item) = db.lookup_resolved_generic_item_by_ptr(lookup_item_id, identifier.stable_ptr()) { @@ -462,41 +462,41 @@ fn find_definition( } } - // Skip variable definition, otherwise we would get parent ModuleItem for variable. - if identifier.as_syntax_node().parent_of_kind(db, SyntaxKind::StatementLet).is_none() { - let item = match lookup_items.first().copied()? { - LookupItemId::ModuleItem(item) => { - ResolvedGenericItem::from_module_item(db, item).to_option()? - } - LookupItemId::TraitItem(trait_item) => { - if let TraitItemId::Function(trait_function_id) = trait_item { - let parent_trait = trait_item.trait_id(db); - let generic_parameters = db.trait_generic_params(parent_trait).to_option()?; - let concrete_trait = ConcreteTraitLongId { - trait_id: parent_trait, - generic_args: generic_params_to_args(&generic_parameters, db), - }; - let concrete_trait = db.intern_concrete_trait(concrete_trait); - - ResolvedGenericItem::GenericFunction(GenericFunctionId::Impl( - ImplGenericFunctionId { - impl_id: ImplLongId::SelfImpl(concrete_trait).intern(db), - function: trait_function_id, - }, - )) - } else { - ResolvedGenericItem::Trait(trait_item.trait_id(db)) - } - } - LookupItemId::ImplItem(impl_item) => { - ResolvedGenericItem::Impl(impl_item.impl_def_id(db)) + if let Some(var_id) = try_extract_variable_declaration(db, identifier, lookup_items) { + let item = ResolvedGenericItem::Variable(var_id); + return Some((ResolvedItem::Generic(item.clone()), resolved_generic_item_def(db, item)?)); + } + + // FIXME(mkaput): This logic always kicks in if we're finding definition of undefined symbol + // which is very wrong in such cases. + let item = match lookup_items.first().copied()? { + LookupItemId::ModuleItem(item) => { + ResolvedGenericItem::from_module_item(db, item).to_option()? + } + LookupItemId::TraitItem(trait_item) => { + if let TraitItemId::Function(trait_function_id) = trait_item { + let parent_trait = trait_item.trait_id(db); + let generic_parameters = db.trait_generic_params(parent_trait).to_option()?; + let concrete_trait = ConcreteTraitLongId { + trait_id: parent_trait, + generic_args: generic_params_to_args(&generic_parameters, db), + }; + let concrete_trait = db.intern_concrete_trait(concrete_trait); + + ResolvedGenericItem::GenericFunction(GenericFunctionId::Impl( + ImplGenericFunctionId { + impl_id: ImplLongId::SelfImpl(concrete_trait).intern(db), + function: trait_function_id, + }, + )) + } else { + ResolvedGenericItem::Trait(trait_item.trait_id(db)) } - }; + } + LookupItemId::ImplItem(impl_item) => ResolvedGenericItem::Impl(impl_item.impl_def_id(db)), + }; - Some((ResolvedItem::Generic(item.clone()), resolved_generic_item_def(db, item)?)) - } else { - None - } + Some((ResolvedItem::Generic(item.clone()), resolved_generic_item_def(db, item)?)) } /// Extracts [`MemberId`] if the [`TerminalIdentifier`] is used as a struct member @@ -567,6 +567,56 @@ fn try_extract_member( } } +/// Lookups if the identifier is a declaration of a variable/param in one of the lookup items. +/// +/// Declaration identifiers aren't kept in `ResolvedData`, which is searched for by +/// `lookup_resolved_generic_item_by_ptr` and `lookup_resolved_concrete_item_by_ptr`. +/// Therefore, we have to look for these ourselves. +fn try_extract_variable_declaration( + db: &AnalysisDatabase, + identifier: &ast::TerminalIdentifier, + lookup_items: &[LookupItemId], +) -> Option { + let function_id = lookup_items.first()?.function_with_body()?; + + // Look at function parameters. + if let Some(param) = identifier.as_syntax_node().parent_of_kind(db, SyntaxKind::Param) { + // Closures have different semantic model structures than regular functions. + let params = if let Some(expr_closure_ast) = param.parent_of_type::(db) { + let expr_id = + db.lookup_expr_by_ptr(function_id, expr_closure_ast.stable_ptr().into()).ok()?; + let Expr::ExprClosure(expr_closure_semantic) = db.expr_semantic(function_id, expr_id) + else { + unreachable!("expected semantic for ast::ExprClosure to be Expr::ExprClosure"); + }; + expr_closure_semantic.params + } else { + let signature = db.function_with_body_signature(function_id).ok()?; + signature.params + }; + + if let Some(param) = + params.into_iter().find(|param| param.stable_ptr == identifier.stable_ptr()) + { + return Some(VarId::Param(param.id)); + } + } + + // Look at patterns in the function body. + if let Some(pattern) = identifier.as_syntax_node().parent_of_type::(db) { + // Bail out if the pattern happens to not exist in the semantic model. + // We don't need semantics for returning, though, due to the way how local variables are + // identified there, so we're happily ignoring the result value. + db.lookup_pattern_by_ptr(function_id, pattern.stable_ptr()).ok()?; + + return Some(VarId::Local( + LocalVarLongId(function_id.module_file_id(db), identifier.stable_ptr()).intern(db), + )); + } + + None +} + fn resolved_concrete_item_def( db: &AnalysisDatabase, item: ResolvedConcreteItem, diff --git a/tests/e2e/find_references/vars.rs b/tests/e2e/find_references/vars.rs index c14041a2..23a7d100 100644 --- a/tests/e2e/find_references/vars.rs +++ b/tests/e2e/find_references/vars.rs @@ -11,7 +11,15 @@ fn var_via_binding() { fn bar() { let foobar = 42; // bad } - "#, @"none response") + "#, @r" + fn main() { + let foobar = 1233; // good + let x = foobar + 1; // good + } + fn bar() { + let foobar = 42; // bad + } + ") } #[test] @@ -133,8 +141,8 @@ fn param_via_binding() { num * num } "#, @r" - fn pow(num: felt252) -> felt252 { - num * num + fn pow(num: felt252) -> felt252 { + num * num } ") } @@ -204,3 +212,16 @@ fn closure_param_via_use() { } ") } + +#[test] +fn closure_param_via_binding() { + test_transform!(find_references, r#" + fn main() { + let f = |abc: felt252| abc + 1; + } + "#, @r" + fn main() { + let f = |abc: felt252| abc + 1; + } + ") +} diff --git a/tests/e2e/goto_definition/vars.rs b/tests/e2e/goto_definition/vars.rs index d98e8394..ff5795a9 100644 --- a/tests/e2e/goto_definition/vars.rs +++ b/tests/e2e/goto_definition/vars.rs @@ -2,7 +2,7 @@ use crate::goto_definition::goto_definition; use crate::support::insta::test_transform; #[test] -fn var_in_expr() { +fn var_via_expr() { test_transform!(goto_definition, r" fn main() { let abc: felt252 = 0; // good @@ -22,6 +22,27 @@ fn var_in_expr() { ") } +#[test] +fn var_via_binding() { + test_transform!(goto_definition, r" + fn main() { + let abc: felt252 = 0; // good + let _ = abc * 2; + } + fn foo() { + let abc: felt252 = 1; // bad + } + ", @r" + fn main() { + let abc: felt252 = 0; // good + let _ = abc * 2; + } + fn foo() { + let abc: felt252 = 1; // bad + } + ") +} + #[test] fn fn_param_via_expr() { test_transform!(goto_definition, r" @@ -37,6 +58,21 @@ fn fn_param_via_expr() { ") } +#[test] +fn fn_param_via_binding() { + test_transform!(goto_definition, r" + fn main(abc: felt252, def: felt252) { // good + let _ = abc * 2; + } + fn foo(abc: felt252) {} // bad + ", @r" + fn main(abc: felt252, def: felt252) { // good + let _ = abc * 2; + } + fn foo(abc: felt252) {} // bad + ") +} + #[test] fn closure_param_via_expr() { test_transform!(goto_definition, r" @@ -59,3 +95,26 @@ fn closure_param_via_expr() { fn foo(abc: felt252) {} // bad ") } + +#[test] +fn closure_param_via_binding() { + test_transform!(goto_definition, r" + fn foo(a: felt252) -> felt252 { + let abc: felt252 = 0; // bad + let c = |abc| { // good + abc + 3 + }; + } + + fn foo(abc: felt252) {} // bad + ", @r" + fn foo(a: felt252) -> felt252 { + let abc: felt252 = 0; // bad + let c = |abc| { // good + abc + 3 + }; + } + + fn foo(abc: felt252) {} // bad + ") +} diff --git a/tests/test_data/hover/basic.txt b/tests/test_data/hover/basic.txt index 89b6b30c..df3277ff 100644 --- a/tests/test_data/hover/basic.txt +++ b/tests/test_data/hover/basic.txt @@ -75,9 +75,11 @@ pub mod front_of_house { // = source context let mut x = 5; // = highlight -No highlight information. + let mut x = 5; // = popover -Type: `core::integer::u32` +```cairo +let mut x: core::integer::u32 +``` //! > hover #1 // = source context @@ -208,9 +210,11 @@ Add to waitlist function. // = source context let mut rect = Rectangle { width: 30, height: 50 }; // = highlight -No highlight information. + let mut rect = Rectangle { width: 30, height: 50 }; // = popover -Type: `hello::Rectangle` +```cairo +let mut rect: hello::Rectangle +``` //! > hover #10 // = source context @@ -254,9 +258,11 @@ Width of the rectangle. // = source context let area = rect.area(); // = highlight -No highlight information. + let area = rect.area(); // = popover -Type: `core::integer::u64` +```cairo +let area: core::integer::u64 +``` //! > hover #13 // = source context @@ -296,14 +302,8 @@ fn add_two(x: u32) -> u32 fn area(self: @Rectangle) -> u64; // = popover ```cairo -hello::RectangleTrait -``` -```cairo -trait RectangleTrait -fn area(self: @Rectangle) -> u64 +self: @hello::Rectangle ``` ---- -Calculate the area of the rectangle. //! > hover #16 // = source context @@ -331,13 +331,8 @@ Rectangle struct. fn area(self: @Rectangle) -> u64 { // = popover ```cairo -hello -``` -```cairo -impl RectangleImpl of RectangleTrait +self: @hello::Rectangle ``` ---- -Implementing the `RectangleTrait` for the `Rectangle` struct. //! > hover #18 // = source context @@ -523,10 +518,7 @@ fn value_in_cents(coin: Coin) -> felt252 { fn value_in_cents(coin: Coin) -> felt252 { // = popover ```cairo -hello -``` -```cairo -fn value_in_cents(coin: Coin) -> felt252 +coin: hello::Coin ``` //! > hover #30 diff --git a/tests/test_data/hover/partial.txt b/tests/test_data/hover/partial.txt index 97d3dc77..a0c66f90 100644 --- a/tests/test_data/hover/partial.txt +++ b/tests/test_data/hover/partial.txt @@ -17,18 +17,23 @@ fn f(abc) -> felt252 { // = source context let mut xyz = unknown_function(); // = highlight -No highlight information. + let mut xyz = unknown_function(); // = popover -Type: `` +```cairo +let mut xyz: +``` //! > hover #1 // = source context let mut xyz = unknown_function(); // = highlight -No highlight information. + let mut xyz = unknown_function(); // = popover ```cairo - +hello +``` +```cairo +fn main() ``` //! > hover #2 @@ -48,10 +53,7 @@ fn f(abc) -> felt252 { fn f(abc) -> felt252 { // = popover ```cairo -hello -``` -```cairo -fn f(abc) -> felt252 +abc: ``` //! > hover #4 diff --git a/tests/test_data/hover/paths.txt b/tests/test_data/hover/paths.txt index 00e5d62f..f0e18b36 100644 --- a/tests/test_data/hover/paths.txt +++ b/tests/test_data/hover/paths.txt @@ -286,10 +286,13 @@ internal_module docstring. // = source context let _ = super::some_module::internal_module::nested_internal_module::PublicStruct {}; // = highlight -No highlight information. + let _ = super::some_module::internal_module::nested_internal_module::PublicStruct {}; // = popover ```cairo -hello::some_module::internal_module::nested_internal_module::PublicStruct +hello::happy_cases +``` +```cairo +fn constructor_with_path() ``` //! > hover #15 diff --git a/tests/test_data/hover/starknet.txt b/tests/test_data/hover/starknet.txt index 21bc3260..88fc4b66 100644 --- a/tests/test_data/hover/starknet.txt +++ b/tests/test_data/hover/starknet.txt @@ -62,10 +62,7 @@ pub fn contract_state_for_testing() -> ContractState fn constructor(ref self: ContractState, value_: u128) { // = popover ```cairo -hello::Balance -``` -```cairo -fn constructor(ref self: ContractState, value_: u128) +ref self: hello::Balance::ContractState ``` //! > hover #2 diff --git a/tests/test_data/hover/structs.txt b/tests/test_data/hover/structs.txt index 33b46246..2175bf54 100644 --- a/tests/test_data/hover/structs.txt +++ b/tests/test_data/hover/structs.txt @@ -143,8 +143,11 @@ Docstring of member1. // = source context let _ = NonExistentStruct { member: 0 }; // = highlight -No highlight information. + let _ = NonExistentStruct { member: 0 }; // = popover ```cairo - +hello::unhappy_cases +``` +```cairo +fn non_existent_struct ``` diff --git a/tests/test_data/hover/variables.txt b/tests/test_data/hover/variables.txt index d93c2582..97ea3105 100644 --- a/tests/test_data/hover/variables.txt +++ b/tests/test_data/hover/variables.txt @@ -15,25 +15,31 @@ fn main() { // = source context let abc: felt252 = 0; // = highlight -No highlight information. + let abc: felt252 = 0; // = popover -Type: `core::felt252` +```cairo +let abc: core::felt252 +``` //! > hover #1 // = source context let xyz = 3; // = highlight -No highlight information. + let xyz = 3; // = popover -Type: `core::felt252` +```cairo +let xyz: core::felt252 +``` //! > hover #2 // = source context let mut def = abc * 2; // = highlight -No highlight information. + let mut def = abc * 2; // = popover -Type: `core::felt252` +```cairo +let mut def: core::felt252 +``` //! > hover #3 // = source context