From 7c0743293e5720a5be7b44b4a781b2982d63152a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 23 Jul 2020 18:57:27 +0200 Subject: Require quotes around key-value cfg flags in rust-project.json This matches rustc command-line flags, as well as the build.rs format. --- crates/ra_project_model/src/cargo_workspace.rs | 18 +++++- crates/ra_project_model/src/cfg_flag.rs | 51 +++++++++++++++++ crates/ra_project_model/src/lib.rs | 77 ++++++++++---------------- crates/ra_project_model/src/project_json.rs | 23 ++------ crates/ra_project_model/src/sysroot.rs | 7 +-- crates/rust-analyzer/tests/heavy_tests/main.rs | 2 +- 6 files changed, 107 insertions(+), 71 deletions(-) create mode 100644 crates/ra_project_model/src/cfg_flag.rs diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index 4182ca156..fb88e0f06 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -14,6 +14,8 @@ use ra_arena::{Arena, Idx}; use ra_db::Edition; use rustc_hash::FxHashMap; +use crate::cfg_flag::CfgFlag; + /// `CargoWorkspace` represents the logical structure of, well, a Cargo /// workspace. It pretty closely mirrors `cargo metadata` output. /// @@ -78,7 +80,7 @@ pub struct PackageData { pub dependencies: Vec, pub edition: Edition, pub features: Vec, - pub cfgs: Vec, + pub cfgs: Vec, pub out_dir: Option, pub proc_macro_dylib_path: Option, } @@ -276,7 +278,7 @@ impl CargoWorkspace { pub struct ExternResources { out_dirs: FxHashMap, proc_dylib_paths: FxHashMap, - cfgs: FxHashMap>, + cfgs: FxHashMap>, } pub fn load_extern_resources( @@ -303,6 +305,18 @@ pub fn load_extern_resources( if let Ok(message) = message { match message { Message::BuildScriptExecuted(BuildScript { package_id, out_dir, cfgs, .. }) => { + let cfgs = { + let mut acc = Vec::new(); + for cfg in cfgs { + match cfg.parse::() { + Ok(it) => acc.push(it), + Err(err) => { + anyhow::bail!("invalid cfg from cargo-metadata: {}", err) + } + }; + } + acc + }; // cargo_metadata crate returns default (empty) path for // older cargos, which is not absolute, so work around that. if out_dir != PathBuf::default() { diff --git a/crates/ra_project_model/src/cfg_flag.rs b/crates/ra_project_model/src/cfg_flag.rs new file mode 100644 index 000000000..1bc5d4832 --- /dev/null +++ b/crates/ra_project_model/src/cfg_flag.rs @@ -0,0 +1,51 @@ +//! Parsing of CfgFlags as command line arguments, as in +//! +//! rustc main.rs --cfg foo --cfg 'feature="bar"' +use std::str::FromStr; + +use ra_cfg::CfgOptions; +use stdx::split_delim; + +#[derive(Clone, Eq, PartialEq, Debug)] +pub enum CfgFlag { + Atom(String), + KeyValue { key: String, value: String }, +} + +impl FromStr for CfgFlag { + type Err = String; + fn from_str(s: &str) -> Result { + let res = match split_delim(s, '=') { + Some((key, value)) => { + if !(value.starts_with('"') && value.ends_with('"')) { + return Err(format!("Invalid cfg ({:?}), value should be in quotes", s)); + } + let key = key.to_string(); + let value = value[1..value.len() - 1].to_string(); + CfgFlag::KeyValue { key, value } + } + None => CfgFlag::Atom(s.into()), + }; + Ok(res) + } +} + +impl<'de> serde::Deserialize<'de> for CfgFlag { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + String::deserialize(deserializer)?.parse().map_err(serde::de::Error::custom) + } +} + +impl Extend for CfgOptions { + fn extend>(&mut self, iter: T) { + for cfg_flag in iter { + match cfg_flag { + CfgFlag::Atom(it) => self.insert_atom(it.into()), + CfgFlag::KeyValue { key, value } => self.insert_key_value(key.into(), value.into()), + } + } + } +} diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 8053712ff..300e75135 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -3,11 +3,12 @@ mod cargo_workspace; mod project_json; mod sysroot; +mod cfg_flag; use std::{ fs::{self, read_dir, ReadDir}, io, - process::{Command, Output}, + process::Command, }; use anyhow::{bail, Context, Result}; @@ -15,13 +16,15 @@ use paths::{AbsPath, AbsPathBuf}; use ra_cfg::CfgOptions; use ra_db::{CrateGraph, CrateId, CrateName, Edition, Env, FileId}; use rustc_hash::{FxHashMap, FxHashSet}; -use stdx::split_delim; + +use crate::cfg_flag::CfgFlag; pub use crate::{ cargo_workspace::{CargoConfig, CargoWorkspace, Package, Target, TargetKind}, project_json::{ProjectJson, ProjectJsonData}, sysroot::Sysroot, }; + pub use ra_proc_macro::ProcMacroClient; #[derive(Debug, Clone, Eq, PartialEq)] @@ -250,7 +253,7 @@ impl ProjectWorkspace { let mut crate_graph = CrateGraph::default(); match self { ProjectWorkspace::Json { project } => { - let mut target_cfg_map = FxHashMap::, CfgOptions>::default(); + let mut cfg_cache: FxHashMap, Vec> = FxHashMap::default(); let crates: FxHashMap<_, _> = project .crates .iter() @@ -266,11 +269,12 @@ impl ProjectWorkspace { .map(|it| proc_macro_client.by_dylib_path(&it)); let target = krate.target.as_deref().or(target); - let target_cfgs = target_cfg_map - .entry(target.clone()) - .or_insert_with(|| get_rustc_cfg_options(target.as_deref())); - let mut cfg_options = krate.cfg.clone(); - cfg_options.append(target_cfgs); + let target_cfgs = cfg_cache + .entry(target) + .or_insert_with(|| get_rustc_cfg_options(target)); + + let mut cfg_options = CfgOptions::default(); + cfg_options.extend(target_cfgs.iter().chain(krate.cfg.iter()).cloned()); // FIXME: No crate name in json definition such that we cannot add OUT_DIR to env Some(( @@ -307,7 +311,8 @@ impl ProjectWorkspace { } } ProjectWorkspace::Cargo { cargo, sysroot } => { - let mut cfg_options = get_rustc_cfg_options(target); + let mut cfg_options = CfgOptions::default(); + cfg_options.extend(get_rustc_cfg_options(target)); let sysroot_crates: FxHashMap<_, _> = sysroot .crates() @@ -354,6 +359,7 @@ impl ProjectWorkspace { // Add test cfg for non-sysroot crates cfg_options.insert_atom("test".into()); + cfg_options.insert_atom("debug_assertions".into()); // Next, create crates for each package, target pair for pkg in cargo.packages() { @@ -367,15 +373,7 @@ impl ProjectWorkspace { for feature in cargo[pkg].features.iter() { opts.insert_key_value("feature".into(), feature.into()); } - for cfg in cargo[pkg].cfgs.iter() { - match cfg.find('=') { - Some(split) => opts.insert_key_value( - cfg[..split].into(), - cfg[split + 1..].trim_matches('"').into(), - ), - None => opts.insert_atom(cfg.into()), - }; - } + opts.extend(cargo[pkg].cfgs.iter().cloned()); opts }; let mut env = Env::default(); @@ -503,51 +501,35 @@ impl ProjectWorkspace { } } -fn get_rustc_cfg_options(target: Option<&str>) -> CfgOptions { - let mut cfg_options = CfgOptions::default(); +fn get_rustc_cfg_options(target: Option<&str>) -> Vec { + let mut res = Vec::new(); // Some nightly-only cfgs, which are required for stdlib - { - cfg_options.insert_atom("target_thread_local".into()); - for &target_has_atomic in ["8", "16", "32", "64", "cas", "ptr"].iter() { - cfg_options.insert_key_value("target_has_atomic".into(), target_has_atomic.into()); - cfg_options - .insert_key_value("target_has_atomic_load_store".into(), target_has_atomic.into()); + res.push(CfgFlag::Atom("target_thread_local".into())); + for &ty in ["8", "16", "32", "64", "cas", "ptr"].iter() { + for &key in ["target_has_atomic", "target_has_atomic_load_store"].iter() { + res.push(CfgFlag::KeyValue { key: key.to_string(), value: ty.into() }); } } - let rustc_cfgs = || -> Result { - // `cfg(test)` and `cfg(debug_assertion)` are handled outside, so we suppress them here. + let rustc_cfgs = { let mut cmd = Command::new(ra_toolchain::rustc()); cmd.args(&["--print", "cfg", "-O"]); if let Some(target) = target { cmd.args(&["--target", target]); } - let output = output(cmd)?; - Ok(String::from_utf8(output.stdout)?) - }(); + utf8_stdout(cmd) + }; match rustc_cfgs { - Ok(rustc_cfgs) => { - for line in rustc_cfgs.lines() { - match split_delim(line, '=') { - None => cfg_options.insert_atom(line.into()), - Some((key, value)) => { - let value = value.trim_matches('"'); - cfg_options.insert_key_value(key.into(), value.into()); - } - } - } - } + Ok(rustc_cfgs) => res.extend(rustc_cfgs.lines().map(|it| it.parse().unwrap())), Err(e) => log::error!("failed to get rustc cfgs: {:#}", e), } - cfg_options.insert_atom("debug_assertions".into()); - - cfg_options + res } -fn output(mut cmd: Command) -> Result { +fn utf8_stdout(mut cmd: Command) -> Result { let output = cmd.output().with_context(|| format!("{:?} failed", cmd))?; if !output.status.success() { match String::from_utf8(output.stderr) { @@ -557,5 +539,6 @@ fn output(mut cmd: Command) -> Result { _ => bail!("{:?} failed, {}", cmd, output.status), } } - Ok(output) + let stdout = String::from_utf8(output.stdout)?; + Ok(stdout) } diff --git a/crates/ra_project_model/src/project_json.rs b/crates/ra_project_model/src/project_json.rs index e9a333191..e3f3163f6 100644 --- a/crates/ra_project_model/src/project_json.rs +++ b/crates/ra_project_model/src/project_json.rs @@ -3,11 +3,11 @@ use std::path::PathBuf; use paths::{AbsPath, AbsPathBuf}; -use ra_cfg::CfgOptions; use ra_db::{CrateId, CrateName, Dependency, Edition}; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashMap; use serde::{de, Deserialize}; -use stdx::split_delim; + +use crate::cfg_flag::CfgFlag; /// Roots and crates that compose this Rust project. #[derive(Clone, Debug, Eq, PartialEq)] @@ -22,7 +22,7 @@ pub struct Crate { pub(crate) root_module: AbsPathBuf, pub(crate) edition: Edition, pub(crate) deps: Vec, - pub(crate) cfg: CfgOptions, + pub(crate) cfg: Vec, pub(crate) target: Option, pub(crate) env: FxHashMap, pub(crate) proc_macro_dylib_path: Option, @@ -65,18 +65,7 @@ impl ProjectJson { name: dep_data.name, }) .collect::>(), - cfg: { - let mut cfg = CfgOptions::default(); - for entry in &crate_data.cfg { - match split_delim(entry, '=') { - Some((key, value)) => { - cfg.insert_key_value(key.into(), value.into()); - } - None => cfg.insert_atom(entry.into()), - } - } - cfg - }, + cfg: crate_data.cfg, target: crate_data.target, env: crate_data.env, proc_macro_dylib_path: crate_data @@ -103,7 +92,7 @@ struct CrateData { edition: EditionData, deps: Vec, #[serde(default)] - cfg: FxHashSet, + cfg: Vec, target: Option, #[serde(default)] env: FxHashMap, diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs index 68d134da4..8a92acea5 100644 --- a/crates/ra_project_model/src/sysroot.rs +++ b/crates/ra_project_model/src/sysroot.rs @@ -6,7 +6,7 @@ use anyhow::{bail, format_err, Result}; use paths::{AbsPath, AbsPathBuf}; use ra_arena::{Arena, Idx}; -use crate::output; +use crate::utf8_stdout; #[derive(Default, Debug, Clone, Eq, PartialEq)] pub struct Sysroot { @@ -92,15 +92,14 @@ fn get_or_install_rust_src(cargo_toml: &AbsPath) -> Result { let current_dir = cargo_toml.parent().unwrap(); let mut rustc = Command::new(ra_toolchain::rustc()); rustc.current_dir(current_dir).args(&["--print", "sysroot"]); - let rustc_output = output(rustc)?; - let stdout = String::from_utf8(rustc_output.stdout)?; + let stdout = utf8_stdout(rustc)?; let sysroot_path = AbsPath::assert(Path::new(stdout.trim())); let src_path = sysroot_path.join("lib/rustlib/src/rust/src"); if !src_path.exists() { let mut rustup = Command::new(ra_toolchain::rustup()); rustup.current_dir(current_dir).args(&["component", "add", "rust-src"]); - let _output = output(rustup)?; + utf8_stdout(rustup)?; } if !src_path.exists() { bail!( diff --git a/crates/rust-analyzer/tests/heavy_tests/main.rs b/crates/rust-analyzer/tests/heavy_tests/main.rs index f12e5a37f..93448834f 100644 --- a/crates/rust-analyzer/tests/heavy_tests/main.rs +++ b/crates/rust-analyzer/tests/heavy_tests/main.rs @@ -318,7 +318,7 @@ fn test_missing_module_code_action_in_json_project() { "root_module": path.join("src/lib.rs"), "deps": [], "edition": "2015", - "cfg": [ "cfg_atom_1", "feature=cfg_1"], + "cfg": [ "cfg_atom_1", "feature=\"cfg_1\""], } ] }); -- cgit v1.2.3