Clean up prototype implementation

Author: Xanewok

src/build.rs

@@ -18,8 +18,7 @@ extern crate syntax;
 
 use cargo::core::{PackageId, Shell, Workspace, Verbosity};
 use cargo::ops::{compile_with_exec, Executor, Context, Packages, CompileOptions, CompileMode, CompileFilter, Unit};
-use cargo::util::{Config as CargoConfig, ProcessBuilder, homedir, ConfigValue};
-use cargo::util::{CargoResult, important_paths};
+use cargo::util::{Config as CargoConfig, ProcessBuilder, homedir, important_paths, ConfigValue, CargoResult};
 
 use data::Analysis;
 use vfs::Vfs;
@@ -256,7 +255,7 @@ impl BuildQueue {
             let result = self.cargo(build_dir.clone());
 
             match result {
-                BuildResult::Err |
+                BuildResult::Err => return BuildResult::Err,
                 _ if workspace_mode => return result,
                 _ => {},
             };
@@ -275,23 +274,25 @@ impl BuildQueue {
             compilation_cx: Arc<Mutex<CompilationContext>>,
             cur_package_id: Mutex<Option<PackageId>>,
             config: Arc<Mutex<Config>>,
-            /// Packages which are directly a member of the workspace, currently
-            /// RLS shouldn't provide analysis for upstream packages to those
             workspace_mode: bool,
+            /// Packages which are directly a member of the workspace, for which
+            /// analysis and diagnostics will be provided
             member_packages: Mutex<HashSet<PackageId>>,
             /// JSON compiler messages emitted for each primary compiled crate
             compiler_messages: Arc<Mutex<Vec<String>>>,
         }
 
         impl RlsExecutor {
             fn new(compilation_cx: Arc<Mutex<CompilationContext>>,
-                   config: Arc<Mutex<Config>>, compiler_messages: Arc<Mutex<Vec<String>>>)
-                -> RlsExecutor {
+                   config: Arc<Mutex<Config>>,
+                   compiler_messages: Arc<Mutex<Vec<String>>>)
+            -> RlsExecutor {
+                let workspace_mode = config.lock().unwrap().workspace_mode;
                 RlsExecutor {
-                    compilation_cx: compilation_cx,
+                    compilation_cx,
                     cur_package_id: Mutex::new(None),
                     config,
-                    workspace_mode: config.lock().unwrap().workspace_mode,
+                    workspace_mode,
                     member_packages: Mutex::new(HashSet::new()),
                     compiler_messages,
                 }
@@ -303,7 +304,7 @@ impl BuildQueue {
                 } else {
                     let cur_package_id = self.cur_package_id.lock().unwrap();
                     id == cur_package_id.as_ref().expect("Executor has not been initialised")
-                };
+                }
             }
         }
 
@@ -417,7 +418,9 @@ impl BuildQueue {
                     for a in &args {
                         // Emitting only dep-info is possible only for final crate type, as
                         // as others may emit required metadata for dependent crate types
-                        if a.starts_with("--emit") && is_final_crate_type && !workspace_mode {
+                        // In workspace_mode we don't run follow-up rustc, so don't omit anything,
+                        // as we need the generated save-analysis file
+                        if a.starts_with("--emit") && is_final_crate_type && !self.workspace_mode {
                             cmd.arg("--emit=dep-info");
                         } else {
                             cmd.arg(a);
@@ -429,8 +432,7 @@ impl BuildQueue {
                     }
                 }
 
-                // Different path for workspace_mode just to be 110% sure it doesn't affect old behaviour
-                if workspace_mode {
+                if self.workspace_mode {
                     let output = cmd.output().expect("Couldn't execute rustc");
                     let mut stderr_json_msg = convert_message_to_json_strings(output.stderr);
                     self.compiler_messages.lock().unwrap().append(&mut stderr_json_msg);
@@ -464,64 +466,27 @@ impl BuildQueue {
         // However, if Cargo doesn't run a separate thread, then we'll just wait
         // forever. Therefore, we spawn an extra thread here to be safe.
         let handle = thread::spawn(move || {
-            #[allow(dead_code)]
             #[derive(Debug)]
             struct CargoOptions {
-                flag_package: Vec<String>,
-                flag_jobs: Option<u32>,
-                flag_features: Vec<String>,
-                flag_all_features: bool,
-                flag_no_default_features: bool,
-                flag_target: Option<String>,
-                flag_manifest_path: Option<String>,
-                flag_verbose: u32,
-                flag_quiet: Option<bool>,
-                flag_color: Option<String>,
-                //flag_message_format: MessageFormat,
-                flag_release: bool,
-                flag_lib: bool,
-                flag_bin: Vec<String>,
-                flag_bins: bool,
-                flag_example: Vec<String>,
-                flag_examples: bool,
-                flag_test: Vec<String>,
-                flag_tests: bool,
-                flag_bench: Vec<String>,
-                flag_benches: bool,
-                flag_locked: bool,
-                flag_frozen: bool,
-                flag_all: bool,
-                flag_exclude: Vec<String>,
+                package: Vec<String>,
+                target: Option<String>,
+                lib: bool,
+                bin: Vec<String>,
+                bins: bool,
+                all: bool,
+                exclude: Vec<String>,
             }
 
             impl CargoOptions {
                 fn default() -> CargoOptions {
                     CargoOptions {
-                        flag_package: vec![],
-                        flag_jobs: None,
-                        flag_features: vec![],
-                        flag_all_features: false,
-                        flag_no_default_features: false,
-                        flag_target: None,
-                        flag_manifest_path: None,
-                        flag_verbose: 0, // u32?
-                        flag_quiet: None,
-                        flag_color: None,
-                        //flag_message_format: MessageFormat::Human,
-                        flag_release: false,
-                        flag_lib: false,
-                        flag_bin: vec![],
-                        flag_bins: false,
-                        flag_example: vec![],
-                        flag_examples: false,
-                        flag_test: vec![],
-                        flag_tests: false,
-                        flag_bench: vec![],
-                        flag_benches: false,
-                        flag_locked: false,
-                        flag_frozen: false,
-                        flag_all: false,
-                        flag_exclude: vec![],
+                        package: vec![],
+                        target: None,
+                        lib: false,
+                        bin: vec![],
+                        bins: false,
+                        all: false,
+                        exclude: vec![],
                     }
                 }
 
@@ -533,13 +498,13 @@ impl BuildQueue {
                         };
 
                         CargoOptions {
-                            flag_package: package,
-                            flag_all: all,
-                            flag_target: config.target.clone(),
+                            package,
+                            all,
+                            target: config.target.clone(),
                             .. CargoOptions::default()
                         }
                     } else {
-                        // In non-workspace mode we currently support only one crate target,
+                        // 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 {
                             true => (true, vec![]),
@@ -553,70 +518,92 @@ impl BuildQueue {
                         };
 
                         CargoOptions {
-                            flag_lib: lib,
-                            flag_bin: bin,
-                            flag_target: config.target.clone(),
+                            lib,
+                            bin,
+                            target: config.target.clone(),
                             .. CargoOptions::default()
                         }
                     }
                 }
             }
 
+            fn prepare_cargo_rustflags(config: &Config) -> String {
+                let mut flags = "-Zunstable-options -Zsave-analysis --error-format=json \
+                                 -Zcontinue-parse-after-error".to_owned();
+
+                if let Some(ref sysroot) = config.sysroot {
+                    flags.push_str(&format!(" --sysroot {}", sysroot));
+                }
+
+                flags = format!("{} {} {}",
+                                     env::var("RUSTFLAGS").unwrap_or(String::new()),
+                                     config.rustflags.as_ref().unwrap_or(&String::new()),
+                                     flags);
+
+                dedup_flags(&flags)
+            }
+
             // 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, &build_dir)
                 .expect(&format!("Couldn't find a root manifest for cwd: {:?}", &build_dir));
             trace!("root manifest_path: {:?}", &manifest_path);
 
             let mut shell = Shell::from_write(Box::new(BufWriter(out.clone())));
             shell.set_verbosity(Verbosity::Quiet);
-            // We construct relative paths from the manifest dir, so we have to omit the filename
-            let config = make_cargo_config(manifest_path.parent().unwrap(), shell);
+
+            // 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");
 
-            // Copy over relevant options without locking whole config, as compilation procedure
-            // will need those and we don't want to lock whole config file during that period
-            // TODO: Keep this structure cached and regenerate it on every relevant configuration change
-            let rls_config = rls_config.lock().unwrap().clone();
+            // TODO: It might be feasible to keep this CargoOptions structure cached and regenerate
+            // it on every relevant configuration change
+            let (opts, rustflags) = {
+                // We mustn't lock configuration for the whole build process
+                let rls_config = rls_config.lock().unwrap();
 
-            let opts = CargoOptions::new(&rls_config);
-            trace!("Current Cargo compilation options:\n{:?}", opts);
+                let opts = CargoOptions::new(&rls_config);
+                trace!("Cargo compilation options:\n{:?}", opts);
 
-            let spec = Packages::from_flags(opts.flag_all, &opts.flag_exclude, &opts.flag_package)
+                // Warn about invalid specified bin target or package depending on current mode
+                // TODO: Return client notifications along with diagnostics to inform the user
+                if !rls_config.workspace_mode {
+                    let cur_pkg_targets = ws.current().unwrap().targets();
+
+                    if let Some(ref build_bin) = rls_config.build_bin {
+                        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); 
+                        }
+                    }
+                } else {
+                    for package in &opts.package {
+                        if let None =  ws.members().find(|x| x.name() == package) {
+                            warn!("cargo - couldn't find member package `{}` specified in \"analyze_package\" configuration", package);
+                        }
+                    }
+                }
+
+                let rustflags = prepare_cargo_rustflags(&rls_config);
+                (opts, rustflags)
+            };
+
+            let spec = Packages::from_flags(opts.all, &opts.exclude, &opts.package)
                 .expect("Couldn't create Packages for Cargo");
 
             let compile_opts = CompileOptions {
-                target: opts.flag_target.as_ref().map(|t| &t[..]),
+                target: opts.target.as_ref().map(|t| &t[..]),
                 spec: spec,
-                filter: CompileFilter::new(opts.flag_lib,
-                                          &opts.flag_bin, opts.flag_bins,
-                                          // TODO: Support more crate target types
-                                          &[], false, &[], false, &[], false),
+                filter: CompileFilter::new(opts.lib,
+                                        &opts.bin, opts.bins,
+                                        // TODO: Support more crate target types
+                                        &[], false, &[], false, &[], false),
                 .. CompileOptions::default(&config, CompileMode::Check)
             };
-
-
-            let rustflags = {
-                let mut flags = "-Zunstable-options -Zsave-analysis --error-format=json \
-                                 -Zcontinue-parse-after-error".to_owned();
-
-                // TODO: Needed? since it's also pushed in exec()
-                if let Some(ref sysroot) = rls_config.sysroot {
-                    flags.push_str(&format!(" --sysroot {}", sysroot));
-                }
-
-                flags = format!("{} {} {}",
-                                     env::var("RUSTFLAGS").unwrap_or(String::new()),
-                                     rls_config.rustflags.as_ref().unwrap_or(&String::new()),
-                                     flags);
-
-                dedup_flags(&flags)
-            };
+            
             env::set_var("RUSTFLAGS", rustflags);
 
-            // let current_package = ws.current().unwrap();
-            // let targets = current_package.targets();
-
             compile_with_exec(&ws, &compile_opts, Arc::new(exec)).expect("could not run cargo");
         });