Merge #9364

9364: Split hover actions config into its own config struct r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <[email protected]>

4 files changed, 157 insertions, 97 deletions

diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 0c1da8774..c6d6bb74a 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -1,5 +1,5 @@
 use either::Either;
-use hir::{AsAssocItem, HasAttrs, HasSource, HirDisplay};
+use hir::{AsAssocItem, HasAttrs, HasSource, HirDisplay, Semantics};
 use ide_db::{
     base_db::SourceDatabase,
     defs::{Definition, NameClass, NameRefClass},
@@ -30,39 +30,20 @@ use crate::{
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct HoverConfig {
-    pub implementations: bool,
-    pub references: bool,
-    pub run: bool,
-    pub debug: bool,
-    pub goto_type_def: bool,
     pub links_in_hover: bool,
-    pub markdown: bool,
-    pub documentation: bool,
+    pub documentation: Option<HoverDocFormat>,
 }
 
 impl HoverConfig {
-    pub const NO_ACTIONS: Self = Self {
-        implementations: false,
-        references: false,
-        run: false,
-        debug: false,
-        goto_type_def: false,
-        links_in_hover: true,
-        markdown: true,
-        documentation: true,
-    };
-
-    pub fn any_actions(&self) -> bool {
-        self.implementations || self.references || self.runnable() || self.goto_type_def
-    }
-
-    pub fn no_actions(&self) -> bool {
-        !self.any_actions()
+    fn markdown(&self) -> bool {
+        matches!(self.documentation, Some(HoverDocFormat::Markdown))
     }
+}
 
-    pub fn runnable(&self) -> bool {
-        self.run || self.debug
-    }
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum HoverDocFormat {
+    Markdown,
+    PlainText,
 }
 
 #[derive(Debug, Clone)]
@@ -95,9 +76,7 @@ pub struct HoverResult {
 pub(crate) fn hover(
     db: &RootDatabase,
     position: FilePosition,
-    links_in_hover: bool,
-    documentation: bool,
-    markdown: bool,
+    config: &HoverConfig,
 ) -> Option<RangeInfo<HoverResult>> {
     let sema = hir::Semantics::new(db);
     let file = sema.parse(position.file_id).syntax().clone();
@@ -156,10 +135,8 @@ pub(crate) fn hover(
             }
             _ => None,
         };
-        if let Some(markup) =
-            hover_for_definition(db, definition, famous_defs.as_ref(), documentation)
-        {
-            res.markup = process_markup(sema.db, definition, &markup, links_in_hover, markdown);
+        if let Some(markup) = hover_for_definition(db, definition, famous_defs.as_ref(), config) {
+            res.markup = process_markup(sema.db, definition, &markup, config);
             if let Some(action) = show_implementations_action(db, definition) {
                 res.actions.push(action);
             }
@@ -181,8 +158,7 @@ pub(crate) fn hover(
         }
     }
 
-    if let res @ Some(_) = hover_for_keyword(&sema, links_in_hover, markdown, documentation, &token)
-    {
+    if let res @ Some(_) = hover_for_keyword(&sema, config, &token) {
         return res;
     }
 
@@ -201,7 +177,7 @@ pub(crate) fn hover(
         }
     };
 
-    res.markup = if markdown {
+    res.markup = if config.markdown() {
         Markup::fenced_block(&ty.display(db))
     } else {
         ty.display(db).to_string().into()
@@ -375,13 +351,12 @@ fn process_markup(
     db: &RootDatabase,
     def: Definition,
     markup: &Markup,
-    links_in_hover: bool,
-    markdown: bool,
+    config: &HoverConfig,
 ) -> Markup {
     let markup = markup.as_str();
-    let markup = if !markdown {
+    let markup = if !config.markdown() {
         remove_markdown(markup)
-    } else if links_in_hover {
+    } else if config.links_in_hover {
         rewrite_links(db, markup, &def)
     } else {
         remove_links(markup)
@@ -428,7 +403,7 @@ fn hover_for_definition(
     db: &RootDatabase,
     def: Definition,
     famous_defs: Option<&FamousDefs>,
-    documentation: bool,
+    config: &HoverConfig,
 ) -> Option<Markup> {
     let mod_path = definition_mod_path(db, &def);
     let (label, docs) = match def {
@@ -466,7 +441,11 @@ fn hover_for_definition(
         Definition::Label(it) => return Some(Markup::fenced_block(&it.name(db))),
     };
 
-    return hover_markup(docs.filter(|_| documentation).map(Into::into), label, mod_path);
+    return hover_markup(
+        docs.filter(|_| config.documentation.is_some()).map(Into::into),
+        label,
+        mod_path,
+    );
 
     fn label_and_docs<D>(db: &RootDatabase, def: D) -> (String, Option<hir::Documentation>)
     where
@@ -502,13 +481,11 @@ fn hover_for_local(it: hir::Local, db: &RootDatabase) -> Option<Markup> {
 }
 
 fn hover_for_keyword(
-    sema: &hir::Semantics<RootDatabase>,
-    links_in_hover: bool,
-    markdown: bool,
-    documentation: bool,
+    sema: &Semantics<RootDatabase>,
+    config: &HoverConfig,
     token: &SyntaxToken,
 ) -> Option<RangeInfo<HoverResult>> {
-    if !token.kind().is_keyword() || !documentation {
+    if !token.kind().is_keyword() || !config.documentation.is_some() {
         return None;
     }
     let famous_defs = FamousDefs(sema, sema.scope(&token.parent()?).krate());
@@ -520,8 +497,7 @@ fn hover_for_keyword(
         sema.db,
         Definition::ModuleDef(doc_owner.into()),
         &hover_markup(Some(docs.into()), token.text().into(), None)?,
-        links_in_hover,
-        markdown,
+        config,
     );
     Some(RangeInfo::new(token.text_range(), HoverResult { markup, actions: Default::default() }))
 }
@@ -561,16 +537,34 @@ mod tests {
     use expect_test::{expect, Expect};
     use ide_db::base_db::FileLoader;
 
-    use crate::fixture;
+    use crate::{fixture, hover::HoverDocFormat, HoverConfig};
 
     fn check_hover_no_result(ra_fixture: &str) {
         let (analysis, position) = fixture::position(ra_fixture);
-        assert!(analysis.hover(position, true, true, true).unwrap().is_none());
+        assert!(analysis
+            .hover(
+                position,
+                &HoverConfig {
+                    links_in_hover: true,
+                    documentation: Some(HoverDocFormat::Markdown)
+                }
+            )
+            .unwrap()
+            .is_none());
     }
 
     fn check(ra_fixture: &str, expect: Expect) {
         let (analysis, position) = fixture::position(ra_fixture);
-        let hover = analysis.hover(position, true, true, true).unwrap().unwrap();
+        let hover = analysis
+            .hover(
+                position,
+                &HoverConfig {
+                    links_in_hover: true,
+                    documentation: Some(HoverDocFormat::Markdown),
+                },
+            )
+            .unwrap()
+            .unwrap();
 
         let content = analysis.db.file_text(position.file_id);
         let hovered_element = &content[hover.range];
@@ -581,7 +575,16 @@ mod tests {
 
     fn check_hover_no_links(ra_fixture: &str, expect: Expect) {
         let (analysis, position) = fixture::position(ra_fixture);
-        let hover = analysis.hover(position, false, true, true).unwrap().unwrap();
+        let hover = analysis
+            .hover(
+                position,
+                &HoverConfig {
+                    links_in_hover: false,
+                    documentation: Some(HoverDocFormat::Markdown),
+                },
+            )
+            .unwrap()
+            .unwrap();
 
         let content = analysis.db.file_text(position.file_id);
         let hovered_element = &content[hover.range];
@@ -592,7 +595,16 @@ mod tests {
 
     fn check_hover_no_markdown(ra_fixture: &str, expect: Expect) {
         let (analysis, position) = fixture::position(ra_fixture);
-        let hover = analysis.hover(position, true, true, false).unwrap().unwrap();
+        let hover = analysis
+            .hover(
+                position,
+                &HoverConfig {
+                    links_in_hover: true,
+                    documentation: Some(HoverDocFormat::PlainText),
+                },
+            )
+            .unwrap()
+            .unwrap();
 
         let content = analysis.db.file_text(position.file_id);
         let hovered_element = &content[hover.range];
@@ -603,7 +615,16 @@ mod tests {
 
     fn check_actions(ra_fixture: &str, expect: Expect) {
         let (analysis, position) = fixture::position(ra_fixture);
-        let hover = analysis.hover(position, true, true, true).unwrap().unwrap();
+        let hover = analysis
+            .hover(
+                position,
+                &HoverConfig {
+                    links_in_hover: true,
+                    documentation: Some(HoverDocFormat::Markdown),
+                },
+            )
+            .unwrap()
+            .unwrap();
         expect.assert_debug_eq(&hover.info.actions)
     }
 
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
index aac084012..b978e36af 100644
--- a/crates/ide/src/lib.rs
+++ b/crates/ide/src/lib.rs
@@ -75,7 +75,7 @@ pub use crate::{
     expand_macro::ExpandedMacro,
     file_structure::{StructureNode, StructureNodeKind},
     folding_ranges::{Fold, FoldKind},
-    hover::{HoverAction, HoverConfig, HoverGotoTypeData, HoverResult},
+    hover::{HoverAction, HoverConfig, HoverDocFormat, HoverGotoTypeData, HoverResult},
     inlay_hints::{InlayHint, InlayHintsConfig, InlayKind},
     markup::Markup,
     move_item::Direction,
@@ -408,11 +408,9 @@ impl Analysis {
     pub fn hover(
         &self,
         position: FilePosition,
-        links_in_hover: bool,
-        documentation: bool,
-        markdown: bool,
+        config: &HoverConfig,
     ) -> Cancellable<Option<RangeInfo<HoverResult>>> {
-        self.with_db(|db| hover::hover(db, position, links_in_hover, documentation, markdown))
+        self.with_db(|db| hover::hover(db, position, config))
     }
 
     /// Return URL(s) for the documentation of the symbol under the cursor.
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index 3aeca8839..b9aa6f0aa 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -10,7 +10,10 @@
 use std::{ffi::OsString, iter, path::PathBuf};
 
 use flycheck::FlycheckConfig;
-use ide::{AssistConfig, CompletionConfig, DiagnosticsConfig, HoverConfig, InlayHintsConfig};
+use ide::{
+    AssistConfig, CompletionConfig, DiagnosticsConfig, HoverConfig, HoverDocFormat,
+    InlayHintsConfig,
+};
 use ide_db::helpers::{
     insert_use::{ImportGranularity, InsertUseConfig, PrefixKind},
     SnippetCap,
@@ -32,6 +35,9 @@ use crate::{
 //
 // However, editor specific config, which the server doesn't know about, should
 // be specified directly in `package.json`.
+//
+// To deprecate an option by replacing it with another name use `new_name | `old_name` so that we keep
+// parsing the old name.
 config_data! {
     struct ConfigData {
         /// How imports should be grouped into use statements.
@@ -309,6 +315,37 @@ impl LensConfig {
     }
 }
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct HoverActionsConfig {
+    pub implementations: bool,
+    pub references: bool,
+    pub run: bool,
+    pub debug: bool,
+    pub goto_type_def: bool,
+}
+
+impl HoverActionsConfig {
+    pub const NO_ACTIONS: Self = Self {
+        implementations: false,
+        references: false,
+        run: false,
+        debug: false,
+        goto_type_def: false,
+    };
+
+    pub fn any(&self) -> bool {
+        self.implementations || self.references || self.runnable() || self.goto_type_def
+    }
+
+    pub fn none(&self) -> bool {
+        !self.any()
+    }
+
+    pub fn runnable(&self) -> bool {
+        self.run || self.debug
+    }
+}
+
 #[derive(Debug, Clone)]
 pub struct FilesConfig {
     pub watcher: FilesWatcher,
@@ -527,7 +564,7 @@ impl Config {
     pub fn code_action_group(&self) -> bool {
         self.experimental("codeActionGroup")
     }
-    pub fn hover_actions(&self) -> bool {
+    pub fn experimental_hover_actions(&self) -> bool {
         self.experimental("hoverActions")
     }
     pub fn server_status_notification(&self) -> bool {
@@ -727,31 +764,41 @@ impl Config {
             refs: self.data.lens_enable && self.data.lens_references,
         }
     }
-    pub fn highlighting_strings(&self) -> bool {
-        self.data.highlighting_strings
-    }
-    pub fn hover(&self) -> HoverConfig {
-        HoverConfig {
+    pub fn hover_actions(&self) -> HoverActionsConfig {
+        HoverActionsConfig {
             implementations: self.data.hoverActions_enable
                 && self.data.hoverActions_implementations,
             references: self.data.hoverActions_enable && self.data.hoverActions_references,
             run: self.data.hoverActions_enable && self.data.hoverActions_run,
             debug: self.data.hoverActions_enable && self.data.hoverActions_debug,
             goto_type_def: self.data.hoverActions_enable && self.data.hoverActions_gotoTypeDef,
+        }
+    }
+    pub fn highlighting_strings(&self) -> bool {
+        self.data.highlighting_strings
+    }
+    pub fn hover(&self) -> HoverConfig {
+        HoverConfig {
             links_in_hover: self.data.hover_linksInHover,
-            markdown: try_or!(
-                self.caps
-                    .text_document
-                    .as_ref()?
-                    .hover
-                    .as_ref()?
-                    .content_format
-                    .as_ref()?
-                    .as_slice(),
-                &[]
-            )
-            .contains(&MarkupKind::Markdown),
-            documentation: self.data.hover_documentation,
+            documentation: self.data.hover_documentation.then(|| {
+                let is_markdown = try_or!(
+                    self.caps
+                        .text_document
+                        .as_ref()?
+                        .hover
+                        .as_ref()?
+                        .content_format
+                        .as_ref()?
+                        .as_slice(),
+                    &[]
+                )
+                .contains(&MarkupKind::Markdown);
+                if is_markdown {
+                    HoverDocFormat::Markdown
+                } else {
+                    HoverDocFormat::PlainText
+                }
+            }),
         }
     }
 
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index eff1e6c93..dcead5f5c 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -861,13 +861,7 @@ pub(crate) fn handle_hover(
 ) -> Result<Option<lsp_ext::Hover>> {
     let _p = profile::span("handle_hover");
     let position = from_proto::file_position(&snap, params.text_document_position_params)?;
-    let hover_config = snap.config.hover();
-    let info = match snap.analysis.hover(
-        position,
-        hover_config.links_in_hover,
-        hover_config.documentation,
-        hover_config.markdown,
-    )? {
+    let info = match snap.analysis.hover(position, &snap.config.hover())? {
         None => return Ok(None),
         Some(info) => info,
     };
@@ -1487,7 +1481,7 @@ fn show_impl_command_link(
     snap: &GlobalStateSnapshot,
     position: &FilePosition,
 ) -> Option<lsp_ext::CommandLinkGroup> {
-    if snap.config.hover().implementations {
+    if snap.config.hover_actions().implementations {
         if let Some(nav_data) = snap.analysis.goto_implementation(*position).unwrap_or(None) {
             let uri = to_proto::url(snap, position.file_id);
             let line_index = snap.file_line_index(position.file_id).ok()?;
@@ -1513,7 +1507,7 @@ fn show_ref_command_link(
     snap: &GlobalStateSnapshot,
     position: &FilePosition,
 ) -> Option<lsp_ext::CommandLinkGroup> {
-    if snap.config.hover().references {
+    if snap.config.hover_actions().references {
         if let Some(ref_search_res) = snap.analysis.find_all_refs(*position, None).unwrap_or(None) {
             let uri = to_proto::url(snap, position.file_id);
             let line_index = snap.file_line_index(position.file_id).ok()?;
@@ -1544,8 +1538,8 @@ fn runnable_action_links(
     runnable: Runnable,
 ) -> Option<lsp_ext::CommandLinkGroup> {
     let cargo_spec = CargoTargetSpec::for_file(snap, runnable.nav.file_id).ok()?;
-    let hover_config = snap.config.hover();
-    if !hover_config.runnable() || should_skip_target(&runnable, cargo_spec.as_ref()) {
+    let hover_actions_config = snap.config.hover_actions();
+    if !hover_actions_config.runnable() || should_skip_target(&runnable, cargo_spec.as_ref()) {
         return None;
     }
 
@@ -1553,12 +1547,12 @@ fn runnable_action_links(
     to_proto::runnable(snap, runnable).ok().map(|r| {
         let mut group = lsp_ext::CommandLinkGroup::default();
 
-        if hover_config.run {
+        if hover_actions_config.run {
             let run_command = to_proto::command::run_single(&r, action.run_title);
             group.commands.push(to_command_link(run_command, r.label.clone()));
         }
 
-        if hover_config.debug {
+        if hover_actions_config.debug {
             let dbg_command = to_proto::command::debug_single(&r);
             group.commands.push(to_command_link(dbg_command, r.label));
         }
@@ -1571,7 +1565,7 @@ fn goto_type_action_links(
     snap: &GlobalStateSnapshot,
     nav_targets: &[HoverGotoTypeData],
 ) -> Option<lsp_ext::CommandLinkGroup> {
-    if !snap.config.hover().goto_type_def || nav_targets.is_empty() {
+    if !snap.config.hover_actions().goto_type_def || nav_targets.is_empty() {
         return None;
     }
 
@@ -1591,7 +1585,7 @@ fn prepare_hover_actions(
     snap: &GlobalStateSnapshot,
     actions: &[HoverAction],
 ) -> Vec<lsp_ext::CommandLinkGroup> {
-    if snap.config.hover().no_actions() || !snap.config.hover_actions() {
+    if snap.config.hover_actions().none() || !snap.config.experimental_hover_actions() {
         return Vec::new();
     }