From d9f570960980a98a1cde705ca38bf957231a53e6 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 19 Sep 2022 16:40:43 +0200 Subject: [PATCH] Simplify feature representation in CargoConfig --- crates/project-model/src/build_scripts.rs | 22 ++--- crates/project-model/src/cargo_workspace.rs | 61 ++++++++------ crates/project-model/src/lib.rs | 4 +- crates/rust-analyzer/src/cargo_target_spec.rs | 82 ++++++++++--------- crates/rust-analyzer/src/config.rs | 30 +++---- 5 files changed, 109 insertions(+), 90 deletions(-) diff --git a/crates/project-model/src/build_scripts.rs b/crates/project-model/src/build_scripts.rs index 837ea016193c..32db42f1db75 100644 --- a/crates/project-model/src/build_scripts.rs +++ b/crates/project-model/src/build_scripts.rs @@ -15,7 +15,7 @@ use rustc_hash::FxHashMap; use semver::Version; use serde::Deserialize; -use crate::{cfg_flag::CfgFlag, CargoConfig, CargoWorkspace, Package}; +use crate::{cfg_flag::CfgFlag, CargoConfig, CargoFeatures, CargoWorkspace, Package}; #[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct WorkspaceBuildScripts { @@ -49,7 +49,6 @@ impl WorkspaceBuildScripts { let mut cmd = Command::new(toolchain::cargo()); cmd.envs(&config.extra_env); - cmd.args(&["check", "--quiet", "--workspace", "--message-format=json"]); // --all-targets includes tests, benches and examples in addition to the @@ -61,15 +60,18 @@ impl WorkspaceBuildScripts { cmd.args(&["--target", target]); } - if config.all_features { - cmd.arg("--all-features"); - } else { - if config.no_default_features { - cmd.arg("--no-default-features"); + match &config.features { + CargoFeatures::All => { + cmd.arg("--all-features"); } - if !config.features.is_empty() { - cmd.arg("--features"); - cmd.arg(config.features.join(" ")); + CargoFeatures::Selected { features, no_default_features } => { + if *no_default_features { + cmd.arg("--no-default-features"); + } + if !features.is_empty() { + cmd.arg("--features"); + cmd.arg(features.join(" ")); + } } } diff --git a/crates/project-model/src/cargo_workspace.rs b/crates/project-model/src/cargo_workspace.rs index 736d80041bd5..8e50fe878ae3 100644 --- a/crates/project-model/src/cargo_workspace.rs +++ b/crates/project-model/src/cargo_workspace.rs @@ -71,35 +71,41 @@ impl Default for UnsetTestCrates { } } -#[derive(Default, Clone, Debug, PartialEq, Eq)] -pub struct CargoConfig { - /// Do not activate the `default` feature. - pub no_default_features: bool, +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum CargoFeatures { + All, + Selected { + /// List of features to activate. + features: Vec, + /// Do not activate the `default` feature. + no_default_features: bool, + }, +} - /// Activate all available features - pub all_features: bool, +impl Default for CargoFeatures { + fn default() -> Self { + CargoFeatures::Selected { features: vec![], no_default_features: false } + } +} +#[derive(Default, Clone, Debug, PartialEq, Eq)] +pub struct CargoConfig { /// List of features to activate. - /// This will be ignored if `cargo_all_features` is true. - pub features: Vec, - + pub features: CargoFeatures, /// rustc target pub target: Option, - /// Don't load sysroot crates (`std`, `core` & friends). Might be useful /// when debugging isolated issues. pub no_sysroot: bool, - /// rustc private crate source pub rustc_source: Option, - /// crates to disable `#[cfg(test)]` on pub unset_test_crates: UnsetTestCrates, - + /// Invoke `cargo check` through the RUSTC_WRAPPER. pub wrap_rustc_in_build_scripts: bool, - + /// The command to run instead of `cargo check` for building build scripts. pub run_build_script_command: Option>, - + /// Extra env vars to set when invoking the cargo command pub extra_env: FxHashMap, } @@ -143,7 +149,7 @@ pub struct PackageData { pub targets: Vec, /// Does this package come from the local filesystem (and is editable)? pub is_local: bool, - // Whether this package is a member of the workspace + /// Whether this package is a member of the workspace pub is_member: bool, /// List of packages this package depends on pub dependencies: Vec, @@ -249,8 +255,8 @@ impl TargetKind { } } +// Deserialize helper for the cargo metadata #[derive(Deserialize, Default)] -// Deserialise helper for the cargo metadata struct PackageMetadata { #[serde(rename = "rust-analyzer")] rust_analyzer: Option, @@ -272,16 +278,19 @@ impl CargoWorkspace { let mut meta = MetadataCommand::new(); meta.cargo_path(toolchain::cargo()); meta.manifest_path(cargo_toml.to_path_buf()); - if config.all_features { - meta.features(CargoOpt::AllFeatures); - } else { - if config.no_default_features { - // FIXME: `NoDefaultFeatures` is mutual exclusive with `SomeFeatures` - // https://github.com/oli-obk/cargo_metadata/issues/79 - meta.features(CargoOpt::NoDefaultFeatures); + match &config.features { + CargoFeatures::All => { + meta.features(CargoOpt::AllFeatures); } - if !config.features.is_empty() { - meta.features(CargoOpt::SomeFeatures(config.features.clone())); + CargoFeatures::Selected { features, no_default_features } => { + if *no_default_features { + // FIXME: `NoDefaultFeatures` is mutual exclusive with `SomeFeatures` + // https://github.com/oli-obk/cargo_metadata/issues/79 + meta.features(CargoOpt::NoDefaultFeatures); + } + if !features.is_empty() { + meta.features(CargoOpt::SomeFeatures(features.clone())); + } } } meta.current_dir(current_dir.as_os_str()); diff --git a/crates/project-model/src/lib.rs b/crates/project-model/src/lib.rs index b81b7432f655..ce78ce85697a 100644 --- a/crates/project-model/src/lib.rs +++ b/crates/project-model/src/lib.rs @@ -42,8 +42,8 @@ use rustc_hash::FxHashSet; pub use crate::{ build_scripts::WorkspaceBuildScripts, cargo_workspace::{ - CargoConfig, CargoWorkspace, Package, PackageData, PackageDependency, RustcSource, Target, - TargetData, TargetKind, UnsetTestCrates, + CargoConfig, CargoFeatures, CargoWorkspace, Package, PackageData, PackageDependency, + RustcSource, Target, TargetData, TargetKind, UnsetTestCrates, }, manifest_path::ManifestPath, project_json::{ProjectJson, ProjectJsonData}, diff --git a/crates/rust-analyzer/src/cargo_target_spec.rs b/crates/rust-analyzer/src/cargo_target_spec.rs index 1c39e9391af2..e1675a030c0f 100644 --- a/crates/rust-analyzer/src/cargo_target_spec.rs +++ b/crates/rust-analyzer/src/cargo_target_spec.rs @@ -4,7 +4,7 @@ use std::mem; use cfg::{CfgAtom, CfgExpr}; use ide::{FileId, RunnableKind, TestId}; -use project_model::{self, ManifestPath, TargetKind}; +use project_model::{self, CargoFeatures, ManifestPath, TargetKind}; use vfs::AbsPathBuf; use crate::{global_state::GlobalStateSnapshot, Result}; @@ -35,41 +35,41 @@ impl CargoTargetSpec { match kind { RunnableKind::Test { test_id, attr } => { - args.push("test".to_string()); + args.push("test".to_owned()); extra_args.push(test_id.to_string()); if let TestId::Path(_) = test_id { - extra_args.push("--exact".to_string()); + extra_args.push("--exact".to_owned()); } - extra_args.push("--nocapture".to_string()); + extra_args.push("--nocapture".to_owned()); if attr.ignore { - extra_args.push("--ignored".to_string()); + extra_args.push("--ignored".to_owned()); } } RunnableKind::TestMod { path } => { - args.push("test".to_string()); - extra_args.push(path.to_string()); - extra_args.push("--nocapture".to_string()); + args.push("test".to_owned()); + extra_args.push(path.clone()); + extra_args.push("--nocapture".to_owned()); } RunnableKind::Bench { test_id } => { - args.push("bench".to_string()); + args.push("bench".to_owned()); extra_args.push(test_id.to_string()); if let TestId::Path(_) = test_id { - extra_args.push("--exact".to_string()); + extra_args.push("--exact".to_owned()); } - extra_args.push("--nocapture".to_string()); + extra_args.push("--nocapture".to_owned()); } RunnableKind::DocTest { test_id } => { - args.push("test".to_string()); - args.push("--doc".to_string()); + args.push("test".to_owned()); + args.push("--doc".to_owned()); extra_args.push(test_id.to_string()); - extra_args.push("--nocapture".to_string()); + extra_args.push("--nocapture".to_owned()); } RunnableKind::Bin => { let subcommand = match spec { Some(CargoTargetSpec { target_kind: TargetKind::Test, .. }) => "test", _ => "run", }; - args.push(subcommand.to_string()); + args.push(subcommand.to_owned()); } } @@ -82,29 +82,35 @@ impl CargoTargetSpec { }; let cargo_config = snap.config.cargo(); - if cargo_config.all_features { - args.push("--all-features".to_string()); - for feature in target_required_features { - args.push("--features".to_string()); - args.push(feature); - } - } else { - let mut features = Vec::new(); - if let Some(cfg) = cfg.as_ref() { - required_features(cfg, &mut features); + match &cargo_config.features { + CargoFeatures::All => { + args.push("--all-features".to_owned()); + for feature in target_required_features { + args.push("--features".to_owned()); + args.push(feature); + } } + CargoFeatures::Selected { features, no_default_features } => { + let mut feats = Vec::new(); + if let Some(cfg) = cfg.as_ref() { + required_features(cfg, &mut feats); + } - features.extend(cargo_config.features); - features.extend(target_required_features); + feats.extend(features.iter().cloned()); + feats.extend(target_required_features); - features.dedup(); - for feature in features { - args.push("--features".to_string()); - args.push(feature); + feats.dedup(); + for feature in feats { + args.push("--features".to_owned()); + args.push(feature); + } + + if *no_default_features { + args.push("--no-default-features".to_owned()); + } } } - Ok((args, extra_args)) } @@ -136,7 +142,7 @@ impl CargoTargetSpec { } pub(crate) fn push_to(self, buf: &mut Vec, kind: &RunnableKind) { - buf.push("--package".to_string()); + buf.push("--package".to_owned()); buf.push(self.package); // Can't mix --doc with other target flags @@ -145,23 +151,23 @@ impl CargoTargetSpec { } match self.target_kind { TargetKind::Bin => { - buf.push("--bin".to_string()); + buf.push("--bin".to_owned()); buf.push(self.target); } TargetKind::Test => { - buf.push("--test".to_string()); + buf.push("--test".to_owned()); buf.push(self.target); } TargetKind::Bench => { - buf.push("--bench".to_string()); + buf.push("--bench".to_owned()); buf.push(self.target); } TargetKind::Example => { - buf.push("--example".to_string()); + buf.push("--example".to_owned()); buf.push(self.target); } TargetKind::Lib => { - buf.push("--lib".to_string()); + buf.push("--lib".to_owned()); } TargetKind::Other | TargetKind::BuildScript => (), } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 9ef79e6f3812..0d0e246029b1 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -22,7 +22,8 @@ use ide_db::{ use itertools::Itertools; use lsp_types::{ClientCapabilities, MarkupKind}; use project_model::{ - CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest, RustcSource, UnsetTestCrates, + CargoConfig, CargoFeatures, ProjectJson, ProjectJsonData, ProjectManifest, RustcSource, + UnsetTestCrates, }; use rustc_hash::{FxHashMap, FxHashSet}; use serde::{de::DeserializeOwned, Deserialize}; @@ -90,7 +91,7 @@ config_data! { /// List of features to activate. /// /// Set this to `"all"` to pass `--all-features` to cargo. - cargo_features: CargoFeatures = "[]", + cargo_features: CargoFeaturesDef = "[]", /// Whether to pass `--no-default-features` to cargo. cargo_noDefaultFeatures: bool = "false", /// Internal config for debugging, disables loading of sysroot crates. @@ -114,7 +115,7 @@ config_data! { /// `#rust-analyzer.cargo.features#`. /// /// Set to `"all"` to pass `--all-features` to Cargo. - checkOnSave_features: Option = "null", + checkOnSave_features: Option = "null", /// Whether to pass `--no-default-features` to Cargo. Defaults to /// `#rust-analyzer.cargo.noDefaultFeatures#`. checkOnSave_noDefaultFeatures: Option = "null", @@ -1028,11 +1029,12 @@ impl Config { }); CargoConfig { - no_default_features: self.data.cargo_noDefaultFeatures, - all_features: matches!(self.data.cargo_features, CargoFeatures::All), features: match &self.data.cargo_features { - CargoFeatures::All => vec![], - CargoFeatures::Listed(it) => it.clone(), + CargoFeaturesDef::All => CargoFeatures::All, + CargoFeaturesDef::Selected(features) => CargoFeatures::Selected { + features: features.clone(), + no_default_features: self.data.cargo_noDefaultFeatures, + }, }, target: self.data.cargo_target.clone(), no_sysroot: self.data.cargo_noSysroot, @@ -1086,7 +1088,7 @@ impl Config { .unwrap_or(self.data.cargo_noDefaultFeatures), all_features: matches!( self.data.checkOnSave_features.as_ref().unwrap_or(&self.data.cargo_features), - CargoFeatures::All + CargoFeaturesDef::All ), features: match self .data @@ -1094,8 +1096,8 @@ impl Config { .clone() .unwrap_or_else(|| self.data.cargo_features.clone()) { - CargoFeatures::All => vec![], - CargoFeatures::Listed(it) => it, + CargoFeaturesDef::All => vec![], + CargoFeaturesDef::Selected(it) => it, }, extra_args: self.data.checkOnSave_extraArgs.clone(), extra_env: self.check_on_save_extra_env(), @@ -1564,10 +1566,10 @@ enum CallableCompletionDef { #[derive(Deserialize, Debug, Clone)] #[serde(untagged)] -enum CargoFeatures { +enum CargoFeaturesDef { #[serde(deserialize_with = "de_unit_v::all")] All, - Listed(Vec), + Selected(Vec), } #[derive(Deserialize, Debug, Clone)] @@ -1912,7 +1914,7 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json "Only show mutable reborrow hints." ] }, - "CargoFeatures" => set! { + "CargoFeaturesDef" => set! { "anyOf": [ { "type": "string", @@ -1929,7 +1931,7 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json } ], }, - "Option" => set! { + "Option" => set! { "anyOf": [ { "type": "string",