Skip to content

Rename checkOnSave settings to check #13799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 40 additions & 53 deletions crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -786,9 +787,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"),
));
}
Expand Down Expand Up @@ -1033,7 +1034,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
}

Expand Down Expand Up @@ -1145,21 +1146,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())
}
Expand All @@ -1168,42 +1169,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 {
Expand Down Expand Up @@ -1862,35 +1863,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:?}");
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already assert this via a separate test


let map = fields
.iter()
.map(|(field, ty, doc, default)| {
Expand Down Expand Up @@ -1964,15 +1960,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"],
Expand Down
17 changes: 15 additions & 2 deletions crates/rust-analyzer/src/config/patch_old_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down
28 changes: 14 additions & 14 deletions docs/user/generated_config.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand Down
Loading