Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #18057 - alibektas:better_ratoml_testing, r=Veykril
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.
| -rw-r--r-- | crates/rust-analyzer/src/config.rs | 95 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/handlers/request.rs | 28 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/lsp/ext.rs | 17 | ||||
| -rw-r--r-- | crates/rust-analyzer/tests/slow-tests/ratoml.rs | 189 | ||||
| -rw-r--r-- | docs/dev/lsp-extensions.md | 2 |
5 files changed, 228 insertions, 103 deletions
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 35be6d0cf7..a7bde20026 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 @@ -442,48 +444,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). @@ -893,7 +853,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() { @@ -1270,7 +1230,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> }, @@ -1902,6 +1862,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/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), + ); } 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: |