Merge pull request #18011 from github/revert-17937-redsun82/rust-cargo-options

Revert "Rust: allow to specify more cargo configuration options"

Author: redsun82

Cargo.lock

@@ -35,22 +35,3 @@ options:
       reduce execution time of consecutive extractor runs. By default, a new scratch
       directory is used for each run.
     type: string
-  cargo_target:
-    title: Target architecture
-    description: >
-      Target architecture to use for analysis, analogous to `cargo --target`. By
-      default the host architecture is used.
-    type: string
-  cargo_features:
-    title: Cargo features to turn on
-    description: >
-      Comma-separated list of features to turn on. If any value is `*` all features
-      are turned on. By default only default cargo features are enabled. Can be
-      repeated.
-    type: array
-  cargo_cfg_overrides:
-    title: Cargo cfg overrides
-    description: >
-      Comma-separated list of cfg settings to enable, or disable if prefixed with `-`.
-      Can be repeated.
-    type: array

rust/codeql-extractor.yml

@@ -22,8 +22,6 @@ ra_ap_syntax = "0.0.232"
 ra_ap_vfs = "0.0.232"
 ra_ap_parser = "0.0.232"
 ra_ap_span = "0.0.232"
-ra_ap_cfg = "0.0.232"
-ra_ap_intern = "0.0.232"
 serde = "1.0.209"
 serde_with = "3.9.0"
 stderrlog = "0.6.0"

rust/extractor/Cargo.toml

@@ -1,74 +1,47 @@
 use proc_macro::TokenStream;
 use quote::{format_ident, quote};
-use syn::{Ident, Type};
-
-fn get_type_tip(t: &Type) -> Option<&Ident> {
-    let syn::Type::Path(path) = t else {
-        return None;
-    };
-    let segment = path.path.segments.last()?;
-    Some(&segment.ident)
-}
 
 /// Allow all fields in the extractor config to be also overrideable by extractor CLI flags
 #[proc_macro_attribute]
 pub fn extractor_cli_config(_attr: TokenStream, item: TokenStream) -> TokenStream {
     let ast = syn::parse_macro_input!(item as syn::ItemStruct);
     let name = &ast.ident;
-    let fields = ast
-        .fields
-        .iter()
-        .map(|f| {
-            if f.ident.as_ref().is_some_and(|i| i != "inputs")
-                && get_type_tip(&f.ty).is_some_and(|i| i == "Vec")
-            {
-                quote! {
-                    #[serde(deserialize_with="deserialize_newline_or_comma_separated")]
-                    #f
-                }
-            } else {
-                quote! { #f }
-            }
-        })
-        .collect::<Vec<_>>();
     let cli_name = format_ident!("Cli{}", name);
     let cli_fields = ast
         .fields
         .iter()
         .map(|f| {
             let id = f.ident.as_ref().unwrap();
             let ty = &f.ty;
-            let type_tip = get_type_tip(ty);
-            if type_tip.is_some_and(|i| i == "bool") {
-                quote! {
-                    #[arg(long)]
-                    #[serde(skip_serializing_if="<&bool>::not")]
-                    #id: bool
+            if let syn::Type::Path(p) = ty {
+                if p.path.is_ident(&format_ident!("bool")) {
+                    return quote! {
+                        #[arg(long)]
+                        #[serde(skip_serializing_if="<&bool>::not")]
+                        #id: bool,
+                    };
                 }
-            } else if type_tip.is_some_and(|i| i == "Option") {
-                quote! {
-                    #[arg(long)]
-                    #f
+                if p.path.segments.len() == 1 && p.path.segments[0].ident == "Option" {
+                    return quote! {
+                        #[arg(long)]
+                        #id: #ty,
+                    };
                 }
-            } else if id == &format_ident!("verbose") {
+            }
+            if id == &format_ident!("verbose") {
                 quote! {
                     #[arg(long, short, action=clap::ArgAction::Count)]
                     #[serde(skip_serializing_if="u8::is_zero")]
-                    #id: u8
+                    #id: u8,
                 }
             } else if id == &format_ident!("inputs") {
                 quote! {
-                    #f
-                }
-            } else if type_tip.is_some_and(|i| i == "Vec") {
-                quote! {
-                    #[arg(long)]
-                    #id: Option<String>
+                    #id: #ty,
                 }
             } else {
                 quote! {
                     #[arg(long)]
-                    #id: Option<#ty>
+                    #id: Option<#ty>,
                 }
             }
         })
@@ -93,9 +66,7 @@ pub fn extractor_cli_config(_attr: TokenStream, item: TokenStream) -> TokenStrea
     let gen = quote! {
         #[serde_with::apply(_ => #[serde(default)])]
         #[derive(Deserialize, Default)]
-        pub struct #name {
-            #(#fields),*
-        }
+        #ast
 
         impl Debug for #name {
             fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -109,7 +80,7 @@ pub fn extractor_cli_config(_attr: TokenStream, item: TokenStream) -> TokenStrea
         #[derive(clap::Parser, Serialize)]
         #[command(about, long_about = None)]
         struct #cli_name {
-            #(#cli_fields),*
+            #(#cli_fields)*
         }
     };
     gen.into()

rust/extractor/macros/src/lib.rs

@@ -7,14 +7,9 @@ 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, Deserializer, Serialize};
+use serde::{Deserialize, Serialize};
 use std::fmt::Debug;
 use std::ops::Not;
 use std::path::PathBuf;
@@ -37,23 +32,12 @@ impl From<Compression> for trap::Compression {
     }
 }
 
-// required by the extractor_cli_config macro.
-fn deserialize_newline_or_comma_separated<'a, D: Deserializer<'a>, T: for<'b> From<&'b str>>(
-    deserializer: D,
-) -> Result<Vec<T>, D::Error> {
-    let value = String::deserialize(deserializer)?;
-    Ok(value.split(['\n', ',']).map(T::from).collect())
-}
-
 #[extractor_cli_config]
 pub struct Config {
     pub scratch_dir: PathBuf,
     pub trap_dir: PathBuf,
     pub source_archive_dir: PathBuf,
     pub cargo_target_dir: Option<PathBuf>,
-    pub cargo_target: Option<String>,
-    pub cargo_features: Vec<String>,
-    pub cargo_cfg_overrides: Vec<String>,
     pub verbose: u8,
     pub compression: Compression,
     pub inputs: Vec<PathBuf>,
@@ -68,7 +52,7 @@ impl Config {
             .context("expanding parameter files")?;
         let cli_args = CliConfig::parse_from(args);
         let mut figment = Figment::new()
-            .merge(Env::raw().only(["CODEQL_VERBOSE"].as_slice()))
+            .merge(Env::prefixed("CODEQL_"))
             .merge(Env::prefixed("CODEQL_EXTRACTOR_RUST_"))
             .merge(Env::prefixed("CODEQL_EXTRACTOR_RUST_OPTION_"))
             .merge(Serialized::defaults(cli_args));
@@ -94,81 +78,4 @@ impl Config {
         }
         figment.extract().context("loading configuration")
     }
-
-    pub fn to_cargo_config(&self) -> CargoConfig {
-        let sysroot = Some(RustLibSource::Discover);
-
-        let target_dir = self
-            .cargo_target_dir
-            .clone()
-            .unwrap_or_else(|| self.scratch_dir.join("target"));
-        let target_dir = Utf8PathBuf::from_path_buf(target_dir).ok();
-
-        let features = if self.cargo_features.is_empty() {
-            Default::default()
-        } else if self.cargo_features.contains(&"*".to_string()) {
-            CargoFeatures::All
-        } else {
-            CargoFeatures::Selected {
-                features: self.cargo_features.clone(),
-                no_default_features: false,
-            }
-        };
-
-        let target = self.cargo_target.clone();
-
-        let cfg_overrides = to_cfg_overrides(&self.cargo_cfg_overrides);
-
-        CargoConfig {
-            sysroot,
-            target_dir,
-            features,
-            target,
-            cfg_overrides,
-            ..Default::default()
-        }
-    }
-}
-
-fn to_cfg_override(spec: &str) -> CfgAtom {
-    if let Some((key, value)) = spec.split_once("=") {
-        CfgAtom::KeyValue {
-            key: Symbol::intern(key),
-            value: Symbol::intern(value),
-        }
-    } else {
-        CfgAtom::Flag(Symbol::intern(spec))
-    }
-}
-
-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;
-    for spec in specs {
-        if spec.starts_with("-") {
-            disabled_cfgs.push(to_cfg_override(&spec[1..]));
-        } else {
-            enabled_cfgs.push(to_cfg_override(spec));
-            if spec == "test" {
-                has_test_explicitly_enabled = true;
-            }
-        }
-    }
-    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()
-        }
-    }
 }

rust/extractor/src/config.rs

@@ -130,9 +130,11 @@ fn main() -> anyhow::Result<()> {
         }
         extractor.extract_without_semantics(file, "no manifest found");
     }
-    let cargo_config = cfg.to_cargo_config();
+    let target_dir = &cfg
+        .cargo_target_dir
+        .unwrap_or_else(|| cfg.scratch_dir.join("target"));
     for (manifest, files) in map.values().filter(|(_, files)| !files.is_empty()) {
-        if let Some((ref db, ref vfs)) = RustAnalyzer::load_workspace(manifest, &cargo_config) {
+        if let Some((ref db, ref vfs)) = RustAnalyzer::load_workspace(manifest, target_dir) {
             let semantics = Semantics::new(db);
             for file in files {
                 let Some(id) = path_to_file_id(file, vfs) else {

rust/extractor/src/main.rs

@@ -7,6 +7,7 @@ use ra_ap_load_cargo::{load_workspace_at, LoadCargoConfig, ProcMacroServerChoice
 use ra_ap_paths::Utf8PathBuf;
 use ra_ap_project_model::CargoConfig;
 use ra_ap_project_model::ProjectManifest;
+use ra_ap_project_model::RustLibSource;
 use ra_ap_span::Edition;
 use ra_ap_span::EditionedFileId;
 use ra_ap_span::TextRange;
@@ -19,7 +20,6 @@ use ra_ap_vfs::{AbsPathBuf, FileId};
 use std::borrow::Cow;
 use std::path::{Path, PathBuf};
 use triomphe::Arc;
-
 pub enum RustAnalyzer<'a> {
     WithSemantics {
         vfs: &'a Vfs,
@@ -45,8 +45,13 @@ pub struct ParseResult<'a> {
 impl<'a> RustAnalyzer<'a> {
     pub fn load_workspace(
         project: &ProjectManifest,
-        config: &CargoConfig,
+        target_dir: &Path,
     ) -> Option<(RootDatabase, Vfs)> {
+        let config = CargoConfig {
+            sysroot: Some(RustLibSource::Discover),
+            target_dir: ra_ap_paths::Utf8PathBuf::from_path_buf(target_dir.to_path_buf()).ok(),
+            ..Default::default()
+        };
         let progress = |t| (log::trace!("progress: {}", t));
         let load_config = LoadCargoConfig {
             load_out_dirs_from_check: true,
@@ -55,7 +60,7 @@ impl<'a> RustAnalyzer<'a> {
         };
         let manifest = project.manifest_path();
 
-        match load_workspace_at(manifest.as_ref(), config, &load_config, &progress) {
+        match load_workspace_at(manifest.as_ref(), &config, &load_config, &progress) {
             Ok((db, vfs, _macro_server)) => Some((db, vfs)),
             Err(err) => {
                 log::error!("failed to load workspace for {}: {}", manifest, err);