Skip to content

Commit 91834ec

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 923cb99 commit 91834ec

File tree

17 files changed

+78
-45
lines changed

17 files changed

+78
-45
lines changed

‎src/tools/rust-analyzer/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
}

‎src/tools/rust-analyzer/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},
@@ -1324,13 +1324,21 @@ impl DefCollector<'_> {
13241324
};
13251325

13261326
// Skip #[test]/#[bench] expansion, which would merely result in more memory usage
1327-
// due to duplicating functions into macro expansions
1327+
// due to duplicating functions into macro expansions, but only if `cfg(test)` is active,
1328+
// otherwise they are expanded to nothing and this can impact e.g. diagnostics (due to things
1329+
// being cfg'ed out).
1330+
// Ideally we will just expand them to nothing here. But we are only collecting macro calls,
1331+
// not expanding them, so we have no way to do that.
13281332
if matches!(
13291333
def.kind,
13301334
MacroDefKind::BuiltInAttr(_, expander)
13311335
if expander.is_test() || expander.is_bench()
13321336
) {
1333-
return recollect_without(self);
1337+
let test_is_active =
1338+
self.cfg_options.check_atom(&CfgAtom::Flag(sym::test.clone()));
1339+
if test_is_active {
1340+
return recollect_without(self);
1341+
}
13341342
}
13351343

13361344
let call_id = || {

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use span::{MacroCallId, Span};
44

55
use crate::{db::ExpandDatabase, name, tt, ExpandResult, MacroCallKind};
66

7+
use super::quote;
8+
79
macro_rules! register_builtin {
810
($(($name:ident, $variant:ident) => $expand:ident),* ) => {
911
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -52,15 +54,15 @@ impl BuiltinAttrExpander {
5254
}
5355

5456
register_builtin! {
55-
(bench, Bench) => dummy_attr_expand,
57+
(bench, Bench) => dummy_gate_test_expand,
5658
(cfg_accessible, CfgAccessible) => dummy_attr_expand,
5759
(cfg_eval, CfgEval) => dummy_attr_expand,
5860
(derive, Derive) => derive_expand,
5961
// derive const is equivalent to derive for our proposes.
6062
(derive_const, DeriveConst) => derive_expand,
6163
(global_allocator, GlobalAllocator) => dummy_attr_expand,
62-
(test, Test) => dummy_attr_expand,
63-
(test_case, TestCase) => dummy_attr_expand
64+
(test, Test) => dummy_gate_test_expand,
65+
(test_case, TestCase) => dummy_gate_test_expand
6466
}
6567

6668
pub fn find_builtin_attr(ident: &name::Name) -> Option<BuiltinAttrExpander> {
@@ -76,6 +78,19 @@ fn dummy_attr_expand(
7678
ExpandResult::ok(tt.clone())
7779
}
7880

81+
fn dummy_gate_test_expand(
82+
_db: &dyn ExpandDatabase,
83+
_id: MacroCallId,
84+
tt: &tt::Subtree,
85+
span: Span,
86+
) -> ExpandResult<tt::Subtree> {
87+
let result = quote::quote! { span=>
88+
#[cfg(test)]
89+
#tt
90+
};
91+
ExpandResult::ok(result)
92+
}
93+
7994
/// We generate a very specific expansion here, as we do not actually expand the `#[derive]` attribute
8095
/// itself in name res, but we do want to expand it to something for the IDE layer, so that the input
8196
/// derive attributes can be downmapped, and resolved as proper paths.

‎src/tools/rust-analyzer/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 {
@@ -528,12 +526,11 @@ mod tests {
528526
#[test]
529527
fn test_loading_rust_analyzer() {
530528
let path = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap().parent().unwrap();
531-
let cargo_config = CargoConfig::default();
529+
let cargo_config = CargoConfig { set_test: true, ..CargoConfig::default() };
532530
let load_cargo_config = LoadCargoConfig {
533531
load_out_dirs_from_check: false,
534532
with_proc_macro_server: ProcMacroServerChoice::None,
535533
prefill_caches: false,
536-
set_test: true,
537534
};
538535
let (db, _vfs, _proc_macro) =
539536
load_workspace_at(path, &cargo_config, &load_cargo_config, &|_| {}).unwrap();

‎src/tools/rust-analyzer/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>;

‎src/tools/rust-analyzer/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
}

‎src/tools/rust-analyzer/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
.field("build_scripts", &build_scripts.error().unwrap_or("ok"))
130134
.finish(),
131135
ProjectWorkspaceKind::Json(project) => {
@@ -137,12 +141,14 @@ impl fmt::Debug for ProjectWorkspace {
137141
.field("toolchain", &toolchain)
138142
.field("data_layout", &target_layout)
139143
.field("n_cfg_overrides", &cfg_overrides.len());
144+
140145
debug_struct.finish()
141146
}
142147
ProjectWorkspaceKind::DetachedFile {
143148
file,
144149
cargo: cargo_script,
145150
cargo_config_extra_env,
151+
set_test,
146152
} => f
147153
.debug_struct("DetachedFiles")
148154
.field("file", &file)
@@ -154,6 +160,7 @@ impl fmt::Debug for ProjectWorkspace {
154160
.field("data_layout", &target_layout)
155161
.field("n_cfg_overrides", &cfg_overrides.len())
156162
.field("cargo_config_extra_env", &cargo_config_extra_env)
163+
.field("set_test", set_test)
157164
.finish(),
158165
}
159166
}
@@ -329,6 +336,7 @@ impl ProjectWorkspace {
329336
rustc,
330337
cargo_config_extra_env,
331338
error: error.map(Arc::new),
339+
set_test: config.set_test,
332340
},
333341
sysroot,
334342
rustc_cfg,
@@ -423,6 +431,7 @@ impl ProjectWorkspace {
423431
file: detached_file.to_owned(),
424432
cargo: cargo_script,
425433
cargo_config_extra_env,
434+
set_test: config.set_test,
426435
},
427436
sysroot,
428437
rustc_cfg,
@@ -609,6 +618,7 @@ impl ProjectWorkspace {
609618
build_scripts,
610619
cargo_config_extra_env: _,
611620
error: _,
621+
set_test: _,
612622
} => {
613623
cargo
614624
.packages()
@@ -728,7 +738,6 @@ impl ProjectWorkspace {
728738
&self,
729739
load: FileLoader<'_>,
730740
extra_env: &FxHashMap<String, String>,
731-
set_test: bool,
732741
) -> (CrateGraph, ProcMacroPaths) {
733742
let _p = tracing::info_span!("ProjectWorkspace::to_crate_graph").entered();
734743

@@ -742,7 +751,6 @@ impl ProjectWorkspace {
742751
sysroot,
743752
extra_env,
744753
cfg_overrides,
745-
set_test,
746754
),
747755
sysroot,
748756
),
@@ -752,6 +760,7 @@ impl ProjectWorkspace {
752760
build_scripts,
753761
cargo_config_extra_env: _,
754762
error: _,
763+
set_test,
755764
} => (
756765
cargo_to_crate_graph(
757766
load,
@@ -761,11 +770,11 @@ impl ProjectWorkspace {
761770
rustc_cfg.clone(),
762771
cfg_overrides,
763772
build_scripts,
764-
set_test,
773+
*set_test,
765774
),
766775
sysroot,
767776
),
768-
ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, .. } => (
777+
ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, set_test, .. } => (
769778
if let Some((cargo, build_scripts, _)) = cargo_script {
770779
cargo_to_crate_graph(
771780
&mut |path| load(path),
@@ -775,7 +784,7 @@ impl ProjectWorkspace {
775784
rustc_cfg.clone(),
776785
cfg_overrides,
777786
build_scripts,
778-
set_test,
787+
*set_test,
779788
)
780789
} else {
781790
detached_file_to_crate_graph(
@@ -784,7 +793,7 @@ impl ProjectWorkspace {
784793
file,
785794
sysroot,
786795
cfg_overrides,
787-
set_test,
796+
*set_test,
788797
)
789798
},
790799
sysroot,
@@ -818,13 +827,15 @@ impl ProjectWorkspace {
818827
cargo_config_extra_env,
819828
build_scripts: _,
820829
error: _,
830+
set_test: _,
821831
},
822832
ProjectWorkspaceKind::Cargo {
823833
cargo: o_cargo,
824834
rustc: o_rustc,
825835
cargo_config_extra_env: o_cargo_config_extra_env,
826836
build_scripts: _,
827837
error: _,
838+
set_test: _,
828839
},
829840
) => {
830841
cargo == o_cargo
@@ -839,11 +850,13 @@ impl ProjectWorkspace {
839850
file,
840851
cargo: Some((cargo_script, _, _)),
841852
cargo_config_extra_env,
853+
set_test: _,
842854
},
843855
ProjectWorkspaceKind::DetachedFile {
844856
file: o_file,
845857
cargo: Some((o_cargo_script, _, _)),
846858
cargo_config_extra_env: o_cargo_config_extra_env,
859+
set_test: _,
847860
},
848861
) => {
849862
file == o_file
@@ -875,12 +888,11 @@ fn project_json_to_crate_graph(
875888
sysroot: &Sysroot,
876889
extra_env: &FxHashMap<String, String>,
877890
override_cfg: &CfgOverrides,
878-
set_test: bool,
879891
) -> (CrateGraph, ProcMacroPaths) {
880892
let mut res = (CrateGraph::default(), ProcMacroPaths::default());
881893
let (crate_graph, proc_macros) = &mut res;
882894
let (public_deps, libproc_macro) =
883-
sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load, set_test);
895+
sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load);
884896

885897
let r_a_cfg_flag = CfgAtom::Flag(sym::rust_analyzer.clone());
886898
let mut cfg_cache: FxHashMap<&str, Vec<CfgAtom>> = FxHashMap::default();
@@ -1000,7 +1012,7 @@ fn cargo_to_crate_graph(
10001012
let crate_graph = &mut res.0;
10011013
let proc_macros = &mut res.1;
10021014
let (public_deps, libproc_macro) =
1003-
sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load, set_test);
1015+
sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load);
10041016

10051017
let cfg_options = CfgOptions::from_iter(rustc_cfg);
10061018

@@ -1187,7 +1199,7 @@ fn detached_file_to_crate_graph(
11871199
let _p = tracing::info_span!("detached_file_to_crate_graph").entered();
11881200
let mut crate_graph = CrateGraph::default();
11891201
let (public_deps, _libproc_macro) =
1190-
sysroot_to_crate_graph(&mut crate_graph, sysroot, rustc_cfg.clone(), load, set_test);
1202+
sysroot_to_crate_graph(&mut crate_graph, sysroot, rustc_cfg.clone(), load);
11911203

11921204
let mut cfg_options = CfgOptions::from_iter(rustc_cfg);
11931205
if set_test {
@@ -1416,7 +1428,6 @@ fn sysroot_to_crate_graph(
14161428
sysroot: &Sysroot,
14171429
rustc_cfg: Vec<CfgAtom>,
14181430
load: FileLoader<'_>,
1419-
set_test: bool,
14201431
) -> (SysrootPublicDeps, Option<CrateId>) {
14211432
let _p = tracing::info_span!("sysroot_to_crate_graph").entered();
14221433
match sysroot.mode() {
@@ -1439,7 +1450,7 @@ fn sysroot_to_crate_graph(
14391450
..Default::default()
14401451
},
14411452
&WorkspaceBuildScripts::default(),
1442-
set_test,
1453+
false,
14431454
);
14441455

14451456
let mut pub_deps = vec![];

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ impl flags::AnalysisStats {
6565
false => Some(RustLibSource::Discover),
6666
},
6767
all_targets: true,
68+
set_test: true,
6869
..Default::default()
6970
};
7071
let no_progress = &|_| ();
@@ -84,7 +85,6 @@ impl flags::AnalysisStats {
8485
ProcMacroServerChoice::Sysroot
8586
},
8687
prefill_caches: false,
87-
set_test: true,
8888
};
8989

9090
let build_scripts_time = if self.disable_build_scripts {

‎src/tools/rust-analyzer/crates/rust-analyzer/src/cli/diagnostics.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ impl flags::Diagnostics {
3939
load_out_dirs_from_check: !self.disable_build_scripts,
4040
with_proc_macro_server,
4141
prefill_caches: false,
42-
// We don't pass `--all-targets` so we also set `cfg(test)` to false.
43-
set_test: false,
4442
};
4543
let (db, _vfs, _proc_macro) =
4644
load_workspace_at(&self.path, &cargo_config, &load_cargo_config, &|_| {})?;

0 commit comments

Comments
 (0)