From be45e3deeded77e645c872323ba8083d769e9048 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Fri, 8 Nov 2024 12:21:25 +0100 Subject: [PATCH] Rust: allow to specify more cargo configuration options This allows to tweak via extractor options some aspects of the cargo configuration: * the target architecture * features (including `*` for all, which we must understand whether to set by default) * cfg overrides Integration tests will be added in a follow-up commit. --- Cargo.lock | 2 + rust/codeql-extractor.yml | 19 +++++++ rust/extractor/Cargo.toml | 2 + rust/extractor/macros/src/lib.rs | 62 ++++++++++++++------- rust/extractor/src/config.rs | 86 ++++++++++++++++++++++++++++- rust/extractor/src/main.rs | 6 +- rust/extractor/src/rust_analyzer.rs | 11 +--- 7 files changed, 155 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9304acd6d2e..97bc7d1c761 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -389,10 +389,12 @@ dependencies = [ "log", "num-traits", "ra_ap_base_db", + "ra_ap_cfg", "ra_ap_hir", "ra_ap_hir_def", "ra_ap_hir_expand", "ra_ap_ide_db", + "ra_ap_intern", "ra_ap_load-cargo", "ra_ap_parser", "ra_ap_paths", diff --git a/rust/codeql-extractor.yml b/rust/codeql-extractor.yml index c7558c17efe..2f726a10ada 100644 --- a/rust/codeql-extractor.yml +++ b/rust/codeql-extractor.yml @@ -35,3 +35,22 @@ 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 diff --git a/rust/extractor/Cargo.toml b/rust/extractor/Cargo.toml index 2cb4e70fc7d..5b7e09e6f3f 100644 --- a/rust/extractor/Cargo.toml +++ b/rust/extractor/Cargo.toml @@ -21,6 +21,8 @@ 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" diff --git a/rust/extractor/macros/src/lib.rs b/rust/extractor/macros/src/lib.rs index c9771a181de..c10747bef01 100644 --- a/rust/extractor/macros/src/lib.rs +++ b/rust/extractor/macros/src/lib.rs @@ -1,11 +1,34 @@ 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 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::>(); let cli_name = format_ident!("Cli{}", name); let cli_fields = ast .fields @@ -13,35 +36,34 @@ pub fn extractor_cli_config(_attr: TokenStream, item: TokenStream) -> TokenStrea .map(|f| { let id = f.ident.as_ref().unwrap(); let ty = &f.ty; - 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, - }; - } - if p.path.segments.len() == 1 && p.path.segments[0].ident == "Option" { - return quote! { - #[arg(long)] - #id: #ty, - }; - } + let type_tip = get_type_tip(ty); + if type_tip.is_some_and(|i| i == "bool") { + return quote! { + #[arg(long)] + #[serde(skip_serializing_if="<&bool>::not")] + #id: bool + }; + } + if type_tip.is_some_and(|i| i == "Option") { + return quote! { + #[arg(long)] + #f + }; } 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! { - #id: #ty, + #f } } else { quote! { #[arg(long)] - #id: Option<#ty>, + #id: Option<#ty> } } }) @@ -66,7 +88,9 @@ pub fn extractor_cli_config(_attr: TokenStream, item: TokenStream) -> TokenStrea let gen = quote! { #[serde_with::apply(_ => #[serde(default)])] #[derive(Deserialize, Default)] - #ast + pub struct #name { + #(#fields),* + } impl Debug for #name { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -80,7 +104,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() diff --git a/rust/extractor/src/config.rs b/rust/extractor/src/config.rs index f477663f607..5e17a458db7 100644 --- a/rust/extractor/src/config.rs +++ b/rust/extractor/src/config.rs @@ -7,9 +7,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 serde::{Deserialize, Deserializer, Serialize}; use std::fmt::Debug; use std::ops::Not; use std::path::PathBuf; @@ -32,12 +37,23 @@ impl From 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, 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, + pub cargo_target: Option, + pub cargo_features: Vec, + pub cargo_cfg_overrides: Vec, pub verbose: u8, pub compression: Compression, pub inputs: Vec, @@ -51,7 +67,7 @@ impl Config { .context("expanding parameter files")?; let cli_args = CliConfig::parse_from(args); let mut figment = Figment::new() - .merge(Env::prefixed("CODEQL_")) + .merge(Env::raw().only(["CODEQL_VERBOSE"].as_slice())) .merge(Env::prefixed("CODEQL_EXTRACTOR_RUST_")) .merge(Env::prefixed("CODEQL_EXTRACTOR_RUST_OPTION_")) .merge(Serialized::defaults(cli_args)); @@ -71,4 +87,70 @@ 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) -> CfgOverrides { + let mut enabled_cfgs = Vec::new(); + let mut disabled_cfgs = Vec::new(); + 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 let Some(global) = CfgDiff::new(enabled_cfgs, disabled_cfgs) { + CfgOverrides { + global, + ..Default::default() + } + } else { + warn!("non-disjoint cfg overrides, ignoring: {}", specs.join(", ")); + CfgOverrides::default() + } } diff --git a/rust/extractor/src/main.rs b/rust/extractor/src/main.rs index 1f963a0990e..ecbdc965f8e 100644 --- a/rust/extractor/src/main.rs +++ b/rust/extractor/src/main.rs @@ -130,11 +130,9 @@ fn main() -> anyhow::Result<()> { } extractor.extract_without_semantics(file, "no manifest found"); } - let target_dir = &cfg - .cargo_target_dir - .unwrap_or_else(|| cfg.scratch_dir.join("target")); + let cargo_config = cfg.to_cargo_config(); for (manifest, files) in map.values().filter(|(_, files)| !files.is_empty()) { - if let Some((ref db, ref vfs)) = RustAnalyzer::load_workspace(manifest, target_dir) { + if let Some((ref db, ref vfs)) = RustAnalyzer::load_workspace(manifest, &cargo_config) { let semantics = Semantics::new(db); for file in files { let Some(id) = path_to_file_id(file, vfs) else { diff --git a/rust/extractor/src/rust_analyzer.rs b/rust/extractor/src/rust_analyzer.rs index ddbc16ab6a3..735bacb27c1 100644 --- a/rust/extractor/src/rust_analyzer.rs +++ b/rust/extractor/src/rust_analyzer.rs @@ -7,7 +7,6 @@ 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; @@ -20,6 +19,7 @@ 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,13 +45,8 @@ pub struct ParseResult<'a> { impl<'a> RustAnalyzer<'a> { pub fn load_workspace( project: &ProjectManifest, - target_dir: &Path, + config: &CargoConfig, ) -> 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, @@ -60,7 +55,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);