Auto merge of #13799 - Veykril:flycheck, r=Veykril

bors · bors · commit fd300eebc9bd · 2023-01-09T15:35:38.000Z
Rename `checkOnSave` settings to `check`

Now that flychecks can be triggered without saving the setting name doesn't make that much sense anymore. This PR renames it to just `check`, but keeps `checkOnSave` as the enabling setting.
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
@@ -124,39 +124,40 @@ config_data! {
         /// Unsets `#[cfg(test)]` for the specified crates.
         cargo_unsetTest: Vec<String>     = "[\"core\"]",
 
+        /// Run the check command for diagnostics on save.
+        checkOnSave | checkOnSave_enable: bool                         = "true",
+
         /// Check all targets and tests (`--all-targets`).
-        checkOnSave_allTargets: bool                     = "true",
+        check_allTargets | checkOnSave_allTargets: bool                  = "true",
         /// Cargo command to use for `cargo check`.
-        checkOnSave_command: String                      = "\"check\"",
-        /// Run specified `cargo check` command for diagnostics on save.
-        checkOnSave_enable: bool                         = "true",
+        check_command | checkOnSave_command: String                      = "\"check\"",
         /// Extra arguments for `cargo check`.
-        checkOnSave_extraArgs: Vec<String>               = "[]",
+        check_extraArgs | checkOnSave_extraArgs: Vec<String>             = "[]",
         /// Extra environment variables that will be set when running `cargo check`.
         /// Extends `#rust-analyzer.cargo.extraEnv#`.
-        checkOnSave_extraEnv: FxHashMap<String, String> = "{}",
+        check_extraEnv | checkOnSave_extraEnv: FxHashMap<String, String> = "{}",
         /// List of features to activate. Defaults to
         /// `#rust-analyzer.cargo.features#`.
         ///
         /// Set to `"all"` to pass `--all-features` to Cargo.
-        checkOnSave_features: Option<CargoFeaturesDef>      = "null",
+        check_features | checkOnSave_features: Option<CargoFeaturesDef>  = "null",
         /// Specifies the working directory for running checks.
         /// - "workspace": run checks for workspaces in the corresponding workspaces' root directories.
         // FIXME: Ideally we would support this in some way
         ///   This falls back to "root" if `#rust-analyzer.cargo.checkOnSave.invocationStrategy#` is set to `once`.
         /// - "root": run checks in the project's root directory.
         /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
         /// is set.
-        checkOnSave_invocationLocation: InvocationLocation = "\"workspace\"",
+        check_invocationLocation | checkOnSave_invocationLocation: InvocationLocation = "\"workspace\"",
         /// Specifies the invocation strategy to use when running the checkOnSave command.
         /// If `per_workspace` is set, the command will be executed for each workspace.
         /// If `once` is set, the command will be executed once.
         /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
         /// is set.
-        checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
+        check_invocationStrategy | checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
         /// Whether to pass `--no-default-features` to Cargo. Defaults to
         /// `#rust-analyzer.cargo.noDefaultFeatures#`.
-        checkOnSave_noDefaultFeatures: Option<bool>      = "null",
+        check_noDefaultFeatures | checkOnSave_noDefaultFeatures: Option<bool>         = "null",
         /// Override the command rust-analyzer uses instead of `cargo check` for
         /// diagnostics on save. The command is required to output json and
         /// should therefore include `--message-format=json` or a similar option.
@@ -175,14 +176,14 @@ config_data! {
         /// cargo check --workspace --message-format=json --all-targets
         /// ```
         /// .
-        checkOnSave_overrideCommand: Option<Vec<String>> = "null",
+        check_overrideCommand | checkOnSave_overrideCommand: Option<Vec<String>>             = "null",
         /// Check for specific targets. Defaults to `#rust-analyzer.cargo.target#` if empty.
         ///
         /// Can be a single target, e.g. `"x86_64-unknown-linux-gnu"` or a list of targets, e.g.
         /// `["aarch64-apple-darwin", "x86_64-apple-darwin"]`.
         ///
         /// Aliased as `"checkOnSave.targets"`.
-        checkOnSave_target | checkOnSave_targets: Option<CheckOnSaveTargets>           = "null",
+        check_targets | checkOnSave_targets | checkOnSave_target: Option<CheckOnSaveTargets> = "null",
 
         /// Toggles the additional completions that automatically add imports when completed.
         /// Note that your client must specify the `additionalTextEdits` LSP client capability to truly have this feature enabled.
@@ -791,9 +792,9 @@ impl Config {
 
     fn validate(&self, error_sink: &mut Vec<(String, serde_json::Error)>) {
         use serde::de::Error;
-        if self.data.checkOnSave_command.is_empty() {
+        if self.data.check_command.is_empty() {
             error_sink.push((
-                "/checkOnSave/command".to_string(),
+                "/check/command".to_string(),
                 serde_json::Error::custom("expected a non-empty string"),
             ));
         }
@@ -1038,7 +1039,7 @@ impl Config {
 
     pub fn check_on_save_extra_env(&self) -> FxHashMap<String, String> {
         let mut extra_env = self.data.cargo_extraEnv.clone();
-        extra_env.extend(self.data.checkOnSave_extraEnv.clone());
+        extra_env.extend(self.data.check_extraEnv.clone());
         extra_env
     }
 
@@ -1150,21 +1151,21 @@ impl Config {
     }
 
     pub fn flycheck(&self) -> FlycheckConfig {
-        match &self.data.checkOnSave_overrideCommand {
+        match &self.data.check_overrideCommand {
             Some(args) if !args.is_empty() => {
                 let mut args = args.clone();
                 let command = args.remove(0);
                 FlycheckConfig::CustomCommand {
                     command,
                     args,
                     extra_env: self.check_on_save_extra_env(),
-                    invocation_strategy: match self.data.checkOnSave_invocationStrategy {
+                    invocation_strategy: match self.data.check_invocationStrategy {
                         InvocationStrategy::Once => flycheck::InvocationStrategy::Once,
                         InvocationStrategy::PerWorkspace => {
                             flycheck::InvocationStrategy::PerWorkspace
                         }
                     },
-                    invocation_location: match self.data.checkOnSave_invocationLocation {
+                    invocation_location: match self.data.check_invocationLocation {
                         InvocationLocation::Root => {
                             flycheck::InvocationLocation::Root(self.root_path.clone())
                         }
@@ -1173,42 +1174,42 @@ impl Config {
                 }
             }
             Some(_) | None => FlycheckConfig::CargoCommand {
-                command: self.data.checkOnSave_command.clone(),
+                command: self.data.check_command.clone(),
                 target_triples: self
                     .data
-                    .checkOnSave_target
+                    .check_targets
                     .clone()
                     .and_then(|targets| match &targets.0[..] {
                         [] => None,
                         targets => Some(targets.into()),
                     })
                     .unwrap_or_else(|| self.data.cargo_target.clone().into_iter().collect()),
-                all_targets: self.data.checkOnSave_allTargets,
+                all_targets: self.data.check_allTargets,
                 no_default_features: self
                     .data
-                    .checkOnSave_noDefaultFeatures
+                    .check_noDefaultFeatures
                     .unwrap_or(self.data.cargo_noDefaultFeatures),
                 all_features: matches!(
-                    self.data.checkOnSave_features.as_ref().unwrap_or(&self.data.cargo_features),
+                    self.data.check_features.as_ref().unwrap_or(&self.data.cargo_features),
                     CargoFeaturesDef::All
                 ),
                 features: match self
                     .data
-                    .checkOnSave_features
+                    .check_features
                     .clone()
                     .unwrap_or_else(|| self.data.cargo_features.clone())
                 {
                     CargoFeaturesDef::All => vec![],
                     CargoFeaturesDef::Selected(it) => it,
                 },
-                extra_args: self.data.checkOnSave_extraArgs.clone(),
+                extra_args: self.data.check_extraArgs.clone(),
                 extra_env: self.check_on_save_extra_env(),
             },
         }
     }
 
     pub fn check_on_save(&self) -> bool {
-        self.data.checkOnSave_enable
+        self.data.checkOnSave
     }
 
     pub fn runnables(&self) -> RunnablesConfig {
@@ -1886,35 +1887,30 @@ fn get_field<T: DeserializeOwned>(
     alias: Option<&'static str>,
     default: &str,
 ) -> T {
-    let default = serde_json::from_str(default).unwrap();
     // XXX: check alias first, to work-around the VS Code where it pre-fills the
     // defaults instead of sending an empty object.
     alias
         .into_iter()
         .chain(iter::once(field))
-        .find_map(move |field| {
+        .filter_map(move |field| {
             let mut pointer = field.replace('_', "/");
             pointer.insert(0, '/');
-            json.pointer_mut(&pointer).and_then(|it| match serde_json::from_value(it.take()) {
-                Ok(it) => Some(it),
-                Err(e) => {
-                    tracing::warn!("Failed to deserialize config field at {}: {:?}", pointer, e);
-                    error_sink.push((pointer, e));
-                    None
-                }
-            })
+            json.pointer_mut(&pointer)
+                .map(|it| serde_json::from_value(it.take()).map_err(|e| (e, pointer)))
+        })
+        .find(Result::is_ok)
+        .and_then(|res| match res {
+            Ok(it) => Some(it),
+            Err((e, pointer)) => {
+                tracing::warn!("Failed to deserialize config field at {}: {:?}", pointer, e);
+                error_sink.push((pointer, e));
+                None
+            }
         })
-        .unwrap_or(default)
+        .unwrap_or_else(|| serde_json::from_str(default).unwrap())
 }
 
 fn schema(fields: &[(&'static str, &'static str, &[&str], &str)]) -> serde_json::Value {
-    for ((f1, ..), (f2, ..)) in fields.iter().zip(&fields[1..]) {
-        fn key(f: &str) -> &str {
-            f.splitn(2, '_').next().unwrap()
-        }
-        assert!(key(f1) <= key(f2), "wrong field order: {f1:?} {f2:?}");
-    }
-
     let map = fields
         .iter()
         .map(|(field, ty, doc, default)| {
@@ -1988,15 +1984,6 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
             "type": ["null", "array"],
             "items": { "type": "string" },
         },
-        "MergeBehaviorDef" => set! {
-            "type": "string",
-            "enum": ["none", "crate", "module"],
-            "enumDescriptions": [
-                "Do not merge imports at all.",
-                "Merge imports from the same crate into a single `use` statement.",
-                "Merge imports from the same module into a single `use` statement."
-            ],
-        },
         "ExprFillDefaultDef" => set! {
             "type": "string",
             "enum": ["todo", "default"],
diff --git a/crates/rust-analyzer/src/config/patch_old_style.rs b/crates/rust-analyzer/src/config/patch_old_style.rs
@@ -4,6 +4,9 @@ use serde_json::{json, Value};
 /// This function patches the json config to the new expected keys.
 /// That is we try to load old known config keys here and convert them to the new ones.
 /// See https://github.com/rust-lang/rust-analyzer/pull/12010
+///
+/// We already have an alias system for simple cases, but if we make structural changes
+/// the alias infra fails down.
 pub(super) fn patch_json_for_outdated_configs(json: &mut Value) {
     let copy = json.clone();
 
@@ -105,9 +108,9 @@ pub(super) fn patch_json_for_outdated_configs(json: &mut Value) {
         merge(json, json!({ "cargo": { "features": "all" } }));
     }
 
-    // checkOnSave_allFeatures, checkOnSave_features -> checkOnSave_features
+    // checkOnSave_allFeatures, checkOnSave_features -> check_features
     if let Some(Value::Bool(true)) = copy.pointer("/checkOnSave/allFeatures") {
-        merge(json, json!({ "checkOnSave": { "features": "all" } }));
+        merge(json, json!({ "check": { "features": "all" } }));
     }
 
     // completion_addCallArgumentSnippets completion_addCallParenthesis -> completion_callable_snippets
@@ -121,6 +124,16 @@ pub(super) fn patch_json_for_outdated_configs(json: &mut Value) {
         (_, _) => return,
     };
     merge(json, json!({ "completion": { "callable": {"snippets": res }} }));
+
+    // We need to do this due to the checkOnSave_enable -> checkOnSave change, as that key now can either be an object or a bool
+    // checkOnSave_* -> check_*
+    if let Some(Value::Object(obj)) = copy.pointer("/checkOnSave") {
+        // checkOnSave_enable -> checkOnSave
+        if let Some(b @ Value::Bool(_)) = obj.get("enable") {
+            merge(json, json!({ "checkOnSave": b }));
+        }
+        merge(json, json!({ "check": obj }));
+    }
 }
 
 fn merge(dst: &mut Value, src: Value) {
diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc
@@ -109,41 +109,41 @@ Compilation target override (target triple).
 --
 Unsets `#[cfg(test)]` for the specified crates.
 --
-[[rust-analyzer.checkOnSave.allTargets]]rust-analyzer.checkOnSave.allTargets (default: `true`)::
+[[rust-analyzer.checkOnSave]]rust-analyzer.checkOnSave (default: `true`)::
 +
 --
-Check all targets and tests (`--all-targets`).
+Run the check command for diagnostics on save.
 --
-[[rust-analyzer.checkOnSave.command]]rust-analyzer.checkOnSave.command (default: `"check"`)::
+[[rust-analyzer.check.allTargets]]rust-analyzer.check.allTargets (default: `true`)::
 +
 --
-Cargo command to use for `cargo check`.
+Check all targets and tests (`--all-targets`).
 --
-[[rust-analyzer.checkOnSave.enable]]rust-analyzer.checkOnSave.enable (default: `true`)::
+[[rust-analyzer.check.command]]rust-analyzer.check.command (default: `"check"`)::
 +
 --
-Run specified `cargo check` command for diagnostics on save.
+Cargo command to use for `cargo check`.
 --
-[[rust-analyzer.checkOnSave.extraArgs]]rust-analyzer.checkOnSave.extraArgs (default: `[]`)::
+[[rust-analyzer.check.extraArgs]]rust-analyzer.check.extraArgs (default: `[]`)::
 +
 --
 Extra arguments for `cargo check`.
 --
-[[rust-analyzer.checkOnSave.extraEnv]]rust-analyzer.checkOnSave.extraEnv (default: `{}`)::
+[[rust-analyzer.check.extraEnv]]rust-analyzer.check.extraEnv (default: `{}`)::
 +
 --
 Extra environment variables that will be set when running `cargo check`.
 Extends `#rust-analyzer.cargo.extraEnv#`.
 --
-[[rust-analyzer.checkOnSave.features]]rust-analyzer.checkOnSave.features (default: `null`)::
+[[rust-analyzer.check.features]]rust-analyzer.check.features (default: `null`)::
 +
 --
 List of features to activate. Defaults to
 `#rust-analyzer.cargo.features#`.
 
 Set to `"all"` to pass `--all-features` to Cargo.
 --
-[[rust-analyzer.checkOnSave.invocationLocation]]rust-analyzer.checkOnSave.invocationLocation (default: `"workspace"`)::
+[[rust-analyzer.check.invocationLocation]]rust-analyzer.check.invocationLocation (default: `"workspace"`)::
 +
 --
 Specifies the working directory for running checks.
@@ -153,7 +153,7 @@ Specifies the working directory for running checks.
 This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
 is set.
 --
-[[rust-analyzer.checkOnSave.invocationStrategy]]rust-analyzer.checkOnSave.invocationStrategy (default: `"per_workspace"`)::
+[[rust-analyzer.check.invocationStrategy]]rust-analyzer.check.invocationStrategy (default: `"per_workspace"`)::
 +
 --
 Specifies the invocation strategy to use when running the checkOnSave command.
@@ -162,13 +162,13 @@ If `once` is set, the command will be executed once.
 This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
 is set.
 --
-[[rust-analyzer.checkOnSave.noDefaultFeatures]]rust-analyzer.checkOnSave.noDefaultFeatures (default: `null`)::
+[[rust-analyzer.check.noDefaultFeatures]]rust-analyzer.check.noDefaultFeatures (default: `null`)::
 +
 --
 Whether to pass `--no-default-features` to Cargo. Defaults to
 `#rust-analyzer.cargo.noDefaultFeatures#`.
 --
-[[rust-analyzer.checkOnSave.overrideCommand]]rust-analyzer.checkOnSave.overrideCommand (default: `null`)::
+[[rust-analyzer.check.overrideCommand]]rust-analyzer.check.overrideCommand (default: `null`)::
 +
 --
 Override the command rust-analyzer uses instead of `cargo check` for
@@ -190,7 +190,7 @@ cargo check --workspace --message-format=json --all-targets
 ```
 .
 --
-[[rust-analyzer.checkOnSave.target]]rust-analyzer.checkOnSave.target (default: `null`)::
+[[rust-analyzer.check.targets]]rust-analyzer.check.targets (default: `null`)::
 +
 --
 Check for specific targets. Defaults to `#rust-analyzer.cargo.target#` if empty.
diff --git a/editors/code/package.json b/editors/code/package.json
