Skip to content

Conversation

@regexident
Copy link
Contributor

This is to allow for efficiently disabling #[cfg(test)] on all crates (by passing unset_test_crates: UnsetTestCrates::All) without having to first load the crate graph, when using rust-analyzer as a library.

(FYI: The change doesn't seem to be covered by any existing tests.)

@matklad
Copy link
Contributor

matklad commented Aug 24, 2021

We actually recently gained some initial testing infra in this area. You can start with the following to test the effect of overrides:

diff --git a/crates/project_model/src/tests.rs b/crates/project_model/src/tests.rs
index f2c998308..c61aec8d3 100644
--- a/crates/project_model/src/tests.rs
+++ b/crates/project_model/src/tests.rs
@@ -14,6 +14,10 @@ use crate::{
 };
 
 fn load_cargo(file: &str) -> CrateGraph {
+    load_cargo_with_overrides(file, CfgOverrides::default())
+}
+
+fn load_cargo_with_overrides(file: &str, cfg_overrides: CfgOverrides) -> CrateGraph {
     let meta = get_test_json_file(file);
     let cargo_workspace = CargoWorkspace::new(meta);
     let project_workspace = ProjectWorkspace::Cargo {
@@ -22,7 +26,7 @@ fn load_cargo(file: &str) -> CrateGraph {
         sysroot: None,
         rustc: None,
         rustc_cfg: Vec::new(),
-        cfg_overrides: CfgOverrides::default(),
+        cfg_overrides,
     };
     to_crate_graph(project_workspace)
 }

@matklad
Copy link
Contributor

matklad commented Aug 24, 2021

The impl looks good to me, but adding a test for some overrides and all overrides would be helpful!

@regexident regexident force-pushed the extend-unset_test_crates branch 2 times, most recently from 4489c15 to eb7b58a Compare August 24, 2021 23:09
@regexident
Copy link
Contributor Author

Hey Aleksey,

I was hoping you [c/]would point towards the appropriate place for adding some tests, so this is perfect!

Running the test I did however realize that by introducing a possibly overlapping overrides (i.e. one could provide a wildcard override, as well as a named one) I was effectively violating the invariant of CfgDiff

pub struct CfgDiff {
    // Invariants: No duplicates, no atom that's both in `enable` and `disable`.
    enable: Vec<CfgAtom>,
    disable: Vec<CfgAtom>,
}

… as now you could effectively have duplicates or even atoms in both, enable and disable. Just spread across two CfgDiff.

So I changed CfgOverrides to the following …

/// A set of cfg-overrides per crate.
///
/// `Wildcard(..)` is useful e.g. disabling `#[cfg(test)]` on all crates,
/// without having to first obtain a list of all crates.
#[derive(Debug, Clone, Eq, PartialEq)]
pub enum CfgOverrides {
    /// A single global set of overrides matching all crates.
    Wildcard(CfgDiff),
    /// A set of overrides matching specific crates.
    Selective(FxHashMap<String, CfgDiff>),
}

… to prevent that issue.

Let me know if you would like me to make any changes. :)

…ling `#[cfg(test)]` on all crates without having to first load the crate graph
@regexident regexident force-pushed the extend-unset_test_crates branch from eb7b58a to 74880a1 Compare August 26, 2021 11:10
@matklad
Copy link
Contributor

matklad commented Aug 30, 2021

lgtm!

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 30, 2021

@bors bors bot merged commit 1636f61 into rust-lang:master Aug 30, 2021
@lnicola lnicola changed the title Extend CargoConfig.unset_test_crates internal: Extend CargoConfig.unset_test_crates Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants