From 3f8d438851bad7ff8d6c66829bbf2f0af5a2ca0a Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 24 Oct 2024 11:51:13 +0200 Subject: [PATCH 1/3] Rust: move `qltest` to rust code, add `options` with cargo check --- Cargo.lock | 97 +++++++++++++++++++ rust/extractor/Cargo.toml | 4 +- rust/extractor/src/config.rs | 28 +++++- rust/extractor/src/main.rs | 41 ++++++-- rust/extractor/src/qltest.rs | 84 ++++++++++++++++ .../generated/Module/Module.expected | 2 +- .../generated/Module/Module_getName.expected | 2 +- .../generated/Name/Name.expected | 2 +- .../generated/Name/Name_getText.expected | 2 +- .../generated/SourceFile/SourceFile.expected | 2 +- .../SourceFile/SourceFile_getItem.expected | 2 +- .../ql/test/extractor-tests/generated/options | 1 + .../ql/test/extractor-tests/utf8/ast.expected | 6 +- .../ql/test/library-tests/controlflow/options | 1 + rust/ql/test/library-tests/variables/options | 1 + rust/ql/test/options | 1 + rust/ql/test/query-tests/options | 1 + rust/tools/qltest.cmd | 7 ++ rust/tools/qltest.sh | 37 +------ 19 files changed, 262 insertions(+), 59 deletions(-) create mode 100644 rust/extractor/src/qltest.rs create mode 100644 rust/ql/test/extractor-tests/generated/options create mode 100644 rust/ql/test/library-tests/controlflow/options create mode 100644 rust/ql/test/library-tests/variables/options create mode 100644 rust/ql/test/options create mode 100644 rust/ql/test/query-tests/options create mode 100644 rust/tools/qltest.cmd diff --git a/Cargo.lock b/Cargo.lock index 327f62e7935b..bd505902fb61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -384,6 +384,8 @@ dependencies = [ "clap", "codeql-extractor", "figment", + "gag", + "glob", "itertools 0.13.0", "log", "num-traits", @@ -622,6 +624,22 @@ 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" @@ -631,10 +649,22 @@ dependencies = [ "atomic", "pear", "serde", + "serde_yaml", "uncased", "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" @@ -693,6 +723,16 @@ 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" @@ -704,6 +744,12 @@ dependencies = [ "wasi", ] +[[package]] +name = "glob" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" + [[package]] name = "globset" version = "0.4.15" @@ -969,6 +1015,12 @@ 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" @@ -1960,6 +2012,19 @@ 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" @@ -2058,6 +2123,19 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_yaml" +version = "0.9.34+deprecated" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" +dependencies = [ + "indexmap 2.5.0", + "itoa", + "ryu", + "serde", + "unsafe-libyaml", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -2141,6 +2219,19 @@ 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" @@ -2381,6 +2472,12 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "229730647fbc343e3a80e463c1db7f78f3855d3f3739bee0dda773c9a037c90a" +[[package]] +name = "unsafe-libyaml" +version = "0.2.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" + [[package]] name = "utf8parse" version = "0.2.2" diff --git a/rust/extractor/Cargo.toml b/rust/extractor/Cargo.toml index 2a7112912582..ac11c6b67362 100644 --- a/rust/extractor/Cargo.toml +++ b/rust/extractor/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" [dependencies] anyhow = "1.0.86" clap = { version = "4.5.16", features = ["derive"] } -figment = { version = "0.10.19", features = ["env"]} +figment = { version = "0.10.19", features = ["env", "yaml"] } log = "0.4.22" num-traits = "0.2.19" ra_ap_base_db = "0.0.232" @@ -29,3 +29,5 @@ argfile = "0.2.1" 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/src/config.rs b/rust/extractor/src/config.rs index 0625968a4af7..d7871ee84a8e 100644 --- a/rust/extractor/src/config.rs +++ b/rust/extractor/src/config.rs @@ -2,9 +2,11 @@ use anyhow::Context; use clap::Parser; use codeql_extractor::trap; use figment::{ - providers::{Env, Serialized}, + providers::{Env, Format, Serialized, Yaml}, + value::Value, Figment, }; +use itertools::Itertools; use num_traits::Zero; use rust_extractor_macros::extractor_cli_config; use serde::{Deserialize, Serialize}; @@ -34,10 +36,13 @@ 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, pub inputs: Vec, + pub qltest: bool, + pub qltest_cargo_check: bool, } impl Config { @@ -45,12 +50,25 @@ impl Config { let args = argfile::expand_args(argfile::parse_fromfile, argfile::PREFIX) .context("expanding parameter files")?; let cli_args = CliConfig::parse_from(args); - Figment::new() + let mut figment = Figment::new() .merge(Env::prefixed("CODEQL_")) .merge(Env::prefixed("CODEQL_EXTRACTOR_RUST_")) .merge(Env::prefixed("CODEQL_EXTRACTOR_RUST_OPTION_")) - .merge(Serialized::defaults(cli_args)) - .extract() - .context("loading configuration") + .merge(Serialized::defaults(cli_args)); + if matches!(figment.find_value("qltest"), Ok(Value::Bool(_, true))) { + let cwd = std::env::current_dir()?; + let mut option_files = cwd + .ancestors() + // only travel up while we're within the test pack + .take_while_inclusive(|p| !p.join("qlpack.yml").exists()) + .map(|p| p.join("options")) + .filter(|p| p.exists()) + .collect_vec(); + option_files.reverse(); + for path in option_files { + figment = figment.merge(Yaml::file_exact(path)); + } + } + figment.extract().context("loading configuration") } } diff --git a/rust/extractor/src/main.rs b/rust/extractor/src/main.rs index adcb6f681ae1..149c75aa6f92 100644 --- a/rust/extractor/src/main.rs +++ b/rust/extractor/src/main.rs @@ -1,16 +1,19 @@ -use std::{ - collections::HashMap, - path::{Path, PathBuf}, -}; - use anyhow::Context; use archive::Archiver; +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}, +}; mod archive; mod config; pub mod generated; +mod qltest; mod rust_analyzer; mod translate; pub mod trap; @@ -65,16 +68,20 @@ fn extract( ) }); } -fn main() -> anyhow::Result<()> { - let cfg = config::Config::extract().context("failed to load configuration")?; + +fn run_extractor(mut cfg: config::Config) -> anyhow::Result<()> { stderrlog::new() .module(module_path!()) - .verbosity(1 + cfg.verbose as usize) + .verbosity(cfg.verbose as usize) .init()?; + if cfg.qltest { + qltest::prepare(&mut cfg)?; + } + info!("configuration: {cfg:#?}\n"); let traps = trap::TrapFileProvider::new(&cfg).context("failed to set up trap files")?; let archiver = archive::Archiver { - root: cfg.source_archive_dir, + root: cfg.source_archive_dir.clone(), }; let files: Vec = cfg .inputs @@ -118,3 +125,19 @@ fn main() -> 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 new file mode 100644 index 000000000000..035043e831f2 --- /dev/null +++ b/rust/extractor/src/qltest.rs @@ -0,0 +1,84 @@ +use crate::config::Config; +use anyhow::Context; +use glob::glob; +use itertools::Itertools; +use log::info; +use std::fs; +use std::process::Command; + +fn dump_lib() -> anyhow::Result<()> { + let path_iterator = glob("*.rs").context("globbing test sources")?; + let paths = path_iterator + .collect::, _>>() + .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())) + .join("\n"); + fs::write("lib.rs", lib).context("writing lib.rs") +} + +fn dump_cargo_manifest() -> anyhow::Result<()> { + let mut manifest = String::from( + r#"[workspace] +[package] +name = "test" +version="0.0.1" +edition="2021" +[lib] +path="lib.rs" +"#, + ); + if fs::exists("main.rs").context("checking existence of main.rs")? { + manifest.push_str( + r#"[[bin]] +name = "main" +path = "main.rs" +"#, + ); + } + fs::write("Cargo.toml", manifest).context("writing Cargo.toml") +} + +fn set_sources(config: &mut Config) -> anyhow::Result<()> { + let path_iterator = glob("*.rs").context("globbing test sources")?; + config.inputs = path_iterator + .collect::, _>>() + .context("fetching test sources")?; + 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()?; + if config.qltest_cargo_check { + let status = Command::new("cargo") + .env("RUSTFLAGS", "-Awarnings") + .arg("check") + .arg("-q") + .status() + .context("spawning cargo check")?; + if status.success() { + info!("cargo check successful"); + } else { + anyhow::bail!("requested cargo check failed"); + } + } + Ok(()) +} diff --git a/rust/ql/test/extractor-tests/generated/Module/Module.expected b/rust/ql/test/extractor-tests/generated/Module/Module.expected index 2020ab72dc17..60e0500fefdf 100644 --- a/rust/ql/test/extractor-tests/generated/Module/Module.expected +++ b/rust/ql/test/extractor-tests/generated/Module/Module.expected @@ -1,3 +1,3 @@ | gen_module.rs:3:1:4:8 | Module | getNumberOfAttrs: | 0 | hasItemList: | no | hasName: | yes | hasVisibility: | no | | gen_module.rs:5:1:7:1 | Module | getNumberOfAttrs: | 0 | hasItemList: | yes | hasName: | yes | hasVisibility: | no | -| lib.rs:2:1:2:15 | Module | getNumberOfAttrs: | 0 | hasItemList: | no | hasName: | yes | hasVisibility: | no | +| lib.rs:1:1:1:15 | Module | getNumberOfAttrs: | 0 | hasItemList: | no | hasName: | yes | hasVisibility: | no | diff --git a/rust/ql/test/extractor-tests/generated/Module/Module_getName.expected b/rust/ql/test/extractor-tests/generated/Module/Module_getName.expected index 3a8692beec65..918f151e292c 100644 --- a/rust/ql/test/extractor-tests/generated/Module/Module_getName.expected +++ b/rust/ql/test/extractor-tests/generated/Module/Module_getName.expected @@ -1,3 +1,3 @@ | gen_module.rs:3:1:4:8 | Module | gen_module.rs:4:5:4:7 | Name | | gen_module.rs:5:1:7:1 | Module | gen_module.rs:5:5:5:7 | Name | -| lib.rs:2:1:2:15 | Module | lib.rs:2:5:2:14 | Name | +| lib.rs:1:1:1:15 | Module | lib.rs:1:5:1:14 | Name | diff --git a/rust/ql/test/extractor-tests/generated/Name/Name.expected b/rust/ql/test/extractor-tests/generated/Name/Name.expected index 556c1dff2dde..38072342a84d 100644 --- a/rust/ql/test/extractor-tests/generated/Name/Name.expected +++ b/rust/ql/test/extractor-tests/generated/Name/Name.expected @@ -1,2 +1,2 @@ | gen_name.rs:3:4:3:12 | Name | hasText: | yes | -| lib.rs:2:5:2:12 | Name | hasText: | yes | +| lib.rs:1:5:1:12 | Name | hasText: | yes | diff --git a/rust/ql/test/extractor-tests/generated/Name/Name_getText.expected b/rust/ql/test/extractor-tests/generated/Name/Name_getText.expected index 89623f2108e4..edc4ae3244d0 100644 --- a/rust/ql/test/extractor-tests/generated/Name/Name_getText.expected +++ b/rust/ql/test/extractor-tests/generated/Name/Name_getText.expected @@ -1,2 +1,2 @@ | gen_name.rs:3:4:3:12 | Name | test_name | -| lib.rs:2:5:2:12 | Name | gen_name | +| lib.rs:1:5:1:12 | Name | gen_name | diff --git a/rust/ql/test/extractor-tests/generated/SourceFile/SourceFile.expected b/rust/ql/test/extractor-tests/generated/SourceFile/SourceFile.expected index 5c1ad52bfc39..9aac17e8e840 100644 --- a/rust/ql/test/extractor-tests/generated/SourceFile/SourceFile.expected +++ b/rust/ql/test/extractor-tests/generated/SourceFile/SourceFile.expected @@ -1,2 +1,2 @@ | gen_source_file.rs:1:1:6:2 | SourceFile | getNumberOfAttrs: | 0 | getNumberOfItems: | 1 | -| lib.rs:1:1:2:21 | SourceFile | getNumberOfAttrs: | 0 | getNumberOfItems: | 1 | +| lib.rs:1:1:1:20 | SourceFile | getNumberOfAttrs: | 0 | getNumberOfItems: | 1 | diff --git a/rust/ql/test/extractor-tests/generated/SourceFile/SourceFile_getItem.expected b/rust/ql/test/extractor-tests/generated/SourceFile/SourceFile_getItem.expected index aba10e75baa9..a256c2b84b20 100644 --- a/rust/ql/test/extractor-tests/generated/SourceFile/SourceFile_getItem.expected +++ b/rust/ql/test/extractor-tests/generated/SourceFile/SourceFile_getItem.expected @@ -1,2 +1,2 @@ | gen_source_file.rs:1:1:6:2 | SourceFile | 0 | gen_source_file.rs:3:1:6:1 | test_source_file | -| lib.rs:1:1:2:21 | SourceFile | 0 | lib.rs:2:1:2:20 | Module | +| lib.rs:1:1:1:20 | SourceFile | 0 | lib.rs:1:1:1:20 | Module | diff --git a/rust/ql/test/extractor-tests/generated/options b/rust/ql/test/extractor-tests/generated/options new file mode 100644 index 000000000000..cf148dd35f86 --- /dev/null +++ b/rust/ql/test/extractor-tests/generated/options @@ -0,0 +1 @@ +qltest_cargo_check: false diff --git a/rust/ql/test/extractor-tests/utf8/ast.expected b/rust/ql/test/extractor-tests/utf8/ast.expected index b0a4dfe00912..5586f36b0ac0 100644 --- a/rust/ql/test/extractor-tests/utf8/ast.expected +++ b/rust/ql/test/extractor-tests/utf8/ast.expected @@ -1,6 +1,6 @@ -| lib.rs:1:1:2:22 | SourceFile | -| lib.rs:2:1:2:21 | Module | -| lib.rs:2:5:2:20 | Name | +| lib.rs:1:1:1:21 | Module | +| lib.rs:1:1:1:21 | SourceFile | +| lib.rs:1:5:1:20 | Name | | utf8_identifiers.rs:1:1:4:6 | foo | | utf8_identifiers.rs:1:1:12:2 | SourceFile | | utf8_identifiers.rs:1:4:1:6 | Name | diff --git a/rust/ql/test/library-tests/controlflow/options b/rust/ql/test/library-tests/controlflow/options new file mode 100644 index 000000000000..cf148dd35f86 --- /dev/null +++ b/rust/ql/test/library-tests/controlflow/options @@ -0,0 +1 @@ +qltest_cargo_check: false diff --git a/rust/ql/test/library-tests/variables/options b/rust/ql/test/library-tests/variables/options new file mode 100644 index 000000000000..c7a0beabb538 --- /dev/null +++ b/rust/ql/test/library-tests/variables/options @@ -0,0 +1 @@ +qltest_cargo_check: true diff --git a/rust/ql/test/options b/rust/ql/test/options new file mode 100644 index 000000000000..c7a0beabb538 --- /dev/null +++ b/rust/ql/test/options @@ -0,0 +1 @@ +qltest_cargo_check: true diff --git a/rust/ql/test/query-tests/options b/rust/ql/test/query-tests/options new file mode 100644 index 000000000000..cf148dd35f86 --- /dev/null +++ b/rust/ql/test/query-tests/options @@ -0,0 +1 @@ +qltest_cargo_check: false diff --git a/rust/tools/qltest.cmd b/rust/tools/qltest.cmd new file mode 100644 index 000000000000..6de274b99015 --- /dev/null +++ b/rust/tools/qltest.cmd @@ -0,0 +1,7 @@ +@echo off + +set "RUST_BACKTRACE=full" + +type NUL && "%CODEQL_EXTRACTOR_RUST_ROOT%/tools/%CODEQL_PLATFORM%/extractor" --qltest + +exit /b %ERRORLEVEL% diff --git a/rust/tools/qltest.sh b/rust/tools/qltest.sh index 58c08a41e01e..bfeb885b8d7f 100755 --- a/rust/tools/qltest.sh +++ b/rust/tools/qltest.sh @@ -1,40 +1,7 @@ #!/bin/bash -mkdir -p "$CODEQL_EXTRACTOR_RUST_TRAP_DIR" +set -eu export RUST_BACKTRACE=full -QLTEST_LOG="$CODEQL_EXTRACTOR_RUST_LOG_DIR"/qltest.log - -EXTRACTOR="$CODEQL_EXTRACTOR_RUST_ROOT/tools/$CODEQL_PLATFORM/extractor" -echo > lib.rs -for src in *.rs; do - if [[ "$src" == "lib.rs" ]]; then - continue - elif [[ "$src" == "main.rs" ]]; then - continue - else - echo "mod ${src%.rs};" >> lib.rs - fi -done -cat > Cargo.toml << EOF -[workspace] -[package] -name = "test" -version="0.0.1" -edition="2021" -[lib] -path="lib.rs" -EOF -if [[ -f "main.rs" ]]; then -cat >> Cargo.toml << EOF -[[bin]] -name = "main" -path = "main.rs" -EOF -fi -"$EXTRACTOR" *.rs >> "$QLTEST_LOG" -if [[ "$?" != 0 ]]; then - cat "$QLTEST_LOG" # Show compiler errors on extraction failure - exit 1 -fi +"$CODEQL_EXTRACTOR_RUST_ROOT/tools/$CODEQL_PLATFORM/extractor" --qltest From c79f8180f3c97cd6d13481ff679539091ed2b8c3 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 24 Oct 2024 17:14:48 +0200 Subject: [PATCH 2/3] Rust: move down `options` in `query-tests` --- rust/ql/test/query-tests/{ => diagnostics}/options | 0 rust/ql/test/query-tests/unusedentities/options | 1 + 2 files changed, 1 insertion(+) rename rust/ql/test/query-tests/{ => diagnostics}/options (100%) create mode 100644 rust/ql/test/query-tests/unusedentities/options diff --git a/rust/ql/test/query-tests/options b/rust/ql/test/query-tests/diagnostics/options similarity index 100% rename from rust/ql/test/query-tests/options rename to rust/ql/test/query-tests/diagnostics/options diff --git a/rust/ql/test/query-tests/unusedentities/options b/rust/ql/test/query-tests/unusedentities/options new file mode 100644 index 000000000000..cf148dd35f86 --- /dev/null +++ b/rust/ql/test/query-tests/unusedentities/options @@ -0,0 +1 @@ +qltest_cargo_check: false From 41d00859180429ebffb21e20940b732a6a6665d3 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 24 Oct 2024 17:54:18 +0200 Subject: [PATCH 3/3] 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 bd505902fb61..9304acd6d2e2 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 ac11c6b67362..2cb4e70fc7dd 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 781d53bd8513..37ca22cda444 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 d7871ee84a8e..a9c27199e37c 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 149c75aa6f92..f9a0f5efd138 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 035043e831f2..169e2c433c71 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 c7a0beabb538..000000000000 --- 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 6de274b99015..533f200edd3a 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 bfeb885b8d7f..df7e7f235638 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