Auto merge of #18085 - ChayimFriedman2:gate-test, r=Veykril

feat: Provide an config option to not set `cfg(test)`

Fixes #17957.

Author: bors

src/tools/rust-analyzer/crates/cfg/src/lib.rs

@@ -49,6 +49,10 @@ impl CfgOptions {
         cfg.fold(&|atom| self.enabled.contains(atom))
     }
 
+    pub fn check_atom(&self, cfg: &CfgAtom) -> bool {
+        self.enabled.contains(cfg)
+    }
+
     pub fn insert_atom(&mut self, key: Symbol) {
         self.enabled.insert(CfgAtom::Flag(key));
     }

src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs

@@ -6,7 +6,7 @@
 use std::{cmp::Ordering, iter, mem, ops::Not};
 
 use base_db::{CrateId, CrateOrigin, Dependency, LangCrateOrigin};
-use cfg::{CfgExpr, CfgOptions};
+use cfg::{CfgAtom, CfgExpr, CfgOptions};
 use either::Either;
 use hir_expand::{
     attrs::{Attr, AttrId},
@@ -1324,13 +1324,21 @@ impl DefCollector<'_> {
                     };
 
                     // Skip #[test]/#[bench] expansion, which would merely result in more memory usage
-                    // due to duplicating functions into macro expansions
+                    // due to duplicating functions into macro expansions, but only if `cfg(test)` is active,
+                    // otherwise they are expanded to nothing and this can impact e.g. diagnostics (due to things
+                    // being cfg'ed out).
+                    // Ideally we will just expand them to nothing here. But we are only collecting macro calls,
+                    // not expanding them, so we have no way to do that.
                     if matches!(
                         def.kind,
                         MacroDefKind::BuiltInAttr(_, expander)
                         if expander.is_test() || expander.is_bench()
                     ) {
-                        return recollect_without(self);
+                        let test_is_active =
+                            self.cfg_options.check_atom(&CfgAtom::Flag(sym::test.clone()));
+                        if test_is_active {
+                            return recollect_without(self);
+                        }
                     }
 
                     let call_id = || {

src/tools/rust-analyzer/crates/hir-expand/src/builtin/attr_macro.rs

@@ -4,6 +4,8 @@ use span::{MacroCallId, Span};
 
 use crate::{db::ExpandDatabase, name, tt, ExpandResult, MacroCallKind};
 
+use super::quote;
+
 macro_rules! register_builtin {
     ($(($name:ident, $variant:ident) => $expand:ident),* ) => {
         #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -52,15 +54,15 @@ impl BuiltinAttrExpander {
 }
 
 register_builtin! {
-    (bench, Bench) => dummy_attr_expand,
+    (bench, Bench) => dummy_gate_test_expand,
     (cfg_accessible, CfgAccessible) => dummy_attr_expand,
     (cfg_eval, CfgEval) => dummy_attr_expand,
     (derive, Derive) => derive_expand,
     // derive const is equivalent to derive for our proposes.
     (derive_const, DeriveConst) => derive_expand,
     (global_allocator, GlobalAllocator) => dummy_attr_expand,
-    (test, Test) => dummy_attr_expand,
-    (test_case, TestCase) => dummy_attr_expand
+    (test, Test) => dummy_gate_test_expand,
+    (test_case, TestCase) => dummy_gate_test_expand
 }
 
 pub fn find_builtin_attr(ident: &name::Name) -> Option<BuiltinAttrExpander> {
@@ -76,6 +78,19 @@ fn dummy_attr_expand(
     ExpandResult::ok(tt.clone())
 }
 
+fn dummy_gate_test_expand(
+    _db: &dyn ExpandDatabase,
+    _id: MacroCallId,
+    tt: &tt::Subtree,
+    span: Span,
+) -> ExpandResult<tt::Subtree> {
+    let result = quote::quote! { span=>
+        #[cfg(test)]
+        #tt
+    };
+    ExpandResult::ok(result)
+}
+
 /// We generate a very specific expansion here, as we do not actually expand the `#[derive]` attribute
 /// itself in name res, but we do want to expand it to something for the IDE layer, so that the input
 /// derive attributes can be downmapped, and resolved as proper paths.

src/tools/rust-analyzer/crates/load-cargo/src/lib.rs

@@ -526,7 +526,7 @@ mod tests {
     #[test]
     fn test_loading_rust_analyzer() {
         let path = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap().parent().unwrap();
-        let cargo_config = CargoConfig::default();
+        let cargo_config = CargoConfig { set_test: true, ..CargoConfig::default() };
         let load_cargo_config = LoadCargoConfig {
             load_out_dirs_from_check: false,
             with_proc_macro_server: ProcMacroServerChoice::None,

src/tools/rust-analyzer/crates/project-model/src/cargo_workspace.rs

@@ -100,6 +100,7 @@ pub struct CargoConfig {
     pub invocation_strategy: InvocationStrategy,
     /// Optional path to use instead of `target` when building
     pub target_dir: Option<Utf8PathBuf>,
+    pub set_test: bool,
 }
 
 pub type Package = Idx<PackageData>;

src/tools/rust-analyzer/crates/project-model/src/tests.rs

@@ -35,6 +35,7 @@ fn load_cargo_with_overrides(
             rustc: Err(None),
             cargo_config_extra_env: Default::default(),
             error: None,
+            set_test: true,
         },
         cfg_overrides,
         sysroot: Sysroot::empty(),
@@ -242,6 +243,7 @@ fn smoke_test_real_sysroot_cargo() {
             rustc: Err(None),
             cargo_config_extra_env: Default::default(),
             error: None,
+            set_test: true,
         },
         sysroot,
         rustc_cfg: Vec::new(),

src/tools/rust-analyzer/crates/project-model/src/workspace.rs

@@ -78,6 +78,7 @@ pub enum ProjectWorkspaceKind {
         rustc: Result<Box<(CargoWorkspace, WorkspaceBuildScripts)>, Option<String>>,
         /// Environment variables set in the `.cargo/config` file.
         cargo_config_extra_env: FxHashMap<String, String>,
+        set_test: bool,
     },
     /// Project workspace was specified using a `rust-project.json` file.
     Json(ProjectJson),
@@ -98,6 +99,7 @@ pub enum ProjectWorkspaceKind {
         cargo: Option<(CargoWorkspace, WorkspaceBuildScripts, Option<Arc<anyhow::Error>>)>,
         /// Environment variables set in the `.cargo/config` file.
         cargo_config_extra_env: FxHashMap<String, String>,
+        set_test: bool,
     },
 }
 
@@ -112,6 +114,7 @@ impl fmt::Debug for ProjectWorkspace {
                 build_scripts,
                 rustc,
                 cargo_config_extra_env,
+                set_test,
             } => f
                 .debug_struct("Cargo")
                 .field("root", &cargo.workspace_root().file_name())
@@ -126,6 +129,7 @@ impl fmt::Debug for ProjectWorkspace {
                 .field("toolchain", &toolchain)
                 .field("data_layout", &target_layout)
                 .field("cargo_config_extra_env", &cargo_config_extra_env)
+                .field("set_test", set_test)
                 .field("build_scripts", &build_scripts.error().unwrap_or("ok"))
                 .finish(),
             ProjectWorkspaceKind::Json(project) => {
@@ -137,12 +141,14 @@ impl fmt::Debug for ProjectWorkspace {
                     .field("toolchain", &toolchain)
                     .field("data_layout", &target_layout)
                     .field("n_cfg_overrides", &cfg_overrides.len());
+
                 debug_struct.finish()
             }
             ProjectWorkspaceKind::DetachedFile {
                 file,
                 cargo: cargo_script,
                 cargo_config_extra_env,
+                set_test,
             } => f
                 .debug_struct("DetachedFiles")
                 .field("file", &file)
@@ -154,6 +160,7 @@ impl fmt::Debug for ProjectWorkspace {
                 .field("data_layout", &target_layout)
                 .field("n_cfg_overrides", &cfg_overrides.len())
                 .field("cargo_config_extra_env", &cargo_config_extra_env)
+                .field("set_test", set_test)
                 .finish(),
         }
     }
@@ -329,6 +336,7 @@ impl ProjectWorkspace {
                         rustc,
                         cargo_config_extra_env,
                         error: error.map(Arc::new),
+                        set_test: config.set_test,
                     },
                     sysroot,
                     rustc_cfg,
@@ -423,6 +431,7 @@ impl ProjectWorkspace {
                 file: detached_file.to_owned(),
                 cargo: cargo_script,
                 cargo_config_extra_env,
+                set_test: config.set_test,
             },
             sysroot,
             rustc_cfg,
@@ -609,6 +618,7 @@ impl ProjectWorkspace {
                 build_scripts,
                 cargo_config_extra_env: _,
                 error: _,
+                set_test: _,
             } => {
                 cargo
                     .packages()
@@ -750,6 +760,7 @@ impl ProjectWorkspace {
                 build_scripts,
                 cargo_config_extra_env: _,
                 error: _,
+                set_test,
             } => (
                 cargo_to_crate_graph(
                     load,
@@ -759,10 +770,11 @@ impl ProjectWorkspace {
                     rustc_cfg.clone(),
                     cfg_overrides,
                     build_scripts,
+                    *set_test,
                 ),
                 sysroot,
             ),
-            ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, .. } => (
+            ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, set_test, .. } => (
                 if let Some((cargo, build_scripts, _)) = cargo_script {
                     cargo_to_crate_graph(
                         &mut |path| load(path),
@@ -772,6 +784,7 @@ impl ProjectWorkspace {
                         rustc_cfg.clone(),
                         cfg_overrides,
                         build_scripts,
+                        *set_test,
                     )
                 } else {
                     detached_file_to_crate_graph(
@@ -780,6 +793,7 @@ impl ProjectWorkspace {
                         file,
                         sysroot,
                         cfg_overrides,
+                        *set_test,
                     )
                 },
                 sysroot,
@@ -813,13 +827,15 @@ impl ProjectWorkspace {
                     cargo_config_extra_env,
                     build_scripts: _,
                     error: _,
+                    set_test: _,
                 },
                 ProjectWorkspaceKind::Cargo {
                     cargo: o_cargo,
                     rustc: o_rustc,
                     cargo_config_extra_env: o_cargo_config_extra_env,
                     build_scripts: _,
                     error: _,
+                    set_test: _,
                 },
             ) => {
                 cargo == o_cargo
@@ -834,11 +850,13 @@ impl ProjectWorkspace {
                     file,
                     cargo: Some((cargo_script, _, _)),
                     cargo_config_extra_env,
+                    set_test: _,
                 },
                 ProjectWorkspaceKind::DetachedFile {
                     file: o_file,
                     cargo: Some((o_cargo_script, _, _)),
                     cargo_config_extra_env: o_cargo_config_extra_env,
+                    set_test: _,
                 },
             ) => {
                 file == o_file
@@ -987,6 +1005,7 @@ fn cargo_to_crate_graph(
     rustc_cfg: Vec<CfgAtom>,
     override_cfg: &CfgOverrides,
     build_scripts: &WorkspaceBuildScripts,
+    set_test: bool,
 ) -> (CrateGraph, ProcMacroPaths) {
     let _p = tracing::info_span!("cargo_to_crate_graph").entered();
     let mut res = (CrateGraph::default(), ProcMacroPaths::default());
@@ -1011,8 +1030,10 @@ fn cargo_to_crate_graph(
             let mut cfg_options = cfg_options.clone();
 
             if cargo[pkg].is_local {
-                // Add test cfg for local crates
-                cfg_options.insert_atom(sym::test.clone());
+                if set_test {
+                    // Add test cfg for local crates
+                    cfg_options.insert_atom(sym::test.clone());
+                }
                 cfg_options.insert_atom(sym::rust_analyzer.clone());
             }
 
@@ -1173,14 +1194,17 @@ fn detached_file_to_crate_graph(
     detached_file: &ManifestPath,
     sysroot: &Sysroot,
     override_cfg: &CfgOverrides,
+    set_test: bool,
 ) -> (CrateGraph, ProcMacroPaths) {
     let _p = tracing::info_span!("detached_file_to_crate_graph").entered();
     let mut crate_graph = CrateGraph::default();
     let (public_deps, _libproc_macro) =
         sysroot_to_crate_graph(&mut crate_graph, sysroot, rustc_cfg.clone(), load);
 
     let mut cfg_options = CfgOptions::from_iter(rustc_cfg);
-    cfg_options.insert_atom(sym::test.clone());
+    if set_test {
+        cfg_options.insert_atom(sym::test.clone());
+    }
     cfg_options.insert_atom(sym::rust_analyzer.clone());
     override_cfg.apply(&mut cfg_options, "");
     let cfg_options = Arc::new(cfg_options);
@@ -1426,6 +1450,7 @@ fn sysroot_to_crate_graph(
                     ..Default::default()
                 },
                 &WorkspaceBuildScripts::default(),
+                false,
             );
 
             let mut pub_deps = vec![];

src/tools/rust-analyzer/crates/rust-analyzer/src/cli/analysis_stats.rs

@@ -65,6 +65,7 @@ impl flags::AnalysisStats {
                 false => Some(RustLibSource::Discover),
             },
             all_targets: true,
+            set_test: true,
             ..Default::default()
         };
         let no_progress = &|_| ();

src/tools/rust-analyzer/crates/rust-analyzer/src/cli/lsif.rs

@@ -277,6 +277,7 @@ impl flags::Lsif {
         let cargo_config = &CargoConfig {
             sysroot: Some(RustLibSource::Discover),
             all_targets: true,
+            set_test: true,
             ..Default::default()
         };
         let no_progress = &|_| ();

src/tools/rust-analyzer/crates/rust-analyzer/src/cli/run_tests.rs

@@ -16,6 +16,7 @@ impl flags::RunTests {
         let cargo_config = CargoConfig {
             sysroot: Some(RustLibSource::Discover),
             all_targets: true,
+            set_test: true,
             ..Default::default()
         };
         let load_cargo_config = LoadCargoConfig {