Skip to content

Commit d5c631b

Browse files
ChayimFriedman2ChayimFriedman2
committed
Gate #[test] expansion under cfg(test).
This will mean users opting to not activate `cfg(test)` will lose IDE experience on them, which is quite unfortunate, but this is unavoidable if we want to avoid false positives on e.g. diagnostics. The real fix is to provide IDE experience even for cfg'ed out code, but this is out of scope for this PR.
1 parent d9550ef commit d5c631b

File tree

18 files changed

+90
-51
lines changed

18 files changed

+90
-51
lines changed

‎crates/cfg/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ impl CfgOptions {
4949
cfg.fold(&|atom| self.enabled.contains(atom))
5050
}
5151

52+
pub fn check_atom(&self, cfg: &CfgAtom) -> bool {
53+
self.enabled.contains(cfg)
54+
}
55+
5256
pub fn insert_atom(&mut self, key: Symbol) {
5357
self.enabled.insert(CfgAtom::Flag(key));
5458
}

‎crates/hir-def/src/nameres/collector.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use std::{cmp::Ordering, iter, mem, ops::Not};
77

88
use base_db::{CrateId, CrateOrigin, Dependency, LangCrateOrigin};
9-
use cfg::{CfgExpr, CfgOptions};
9+
use cfg::{CfgAtom, CfgExpr, CfgOptions};
1010
use either::Either;
1111
use hir_expand::{
1212
attrs::{Attr, AttrId},
@@ -1307,13 +1307,21 @@ impl DefCollector<'_> {
13071307
};
13081308

13091309
// Skip #[test]/#[bench] expansion, which would merely result in more memory usage
1310-
// due to duplicating functions into macro expansions
1310+
// due to duplicating functions into macro expansions, but only if `cfg(test)` is active,
1311+
// otherwise they are expanded to nothing and this can impact e.g. diagnostics (due to things
1312+
// being cfg'ed out).
1313+
// Ideally we will just expand them to nothing here. But we are only collecting macro calls,
1314+
// not expanding them, so we have no way to do that.
13111315
if matches!(
13121316
def.kind,
13131317
MacroDefKind::BuiltInAttr(_, expander)
13141318
if expander.is_test() || expander.is_bench()
13151319
) {
1316-
return recollect_without(self);
1320+
let test_is_active =
1321+
self.cfg_options.check_atom(&CfgAtom::Flag(sym::test.clone()));
1322+
if test_is_active {
1323+
return recollect_without(self);
1324+
}
13171325
}
13181326

13191327
let call_id = || {

‎crates/hir-expand/src/builtin/attr_macro.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
//! Builtin attributes.
2+
use cfg::CfgAtom;
23
use intern::sym;
34
use span::{MacroCallId, Span};
45

56
use crate::{db::ExpandDatabase, name, tt, ExpandResult, MacroCallKind};
67

8+
use super::quote;
9+
710
macro_rules! register_builtin {
811
($(($name:ident, $variant:ident) => $expand:ident),* ) => {
912
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -52,15 +55,15 @@ impl BuiltinAttrExpander {
5255
}
5356

5457
register_builtin! {
55-
(bench, Bench) => dummy_attr_expand,
58+
(bench, Bench) => dummy_gate_test_expand,
5659
(cfg_accessible, CfgAccessible) => dummy_attr_expand,
5760
(cfg_eval, CfgEval) => dummy_attr_expand,
5861
(derive, Derive) => derive_expand,
5962
// derive const is equivalent to derive for our proposes.
6063
(derive_const, DeriveConst) => derive_expand,
6164
(global_allocator, GlobalAllocator) => dummy_attr_expand,
62-
(test, Test) => dummy_attr_expand,
63-
(test_case, TestCase) => dummy_attr_expand
65+
(test, Test) => dummy_gate_test_expand,
66+
(test_case, TestCase) => dummy_gate_test_expand
6467
}
6568

6669
pub fn find_builtin_attr(ident: &name::Name) -> Option<BuiltinAttrExpander> {
@@ -76,6 +79,27 @@ fn dummy_attr_expand(
7679
ExpandResult::ok(tt.clone())
7780
}
7881

82+
fn dummy_gate_test_expand(
83+
db: &dyn ExpandDatabase,
84+
id: MacroCallId,
85+
tt: &tt::Subtree,
86+
span: Span,
87+
) -> ExpandResult<tt::Subtree> {
88+
let loc = db.lookup_intern_macro_call(id);
89+
let crate_graph = db.crate_graph();
90+
let test_is_active =
91+
crate_graph[loc.krate].cfg_options.check_atom(&CfgAtom::Flag(sym::test.clone()));
92+
if test_is_active {
93+
ExpandResult::ok(tt.clone())
94+
} else {
95+
let result = quote::quote! { span=>
96+
#[cfg(test)]
97+
#tt
98+
};
99+
ExpandResult::ok(result)
100+
}
101+
}
102+
79103
/// We generate a very specific expansion here, as we do not actually expand the `#[derive]` attribute
80104
/// itself in name res, but we do want to expand it to something for the IDE layer, so that the input
81105
/// derive attributes can be downmapped, and resolved as proper paths.

‎crates/hir-expand/src/builtin/quote.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ macro_rules! quote_impl__ {
118118
($span:ident < ) => {$crate::builtin::quote::__quote!(@PUNCT($span) '<')};
119119
($span:ident > ) => {$crate::builtin::quote::__quote!(@PUNCT($span) '>')};
120120
($span:ident ! ) => {$crate::builtin::quote::__quote!(@PUNCT($span) '!')};
121+
($span:ident # ) => {$crate::builtin::quote::__quote!(@PUNCT($span) '#')};
121122

122123
($span:ident $first:tt $($tail:tt)+ ) => {
123124
{

‎crates/load-cargo/src/lib.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ pub struct LoadCargoConfig {
3030
pub load_out_dirs_from_check: bool,
3131
pub with_proc_macro_server: ProcMacroServerChoice,
3232
pub prefill_caches: bool,
33-
pub set_test: bool,
3433
}
3534

3635
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -100,7 +99,6 @@ pub fn load_workspace(
10099
vfs.file_id(&path)
101100
},
102101
extra_env,
103-
load_config.set_test,
104102
);
105103
let proc_macros = {
106104
let proc_macro_server = match &proc_macro_server {
@@ -523,12 +521,11 @@ mod tests {
523521
#[test]
524522
fn test_loading_rust_analyzer() {
525523
let path = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap().parent().unwrap();
526-
let cargo_config = CargoConfig::default();
524+
let cargo_config = CargoConfig { set_test: true, ..CargoConfig::default() };
527525
let load_cargo_config = LoadCargoConfig {
528526
load_out_dirs_from_check: false,
529527
with_proc_macro_server: ProcMacroServerChoice::None,
530528
prefill_caches: false,
531-
set_test: true,
532529
};
533530
let (db, _vfs, _proc_macro) =
534531
load_workspace_at(path, &cargo_config, &load_cargo_config, &|_| {}).unwrap();

‎crates/project-model/src/cargo_workspace.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ pub struct CargoConfig {
100100
pub invocation_strategy: InvocationStrategy,
101101
/// Optional path to use instead of `target` when building
102102
pub target_dir: Option<Utf8PathBuf>,
103+
pub set_test: bool,
103104
}
104105

105106
pub type Package = Idx<PackageData>;

‎crates/project-model/src/tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ fn load_cargo_with_overrides(
3535
rustc: Err(None),
3636
cargo_config_extra_env: Default::default(),
3737
error: None,
38+
set_test: true,
3839
},
3940
cfg_overrides,
4041
sysroot: Sysroot::empty(),
@@ -136,7 +137,6 @@ fn to_crate_graph(project_workspace: ProjectWorkspace) -> (CrateGraph, ProcMacro
136137
}
137138
},
138139
&Default::default(),
139-
true,
140140
)
141141
}
142142

@@ -243,6 +243,7 @@ fn smoke_test_real_sysroot_cargo() {
243243
rustc: Err(None),
244244
cargo_config_extra_env: Default::default(),
245245
error: None,
246+
set_test: true,
246247
},
247248
sysroot,
248249
rustc_cfg: Vec::new(),
@@ -258,6 +259,5 @@ fn smoke_test_real_sysroot_cargo() {
258259
}
259260
},
260261
&Default::default(),
261-
true,
262262
);
263263
}

‎crates/project-model/src/workspace.rs

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ pub enum ProjectWorkspaceKind {
7878
rustc: Result<Box<(CargoWorkspace, WorkspaceBuildScripts)>, Option<String>>,
7979
/// Environment variables set in the `.cargo/config` file.
8080
cargo_config_extra_env: FxHashMap<String, String>,
81+
set_test: bool,
8182
},
8283
/// Project workspace was specified using a `rust-project.json` file.
8384
Json(ProjectJson),
@@ -98,6 +99,7 @@ pub enum ProjectWorkspaceKind {
9899
cargo: Option<(CargoWorkspace, WorkspaceBuildScripts, Option<Arc<anyhow::Error>>)>,
99100
/// Environment variables set in the `.cargo/config` file.
100101
cargo_config_extra_env: FxHashMap<String, String>,
102+
set_test: bool,
101103
},
102104
}
103105

@@ -112,6 +114,7 @@ impl fmt::Debug for ProjectWorkspace {
112114
build_scripts: _,
113115
rustc,
114116
cargo_config_extra_env,
117+
set_test,
115118
} => f
116119
.debug_struct("Cargo")
117120
.field("root", &cargo.workspace_root().file_name())
@@ -126,6 +129,7 @@ impl fmt::Debug for ProjectWorkspace {
126129
.field("toolchain", &toolchain)
127130
.field("data_layout", &target_layout)
128131
.field("cargo_config_extra_env", &cargo_config_extra_env)
132+
.field("set_test", set_test)
129133
.finish(),
130134
ProjectWorkspaceKind::Json(project) => {
131135
let mut debug_struct = f.debug_struct("Json");
@@ -136,12 +140,14 @@ impl fmt::Debug for ProjectWorkspace {
136140
.field("toolchain", &toolchain)
137141
.field("data_layout", &target_layout)
138142
.field("n_cfg_overrides", &cfg_overrides.len());
143+
139144
debug_struct.finish()
140145
}
141146
ProjectWorkspaceKind::DetachedFile {
142147
file,
143148
cargo: cargo_script,
144149
cargo_config_extra_env,
150+
set_test,
145151
} => f
146152
.debug_struct("DetachedFiles")
147153
.field("file", &file)
@@ -153,6 +159,7 @@ impl fmt::Debug for ProjectWorkspace {
153159
.field("data_layout", &target_layout)
154160
.field("n_cfg_overrides", &cfg_overrides.len())
155161
.field("cargo_config_extra_env", &cargo_config_extra_env)
162+
.field("set_test", set_test)
156163
.finish(),
157164
}
158165
}
@@ -328,6 +335,7 @@ impl ProjectWorkspace {
328335
rustc,
329336
cargo_config_extra_env,
330337
error: error.map(Arc::new),
338+
set_test: config.set_test,
331339
},
332340
sysroot,
333341
rustc_cfg,
@@ -422,6 +430,7 @@ impl ProjectWorkspace {
422430
file: detached_file.to_owned(),
423431
cargo: cargo_script,
424432
cargo_config_extra_env,
433+
set_test: config.set_test,
425434
},
426435
sysroot,
427436
rustc_cfg,
@@ -597,6 +606,7 @@ impl ProjectWorkspace {
597606
build_scripts,
598607
cargo_config_extra_env: _,
599608
error: _,
609+
set_test: _,
600610
} => {
601611
cargo
602612
.packages()
@@ -716,7 +726,6 @@ impl ProjectWorkspace {
716726
&self,
717727
load: FileLoader<'_>,
718728
extra_env: &FxHashMap<String, String>,
719-
set_test: bool,
720729
) -> (CrateGraph, ProcMacroPaths) {
721730
let _p = tracing::info_span!("ProjectWorkspace::to_crate_graph").entered();
722731

@@ -730,7 +739,6 @@ impl ProjectWorkspace {
730739
sysroot,
731740
extra_env,
732741
cfg_overrides,
733-
set_test,
734742
),
735743
sysroot,
736744
),
@@ -740,6 +748,7 @@ impl ProjectWorkspace {
740748
build_scripts,
741749
cargo_config_extra_env: _,
742750
error: _,
751+
set_test,
743752
} => (
744753
cargo_to_crate_graph(
745754
load,
@@ -749,11 +758,11 @@ impl ProjectWorkspace {
749758
rustc_cfg.clone(),
750759
cfg_overrides,
751760
build_scripts,
752-
set_test,
761+
*set_test,
753762
),
754763
sysroot,
755764
),
756-
ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, .. } => (
765+
ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, set_test, .. } => (
757766
if let Some((cargo, build_scripts, _)) = cargo_script {
758767
cargo_to_crate_graph(
759768
&mut |path| load(path),
@@ -763,7 +772,7 @@ impl ProjectWorkspace {
763772
rustc_cfg.clone(),
764773
cfg_overrides,
765774
build_scripts,
766-
set_test,
775+
*set_test,
767776
)
768777
} else {
769778
detached_file_to_crate_graph(
@@ -772,7 +781,7 @@ impl ProjectWorkspace {
772781
file,
773782
sysroot,
774783
cfg_overrides,
775-
set_test,
784+
*set_test,
776785
)
777786
},
778787
sysroot,
@@ -806,13 +815,15 @@ impl ProjectWorkspace {
806815
cargo_config_extra_env,
807816
build_scripts: _,
808817
error: _,
818+
set_test: _,
809819
},
810820
ProjectWorkspaceKind::Cargo {
811821
cargo: o_cargo,
812822
rustc: o_rustc,
813823
cargo_config_extra_env: o_cargo_config_extra_env,
814824
build_scripts: _,
815825
error: _,
826+
set_test: _,
816827
},
817828
) => {
818829
cargo == o_cargo
@@ -827,11 +838,13 @@ impl ProjectWorkspace {
827838
file,
828839
cargo: Some((cargo_script, _, _)),
829840
cargo_config_extra_env,
841+
set_test: _,
830842
},
831843
ProjectWorkspaceKind::DetachedFile {
832844
file: o_file,
833845
cargo: Some((o_cargo_script, _, _)),
834846
cargo_config_extra_env: o_cargo_config_extra_env,
847+
set_test: _,
835848
},
836849
) => {
837850
file == o_file
@@ -863,12 +876,11 @@ fn project_json_to_crate_graph(
863876
sysroot: &Sysroot,
864877
extra_env: &FxHashMap<String, String>,
865878
override_cfg: &CfgOverrides,
866-
set_test: bool,
867879
) -> (CrateGraph, ProcMacroPaths) {
868880
let mut res = (CrateGraph::default(), ProcMacroPaths::default());
869881
let (crate_graph, proc_macros) = &mut res;
870882
let (public_deps, libproc_macro) =
871-
sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load, set_test);
883+
sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load);
872884

873885
let r_a_cfg_flag = CfgAtom::Flag(sym::rust_analyzer.clone());
874886
let mut cfg_cache: FxHashMap<&str, Vec<CfgAtom>> = FxHashMap::default();
@@ -988,7 +1000,7 @@ fn cargo_to_crate_graph(
9881000
let crate_graph = &mut res.0;
9891001
let proc_macros = &mut res.1;
9901002
let (public_deps, libproc_macro) =
991-
sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load, set_test);
1003+
sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load);
9921004

9931005
let cfg_options = CfgOptions::from_iter(rustc_cfg);
9941006

@@ -1175,7 +1187,7 @@ fn detached_file_to_crate_graph(
11751187
let _p = tracing::info_span!("detached_file_to_crate_graph").entered();
11761188
let mut crate_graph = CrateGraph::default();
11771189
let (public_deps, _libproc_macro) =
1178-
sysroot_to_crate_graph(&mut crate_graph, sysroot, rustc_cfg.clone(), load, set_test);
1190+
sysroot_to_crate_graph(&mut crate_graph, sysroot, rustc_cfg.clone(), load);
11791191

11801192
let mut cfg_options = CfgOptions::from_iter(rustc_cfg);
11811193
if set_test {
@@ -1404,7 +1416,6 @@ fn sysroot_to_crate_graph(
14041416
sysroot: &Sysroot,
14051417
rustc_cfg: Vec<CfgAtom>,
14061418
load: FileLoader<'_>,
1407-
set_test: bool,
14081419
) -> (SysrootPublicDeps, Option<CrateId>) {
14091420
let _p = tracing::info_span!("sysroot_to_crate_graph").entered();
14101421
match sysroot.mode() {
@@ -1427,7 +1438,7 @@ fn sysroot_to_crate_graph(
14271438
..Default::default()
14281439
},
14291440
&WorkspaceBuildScripts::default(),
1430-
set_test,
1441+
false,
14311442
);
14321443

14331444
let mut pub_deps = vec![];

0 commit comments

Comments
 (0)