2 files changed, 123 insertions, 117 deletions

diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs
index 291594e2a..b50cda06f 100644
--- a/crates/ra_project_model/src/cargo_workspace.rs
+++ b/crates/ra_project_model/src/cargo_workspace.rs
@@ -1,17 +1,18 @@
 //! FIXME: write short doc here
 
 use std::{
+    env,
+    ffi::OsStr,
     ops,
     path::{Path, PathBuf},
+    process::Command,
 };
 
 use anyhow::{Context, Result};
 use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId};
 use ra_arena::{Arena, Idx};
-use ra_cargo_watch::run_cargo;
 use ra_db::Edition;
 use rustc_hash::FxHashMap;
-use serde::Deserialize;
 
 /// `CargoWorkspace` represents the logical structure of, well, a Cargo
 /// workspace. It pretty closely mirrors `cargo metadata` output.
@@ -41,9 +42,8 @@ impl ops::Index<Target> for CargoWorkspace {
     }
 }
 
-#[derive(Deserialize, Clone, Debug, PartialEq, Eq)]
-#[serde(rename_all = "camelCase", default)]
-pub struct CargoFeatures {
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct CargoConfig {
     /// Do not activate the `default` feature.
     pub no_default_features: bool,
 
@@ -58,9 +58,9 @@ pub struct CargoFeatures {
     pub load_out_dirs_from_check: bool,
 }
 
-impl Default for CargoFeatures {
+impl Default for CargoConfig {
     fn default() -> Self {
-        CargoFeatures {
+        CargoConfig {
             no_default_features: false,
             all_features: true,
             features: Vec::new(),
@@ -75,6 +75,7 @@ pub type Target = Idx<TargetData>;
 
 #[derive(Debug, Clone)]
 pub struct PackageData {
+    pub version: String,
     pub name: String,
     pub manifest: PathBuf,
     pub targets: Vec<Target>,
@@ -138,7 +139,7 @@ impl PackageData {
 impl CargoWorkspace {
     pub fn from_cargo_metadata(
         cargo_toml: &Path,
-        cargo_features: &CargoFeatures,
+        cargo_features: &CargoConfig,
     ) -> Result<CargoWorkspace> {
         let mut meta = MetadataCommand::new();
         meta.manifest_path(cargo_toml);
@@ -161,7 +162,7 @@ impl CargoWorkspace {
         let mut out_dir_by_id = FxHashMap::default();
         let mut proc_macro_dylib_paths = FxHashMap::default();
         if cargo_features.load_out_dirs_from_check {
-            let resources = load_extern_resources(cargo_toml, cargo_features);
+            let resources = load_extern_resources(cargo_toml, cargo_features)?;
             out_dir_by_id = resources.out_dirs;
             proc_macro_dylib_paths = resources.proc_dylib_paths;
         }
@@ -173,13 +174,15 @@ impl CargoWorkspace {
         let ws_members = &meta.workspace_members;
 
         for meta_pkg in meta.packages {
-            let cargo_metadata::Package { id, edition, name, manifest_path, .. } = meta_pkg;
+            let cargo_metadata::Package { id, edition, name, manifest_path, version, .. } =
+                meta_pkg;
             let is_member = ws_members.contains(&id);
             let edition = edition
                 .parse::<Edition>()
                 .with_context(|| format!("Failed to parse edition {}", edition))?;
             let pkg = packages.alloc(PackageData {
                 name,
+                version: version.to_string(),
                 manifest: manifest_path,
                 targets: Vec::new(),
                 is_member,
@@ -249,6 +252,18 @@ impl CargoWorkspace {
     pub fn workspace_root(&self) -> &Path {
         &self.workspace_root
     }
+
+    pub fn package_flag(&self, package: &PackageData) -> String {
+        if self.is_unique(&*package.name) {
+            package.name.clone()
+        } else {
+            format!("{}:{}", package.name, package.version)
+        }
+    }
+
+    fn is_unique(&self, name: &str) -> bool {
+        self.packages.iter().filter(|(_, v)| v.name == name).count() == 1
+    }
 }
 
 #[derive(Debug, Clone, Default)]
@@ -257,48 +272,60 @@ pub struct ExternResources {
     proc_dylib_paths: FxHashMap<PackageId, PathBuf>,
 }
 
-pub fn load_extern_resources(cargo_toml: &Path, cargo_features: &CargoFeatures) -> ExternResources {
-    let mut args: Vec<String> = vec![
-        "check".to_string(),
-        "--message-format=json".to_string(),
-        "--manifest-path".to_string(),
-        cargo_toml.display().to_string(),
-    ];
-
+pub fn load_extern_resources(
+    cargo_toml: &Path,
+    cargo_features: &CargoConfig,
+) -> Result<ExternResources> {
+    let mut cmd = Command::new(cargo_binary());
+    cmd.args(&["check", "--message-format=json", "--manifest-path"]).arg(cargo_toml);
     if cargo_features.all_features {
-        args.push("--all-features".to_string());
+        cmd.arg("--all-features");
     } else if cargo_features.no_default_features {
         // FIXME: `NoDefaultFeatures` is mutual exclusive with `SomeFeatures`
         // https://github.com/oli-obk/cargo_metadata/issues/79
-        args.push("--no-default-features".to_string());
+        cmd.arg("--no-default-features");
     } else {
-        args.extend(cargo_features.features.iter().cloned());
+        cmd.args(&cargo_features.features);
     }
 
-    let mut acc = ExternResources::default();
-    let res = run_cargo(&args, cargo_toml.parent(), &mut |message| {
-        match message {
-            Message::BuildScriptExecuted(BuildScript { package_id, out_dir, .. }) => {
-                acc.out_dirs.insert(package_id, out_dir);
-            }
+    let output = cmd.output()?;
 
-            Message::CompilerArtifact(message) => {
-                if message.target.kind.contains(&"proc-macro".to_string()) {
-                    let package_id = message.package_id;
-                    if let Some(filename) = message.filenames.get(0) {
-                        acc.proc_dylib_paths.insert(package_id, filename.clone());
+    let mut res = ExternResources::default();
+
+    for message in cargo_metadata::parse_messages(output.stdout.as_slice()) {
+        if let Ok(message) = message {
+            match message {
+                Message::BuildScriptExecuted(BuildScript { package_id, out_dir, .. }) => {
+                    res.out_dirs.insert(package_id, out_dir);
+                }
+
+                Message::CompilerArtifact(message) => {
+                    if message.target.kind.contains(&"proc-macro".to_string()) {
+                        let package_id = message.package_id;
+                        // Skip rmeta file
+                        if let Some(filename) =
+                            message.filenames.iter().filter(|name| is_dylib(name)).next()
+                        {
+                            res.proc_dylib_paths.insert(package_id, filename.clone());
+                        }
                     }
                 }
+                Message::CompilerMessage(_) => (),
+                Message::Unknown => (),
             }
-            Message::CompilerMessage(_) => (),
-            Message::Unknown => (),
         }
-        true
-    });
+    }
+    Ok(res)
+}
 
-    if let Err(err) = res {
-        log::error!("Failed to load outdirs: {:?}", err);
+// FIXME: File a better way to know if it is a dylib
+fn is_dylib(path: &Path) -> bool {
+    match path.extension().and_then(OsStr::to_str).map(|it| it.to_string().to_lowercase()) {
+        None => false,
+        Some(ext) => matches!(ext.as_str(), "dll" | "dylib" | "so"),
     }
+}
 
-    acc
+fn cargo_binary() -> String {
+    env::var("CARGO").unwrap_or_else(|_| "cargo".to_string())
 }
diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs
index 444d3bb3f..0ab64a1e0 100644
--- a/crates/ra_project_model/src/lib.rs
+++ b/crates/ra_project_model/src/lib.rs
@@ -19,7 +19,7 @@ use rustc_hash::FxHashMap;
 use serde_json::from_reader;
 
 pub use crate::{
-    cargo_workspace::{CargoFeatures, CargoWorkspace, Package, Target, TargetKind},
+    cargo_workspace::{CargoConfig, CargoWorkspace, Package, Target, TargetKind},
     json_project::JsonProject,
     sysroot::Sysroot,
 };
@@ -62,30 +62,30 @@ pub struct PackageRoot {
     /// Is a member of the current workspace
     is_member: bool,
 }
-
 impl PackageRoot {
-    pub fn new(path: PathBuf, is_member: bool) -> PackageRoot {
-        PackageRoot { path, is_member }
+    pub fn new_member(path: PathBuf) -> PackageRoot {
+        Self { path, is_member: true }
     }
-
-    pub fn path(&self) -> &PathBuf {
+    pub fn new_non_member(path: PathBuf) -> PackageRoot {
+        Self { path, is_member: false }
+    }
+    pub fn path(&self) -> &Path {
         &self.path
     }
-
     pub fn is_member(&self) -> bool {
         self.is_member
     }
 }
 
 impl ProjectWorkspace {
-    pub fn discover(path: &Path, cargo_features: &CargoFeatures) -> Result<ProjectWorkspace> {
+    pub fn discover(path: &Path, cargo_features: &CargoConfig) -> Result<ProjectWorkspace> {
         ProjectWorkspace::discover_with_sysroot(path, true, cargo_features)
     }
 
     pub fn discover_with_sysroot(
         path: &Path,
         with_sysroot: bool,
-        cargo_features: &CargoFeatures,
+        cargo_features: &CargoConfig,
     ) -> Result<ProjectWorkspace> {
         match find_rust_project_json(path) {
             Some(json_path) => {
@@ -130,70 +130,45 @@ impl ProjectWorkspace {
     pub fn to_roots(&self) -> Vec<PackageRoot> {
         match self {
             ProjectWorkspace::Json { project } => {
-                let mut roots = Vec::with_capacity(project.roots.len());
-                for root in &project.roots {
-                    roots.push(PackageRoot::new(root.path.clone(), true));
-                }
-                roots
-            }
-            ProjectWorkspace::Cargo { cargo, sysroot } => {
-                let mut roots = Vec::with_capacity(cargo.packages().len() + sysroot.crates().len());
-                for pkg in cargo.packages() {
-                    let root = cargo[pkg].root().to_path_buf();
-                    let member = cargo[pkg].is_member;
-                    roots.push(PackageRoot::new(root, member));
-                }
-                for krate in sysroot.crates() {
-                    roots.push(PackageRoot::new(sysroot[krate].root_dir().to_path_buf(), false))
-                }
-                roots
+                project.roots.iter().map(|r| PackageRoot::new_member(r.path.clone())).collect()
             }
+            ProjectWorkspace::Cargo { cargo, sysroot } => cargo
+                .packages()
+                .map(|pkg| PackageRoot {
+                    path: cargo[pkg].root().to_path_buf(),
+                    is_member: cargo[pkg].is_member,
+                })
+                .chain(sysroot.crates().map(|krate| {
+                    PackageRoot::new_non_member(sysroot[krate].root_dir().to_path_buf())
+                }))
+                .collect(),
         }
     }
 
     pub fn out_dirs(&self) -> Vec<PathBuf> {
         match self {
             ProjectWorkspace::Json { project } => {
-                let mut out_dirs = Vec::with_capacity(project.crates.len());
-                for krate in &project.crates {
-                    if let Some(out_dir) = &krate.out_dir {
-                        out_dirs.push(out_dir.to_path_buf());
-                    }
-                }
-                out_dirs
+                project.crates.iter().filter_map(|krate| krate.out_dir.as_ref()).cloned().collect()
             }
-            ProjectWorkspace::Cargo { cargo, sysroot: _sysroot } => {
-                let mut out_dirs = Vec::with_capacity(cargo.packages().len());
-                for pkg in cargo.packages() {
-                    if let Some(out_dir) = &cargo[pkg].out_dir {
-                        out_dirs.push(out_dir.to_path_buf());
-                    }
-                }
-                out_dirs
+            ProjectWorkspace::Cargo { cargo, sysroot: _ } => {
+                cargo.packages().filter_map(|pkg| cargo[pkg].out_dir.as_ref()).cloned().collect()
             }
         }
     }
 
     pub fn proc_macro_dylib_paths(&self) -> Vec<PathBuf> {
         match self {
-            ProjectWorkspace::Json { project } => {
-                let mut proc_macro_dylib_paths = Vec::with_capacity(project.crates.len());
-                for krate in &project.crates {
-                    if let Some(out_dir) = &krate.proc_macro_dylib_path {
-                        proc_macro_dylib_paths.push(out_dir.to_path_buf());
-                    }
-                }
-                proc_macro_dylib_paths
-            }
-            ProjectWorkspace::Cargo { cargo, sysroot: _sysroot } => {
-                let mut proc_macro_dylib_paths = Vec::with_capacity(cargo.packages().len());
-                for pkg in cargo.packages() {
-                    if let Some(dylib_path) = &cargo[pkg].proc_macro_dylib_path {
-                        proc_macro_dylib_paths.push(dylib_path.to_path_buf());
-                    }
-                }
-                proc_macro_dylib_paths
-            }
+            ProjectWorkspace::Json { project } => project
+                .crates
+                .iter()
+                .filter_map(|krate| krate.proc_macro_dylib_path.as_ref())
+                .cloned()
+                .collect(),
+            ProjectWorkspace::Cargo { cargo, sysroot: _sysroot } => cargo
+                .packages()
+                .filter_map(|pkg| cargo[pkg].proc_macro_dylib_path.as_ref())
+                .cloned()
+                .collect(),
         }
     }
 
@@ -216,10 +191,12 @@ impl ProjectWorkspace {
         let mut crate_graph = CrateGraph::default();
         match self {
             ProjectWorkspace::Json { project } => {
-                let mut crates = FxHashMap::default();
-                for (id, krate) in project.crates.iter().enumerate() {
-                    let crate_id = json_project::CrateId(id);
-                    if let Some(file_id) = load(&krate.root_module) {
+                let crates: FxHashMap<_, _> = project
+                    .crates
+                    .iter()
+                    .enumerate()
+                    .filter_map(|(seq_index, krate)| {
+                        let file_id = load(&krate.root_module)?;
                         let edition = match krate.edition {
                             json_project::Edition::Edition2015 => Edition::Edition2015,
                             json_project::Edition::Edition2018 => Edition::Edition2018,
@@ -249,8 +226,8 @@ impl ProjectWorkspace {
                             .clone()
                             .map(|it| proc_macro_client.by_dylib_path(&it));
                         // FIXME: No crate name in json definition such that we cannot add OUT_DIR to env
-                        crates.insert(
-                            crate_id,
+                        Some((
+                            json_project::CrateId(seq_index),
                             crate_graph.add_crate_root(
                                 file_id,
                                 edition,
@@ -261,9 +238,9 @@ impl ProjectWorkspace {
                                 extern_source,
                                 proc_macro.unwrap_or_default(),
                             ),
-                        );
-                    }
-                }
+                        ))
+                    })
+                    .collect();
 
                 for (id, krate) in project.crates.iter().enumerate() {
                     for dep in &krate.deps {
@@ -287,9 +264,11 @@ impl ProjectWorkspace {
                 }
             }
             ProjectWorkspace::Cargo { cargo, sysroot } => {
-                let mut sysroot_crates = FxHashMap::default();
-                for krate in sysroot.crates() {
-                    if let Some(file_id) = load(&sysroot[krate].root) {
+                let sysroot_crates: FxHashMap<_, _> = sysroot
+                    .crates()
+                    .filter_map(|krate| {
+                        let file_id = load(&sysroot[krate].root)?;
+
                         // Crates from sysroot have `cfg(test)` disabled
                         let cfg_options = {
                             let mut opts = default_cfg_options.clone();
@@ -300,22 +279,22 @@ impl ProjectWorkspace {
                         let env = Env::default();
                         let extern_source = ExternSource::default();
                         let proc_macro = vec![];
+                        let crate_name = CrateName::new(&sysroot[krate].name)
+                            .expect("Sysroot crate names should not contain dashes");
 
                         let crate_id = crate_graph.add_crate_root(
                             file_id,
                             Edition::Edition2018,
-                            Some(
-                                CrateName::new(&sysroot[krate].name)
-                                    .expect("Sysroot crate names should not contain dashes"),
-                            ),
+                            Some(crate_name),
                             cfg_options,
                             env,
                             extern_source,
                             proc_macro,
                         );
-                        sysroot_crates.insert(krate, crate_id);
-                    }
-                }
+                        Some((krate, crate_id))
+                    })
+                    .collect();
+
                 for from in sysroot.crates() {
                     for &to in sysroot[from].deps.iter() {
                         let name = &sysroot[to].name;