Merge #5271

5271: Tidyup hover r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

1 files changed, 37 insertions, 51 deletions

diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs
index eaba2b61e..61359c770 100644
--- a/crates/ra_ide/src/hover.rs
+++ b/crates/ra_ide/src/hover.rs
@@ -77,10 +77,6 @@ impl HoverResult {
         Self::default()
     }
 
-    pub fn extend(&mut self, item: Option<String>) {
-        self.results.extend(item);
-    }
-
     pub fn is_empty(&self) -> bool {
         self.results.is_empty()
     }
@@ -89,22 +85,9 @@ impl HoverResult {
         self.results.len()
     }
 
-    pub fn first(&self) -> Option<&str> {
-        self.results.first().map(String::as_str)
-    }
-
-    pub fn results(&self) -> &[String] {
-        &self.results
-    }
-
     pub fn actions(&self) -> &[HoverAction] {
         &self.actions
     }
-
-    pub fn push_action(&mut self, action: HoverAction) {
-        self.actions.push(action);
-    }
-
     /// Returns the results converted into markup
     /// for displaying in a UI
     ///
@@ -112,6 +95,10 @@ impl HoverResult {
     pub fn to_markup(&self) -> String {
         self.results.join("\n\n___\n")
     }
+
+    fn push_action(&mut self, action: HoverAction) {
+        self.actions.push(action);
+    }
 }
 
 // Feature: Hover
@@ -126,30 +113,33 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeIn
 
     let mut res = HoverResult::new();
 
-    if let Some((node, name_kind)) = match_ast! {
-        match (token.parent()) {
+    let node = token.parent();
+    let definition = match_ast! {
+        match node {
             ast::NameRef(name_ref) => {
-                classify_name_ref(&sema, &name_ref).map(|d| (name_ref.syntax().clone(), d.definition()))
+                classify_name_ref(&sema, &name_ref).map(|d| d.definition())
             },
             ast::Name(name) => {
-                classify_name(&sema, &name).map(|d| (name.syntax().clone(), d.definition()))
+                classify_name(&sema, &name).map(|d| d.definition())
             },
             _ => None,
         }
-    } {
+    };
+    if let Some(definition) = definition {
         let range = sema.original_range(&node).range;
-        res.extend(hover_text_from_name_kind(db, name_kind));
-
+        if let Some(text) = hover_text_from_name_kind(db, definition) {
+            res.results.push(text);
+        }
         if !res.is_empty() {
-            if let Some(action) = show_implementations_action(db, name_kind) {
+            if let Some(action) = show_implementations_action(db, definition) {
                 res.push_action(action);
             }
 
-            if let Some(action) = runnable_action(&sema, name_kind, position.file_id) {
+            if let Some(action) = runnable_action(&sema, definition, position.file_id) {
                 res.push_action(action);
             }
 
-            if let Some(action) = goto_type_action(db, name_kind) {
+            if let Some(action) = goto_type_action(db, definition) {
                 res.push_action(action);
             }
 
@@ -178,7 +168,7 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeIn
         }
     }?;
 
-    res.extend(Some(rust_code_markup(&ty.display(db))));
+    res.results.push(rust_code_markup(&ty.display(db)));
     let range = sema.original_range(&node).range;
     Some(RangeInfo::new(range, res))
 }
@@ -405,10 +395,6 @@ mod tests {
         s.trim_start_matches("```rust\n").trim_end_matches("\n```")
     }
 
-    fn trim_markup_opt(s: Option<&str>) -> Option<&str> {
-        s.map(trim_markup)
-    }
-
     fn assert_impl_action(action: &HoverAction, position: u32) {
         let offset = match action {
             HoverAction::Implementaion(pos) => pos.offset,
@@ -420,7 +406,7 @@ mod tests {
     fn check_hover_result(ra_fixture: &str, expected: &[&str]) -> (String, Vec<HoverAction>) {
         let (analysis, position) = analysis_and_position(ra_fixture);
         let hover = analysis.hover(position).unwrap().unwrap();
-        let mut results = Vec::from(hover.info.results());
+        let mut results = hover.info.results.clone();
         results.sort();
 
         for (markup, expected) in
@@ -453,7 +439,7 @@ fn main() {
         );
         let hover = analysis.hover(position).unwrap().unwrap();
         assert_eq!(hover.range, TextRange::new(58.into(), 63.into()));
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("u32"));
+        assert_eq!(trim_markup(&hover.info.results[0]), ("u32"));
     }
 
     #[test]
@@ -652,7 +638,7 @@ fn main() {
             ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("Option\n```\n\n```rust\nSome"));
+        assert_eq!(trim_markup(&hover.info.results[0]), ("Option\n```\n\n```rust\nSome"));
 
         let (analysis, position) = analysis_and_position(
             "
@@ -665,7 +651,7 @@ fn main() {
             ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("Option<i32>"));
+        assert_eq!(trim_markup(&hover.info.results[0]), ("Option<i32>"));
     }
 
     #[test]
@@ -722,14 +708,14 @@ The Some variant
     fn hover_for_local_variable() {
         let (analysis, position) = analysis_and_position("fn func(foo: i32) { fo<|>o; }");
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("i32"));
+        assert_eq!(trim_markup(&hover.info.results[0]), "i32");
     }
 
     #[test]
     fn hover_for_local_variable_pat() {
         let (analysis, position) = analysis_and_position("fn func(fo<|>o: i32) {}");
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("i32"));
+        assert_eq!(trim_markup(&hover.info.results[0]), "i32");
     }
 
     #[test]
@@ -740,14 +726,14 @@ fn func(foo: i32) { if true { <|>foo; }; }
 ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("i32"));
+        assert_eq!(trim_markup(&hover.info.results[0]), "i32");
     }
 
     #[test]
     fn hover_for_param_edge() {
         let (analysis, position) = analysis_and_position("fn func(<|>foo: i32) {}");
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("i32"));
+        assert_eq!(trim_markup(&hover.info.results[0]), "i32");
     }
 
     #[test]
@@ -768,7 +754,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
             ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("Thing"));
+        assert_eq!(trim_markup(&hover.info.results[0]), ("Thing"));
     }
 
     #[test]
@@ -792,8 +778,8 @@ fn func(foo: i32) { if true { <|>foo; }; }
         );
         let hover = analysis.hover(position).unwrap().unwrap();
         assert_eq!(
-            trim_markup_opt(hover.info.first()),
-            Some("wrapper::Thing\n```\n\n```rust\nfn new() -> Thing")
+            trim_markup(&hover.info.results[0]),
+            ("wrapper::Thing\n```\n\n```rust\nfn new() -> Thing")
         );
     }
 
@@ -816,7 +802,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
             ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("const C: u32"));
+        assert_eq!(trim_markup(&hover.info.results[0]), ("const C: u32"));
     }
 
     #[test]
@@ -832,7 +818,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
         ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("Thing"));
+        assert_eq!(trim_markup(&hover.info.results[0]), ("Thing"));
 
         /* FIXME: revive these tests
                 let (analysis, position) = analysis_and_position(
@@ -847,7 +833,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
                 );
 
                 let hover = analysis.hover(position).unwrap().unwrap();
-                assert_eq!(trim_markup_opt(hover.info.first()), Some("Thing"));
+                assert_eq!(trim_markup(&hover.info.results[0]), ("Thing"));
 
                 let (analysis, position) = analysis_and_position(
                     "
@@ -860,7 +846,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
                     ",
                 );
                 let hover = analysis.hover(position).unwrap().unwrap();
-                assert_eq!(trim_markup_opt(hover.info.first()), Some("enum Thing"));
+                assert_eq!(trim_markup(&hover.info.results[0]), ("enum Thing"));
 
                 let (analysis, position) = analysis_and_position(
                     "
@@ -872,7 +858,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
                     ",
                 );
                 let hover = analysis.hover(position).unwrap().unwrap();
-                assert_eq!(trim_markup_opt(hover.info.first()), Some("enum Thing"));
+                assert_eq!(trim_markup(&hover.info.results[0]), ("enum Thing"));
         */
     }
 
@@ -889,7 +875,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
             ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("i32"));
+        assert_eq!(trim_markup(&hover.info.results[0]), "i32");
     }
 
     #[test]
@@ -906,7 +892,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
             ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("macro_rules! foo"));
+        assert_eq!(trim_markup(&hover.info.results[0]), ("macro_rules! foo"));
     }
 
     #[test]
@@ -917,7 +903,7 @@ fn func(foo: i32) { if true { <|>foo; }; }
             ",
         );
         let hover = analysis.hover(position).unwrap().unwrap();
-        assert_eq!(trim_markup_opt(hover.info.first()), Some("i32"));
+        assert_eq!(trim_markup(&hover.info.results[0]), "i32");
     }
 
     #[test]