Rust: address review

2024-10-24 17:54:18 +02:00 · 2024-10-24 17:54:18 +02:00 · 41d0085918
--- 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"
--- 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"
--- 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::<Vec<_>>();
+    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::<Vec<_>>();
+
    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()
--- 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()
--- 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()
-}
--- 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()?;
--- a/rust/ql/test/library-tests/variables/options
+++ b/rust/ql/test/library-tests/variables/options
@ -1 +0,0 @@
-qltest_cargo_check: true
--- 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
+)
--- 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