Apply suggestions from code review

Author: vsrs

crates/ra_ide/src/hover.rs

@@ -17,6 +17,7 @@ use crate::{
     runnables::runnable,
     FileId, FilePosition, NavigationTarget, RangeInfo, Runnable,
 };
+use test_utils::mark;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct HoverConfig {
@@ -202,10 +203,9 @@ fn runnable_action(
             ModuleDef::Function(it) => {
                 let src = it.source(sema.db);
                 if src.file_id != file_id.into() {
-                    // Don't try to find runnables in a macro generated code.
-                    // See tests below:
-                    //   test_hover_macro_generated_struct_fn_doc_comment
-                    //   test_hover_macro_generated_struct_fn_doc_attr
+                    mark::hit!(hover_macro_generated_struct_fn_doc_comment);
+                    mark::hit!(hover_macro_generated_struct_fn_doc_attr);
+
                     return None;
                 }
 
@@ -1121,6 +1121,8 @@ fn func(foo: i32) { if true { <|>foo; }; }
 
     #[test]
     fn test_hover_macro_generated_struct_fn_doc_comment() {
+        mark::check!(hover_macro_generated_struct_fn_doc_comment);
+
         check_hover_result(
             r#"
             //- /lib.rs
@@ -1147,6 +1149,8 @@ fn func(foo: i32) { if true { <|>foo; }; }
 
     #[test]
     fn test_hover_macro_generated_struct_fn_doc_attr() {
+        mark::check!(hover_macro_generated_struct_fn_doc_attr);
+
         check_hover_result(
             r#"
             //- /lib.rs

crates/rust-analyzer/src/main_loop/handlers.rs

@@ -408,7 +408,7 @@ pub fn handle_runnables(
             continue;
         }
 
-        res.push(to_proto::runnable(&snap, file_id, &runnable)?);
+        res.push(to_proto::runnable(&snap, file_id, runnable)?);
     }
 
     // Add `cargo check` and `cargo test` for the whole package
@@ -818,7 +818,7 @@ pub fn handle_code_lens(
 
             let action = runnable.action();
             let range = to_proto::range(&line_index, runnable.nav.range());
-            let r = to_proto::runnable(&snap, file_id, &runnable)?;
+            let r = to_proto::runnable(&snap, file_id, runnable)?;
             if snap.config.lens.run {
                 let lens = CodeLens {
                     range,
@@ -830,7 +830,7 @@ pub fn handle_code_lens(
 
             if action.debugee && snap.config.lens.debug {
                 let debug_lens =
-                    CodeLens { range, command: Some(debug_single_command(r)), data: None };
+                    CodeLens { range, command: Some(debug_single_command(&r)), data: None };
                 lenses.push(debug_lens);
             }
         }
@@ -1142,7 +1142,7 @@ fn run_single_command(runnable: &lsp_ext::Runnable, title: &str) -> Command {
     }
 }
 
-fn debug_single_command(runnable: lsp_ext::Runnable) -> Command {
+fn debug_single_command(runnable: &lsp_ext::Runnable) -> Command {
     Command {
         title: "Debug".into(),
         command: "rust-analyzer.debugSingle".into(),
@@ -1183,26 +1183,25 @@ fn show_impl_command_link(
 fn to_runnable_action(
     snap: &GlobalStateSnapshot,
     file_id: FileId,
-    runnable: &Runnable,
+    runnable: Runnable,
 ) -> Option<lsp_ext::CommandLinkGroup> {
     let cargo_spec = CargoTargetSpec::for_file(&snap, file_id).ok()?;
-    if should_skip_target(runnable, cargo_spec.as_ref()) {
+    if should_skip_target(&runnable, cargo_spec.as_ref()) {
         return None;
     }
 
+    let action: &'static _ = runnable.action();
     to_proto::runnable(snap, file_id, runnable).ok().map(|r| {
         let mut group = lsp_ext::CommandLinkGroup::default();
 
-        let action = runnable.action();
         if snap.config.hover.run {
             let run_command = run_single_command(&r, action.run_title);
             group.commands.push(to_command_link(run_command, r.label.clone()));
         }
 
         if snap.config.hover.debug {
-            let hint = r.label.clone();
-            let dbg_command = debug_single_command(r);
-            group.commands.push(to_command_link(dbg_command, hint));
+            let dbg_command = debug_single_command(&r);
+            group.commands.push(to_command_link(dbg_command, r.label));
         }
 
         group
@@ -1222,7 +1221,7 @@ fn prepare_hover_actions(
         .iter()
         .filter_map(|it| match it {
             HoverAction::Implementaion(position) => show_impl_command_link(snap, position),
-            HoverAction::Runnable(r) => to_runnable_action(snap, file_id, r),
+            HoverAction::Runnable(r) => to_runnable_action(snap, file_id, r.clone()),
         })
         .collect()
 }
@@ -1232,10 +1231,7 @@ fn should_skip_target(runnable: &Runnable, cargo_spec: Option<&CargoTargetSpec>)
         RunnableKind::Bin => {
             // Do not suggest binary run on other target than binary
             match &cargo_spec {
-                Some(spec) => match spec.target_kind {
-                    TargetKind::Bin => false,
-                    _ => true,
-                },
+                Some(spec) => spec.target_kind != TargetKind::Bin,
                 None => true,
             }
         }

crates/rust-analyzer/src/to_proto.rs

@@ -656,14 +656,14 @@ pub(crate) fn resolved_code_action(
 pub(crate) fn runnable(
     snap: &GlobalStateSnapshot,
     file_id: FileId,
-    runnable: &Runnable,
+    runnable: Runnable,
 ) -> Result<lsp_ext::Runnable> {
     let spec = CargoTargetSpec::for_file(snap, file_id)?;
     let target = spec.as_ref().map(|s| s.target.clone());
     let (cargo_args, executable_args) =
         CargoTargetSpec::runnable_args(spec, &runnable.kind, &runnable.cfg_exprs)?;
     let label = runnable.label(target);
-    let location = location_link(snap, None, runnable.nav.clone())?;
+    let location = location_link(snap, None, runnable.nav)?;
 
     Ok(lsp_ext::Runnable {
         label,