Rust: reinstate extraction of test code

Users will still be able to opt out:
* for unit tests, by providing the `cargo_cfg_overrides=-test` extractor
  option
* for integration tests, by excluding the test files from the analysis
  using `paths-ignore` in the codescanning configuration file

We may want to revisit whether we want a single option for both. Also
further work will be needed to restrict our security queries to non-test
code on the QL side.

Author: Paolo Tranquilli

rust/autobuild/src/main.rs

@@ -1,13 +1,13 @@
-use std::path::PathBuf;
 use codeql_extractor::autobuilder;
+use std::path::PathBuf;
 
 fn main() -> std::io::Result<()> {
     let database = std::env::var("CODEQL_EXTRACTOR_RUST_WIP_DATABASE")
         .expect("CODEQL_EXTRACTOR_RUST_WIP_DATABASE not set");
 
     autobuilder::Autobuilder::new("rust", PathBuf::from(database))
         .include_extensions(&[".rs"])
-        .exclude_globs(&["**/.git", "**/tests/**"])
+        .exclude_globs(&["**/.git"])
         .size_limit("5m")
         .run()
 }

rust/extractor/src/config.rs

@@ -10,14 +10,14 @@ use figment::{
     Figment,
 };
 use itertools::Itertools;
-use log::warn;
 use num_traits::Zero;
 use ra_ap_cfg::{CfgAtom, CfgDiff};
 use ra_ap_intern::Symbol;
 use ra_ap_paths::Utf8PathBuf;
 use ra_ap_project_model::{CargoConfig, CargoFeatures, CfgOverrides, RustLibSource};
 use rust_extractor_macros::extractor_cli_config;
 use serde::{Deserialize, Serialize};
+use std::collections::HashSet;
 use std::fmt::Debug;
 use std::ops::Not;
 use std::path::PathBuf;
@@ -132,33 +132,26 @@ fn to_cfg_override(spec: &str) -> CfgAtom {
 }
 
 fn to_cfg_overrides(specs: &Vec<String>) -> CfgOverrides {
-    let mut enabled_cfgs = Vec::new();
-    let mut disabled_cfgs = Vec::new();
-    let mut has_test_explicitly_enabled = false;
+    let mut enabled_cfgs = HashSet::new();
+    enabled_cfgs.insert(to_cfg_override("test"));
+    let mut disabled_cfgs = HashSet::new();
     for spec in specs {
         if let Some(spec) = spec.strip_prefix("-") {
-            disabled_cfgs.push(to_cfg_override(spec));
+            let cfg = to_cfg_override(spec);
+            enabled_cfgs.remove(&cfg);
+            disabled_cfgs.insert(cfg);
         } else {
-            enabled_cfgs.push(to_cfg_override(spec));
-            if spec == "test" {
-                has_test_explicitly_enabled = true;
-            }
+            let cfg = to_cfg_override(spec);
+            disabled_cfgs.remove(&cfg);
+            enabled_cfgs.insert(cfg);
         }
     }
-    if !has_test_explicitly_enabled {
-        disabled_cfgs.push(to_cfg_override("test"));
-    }
-    if let Some(global) = CfgDiff::new(enabled_cfgs, disabled_cfgs) {
-        CfgOverrides {
-            global,
-            ..Default::default()
-        }
-    } else {
-        warn!("non-disjoint cfg overrides, ignoring: {}", specs.join(", "));
-        CfgOverrides {
-            global: CfgDiff::new(Vec::new(), vec![to_cfg_override("test")])
-                .expect("disabling one cfg should always succeed"),
-            ..Default::default()
-        }
+    let enabled_cfgs = enabled_cfgs.into_iter().collect();
+    let disabled_cfgs = disabled_cfgs.into_iter().collect();
+    let global = CfgDiff::new(enabled_cfgs, disabled_cfgs)
+        .expect("There should be no duplicate cfgs by construction");
+    CfgOverrides {
+        global,
+        ..Default::default()
     }
 }

rust/ql/integration-tests/file-exclusions/codeql-config.yml

@@ -1,2 +1,3 @@
 paths-ignore:
   - '/src/maybe_ignore'
+  - '**/tests'

rust/ql/integration-tests/file-exclusions/source_archive.default.expected

@@ -1,3 +1,4 @@
 src/lib.rs
 src/maybe_ignore/a_source.rs
 src/maybe_ignore/mod.rs
+tests/integration.rs

rust/ql/integration-tests/options/cfg/functions.expected

@@ -1,3 +1,4 @@
 | src/lib.rs:7:1:8:19 | fn cfg_no_flag |
 | src/lib.rs:10:1:11:18 | fn cfg_no_key |
-| src/lib.rs:16:1:17:24 | fn pointer_width_64 |
+| src/lib.rs:15:5:16:16 | fn test |
+| src/lib.rs:19:1:20:24 | fn pointer_width_64 |

rust/ql/integration-tests/options/cfg/functions.override.expected

@@ -1,4 +1,3 @@
 | src/lib.rs:1:1:2:16 | fn cfg_flag |
 | src/lib.rs:4:1:5:15 | fn cfg_key |
-| src/lib.rs:13:1:14:12 | fn test |
-| src/lib.rs:19:1:20:24 | fn pointer_width_32 |
+| src/lib.rs:22:1:23:24 | fn pointer_width_32 |

rust/ql/integration-tests/options/cfg/src/lib.rs

@@ -11,7 +11,10 @@ fn cfg_no_flag() {}
 fn cfg_no_key() {}
 
 #[cfg(test)]
-fn test() {}
+mod tests {
+    #[test]
+    fn test() {}
+}
 
 #[cfg(target_pointer_width = "64")]
 fn pointer_width_64() {}

rust/ql/integration-tests/options/cfg/test_cfg_overrides_option.py

@@ -14,6 +14,24 @@ def test_cfg_overrides(codeql, rust):
         "cfg_key=value",
         "-target_pointer_width=64",
         "target_pointer_width=32",
+        "-test",
+    ))
+    codeql.database.create(extractor_option=f"cargo_cfg_overrides={overrides}")
+
+@pytest.mark.ql_test(expected=".override.expected")
+def test_ducplicate_cfg_overrides(codeql, rust):
+    overrides = ",".join((
+        "cfg_flag",
+        "cfg_key=value",
+        "-cfg_flag",
+        "target_pointer_width=32",
+        "-target_pointer_width=64",
+        "cfg_flag",
+        "cfg_flag",
+        "target_pointer_width=32",
+        "-test",
+        "-test",
         "test",
+        "-test",
     ))
     codeql.database.create(extractor_option=f"cargo_cfg_overrides={overrides}")