Merge pull request #438 from Xanewok/infer-targets

Infer appropriate crate target from workspace

Author: nrc

src/actions/mod.rs

@@ -10,13 +10,17 @@
 
 mod compiler_message_parsing;
 
+use cargo::CargoResult;
+use cargo::util::important_paths;
+use cargo::core::{Shell, Workspace};
+
 use analysis::{AnalysisHost};
 use url::Url;
 use vfs::{Vfs, Change, FileContents};
 use racer;
 use rustfmt::{Input as FmtInput, format_input};
 use rustfmt::file_lines::{Range as RustfmtRange, FileLines};
-use config::{Config, FmtConfig};
+use config::{Config, FmtConfig, Inferrable};
 use serde::Deserialize;
 use serde::de::Error;
 use serde_json;
@@ -32,6 +36,7 @@ use jsonrpc_core::types::ErrorCode;
 use std::collections::HashMap;
 use std::panic;
 use std::path::{Path, PathBuf};
+use std::io::sink;
 use std::sync::{Arc, Mutex};
 use std::thread;
 use std::time::Duration;
@@ -92,6 +97,19 @@ impl ActionHandler {
     pub fn init<O: Output>(&self, init_options: InitializationOptions, out: O) {
         trace!("init: {:?}", init_options);
 
+        let project_dir = self.current_project.clone();
+        let config = self.config.clone();
+        // Spawn another thread since we're shelling out to Cargo and this can
+        // cause a non-trivial amount of time due to disk access
+        thread::spawn(move || {
+            let mut config = config.lock().unwrap();
+            if let Err(e)  = infer_config_defaults(&project_dir, &mut *config) {
+                debug!("Encountered an error while trying to infer config \
+                    defaults: {:?}", e);
+            }
+
+        });
+
         if !init_options.omit_init_build {
             self.build_current_project(BuildPriority::Cargo, out);
         }
@@ -748,7 +766,27 @@ impl ActionHandler {
 
         {
             let mut config = self.config.lock().unwrap();
-            *config = new_config;
+
+            // User may specify null (to be inferred) options, in which case
+            // we schedule further inference on a separate thread not to block
+            // the main thread
+            let needs_inference = new_config.needs_inference();
+            // In case of null options, we provide default values for now
+            config.update(new_config);
+            trace!("Updated config: {:?}", *config);
+
+            if needs_inference {
+                let project_dir = self.current_project.clone();
+                let config = self.config.clone();
+                // Will lock and access Config just outside the current scope
+                thread::spawn(move || {
+                    let mut config = config.lock().unwrap();
+                    if let Err(e)  = infer_config_defaults(&project_dir, &mut *config) {
+                        debug!("Encountered an error while trying to infer config \
+                            defaults: {:?}", e);
+                    }
+                });
+            }
         }
         // We do a clean build so that if we've changed any relevant options
         // for Cargo, we'll notice them. But if nothing relevant changes
@@ -812,6 +850,76 @@ impl ActionHandler {
     }
 }
 
+fn infer_config_defaults(project_dir: &Path, config: &mut Config) -> CargoResult<()> {
+    // Note that this may not be equal build_dir when inside a workspace member
+    let manifest_path = important_paths::find_root_manifest_for_wd(None, project_dir)?;
+    trace!("root manifest_path: {:?}", &manifest_path);
+
+    // Cargo constructs relative paths from the manifest dir, so we have to pop "Cargo.toml"
+    let manifest_dir = manifest_path.parent().unwrap();
+    let shell = Shell::from_write(Box::new(sink()));
+    let cargo_config = make_cargo_config(manifest_dir, shell);
+
+    let ws = Workspace::new(&manifest_path, &cargo_config)?;
+
+    // Auto-detect --lib/--bin switch if working under single package mode
+    // or under workspace mode with `analyze_package` specified
+    let package = match config.workspace_mode {
+        true => {
+            let package_name = match config.analyze_package {
+                // No package specified, nothing to do
+                None => { return Ok(()); },
+                Some(ref package) => package,
+            };
+
+            ws.members()
+              .find(move |x| x.name() == package_name)
+              .ok_or(
+                  format!("Couldn't find specified `{}` package via \
+                      `analyze_package` in the workspace", package_name)
+              )?
+        },
+        false => ws.current()?,
+    };
+
+    trace!("infer_config_defaults: Auto-detected `{}` package", package.name());
+
+    let targets = package.targets();
+    let (lib, bin) = if targets.iter().any(|x| x.is_lib()) {
+        (true, None)
+    } else {
+        let mut bins = targets.iter().filter(|x| x.is_bin());
+        // No `lib` detected, but also can't find any `bin` target - there's
+        // no sensible target here, so just Err out
+        let first = bins.nth(0)
+            .ok_or("No `bin` or `lib` targets in the package")?;
+
+        let mut bins = targets.iter().filter(|x| x.is_bin());
+        let target = match bins.find(|x| x.src_path().ends_with("main.rs")) {
+            Some(main_bin) => main_bin,
+            None => first,
+        };
+
+        (false, Some(target.name().to_owned()))
+    };
+
+    trace!("infer_config_defaults: build_lib: {:?}, build_bin: {:?}", lib, bin);
+
+    // Unless crate target is explicitly specified, mark the values as
+    // inferred, so they're not simply ovewritten on config change without
+    // any specified value
+    let (lib, bin) = match (&config.build_lib, &config.build_bin) {
+        (&Inferrable::Specified(true), _) => (lib, None),
+        (_, &Inferrable::Specified(Some(_))) => (false, bin),
+        _ => (lib, bin),
+    };
+
+    config.build_lib.infer(lib);
+    config.build_bin.infer(bin);
+
+    Ok(())
+}
+
 fn racer_coord(line: span::Row<span::OneIndexed>,
                column: span::Column<span::ZeroIndexed>)
                -> racer::Coordinate {

src/build/cargo.rs

@@ -97,12 +97,12 @@ fn run_cargo(compilation_cx: Arc<Mutex<CompilationContext>>,
     let manifest_path = important_paths::find_root_manifest_for_wd(None, &build_dir)
         .expect(&format!("Couldn't find a root manifest for cwd: {:?}", &build_dir));
     trace!("root manifest_path: {:?}", &manifest_path);
+    // Cargo constructs relative paths from the manifest dir, so we have to pop "Cargo.toml"
+    let manifest_dir = manifest_path.parent().unwrap();
 
     let mut shell = Shell::from_write(Box::new(BufWriter(out.clone())));
     shell.set_verbosity(Verbosity::Quiet);
 
-    // Cargo constructs relative paths from the manifest dir, so we have to pop "Cargo.toml"
-    let manifest_dir = manifest_path.parent().unwrap();
     let config = make_cargo_config(manifest_dir, shell);
 
     let ws = Workspace::new(&manifest_path, &config).expect("could not create cargo workspace");
@@ -128,7 +128,7 @@ fn run_cargo(compilation_cx: Arc<Mutex<CompilationContext>>,
         if !rls_config.workspace_mode {
             let cur_pkg_targets = ws.current().unwrap().targets();
 
-            if let Some(ref build_bin) = rls_config.build_bin {
+            if let &Some(ref build_bin) = rls_config.build_bin.as_ref() {
                 let mut bins = cur_pkg_targets.iter().filter(|x| x.is_bin());
                 if let None = bins.find(|x| x.name() == build_bin) {
                     warn!("cargo - couldn't find binary `{}` specified in `build_bin` configuration", build_bin);
@@ -329,7 +329,8 @@ impl Executor for RlsExecutor {
             // assume crate_type arg (i.e. in `cargo test` it isn't specified for --test targets)
             // and build test harness only for final crate type
             let crate_type = crate_type.expect("no crate-type in rustc command line");
-            let is_final_crate_type = crate_type == "bin" || (crate_type == "lib" && config.build_lib);
+            let build_lib = *config.build_lib.as_ref();
+            let is_final_crate_type = crate_type == "bin" || (crate_type == "lib" && build_lib);
 
             if config.cfg_test {
                 // FIXME(#351) allow passing --test to lib crate-type when building a dependency
@@ -448,10 +449,10 @@ impl CargoOptions {
         } else {
             // In single-crate mode we currently support only one crate target,
             // and if lib is set, then we ignore bin target config
-            let (lib, bin) = match config.build_lib {
+            let (lib, bin) = match *config.build_lib.as_ref() {
                 true => (true, vec![]),
                 false => {
-                    let bin = match config.build_bin {
+                    let bin = match *config.build_bin.as_ref() {
                         Some(ref bin) => vec![bin.clone()],
                         None => vec![],
                     };
@@ -485,7 +486,7 @@ fn prepare_cargo_rustflags(config: &Config) -> String {
     dedup_flags(&flags)
 }
 
-fn make_cargo_config(build_dir: &Path, shell: Shell) -> CargoConfig {
+pub fn make_cargo_config(build_dir: &Path, shell: Shell) -> CargoConfig {
     let config = CargoConfig::new(shell,
                                   // This is Cargo's cwd. We're using the actual cwd,
                                   // because Cargo will generate relative paths based

src/build/mod.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+pub use self::cargo::make_cargo_config;
+
 use data::Analysis;
 use vfs::Vfs;
 use config::Config;

src/config.rs

@@ -9,20 +9,92 @@
 // except according to those terms.
 
 use std::path::Path;
+use std::fmt::Debug;
+
+use serde::de::{Deserialize, Deserializer};
 
 use rustfmt::config::Config as RustfmtConfig;
 use rustfmt::config::WriteMode;
 
 const DEFAULT_WAIT_TO_BUILD: u64 = 500;
 
+/// Some values in the config can be inferred without an explicit value set by
+/// the user. There are no guarantees which values will or will not be passed
+/// to the server, so we treat deserialized values effectively as `Option<T>`
+/// and use `None` to mark the values as unspecified, otherwise we always use
+/// `Specified` variant for the deserialized values. For user-provided `None`
+/// values, they must be `Inferred` prior to usage (and can be further
+/// `Specified` by the user).
+#[derive(Clone, Debug, Serialize)]
+pub enum Inferrable<T> {
+    /// Explicitly specified value by the user. Retrieved by deserializing a
+    /// non-`null` value. Can replace every other variant.
+    Specified(T),
+    /// Value that's inferred by the server. Can't replace a `Specified` variant.
+    Inferred(T),
+    /// Marker value that's retrieved when deserializing a user-specified `null`
+    /// value. Can't be used alone and has to be replaced by server-`Inferred`
+    /// or user-`Specified` value.
+    None
+}
+
+// Deserialize as if it's `Option<T>` and use `None` variant if it's `None`,
+// otherwise use `Specified` variant for deserialized value.
+impl<'de, T: Deserialize<'de>> Deserialize<'de> for Inferrable<T> {
+    fn deserialize<D>(deserializer: D) -> Result<Inferrable<T>, D::Error>
+        where D: Deserializer<'de>
+    {
+        let value = Option::<T>::deserialize(deserializer)?;
+        Ok(match value {
+            None => Inferrable::None,
+            Some(value) => Inferrable::Specified(value),
+        })
+    }
+}
+
+impl<T: Clone + Debug> Inferrable<T> {
+    pub fn combine_with_default(&self, new: &Self, default: T) -> Self {
+        match (self, new) {
+            // Don't allow to update a Specified value with an Inferred one
+            (&Inferrable::Specified(_), &Inferrable::Inferred(_)) => self.clone(),
+            // When trying to update with a `None`, use Inferred variant with
+            // a specified default value, as `None` value can't be used directly
+            (_, &Inferrable::None) => Inferrable::Inferred(default),
+            _ => new.clone(),
+        }
+    }
+
+    pub fn infer(&mut self, value: T) {
+        if let &mut Inferrable::Specified(_) = self {
+            trace!("Trying to infer {:?} on a {:?}", value, self);
+            return;
+        }
+
+        *self = Inferrable::Inferred(value);
+    }
+}
+
+impl<T> AsRef<T> for Inferrable<T> {
+    fn as_ref(&self) -> &T {
+        match *self {
+            Inferrable::Inferred(ref value) |
+            Inferrable::Specified(ref value) => value,
+            // Default values should always be initialized as `Inferred` even
+            // before actual inference takes place, `None` variant is only used
+            // when deserializing and should not be read directly (via `as_ref`)
+            Inferrable::None => unreachable!(),
+        }
+    }
+}
+
 #[derive(Clone, Debug, Deserialize, Serialize)]
 #[serde(default)]
 pub struct Config {
     pub sysroot: Option<String>,
     pub target: Option<String>,
     pub rustflags: Option<String>,
-    pub build_lib: bool,
-    pub build_bin: Option<String>,
+    pub build_lib: Inferrable<bool>,
+    pub build_bin: Inferrable<Option<String>>,
     pub cfg_test: bool,
     pub unstable_features: bool,
     pub wait_to_build: u64,
@@ -43,8 +115,8 @@ impl Default for Config {
             sysroot: None,
             target: None,
             rustflags: None,
-            build_lib: false,
-            build_bin: None,
+            build_lib: Inferrable::Inferred(false),
+            build_bin: Inferrable::Inferred(None),
             cfg_test: false,
             unstable_features: false,
             wait_to_build: DEFAULT_WAIT_TO_BUILD,
@@ -59,6 +131,23 @@ impl Default for Config {
     }
 }
 
+impl Config {
+    pub fn update(&mut self, mut new: Config) {
+        new.build_lib = self.build_lib.combine_with_default(&new.build_lib, false);
+        new.build_bin = self.build_bin.combine_with_default(&new.build_bin, None);
+
+        *self = new;
+    }
+
+    pub fn needs_inference(&self) -> bool {
+        match (&self.build_lib, &self.build_bin) {
+            (&Inferrable::None, _) |
+            (_, &Inferrable::None) => true,
+            _ => false,
+        }
+    }
+}
+
 /// A rustfmt config (typically specified via rustfmt.toml)
 /// The FmtConfig is not an exact translation of the config
 /// rustfmt generates from the user's toml file, since when