From 41d00859180429ebffb21e20940b732a6a6665d3 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 24 Oct 2024 17:54:18 +0200 Subject: [PATCH] Rust: address review --- Cargo.lock | 70 -------------------- rust/extractor/Cargo.toml | 1 - rust/extractor/macros/src/lib.rs | 37 +++++++++-- rust/extractor/src/config.rs | 4 +- rust/extractor/src/main.rs | 25 ++----- rust/extractor/src/qltest.rs | 20 ++---- rust/ql/test/library-tests/variables/options | 1 - rust/tools/qltest.cmd | 8 ++- rust/tools/qltest.sh | 7 +- 9 files changed, 52 insertions(+), 121 deletions(-) delete mode 100644 rust/ql/test/library-tests/variables/options diff --git a/Cargo.lock b/Cargo.lock index bd505902fb6..9304acd6d2e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -384,7 +384,6 @@ dependencies = [ "clap", "codeql-extractor", "figment", - "gag", "glob", "itertools 0.13.0", "log", @@ -624,22 +623,6 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" -[[package]] -name = "errno" -version = "0.3.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" -dependencies = [ - "libc", - "windows-sys 0.52.0", -] - -[[package]] -name = "fastrand" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e8c02a5121d4ea3eb16a80748c74f5549a5665e4c21333c6098f283870fbdea6" - [[package]] name = "figment" version = "0.10.19" @@ -654,17 +637,6 @@ dependencies = [ "version_check", ] -[[package]] -name = "filedescriptor" -version = "0.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7199d965852c3bac31f779ef99cbb4537f80e952e2d6aa0ffeb30cce00f4f46e" -dependencies = [ - "libc", - "thiserror", - "winapi", -] - [[package]] name = "filetime" version = "0.2.25" @@ -723,16 +695,6 @@ version = "0.4.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ab85b9b05e3978cc9a9cf8fea7f01b494e1a09ed3037e16ba39edc7a29eb61a" -[[package]] -name = "gag" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a713bee13966e9fbffdf7193af71d54a6b35a0bb34997cd6c9519ebeb5005972" -dependencies = [ - "filedescriptor", - "tempfile", -] - [[package]] name = "getrandom" version = "0.2.15" @@ -1015,12 +977,6 @@ dependencies = [ "text-size", ] -[[package]] -name = "linux-raw-sys" -version = "0.4.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" - [[package]] name = "lock_api" version = "0.4.12" @@ -2012,19 +1968,6 @@ dependencies = [ "smallvec", ] -[[package]] -name = "rustix" -version = "0.38.37" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8acb788b847c24f28525660c4d7758620a7210875711f79e7f663cc152726811" -dependencies = [ - "bitflags 2.6.0", - "errno", - "libc", - "linux-raw-sys", - "windows-sys 0.52.0", -] - [[package]] name = "ryu" version = "1.0.18" @@ -2219,19 +2162,6 @@ dependencies = [ "syn", ] -[[package]] -name = "tempfile" -version = "3.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0f2c9fc62d0beef6951ccffd757e241266a2c833136efbe35af6cd2567dca5b" -dependencies = [ - "cfg-if", - "fastrand", - "once_cell", - "rustix", - "windows-sys 0.59.0", -] - [[package]] name = "termcolor" version = "1.1.3" diff --git a/rust/extractor/Cargo.toml b/rust/extractor/Cargo.toml index ac11c6b6736..2cb4e70fc7d 100644 --- a/rust/extractor/Cargo.toml +++ b/rust/extractor/Cargo.toml @@ -30,4 +30,3 @@ codeql-extractor = { path = "../../shared/tree-sitter-extractor" } rust-extractor-macros = { path = "macros" } itertools = "0.13.0" glob = "0.3.1" -gag = "1.0.0" diff --git a/rust/extractor/macros/src/lib.rs b/rust/extractor/macros/src/lib.rs index 781d53bd851..37ca22cda44 100644 --- a/rust/extractor/macros/src/lib.rs +++ b/rust/extractor/macros/src/lib.rs @@ -6,8 +6,8 @@ use quote::{format_ident, quote}; 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 new_name = format_ident!("Cli{}", name); - let fields: Vec<_> = ast + let cli_name = format_ident!("Cli{}", name); + let cli_fields = ast .fields .iter() .map(|f| { @@ -39,17 +39,42 @@ pub fn extractor_cli_config(_attr: TokenStream, item: TokenStream) -> TokenStrea } } }) - .collect(); + .collect::>(); + let debug_fields = ast + .fields + .iter() + .map(|f| { + let id = f.ident.as_ref().unwrap(); + if id == &format_ident!("inputs") { + quote! { + .field("number of inputs", &self.#id.len()) + } + } else { + quote! { + .field(stringify!(#id), &self.#id) + } + } + }) + .collect::>(); + let gen = quote! { #[serde_with::apply(_ => #[serde(default)])] - #[derive(Debug, Deserialize, Default)] + #[derive(Deserialize, Default)] #ast + impl Debug for #name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("configuration:") + #(#debug_fields)* + .finish() + } + } + #[serde_with::skip_serializing_none] #[derive(clap::Parser, Serialize)] #[command(about, long_about = None)] - struct #new_name { - #(#fields)* + struct #cli_name { + #(#cli_fields)* } }; gen.into() diff --git a/rust/extractor/src/config.rs b/rust/extractor/src/config.rs index d7871ee84a8..a9c27199e37 100644 --- a/rust/extractor/src/config.rs +++ b/rust/extractor/src/config.rs @@ -10,6 +10,7 @@ use itertools::Itertools; use num_traits::Zero; use rust_extractor_macros::extractor_cli_config; use serde::{Deserialize, Serialize}; +use std::fmt::Debug; use std::ops::Not; use std::path::PathBuf; @@ -36,7 +37,6 @@ pub struct Config { pub scratch_dir: PathBuf, pub trap_dir: PathBuf, pub source_archive_dir: PathBuf, - pub log_dir: PathBuf, pub extract_dependencies: bool, pub verbose: u8, pub compression: Compression, @@ -55,7 +55,7 @@ impl Config { .merge(Env::prefixed("CODEQL_EXTRACTOR_RUST_")) .merge(Env::prefixed("CODEQL_EXTRACTOR_RUST_OPTION_")) .merge(Serialized::defaults(cli_args)); - if matches!(figment.find_value("qltest"), Ok(Value::Bool(_, true))) { + if let Ok(Value::Bool(_, true)) = figment.find_value("qltest") { let cwd = std::env::current_dir()?; let mut option_files = cwd .ancestors() diff --git a/rust/extractor/src/main.rs b/rust/extractor/src/main.rs index 149c75aa6f9..f9a0f5efd13 100644 --- a/rust/extractor/src/main.rs +++ b/rust/extractor/src/main.rs @@ -4,8 +4,6 @@ use log::info; use ra_ap_ide_db::line_index::{LineCol, LineIndex}; use ra_ap_project_model::ProjectManifest; use rust_analyzer::{ParseResult, RustAnalyzer}; -use std::fs::File; -use std::io::{BufRead, BufReader}; use std::{ collections::HashMap, path::{Path, PathBuf}, @@ -69,15 +67,16 @@ fn extract( }); } -fn run_extractor(mut cfg: config::Config) -> anyhow::Result<()> { +fn main() -> anyhow::Result<()> { + let mut cfg = config::Config::extract().context("failed to load configuration")?; stderrlog::new() .module(module_path!()) - .verbosity(cfg.verbose as usize) + .verbosity(2 + cfg.verbose as usize) .init()?; if cfg.qltest { qltest::prepare(&mut cfg)?; } - info!("configuration: {cfg:#?}\n"); + info!("{cfg:#?}\n"); let traps = trap::TrapFileProvider::new(&cfg).context("failed to set up trap files")?; let archiver = archive::Archiver { @@ -125,19 +124,3 @@ fn run_extractor(mut cfg: config::Config) -> anyhow::Result<()> { Ok(()) } - -fn main() -> anyhow::Result<()> { - let cfg = config::Config::extract().context("failed to load configuration")?; - let qltest = cfg.qltest; - let qltest_log = cfg.log_dir.join("qltest.log"); - let result = std::panic::catch_unwind(|| run_extractor(cfg)); - if qltest && matches!(result, Err(_) | Ok(Err(_))) { - // in case of failure, print out the full log - let log = File::open(qltest_log).context("opening qltest.log")?; - let reader = BufReader::new(log); - for line in reader.lines() { - println!("{}", line.context("reading qltest.log")?); - } - } - result.unwrap() -} diff --git a/rust/extractor/src/qltest.rs b/rust/extractor/src/qltest.rs index 035043e831f..169e2c433c7 100644 --- a/rust/extractor/src/qltest.rs +++ b/rust/extractor/src/qltest.rs @@ -3,6 +3,7 @@ use anyhow::Context; use glob::glob; use itertools::Itertools; use log::info; +use std::ffi::OsStr; use std::fs; use std::process::Command; @@ -13,8 +14,9 @@ fn dump_lib() -> anyhow::Result<()> { .context("fetching test sources")?; let lib = paths .iter() - .filter(|p| !["lib.rs", "main.rs"].contains(&p.file_name().unwrap().to_str().unwrap())) - .map(|p| format!("mod {};", p.file_stem().unwrap().to_str().unwrap())) + .map(|p| p.file_stem().expect("results of glob must have a name")) + .filter(|&p| !["main", "lib"].map(OsStr::new).contains(&p)) + .map(|p| format!("mod {};", p.to_string_lossy())) .join("\n"); fs::write("lib.rs", lib).context("writing lib.rs") } @@ -49,21 +51,7 @@ fn set_sources(config: &mut Config) -> anyhow::Result<()> { Ok(()) } -fn redirect_output(config: &Config) -> anyhow::Result<()> { - let log_path = config.log_dir.join("qltest.log"); - let log = fs::OpenOptions::new() - .append(true) - .create(true) - .open(&log_path) - .context("opening qltest.log")?; - Box::leak(Box::new( - gag::Redirect::stderr(log).context("redirecting stderr")?, - )); - Ok(()) -} - pub(crate) fn prepare(config: &mut Config) -> anyhow::Result<()> { - redirect_output(config)?; dump_lib()?; set_sources(config)?; dump_cargo_manifest()?; diff --git a/rust/ql/test/library-tests/variables/options b/rust/ql/test/library-tests/variables/options deleted file mode 100644 index c7a0beabb53..00000000000 --- a/rust/ql/test/library-tests/variables/options +++ /dev/null @@ -1 +0,0 @@ -qltest_cargo_check: true diff --git a/rust/tools/qltest.cmd b/rust/tools/qltest.cmd index 6de274b9901..533f200edd3 100644 --- a/rust/tools/qltest.cmd +++ b/rust/tools/qltest.cmd @@ -1,7 +1,11 @@ @echo off set "RUST_BACKTRACE=full" +set "QLTEST_LOG=%CODEQL_EXTRACTOR_RUST_LOG_DIR%/qltest.log" -type NUL && "%CODEQL_EXTRACTOR_RUST_ROOT%/tools/%CODEQL_PLATFORM%/extractor" --qltest +type NUL && "%CODEQL_EXTRACTOR_RUST_ROOT%/tools/%CODEQL_PLATFORM%/extractor" --qltest >"%QLTEST_LOG%" -exit /b %ERRORLEVEL% +if %ERRORLEVEL% neq 0 ( + type "%QLTEST_LOG%" + exit /b 1 +) diff --git a/rust/tools/qltest.sh b/rust/tools/qltest.sh index bfeb885b8d7..df7e7f23563 100755 --- a/rust/tools/qltest.sh +++ b/rust/tools/qltest.sh @@ -3,5 +3,8 @@ set -eu export RUST_BACKTRACE=full - -"$CODEQL_EXTRACTOR_RUST_ROOT/tools/$CODEQL_PLATFORM/extractor" --qltest +QLTEST_LOG="$CODEQL_EXTRACTOR_RUST_LOG_DIR"/qltest.log +if ! "$CODEQL_EXTRACTOR_RUST_ROOT/tools/$CODEQL_PLATFORM/extractor" --qltest &>> "$QLTEST_LOG"; then + cat "$QLTEST_LOG" + exit 1 +fi