Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/cfg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
14 changes: 11 additions & 3 deletions crates/hir-def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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 = || {
Expand Down
21 changes: 18 additions & 3 deletions crates/hir-expand/src/builtin/attr_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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> {
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion crates/load-cargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/project-model/src/cargo_workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
2 changes: 2 additions & 0 deletions crates/project-model/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down
33 changes: 29 additions & 4 deletions crates/project-model/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
},
}

Expand All @@ -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())
Expand All @@ -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) => {
Expand All @@ -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)
Expand All @@ -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(),
}
}
Expand Down Expand Up @@ -329,6 +336,7 @@ impl ProjectWorkspace {
rustc,
cargo_config_extra_env,
error: error.map(Arc::new),
set_test: config.set_test,
},
sysroot,
rustc_cfg,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -609,6 +618,7 @@ impl ProjectWorkspace {
build_scripts,
cargo_config_extra_env: _,
error: _,
set_test: _,
} => {
cargo
.packages()
Expand Down Expand Up @@ -750,6 +760,7 @@ impl ProjectWorkspace {
build_scripts,
cargo_config_extra_env: _,
error: _,
set_test,
} => (
cargo_to_crate_graph(
load,
Expand All @@ -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),
Expand All @@ -772,6 +784,7 @@ impl ProjectWorkspace {
rustc_cfg.clone(),
cfg_overrides,
build_scripts,
*set_test,
)
} else {
detached_file_to_crate_graph(
Expand All @@ -780,6 +793,7 @@ impl ProjectWorkspace {
file,
sysroot,
cfg_overrides,
*set_test,
)
},
sysroot,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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());
Expand All @@ -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());
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1426,6 +1450,7 @@ fn sysroot_to_crate_graph(
..Default::default()
},
&WorkspaceBuildScripts::default(),
false,
);

let mut pub_deps = vec![];
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/cli/analysis_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ impl flags::AnalysisStats {
false => Some(RustLibSource::Discover),
},
all_targets: true,
set_test: true,
..Default::default()
};
let no_progress = &|_| ();
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/cli/lsif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = &|_| ();
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/cli/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions crates/rust-analyzer/src/cli/rustc_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl Tester {
let cargo_config = CargoConfig {
sysroot: Some(RustLibSource::Discover),
all_targets: true,
set_test: true,
..Default::default()
};

Expand All @@ -85,6 +86,7 @@ impl Tester {
file: ManifestPath::try_from(tmp_file).unwrap(),
cargo: None,
cargo_config_extra_env: Default::default(),
set_test: true,
},
sysroot,
rustc_cfg: vec![],
Expand Down
10 changes: 5 additions & 5 deletions crates/rust-analyzer/src/cli/scip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ impl flags::Scip {
let now = Instant::now();

let no_progress = &|s| (eprintln!("rust-analyzer: Loading {s}"));
let load_cargo_config = LoadCargoConfig {
load_out_dirs_from_check: true,
with_proc_macro_server: ProcMacroServerChoice::Sysroot,
prefill_caches: true,
};
let root =
vfs::AbsPathBuf::assert_utf8(std::env::current_dir()?.join(&self.path)).normalize();

Expand All @@ -51,6 +46,11 @@ impl flags::Scip {
// FIXME @alibektas : What happens to errors without logging?
error!(?error_sink, "Config Error(s)");
}
let load_cargo_config = LoadCargoConfig {
load_out_dirs_from_check: true,
with_proc_macro_server: ProcMacroServerChoice::Sysroot,
prefill_caches: true,
};
let cargo_config = config.cargo(None);
let (db, vfs, _) = load_workspace_at(
root.as_path().as_ref(),
Expand Down
Loading