Skip to content

Commit

Permalink
LS: Fix hover definition lookup logic
Browse files Browse the repository at this point in the history
Since ba74ab1 the hover logic wrongly
tried to find definition lookup item by inspecting originating location
within `get_definition_location`. The bug was introduced by making an
attempt to share the definition lookup logic with _goto definition_,
where actually both features need different results.

commit-id:f3e136e6
  • Loading branch information
mkaput committed May 22, 2024
1 parent 74ac649 commit bb1358e
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 15 deletions.
20 changes: 11 additions & 9 deletions crates/cairo-lang-language-server/src/ide/hover.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use cairo_lang_compiler::db::RootDatabase;
use cairo_lang_defs::db::DefsGroup;
use cairo_lang_defs::ids::LookupItemId;
use cairo_lang_syntax::node::TypedSyntaxNode;
use cairo_lang_utils::Upcast;
use tower_lsp::lsp_types::{Hover, HoverContents, HoverParams, MarkedString};

use crate::get_definition_location;
use crate::find_definition;
use crate::lang::lsp::{LsProtoGroup, ToCairo};
use crate::lang::semantic::LsSemanticGroup;
use crate::lang::syntax::LsSyntaxGroup;
Expand All @@ -18,14 +19,15 @@ use crate::lang::syntax::LsSyntaxGroup;
pub fn hover(params: HoverParams, db: &RootDatabase) -> Option<Hover> {
let file_id = db.file_for_url(&params.text_document_position_params.text_document.uri);
let position = params.text_document_position_params.position.to_cairo();
// Get the item id of the definition.
let (found_file, span) = get_definition_location(db, file_id, position)?;
// Get the documentation and declaration of the item.
let node = db.find_syntax_node_at_position(
found_file,
span.start.position_in_file(db.upcast(), found_file)?,
)?;
let lookup_item = db.find_lookup_item(&node)?;
// Get the syntax node of the definition.
let definition_node = {
let identifier = db.find_identifier_at_position(file_id, position)?;
let lookup_items = db.collect_lookup_items_stack(&identifier.as_syntax_node())?;
let stable_ptr = find_definition(db, &identifier, &lookup_items)?;
stable_ptr.lookup(db.upcast())
};
// Get the lookup item representing the defining item.
let lookup_item = db.find_lookup_item(&definition_node)?;
// Build texts.
let mut hints = Vec::new();
if let Some(hint) = get_expr_hint(db.upcast(), lookup_item) {
Expand Down
4 changes: 2 additions & 2 deletions crates/cairo-lang-language-server/tests/e2e/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn starknet() {
#[constructor]
fn constructor(ref se<caret>lf: Cont<caret>ractState, value_: u128) {
self.va<caret>lue.write(value_);
self.va<caret>lue.write(v<caret>alue_);

Check warning on line 124 in crates/cairo-lang-language-server/tests/e2e/hover.rs

View workflow job for this annotation

GitHub Actions / typos

"alue" should be "value".
}
#[abi(embed_v0)]
Expand All @@ -130,7 +130,7 @@ fn starknet() {
self.value.r<caret>ead()
}
fn increase(ref self: ContractState, a: u128) {
self.value.wr<caret>ite( self.value.read() + a );
self.value.wr<caret>ite( self.value.read() + <caret>a );
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use Balance::contract_state_for_testing;

//! > popover
No hover information.

```cairo
pub fn contract_state_for_testing() -> ContractState
```
=========

//! > hover at Position { line: 23, character: 25 }
Expand All @@ -27,8 +28,14 @@ No hover information.

//! > popover
No hover information.
```cairo


pub struct ContractState {
pub value: __member_module_value::ContractMemberState,
}

```
=========

//! > hover at Position { line: 24, character: 15 }
Expand All @@ -42,6 +49,18 @@ No hover information.

=========

//! > hover at Position { line: 24, character: 26 }

//! > source context
self.value.write(value_);

//! > popover
```cairo
fn constructor(ref self: ContractState, value_: u128)
```
=========

//! > hover at Position { line: 28, character: 30 }

//! > source context
Expand All @@ -64,8 +83,14 @@ No hover information.

//! > popover
No hover information.
```cairo


pub struct ContractState {
pub value: __member_module_value::ContractMemberState,
}

```
=========

//! > hover at Position { line: 30, character: 24 }
Expand All @@ -92,3 +117,15 @@ fn write(ref self: TMemberState, value: Self::Value);
```
=========

//! > hover at Position { line: 33, character: 50 }

//! > source context
self.value.write( self.value.read() + a );

//! > popover
```cairo
fn increase(ref self: ContractState, a: u128)
```
=========

0 comments on commit bb1358e

Please sign in to comment.