Repository: rust-lang/rust-analyzer
Pull request #18057 (2024-09-05 20:51:00)

internal: Better testing infra for ratoml
This PR makes some improvements on how we test configs that come from `rust-analyzer.toml` files.
It was primarily used to solve #18021 but along the way I could not really determine the cause of the said issue which makes me think that it may not be related to the changes that I made earlier to the ratoml infra. In either way `custom_snippets` are now made `global` because we still don't have a tree that maps a `SourceRootId` to a set of `Snippet`s. 


Patch:
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index e4e8c9c6d9..b6a1bd1586 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -75,6 +75,8 @@ config_data! {
         /// How many worker threads to handle priming caches. The default `0` means to pick automatically.
         cachePriming_numThreads: NumThreads = NumThreads::Physical,
 
+        /// Custom completion snippets.
+        completion_snippets_custom: FxHashMap<String, SnippetDef> = Config::completion_snippets_default(),
 
 
         /// These directories will be ignored by rust-analyzer. They are
@@ -438,48 +440,6 @@ config_data! {
         completion_postfix_enable: bool         = true,
         /// Enables completions of private items and fields that are defined in the current workspace even if they are not visible at the current position.
         completion_privateEditable_enable: bool = false,
-        /// Custom completion snippets.
-        completion_snippets_custom: FxHashMap<String, SnippetDef> = serde_json::from_str(r#"{
-            "Arc::new": {
-                "postfix": "arc",
-                "body": "Arc::new(${receiver})",
-                "requires": "std::sync::Arc",
-                "description": "Put the expression into an `Arc`",
-                "scope": "expr"
-            },
-            "Rc::new": {
-                "postfix": "rc",
-                "body": "Rc::new(${receiver})",
-                "requires": "std::rc::Rc",
-                "description": "Put the expression into an `Rc`",
-                "scope": "expr"
-            },
-            "Box::pin": {
-                "postfix": "pinbox",
-                "body": "Box::pin(${receiver})",
-                "requires": "std::boxed::Box",
-                "description": "Put the expression into a pinned `Box`",
-                "scope": "expr"
-            },
-            "Ok": {
-                "postfix": "ok",
-                "body": "Ok(${receiver})",
-                "description": "Wrap the expression in a `Result::Ok`",
-                "scope": "expr"
-            },
-            "Err": {
-                "postfix": "err",
-                "body": "Err(${receiver})",
-                "description": "Wrap the expression in a `Result::Err`",
-                "scope": "expr"
-            },
-            "Some": {
-                "postfix": "some",
-                "body": "Some(${receiver})",
-                "description": "Wrap the expression in an `Option::Some`",
-                "scope": "expr"
-            }
-        }"#).unwrap(),
         /// Whether to enable term search based snippets like `Some(foo.bar().baz())`.
         completion_termSearch_enable: bool = false,
         /// Term search fuel in "units of work" for autocompletion (Defaults to 1000).
@@ -889,7 +849,7 @@ impl Config {
                 // IMPORTANT : This holds as long as ` completion_snippets_custom` is declared `client`.
                 config.snippets.clear();
 
-                let snips = self.completion_snippets_custom(None).to_owned();
+                let snips = self.completion_snippets_custom().to_owned();
 
                 for (name, def) in snips.iter() {
                     if def.prefix.is_empty() && def.postfix.is_empty() {
@@ -1266,7 +1226,7 @@ pub struct NotificationsConfig {
     pub cargo_toml_not_found: bool,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Deserialize, Serialize, Debug, Clone)]
 pub enum RustfmtConfig {
     Rustfmt { extra_args: Vec<String>, enable_range_formatting: bool },
     CustomCommand { command: String, args: Vec<String> },
@@ -1897,6 +1857,53 @@ impl Config {
         }
     }
 
+    pub(crate) fn completion_snippets_default() -> FxHashMap<String, SnippetDef> {
+        serde_json::from_str(
+            r#"{
+            "Arc::new": {
+                "postfix": "arc",
+                "body": "Arc::new(${receiver})",
+                "requires": "std::sync::Arc",
+                "description": "Put the expression into an `Arc`",
+                "scope": "expr"
+            },
+            "Rc::new": {
+                "postfix": "rc",
+                "body": "Rc::new(${receiver})",
+                "requires": "std::rc::Rc",
+                "description": "Put the expression into an `Rc`",
+                "scope": "expr"
+            },
+            "Box::pin": {
+                "postfix": "pinbox",
+                "body": "Box::pin(${receiver})",
+                "requires": "std::boxed::Box",
+                "description": "Put the expression into a pinned `Box`",
+                "scope": "expr"
+            },
+            "Ok": {
+                "postfix": "ok",
+                "body": "Ok(${receiver})",
+                "description": "Wrap the expression in a `Result::Ok`",
+                "scope": "expr"
+            },
+            "Err": {
+                "postfix": "err",
+                "body": "Err(${receiver})",
+                "description": "Wrap the expression in a `Result::Err`",
+                "scope": "expr"
+            },
+            "Some": {
+                "postfix": "some",
+                "body": "Some(${receiver})",
+                "description": "Wrap the expression in an `Option::Some`",
+                "scope": "expr"
+            }
+        }"#,
+        )
+        .unwrap()
+    }
+
     pub fn rustfmt(&self, source_root_id: Option<SourceRootId>) -> RustfmtConfig {
         match &self.rustfmt_overrideCommand(source_root_id) {
             Some(args) if !args.is_empty() => {
diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs
index 09d5086201..bcbd970a0d 100644
--- a/crates/rust-analyzer/src/handlers/request.rs
+++ b/crates/rust-analyzer/src/handlers/request.rs
@@ -40,7 +40,10 @@ use crate::{
     hack_recover_crate_name,
     line_index::LineEndings,
     lsp::{
-        ext::InternalTestingFetchConfigParams,
+        ext::{
+            InternalTestingFetchConfigOption, InternalTestingFetchConfigParams,
+            InternalTestingFetchConfigResponse,
+        },
         from_proto, to_proto,
         utils::{all_edits_are_disjoint, invalid_params_error},
         LspError,
@@ -2292,7 +2295,7 @@ pub(crate) fn fetch_dependency_list(
 pub(crate) fn internal_testing_fetch_config(
     state: GlobalStateSnapshot,
     params: InternalTestingFetchConfigParams,
-) -> anyhow::Result<serde_json::Value> {
+) -> anyhow::Result<Option<InternalTestingFetchConfigResponse>> {
     let source_root = params
         .text_document
         .map(|it| {
@@ -2302,15 +2305,18 @@ pub(crate) fn internal_testing_fetch_config(
                 .map_err(anyhow::Error::from)
         })
         .transpose()?;
-    serde_json::to_value(match &*params.config {
-        "local" => state.config.assist(source_root).assist_emit_must_use,
-        "workspace" => matches!(
-            state.config.rustfmt(source_root),
-            RustfmtConfig::Rustfmt { enable_range_formatting: true, .. }
-        ),
-        _ => return Err(anyhow::anyhow!("Unknown test config key: {}", params.config)),
-    })
-    .map_err(Into::into)
+    Ok(Some(match params.config {
+        InternalTestingFetchConfigOption::AssistEmitMustUse => {
+            InternalTestingFetchConfigResponse::AssistEmitMustUse(
+                state.config.assist(source_root).assist_emit_must_use,
+            )
+        }
+        InternalTestingFetchConfigOption::CheckWorkspace => {
+            InternalTestingFetchConfigResponse::CheckWorkspace(
+                state.config.flycheck_workspace(source_root),
+            )
+        }
+    }))
 }
 
 /// Searches for the directory of a Rust crate given this crate's root file path.
diff --git a/crates/rust-analyzer/src/lsp/ext.rs b/crates/rust-analyzer/src/lsp/ext.rs
index ed6f16a173..618481bbc6 100644
--- a/crates/rust-analyzer/src/lsp/ext.rs
+++ b/crates/rust-analyzer/src/lsp/ext.rs
@@ -16,9 +16,22 @@ use serde::{Deserialize, Serialize};
 
 pub enum InternalTestingFetchConfig {}
 
+#[derive(Deserialize, Serialize, Debug)]
+pub enum InternalTestingFetchConfigOption {
+    AssistEmitMustUse,
+    CheckWorkspace,
+}
+
+#[derive(Deserialize, Serialize, Debug, PartialEq, Eq)]
+pub enum InternalTestingFetchConfigResponse {
+    AssistEmitMustUse(bool),
+    CheckWorkspace(bool),
+}
+
 impl Request for InternalTestingFetchConfig {
     type Params = InternalTestingFetchConfigParams;
-    type Result = serde_json::Value;
+    // Option is solely to circumvent Default bound.
+    type Result = Option<InternalTestingFetchConfigResponse>;
     const METHOD: &'static str = "rust-analyzer-internal/internalTestingFetchConfig";
 }
 
@@ -26,7 +39,7 @@ impl Request for InternalTestingFetchConfig {
 #[serde(rename_all = "camelCase")]
 pub struct InternalTestingFetchConfigParams {
     pub text_document: Option<TextDocumentIdentifier>,
-    pub config: String,
+    pub config: InternalTestingFetchConfigOption,
 }
 pub enum AnalyzerStatus {}
 
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md
index 68afacf2db..b7bac4d29f 100644
--- a/docs/dev/lsp-extensions.md
+++ b/docs/dev/lsp-extensions.md
@@ -1,5 +1,5 @@
 <!---
-lsp/ext.rs hash: c6e83d3d08d993de
+lsp/ext.rs hash: 6292ee8d88d4c9ec
 
 If you need to change the above hash to make the test pass, please check if you
 need to adjust this doc as well and ping this issue:


Test patch:
diff --git a/crates/rust-analyzer/tests/slow-tests/ratoml.rs b/crates/rust-analyzer/tests/slow-tests/ratoml.rs
index 295d1d4e8e..77048ad83d 100644
--- a/crates/rust-analyzer/tests/slow-tests/ratoml.rs
+++ b/crates/rust-analyzer/tests/slow-tests/ratoml.rs
@@ -9,18 +9,13 @@ use lsp_types::{
 use paths::Utf8PathBuf;
 
 use rust_analyzer::config::Config;
-use rust_analyzer::lsp::ext::{InternalTestingFetchConfig, InternalTestingFetchConfigParams};
+use rust_analyzer::lsp::ext::{
+    InternalTestingFetchConfig, InternalTestingFetchConfigOption, InternalTestingFetchConfigParams,
+    InternalTestingFetchConfigResponse,
+};
 use serde_json::json;
 use test_utils::skip_slow_tests;
 
-enum QueryType {
-    Local,
-    /// A query whose config key is a part of the global configs, so that
-    /// testing for changes to this config means testing if global changes
-    /// take affect.
-    Workspace,
-}
-
 struct RatomlTest {
     urls: Vec<Url>,
     server: Server,
@@ -158,20 +153,24 @@ impl RatomlTest {
         });
     }
 
-    fn query(&self, query: QueryType, source_file_idx: usize) -> bool {
-        let config = match query {
-            QueryType::Local => "local".to_owned(),
-            QueryType::Workspace => "workspace".to_owned(),
-        };
+    fn query(
+        &self,
+        query: InternalTestingFetchConfigOption,
+        source_file_idx: usize,
+        expected: InternalTestingFetchConfigResponse,
+    ) {
         let res = self.server.send_request::<InternalTestingFetchConfig>(
             InternalTestingFetchConfigParams {
                 text_document: Some(TextDocumentIdentifier {
                     uri: self.urls[source_file_idx].clone(),
                 }),
-                config,
+                config: query,
             },
         );
-        res.as_bool().unwrap()
+        assert_eq!(
+            serde_json::from_value::<InternalTestingFetchConfigResponse>(res).unwrap(),
+            expected
+        )
     }
 }
 
@@ -206,7 +205,11 @@ enum Value {
         })),
     );
 
-    assert!(server.query(QueryType::Local, 1));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        1,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
 }
 
 /// Checks if client config can be modified.
@@ -311,7 +314,11 @@ enum Value {
         None,
     );
 
-    assert!(server.query(QueryType::Local, 2));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        2,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
 }
 
 #[test]
@@ -341,12 +348,20 @@ enum Value {
         None,
     );
 
-    assert!(!server.query(QueryType::Local, 1));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        1,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
+    );
     server.create(
         "//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml",
         RatomlTest::EMIT_MUST_USE.to_owned(),
     );
-    assert!(server.query(QueryType::Local, 1));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        1,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
 }
 
 #[test]
@@ -378,9 +393,17 @@ assist.emitMustUse = true"#,
         None,
     );
 
-    assert!(server.query(QueryType::Local, 1));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        1,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
     server.edit(2, String::new());
-    assert!(!server.query(QueryType::Local, 1));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        1,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
+    );
 }
 
 #[test]
@@ -412,9 +435,17 @@ assist.emitMustUse = true"#,
         None,
     );
 
-    assert!(server.query(QueryType::Local, 1));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        1,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
     server.delete(2);
-    assert!(!server.query(QueryType::Local, 1));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        1,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
+    );
 }
 
 #[test]
@@ -461,7 +492,11 @@ pub fn add(left: usize, right: usize) -> usize {
         None,
     );
 
-    assert!(server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
 }
 
 #[test]
@@ -508,9 +543,17 @@ pub fn add(left: usize, right: usize) -> usize {
         None,
     );
 
-    assert!(!server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
+    );
     server.edit(1, "assist.emitMustUse = true".to_owned());
-    assert!(server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
 }
 
 #[test]
@@ -557,9 +600,17 @@ pub fn add(left: usize, right: usize) -> usize {
         None,
     );
 
-    assert!(server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
     server.delete(1);
-    assert!(!server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
+    );
 }
 
 #[test]
@@ -606,9 +657,17 @@ pub fn add(left: usize, right: usize) -> usize {
         None,
     );
 
-    assert!(server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
     server.create("//- /p1/p2/rust-analyzer.toml", RatomlTest::EMIT_MUST_NOT_USE.to_owned());
-    assert!(!server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
+    );
 }
 
 #[test]
@@ -656,9 +715,17 @@ pub fn add(left: usize, right: usize) -> usize {
         None,
     );
 
-    assert!(server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
     server.delete(1);
-    assert!(!server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(false),
+    );
 }
 
 #[test]
@@ -705,8 +772,16 @@ enum Value {
         None,
     );
 
-    assert!(server.query(QueryType::Local, 3));
-    assert!(server.query(QueryType::Local, 4));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        4,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
 }
 
 #[test]
@@ -744,7 +819,11 @@ fn ratoml_multiple_ratoml_in_single_source_root() {
         None,
     );
 
-    assert!(server.query(QueryType::Local, 3));
+    server.query(
+        InternalTestingFetchConfigOption::AssistEmitMustUse,
+        3,
+        InternalTestingFetchConfigResponse::AssistEmitMustUse(true),
+    );
 }
 
 /// If a root is non-local, so we cannot find what its parent is
@@ -836,7 +915,7 @@ edition = "2021"
         "#,
             r#"
 //- /p1/rust-analyzer.toml
-rustfmt.rangeFormatting.enable = true
+check.workspace = false
         "#,
             r#"
 //- /p1/src/lib.rs
@@ -848,7 +927,11 @@ fn main() {
         None,
     );
 
-    assert!(server.query(QueryType::Workspace, 2));
+    server.query(
+        InternalTestingFetchConfigOption::CheckWorkspace,
+        2,
+        InternalTestingFetchConfigResponse::CheckWorkspace(false),
+    )
 }
 
 #[test]
@@ -868,7 +951,7 @@ edition = "2021"
         "#,
             r#"
 //- /p1/rust-analyzer.toml
-rustfmt.rangeFormatting.enable = true
+check.workspace = false
     "#,
             r#"
 //- /p1/src/lib.rs
@@ -880,9 +963,17 @@ fn main() {
         None,
     );
 
-    assert!(server.query(QueryType::Workspace, 2));
-    server.edit(1, "rustfmt.rangeFormatting.enable = false".to_owned());
-    assert!(!server.query(QueryType::Workspace, 2));
+    server.query(
+        InternalTestingFetchConfigOption::CheckWorkspace,
+        2,
+        InternalTestingFetchConfigResponse::CheckWorkspace(false),
+    );
+    server.edit(1, "check.workspace = true".to_owned());
+    server.query(
+        InternalTestingFetchConfigOption::CheckWorkspace,
+        2,
+        InternalTestingFetchConfigResponse::CheckWorkspace(true),
+    );
 }
 
 #[test]
@@ -902,7 +993,7 @@ edition = "2021"
         "#,
             r#"
 //- /p1/rust-analyzer.toml
-rustfmt.rangeFormatting.enable = true
+check.workspace = false
        "#,
             r#"
 //- /p1/src/lib.rs
@@ -914,7 +1005,15 @@ fn main() {
         None,
     );
 
-    assert!(server.query(QueryType::Workspace, 2));
+    server.query(
+        InternalTestingFetchConfigOption::CheckWorkspace,
+        2,
+        InternalTestingFetchConfigResponse::CheckWorkspace(false),
+    );
     server.delete(1);
-    assert!(!server.query(QueryType::Workspace, 2));
+    server.query(
+        InternalTestingFetchConfigOption::CheckWorkspace,
+        2,
+        InternalTestingFetchConfigResponse::CheckWorkspace(true),
+    );
 }