Merge #2552

2552: fix goto definition when inbetween tokens r=matklad a=succcubbus

fixes both goto_definition and goto_type_definition.
before, when running goto between some non-trivia token and an
identifier, goto would be attempted for the non-trivia token.
but this does not make sense for e.g. L_PAREN or COLONCOLON only for
IDENTs.

this resulted in goto actions not working when running them on the first
character of some identifier e.g. for `module::<|>method()` or
`method(<|>parameter)`.

now only IDENTs will be searched for in goto actions, though i'm not sure
if this is correct or if goto should also work for some other token types.  

Co-authored-by: succcubbus <16743652+succcubbus@users.noreply.github.com>

crates/ra_ide/src/goto_definition.rs

@@ -3,7 +3,9 @@
 use hir::{db::AstDatabase, InFile};
 use ra_syntax::{
     ast::{self, DocCommentsOwner},
-    match_ast, AstNode, SyntaxNode,
+    match_ast, AstNode,
+    SyntaxKind::*,
+    SyntaxNode, SyntaxToken, TokenAtOffset,
 };
 
 use crate::{
@@ -19,8 +21,7 @@ pub(crate) fn goto_definition(
     position: FilePosition,
 ) -> Option<RangeInfo<Vec<NavigationTarget>>> {
     let file = db.parse_or_expand(position.file_id.into())?;
-    let original_token =
+    let original_token = pick_best(file.token_at_offset(position.offset))?;
-        file.token_at_offset(position.offset).filter(|it| !it.kind().is_trivia()).next()?;
     let token = descend_into_macros(db, position.file_id, original_token.clone());
 
     let nav_targets = match_ast! {
@@ -38,6 +39,17 @@ pub(crate) fn goto_definition(
     Some(RangeInfo::new(original_token.text_range(), nav_targets))
 }
 
+fn pick_best(tokens: TokenAtOffset<SyntaxToken>) -> Option<SyntaxToken> {
+    return tokens.max_by_key(priority);
+    fn priority(n: &SyntaxToken) -> usize {
+        match n.kind() {
+            IDENT | INT_NUMBER => 2,
+            kind if kind.is_trivia() => 0,
+            _ => 1,
+        }
+    }
+}
+
 #[derive(Debug)]
 pub(crate) enum ReferenceResult {
     Exact(NavigationTarget),
@@ -234,6 +246,18 @@ mod tests {
         );
     }
 
+    #[test]
+    fn goto_definition_works_at_start_of_item() {
+        check_goto(
+            "
+            //- /lib.rs
+            struct Foo;
+            enum E { X(<|>Foo) }
+            ",
+            "Foo STRUCT_DEF FileId(1) [0; 11) [7; 10)",
+        );
+    }
+
     #[test]
     fn goto_definition_resolves_correct_name() {
         check_goto(
@@ -434,6 +458,22 @@ mod tests {
         );
     }
 
+    #[test]
+    fn goto_for_tuple_fields() {
+        check_goto(
+            "
+            //- /lib.rs
+            struct Foo(u32);
+
+            fn bar() {
+                let foo = Foo(0);
+                foo.<|>0;
+            }
+            ",
+            "TUPLE_FIELD_DEF FileId(1) [11; 14)",
+        );
+    }
+
     #[test]
     fn goto_definition_works_for_ufcs_inherent_methods() {
         check_goto(

crates/ra_ide/src/goto_type_definition.rs

@@ -1,7 +1,7 @@
 //! FIXME: write short doc here
 
 use hir::db::AstDatabase;
-use ra_syntax::{ast, AstNode};
+use ra_syntax::{ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset};
 
 use crate::{
     db::RootDatabase, display::ToNav, expand::descend_into_macros, FilePosition, NavigationTarget,
@@ -13,7 +13,7 @@ pub(crate) fn goto_type_definition(
     position: FilePosition,
 ) -> Option<RangeInfo<Vec<NavigationTarget>>> {
     let file = db.parse_or_expand(position.file_id.into())?;
-    let token = file.token_at_offset(position.offset).filter(|it| !it.kind().is_trivia()).next()?;
+    let token = pick_best(file.token_at_offset(position.offset))?;
     let token = descend_into_macros(db, position.file_id, token);
 
     let node = token.value.ancestors().find_map(|token| {
@@ -41,6 +41,17 @@ pub(crate) fn goto_type_definition(
     Some(RangeInfo::new(node.text_range(), vec![nav]))
 }
 
+fn pick_best(tokens: TokenAtOffset<SyntaxToken>) -> Option<SyntaxToken> {
+    return tokens.max_by_key(priority);
+    fn priority(n: &SyntaxToken) -> usize {
+        match n.kind() {
+            IDENT | INT_NUMBER => 2,
+            kind if kind.is_trivia() => 0,
+            _ => 1,
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::mock_analysis::analysis_and_position;
@@ -102,4 +113,32 @@ mod tests {
             "Foo STRUCT_DEF FileId(1) [52; 65) [59; 62)",
         );
     }
+
+    #[test]
+    fn goto_type_definition_for_param() {
+        check_goto(
+            "
+            //- /lib.rs
+            struct Foo;
+            fn foo(<|>f: Foo) {}
+            ",
+            "Foo STRUCT_DEF FileId(1) [0; 11) [7; 10)",
+        );
+    }
+
+    #[test]
+    fn goto_type_definition_for_tuple_field() {
+        check_goto(
+            "
+            //- /lib.rs
+            struct Foo;
+            struct Bar(Foo);
+            fn foo() {
+                let bar = Bar(Foo);
+                bar.<|>0;
+            }
+            ",
+            "Foo STRUCT_DEF FileId(1) [0; 11) [7; 10)",
+        );
+    }
 }

crates/ra_ide/src/hover.rs

@@ -6,6 +6,8 @@ use ra_syntax::{
     algo::find_covering_element,
     ast::{self, DocCommentsOwner},
     match_ast, AstNode,
+    SyntaxKind::*,
+    SyntaxToken, TokenAtOffset,
 };
 
 use crate::{
@@ -156,7 +158,7 @@ fn hover_text_from_name_kind(
 
 pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeInfo<HoverResult>> {
     let file = db.parse_or_expand(position.file_id.into())?;
-    let token = file.token_at_offset(position.offset).filter(|it| !it.kind().is_trivia()).next()?;
+    let token = pick_best(file.token_at_offset(position.offset))?;
     let token = descend_into_macros(db, position.file_id, token);
 
     let mut res = HoverResult::new();
@@ -218,6 +220,18 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeIn
     Some(RangeInfo::new(range, res))
 }
 
+fn pick_best(tokens: TokenAtOffset<SyntaxToken>) -> Option<SyntaxToken> {
+    return tokens.max_by_key(priority);
+    fn priority(n: &SyntaxToken) -> usize {
+        match n.kind() {
+            IDENT | INT_NUMBER => 3,
+            L_PAREN | R_PAREN => 2,
+            kind if kind.is_trivia() => 0,
+            _ => 1,
+        }
+    }
+}
+
 pub(crate) fn type_of(db: &RootDatabase, frange: FileRange) -> Option<String> {
     let parse = db.parse(frange.file_id);
     let leaf_node = find_covering_element(parse.tree().syntax(), frange.range);
@@ -504,6 +518,13 @@ fn func(foo: i32) { if true { <|>foo; }; }
         assert_eq!(trim_markup_opt(hover.info.first()), Some("i32"));
     }
 
+    #[test]
+    fn hover_for_param_edge() {
+        let (analysis, position) = single_file_with_position("fn func(<|>foo: i32) {}");
+        let hover = analysis.hover(position).unwrap().unwrap();
+        assert_eq!(trim_markup_opt(hover.info.first()), Some("i32"));
+    }
+
     #[test]
     fn test_type_of_for_function() {
         let (analysis, range) = single_file_with_range(