Merge #4784

4784: Change management of test cfg to better support json projects r=Nashenas88 a=Nashenas88

This helps support json projects where they can decide whether to add the `test` cfg or not. One alternative is to add support for marking json project crates as a sysroot crate, and adding logic to remove the `test` cfg in those cases. In my opinion, that option gives less flexibility to json projects and leads to more functionality that needs to be maintained.

Fixes #4508 
cc @woody77 

Co-authored-by: Paul Daniel Faria <Nashenas88@users.noreply.github.com>
Co-authored-by: Paul Daniel Faria <nashenas88@users.noreply.github.com>

Author: bors[bot]

crates/ra_project_model/src/lib.rs

@@ -250,7 +250,7 @@ impl ProjectWorkspace {
 
     pub fn to_crate_graph(
         &self,
-        default_cfg_options: &CfgOptions,
+        target: Option<&str>,
         extern_source_roots: &FxHashMap<PathBuf, ExternSourceId>,
         proc_macro_client: &ProcMacroClient,
         load: &mut dyn FnMut(&Path) -> Option<FileId>,
@@ -269,7 +269,7 @@ impl ProjectWorkspace {
                             json_project::Edition::Edition2018 => Edition::Edition2018,
                         };
                         let cfg_options = {
-                            let mut opts = default_cfg_options.clone();
+                            let mut opts = CfgOptions::default();
                             for cfg in &krate.cfg {
                                 match cfg.find('=') {
                                     None => opts.insert_atom(cfg.into()),
@@ -343,18 +343,13 @@ impl ProjectWorkspace {
                 }
             }
             ProjectWorkspace::Cargo { cargo, sysroot } => {
+                let mut cfg_options = get_rustc_cfg_options(target);
+
                 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();
-                            opts.remove_atom("test");
-                            opts
-                        };
-
                         let env = Env::default();
                         let extern_source = ExternSource::default();
                         let proc_macro = vec![];
@@ -365,7 +360,7 @@ impl ProjectWorkspace {
                             file_id,
                             Edition::Edition2018,
                             Some(crate_name),
-                            cfg_options,
+                            cfg_options.clone(),
                             env,
                             extern_source,
                             proc_macro,
@@ -396,6 +391,10 @@ impl ProjectWorkspace {
 
                 let mut pkg_to_lib_crate = FxHashMap::default();
                 let mut pkg_crates = FxHashMap::default();
+
+                // Add test cfg for non-sysroot crates
+                cfg_options.insert_atom("test".into());
+
                 // Next, create crates for each package, target pair
                 for pkg in cargo.packages() {
                     let mut lib_tgt = None;
@@ -404,7 +403,7 @@ impl ProjectWorkspace {
                         if let Some(file_id) = load(root) {
                             let edition = cargo[pkg].edition;
                             let cfg_options = {
-                                let mut opts = default_cfg_options.clone();
+                                let mut opts = cfg_options.clone();
                                 for feature in cargo[pkg].features.iter() {
                                     opts.insert_key_value("feature".into(), feature.into());
                                 }
@@ -561,7 +560,7 @@ impl ProjectWorkspace {
     }
 }
 
-pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions {
+fn get_rustc_cfg_options(target: Option<&str>) -> CfgOptions {
     let mut cfg_options = CfgOptions::default();
 
     // Some nightly-only cfgs, which are required for stdlib
@@ -579,7 +578,7 @@ pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions {
         let mut cmd = Command::new(ra_toolchain::rustc());
         cmd.args(&["--print", "cfg", "-O"]);
         if let Some(target) = target {
-            cmd.args(&["--target", target.as_str()]);
+            cmd.args(&["--target", target]);
         }
         let output = output(cmd)?;
         Ok(String::from_utf8(output.stdout)?)
@@ -601,6 +600,8 @@ pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions {
         Err(e) => log::error!("failed to get rustc cfgs: {:#}", e),
     }
 
+    cfg_options.insert_atom("debug_assertion".into());
+
     cfg_options
 }
 

crates/rust-analyzer/src/cli/load_cargo.rs

@@ -8,8 +8,7 @@ use crossbeam_channel::{unbounded, Receiver};
 use ra_db::{ExternSourceId, FileId, SourceRootId};
 use ra_ide::{AnalysisChange, AnalysisHost};
 use ra_project_model::{
-    get_rustc_cfg_options, CargoConfig, PackageRoot, ProcMacroClient, ProjectManifest,
-    ProjectWorkspace,
+    CargoConfig, PackageRoot, ProcMacroClient, ProjectManifest, ProjectWorkspace,
 };
 use ra_vfs::{RootEntry, Vfs, VfsChange, VfsTask, Watch};
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -148,26 +147,14 @@ pub(crate) fn load(
         }
     }
 
-    // FIXME: cfg options?
-    let default_cfg_options = {
-        let mut opts = get_rustc_cfg_options(None);
-        opts.insert_atom("test".into());
-        opts.insert_atom("debug_assertion".into());
-        opts
-    };
-
-    let crate_graph = ws.to_crate_graph(
-        &default_cfg_options,
-        &extern_source_roots,
-        proc_macro_client,
-        &mut |path: &Path| {
+    let crate_graph =
+        ws.to_crate_graph(None, &extern_source_roots, proc_macro_client, &mut |path: &Path| {
             // Some path from metadata will be non canonicalized, e.g. /foo/../bar/lib.rs
             let path = path.canonicalize().ok()?;
             let vfs_file = vfs.load(&path);
             log::debug!("vfs file {:?} -> {:?}", path, vfs_file);
             vfs_file.map(vfs_file_to_id)
-        },
-    );
+        });
     log::debug!("crate graph: {:?}", crate_graph);
     analysis_change.set_crate_graph(crate_graph);
 

crates/rust-analyzer/src/global_state.rs

@@ -15,7 +15,7 @@ use ra_flycheck::{Flycheck, FlycheckConfig};
 use ra_ide::{
     Analysis, AnalysisChange, AnalysisHost, CrateGraph, FileId, LibraryData, SourceRootId,
 };
-use ra_project_model::{get_rustc_cfg_options, ProcMacroClient, ProjectWorkspace};
+use ra_project_model::{ProcMacroClient, ProjectWorkspace};
 use ra_vfs::{LineEndings, RootEntry, Vfs, VfsChange, VfsFile, VfsRoot, VfsTask, Watch};
 use relative_path::RelativePathBuf;
 use stdx::format_to;
@@ -135,14 +135,6 @@ impl GlobalState {
             }
         }
 
-        // FIXME: Read default cfgs from config
-        let default_cfg_options = {
-            let mut opts = get_rustc_cfg_options(config.cargo.target.as_ref());
-            opts.insert_atom("test".into());
-            opts.insert_atom("debug_assertion".into());
-            opts
-        };
-
         let proc_macro_client = match &config.proc_macro_srv {
             None => ProcMacroClient::dummy(),
             Some((path, args)) => match ProcMacroClient::extern_process(path.into(), args) {
@@ -168,7 +160,7 @@ impl GlobalState {
         };
         for ws in workspaces.iter() {
             crate_graph.extend(ws.to_crate_graph(
-                &default_cfg_options,
+                config.cargo.target.as_deref(),
                 &extern_source_roots,
                 &proc_macro_client,
                 &mut load,