diff options
author | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-04-28 15:11:07 +0100 |
---|---|---|
committer | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-04-28 15:11:07 +0100 |
commit | 8138b1da4f1564913a1a22407c65e77aa5320d56 (patch) | |
tree | 6b037199384ddf1021c2ad81886425dfcb8271a8 /crates | |
parent | 1dd3d9bc2d328e164c594f3d2022dcf174f907a9 (diff) | |
parent | 85633656dfc3ad4516a99e3062599e3ee85a578d (diff) |
Merge #1215
1215: Fix hover on the beginning of a nested expression r=matklad a=flodiebold
E.g. in
```
let foo = 1u32;
if true {
<|>foo;
}
```
the hover shows `()`, the type of the whole if expression, instead of the more
sensible `u32`. The reason for this was that the search for an expression was
slightly left-biased: When on the edge between two tokens, it first looked at
all ancestors of the left token and then of the right token. Instead merge the
ancestors in ascending order, so that we get the smaller of the two possible
expressions.
This might seem rather inconsequential, but emacs-lsp's sideline requests hover for the beginning of each word in the current line, so it tends to hit this a lot :smile: Actually, as I think about this now, another solution might be just making hover right-biased, since if I'm hovering a certain character I really mean that character and not the one to its left...
Co-authored-by: Florian Diebold <[email protected]>
Diffstat (limited to 'crates')
-rw-r--r-- | crates/ra_ide_api/src/goto_definition.rs | 1 | ||||
-rw-r--r-- | crates/ra_ide_api/src/hover.rs | 21 | ||||
-rw-r--r-- | crates/ra_syntax/src/algo.rs | 21 |
3 files changed, 33 insertions, 10 deletions
diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs index 40a2bd148..163781f88 100644 --- a/crates/ra_ide_api/src/goto_definition.rs +++ b/crates/ra_ide_api/src/goto_definition.rs | |||
@@ -25,6 +25,7 @@ pub(crate) fn goto_definition( | |||
25 | None | 25 | None |
26 | } | 26 | } |
27 | 27 | ||
28 | #[derive(Debug)] | ||
28 | pub(crate) enum ReferenceResult { | 29 | pub(crate) enum ReferenceResult { |
29 | Exact(NavigationTarget), | 30 | Exact(NavigationTarget), |
30 | Approximate(Vec<NavigationTarget>), | 31 | Approximate(Vec<NavigationTarget>), |
diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs index 397b56786..6545a2581 100644 --- a/crates/ra_ide_api/src/hover.rs +++ b/crates/ra_ide_api/src/hover.rs | |||
@@ -1,7 +1,7 @@ | |||
1 | use ra_db::SourceDatabase; | 1 | use ra_db::SourceDatabase; |
2 | use ra_syntax::{ | 2 | use ra_syntax::{ |
3 | AstNode, ast, | 3 | AstNode, ast, |
4 | algo::{find_covering_element, find_node_at_offset, find_token_at_offset}, | 4 | algo::{find_covering_element, find_node_at_offset, ancestors_at_offset}, |
5 | }; | 5 | }; |
6 | use hir::HirDisplay; | 6 | use hir::HirDisplay; |
7 | 7 | ||
@@ -104,12 +104,8 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeIn | |||
104 | } | 104 | } |
105 | 105 | ||
106 | if range.is_none() { | 106 | if range.is_none() { |
107 | let node = find_token_at_offset(file.syntax(), position.offset).find_map(|token| { | 107 | let node = ancestors_at_offset(file.syntax(), position.offset) |
108 | token | 108 | .find(|n| ast::Expr::cast(*n).is_some() || ast::Pat::cast(*n).is_some())?; |
109 | .parent() | ||
110 | .ancestors() | ||
111 | .find(|n| ast::Expr::cast(*n).is_some() || ast::Pat::cast(*n).is_some()) | ||
112 | })?; | ||
113 | let frange = FileRange { file_id: position.file_id, range: node.range() }; | 109 | let frange = FileRange { file_id: position.file_id, range: node.range() }; |
114 | res.extend(type_of(db, frange).map(rust_code_markup)); | 110 | res.extend(type_of(db, frange).map(rust_code_markup)); |
115 | range = Some(node.range()); | 111 | range = Some(node.range()); |
@@ -398,6 +394,17 @@ The Some variant | |||
398 | } | 394 | } |
399 | 395 | ||
400 | #[test] | 396 | #[test] |
397 | fn hover_local_var_edge() { | ||
398 | let (analysis, position) = single_file_with_position( | ||
399 | " | ||
400 | fn func(foo: i32) { if true { <|>foo; }; } | ||
401 | ", | ||
402 | ); | ||
403 | let hover = analysis.hover(position).unwrap().unwrap(); | ||
404 | assert_eq!(trim_markup_opt(hover.info.first()), Some("i32")); | ||
405 | } | ||
406 | |||
407 | #[test] | ||
401 | fn test_type_of_for_function() { | 408 | fn test_type_of_for_function() { |
402 | let (analysis, range) = single_file_with_range( | 409 | let (analysis, range) = single_file_with_range( |
403 | " | 410 | " |
diff --git a/crates/ra_syntax/src/algo.rs b/crates/ra_syntax/src/algo.rs index 1f68fe467..d31d00343 100644 --- a/crates/ra_syntax/src/algo.rs +++ b/crates/ra_syntax/src/algo.rs | |||
@@ -1,5 +1,7 @@ | |||
1 | pub mod visit; | 1 | pub mod visit; |
2 | 2 | ||
3 | use itertools::Itertools; | ||
4 | |||
3 | use crate::{SyntaxNode, TextRange, TextUnit, AstNode, Direction, SyntaxToken, SyntaxElement}; | 5 | use crate::{SyntaxNode, TextRange, TextUnit, AstNode, Direction, SyntaxToken, SyntaxElement}; |
4 | 6 | ||
5 | pub use rowan::TokenAtOffset; | 7 | pub use rowan::TokenAtOffset; |
@@ -12,6 +14,20 @@ pub fn find_token_at_offset(node: &SyntaxNode, offset: TextUnit) -> TokenAtOffse | |||
12 | } | 14 | } |
13 | } | 15 | } |
14 | 16 | ||
17 | /// Returns ancestors of the node at the offset, sorted by length. This should | ||
18 | /// do the right thing at an edge, e.g. when searching for expressions at `{ | ||
19 | /// <|>foo }` we will get the name reference instead of the whole block, which | ||
20 | /// we would get if we just did `find_token_at_offset(...).flat_map(|t| | ||
21 | /// t.parent().ancestors())`. | ||
22 | pub fn ancestors_at_offset( | ||
23 | node: &SyntaxNode, | ||
24 | offset: TextUnit, | ||
25 | ) -> impl Iterator<Item = &SyntaxNode> { | ||
26 | find_token_at_offset(node, offset) | ||
27 | .map(|token| token.parent().ancestors()) | ||
28 | .kmerge_by(|node1, node2| node1.range().len() < node2.range().len()) | ||
29 | } | ||
30 | |||
15 | /// Finds a node of specific Ast type at offset. Note that this is slightly | 31 | /// Finds a node of specific Ast type at offset. Note that this is slightly |
16 | /// imprecise: if the cursor is strictly between two nodes of the desired type, | 32 | /// imprecise: if the cursor is strictly between two nodes of the desired type, |
17 | /// as in | 33 | /// as in |
@@ -20,10 +36,9 @@ pub fn find_token_at_offset(node: &SyntaxNode, offset: TextUnit) -> TokenAtOffse | |||
20 | /// struct Foo {}|struct Bar; | 36 | /// struct Foo {}|struct Bar; |
21 | /// ``` | 37 | /// ``` |
22 | /// | 38 | /// |
23 | /// then the left node will be silently preferred. | 39 | /// then the shorter node will be silently preferred. |
24 | pub fn find_node_at_offset<N: AstNode>(syntax: &SyntaxNode, offset: TextUnit) -> Option<&N> { | 40 | pub fn find_node_at_offset<N: AstNode>(syntax: &SyntaxNode, offset: TextUnit) -> Option<&N> { |
25 | find_token_at_offset(syntax, offset) | 41 | ancestors_at_offset(syntax, offset).find_map(N::cast) |
26 | .find_map(|leaf| leaf.parent().ancestors().find_map(N::cast)) | ||
27 | } | 42 | } |
28 | 43 | ||
29 | /// Finds the first sibling in the given direction which is not `trivia` | 44 | /// Finds the first sibling in the given direction which is not `trivia` |