From e578ac595b0a905aab91e38a48f3bc299157e783 Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 19 Jun 2024 15:17:59 -0400 Subject: [PATCH 1/5] chore: add flamegraph file to gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 9a6cc54c..535ff82a 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,6 @@ # We generate a file with this name when prepping each release, and don't # want it to be checked in. CHANGELOG-NEXT.md + +# File generated by cargo-flamegraph +flamegraph.svg From 1edba489f96b2ebae9a3c54bc646fe5b4cfb75da Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 19 Jun 2024 15:18:57 -0400 Subject: [PATCH 2/5] chore(deps): Add `rayon` and `tempdir` dependencies. --- Cargo.lock | 176 +++++++++++++++++++++++++++++++++++--------- hipcheck/Cargo.toml | 11 ++- 2 files changed, 147 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aac24679..9516fd50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -58,9 +58,9 @@ dependencies = [ [[package]] name = "anstyle-query" -version = "1.0.3" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a64c907d4e79225ac72e2a354c9ce84d50ebb4586dee56c82b3ee73004f537f5" +checksum = "ad186efb764318d35165f1758e7dcef3b10628e26d41a44bc5550652e6804391" dependencies = [ "windows-sys 0.52.0", ] @@ -125,9 +125,9 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "cc" -version = "1.0.98" +version = "1.0.99" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41c270e7540d725e65ac7f1b212ac8ce349719624d7bcff99f8e2e488e8cf03f" +checksum = "96c51067fd44124faa7f870b4b1c969379ad32b2ba805aa959430ceaa384f695" [[package]] name = "cfg-if" @@ -152,9 +152,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.4" +version = "4.5.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "90bc066a67923782aa8515dbaea16946c5bcc5addbd668bb80af688e53e548a0" +checksum = "5db83dced34638ad474f39f250d7fea9598bdd239eaced1bdf45d597da0f433f" dependencies = [ "clap_builder", "clap_derive", @@ -172,9 +172,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.2" +version = "4.5.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae129e2e766ae0ec03484e609954119f123cc1fe650337e155d03b022f24f7b4" +checksum = "f7e204572485eb3fbf28f871612191521df159bc3e15a9f5064c66dba3a8c05f" dependencies = [ "anstream", "anstyle", @@ -184,9 +184,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.5.4" +version = "4.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "528131438037fd55894f62d6e9f068b8f45ac57ffa77517819645d10aed04f64" +checksum = "c780290ccf4fb26629baa7a1081e68ced113f1d3ec302fa5948f1c381ebf06c6" dependencies = [ "heck 0.5.0", "proc-macro2", @@ -196,9 +196,9 @@ dependencies = [ [[package]] name = "clap_lex" -version = "0.7.0" +version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "98cc8fbded0c607b7ba9dd60cd98df59af97e84d24e49c8557331cfc26d301ce" +checksum = "4b82cf0babdbd58558212896d1a4272303a57bdb245c2bf1147185fb45640e70" [[package]] name = "colorchoice" @@ -253,6 +253,25 @@ version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06ea2b9bc92be3c2baa9334a323ebca2d6f074ff852cd1d7b11064035cd3868f" +[[package]] +name = "crossbeam-deque" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "613f8cc01fe9cf1a3eb3d7f488fd2fa8388403e97039e2f73692932e291a770d" +dependencies = [ + "crossbeam-epoch", + "crossbeam-utils", +] + +[[package]] +name = "crossbeam-epoch" +version = "0.9.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "crossbeam-utils" version = "0.8.20" @@ -370,6 +389,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fuchsia-cprng" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba" + [[package]] name = "getrandom" version = "0.2.15" @@ -492,6 +517,7 @@ dependencies = [ "env_logger", "graphql_client", "hipcheck-macros", + "indexmap 2.2.6", "lazy_static", "libc", "log", @@ -502,6 +528,7 @@ dependencies = [ "paste", "pathbuf", "petgraph", + "rayon", "regex", "rustls", "rustls-native-certs", @@ -513,6 +540,7 @@ dependencies = [ "serde_json", "smart-default", "spdx-rs", + "tempdir", "tempfile", "termcolor", "toml", @@ -686,9 +714,9 @@ checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d" [[package]] name = "memchr" -version = "2.7.2" +version = "2.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c8640c5d730cb13ebd907d8d04b52f55ac9a2eec55b440c8892f40d56c76c1d" +checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "minimal-lexical" @@ -746,15 +774,15 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a76df7075c7d4d01fdcb46c912dd17fba5b60c78ea480b475f2b6ab6f666584e" dependencies = [ "num-traits", - "rand", + "rand 0.8.5", "serde", ] [[package]] name = "os_pipe" -version = "1.1.5" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57119c3b893986491ec9aa85056780d3a0f3cf4da7cc09dd3650dbd6c6738fb9" +checksum = "29d73ba8daf8fac13b0501d1abeddcfe21ba7401ada61a819144b6c2a4f32209" dependencies = [ "libc", "windows-sys 0.52.0", @@ -839,16 +867,44 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "552840b97013b1a26992c11eac34bdd778e464601a4c2054b5f0bff7c6761293" +dependencies = [ + "fuchsia-cprng", + "libc", + "rand_core 0.3.1", + "rdrand", + "winapi", +] + [[package]] name = "rand" version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" dependencies = [ - "rand_core", + "rand_core 0.6.4", "serde", ] +[[package]] +name = "rand_core" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a6fdeb83b075e8266dcc8762c22776f6877a63111121f5f8c7411e5be7eed4b" +dependencies = [ + "rand_core 0.4.2", +] + +[[package]] +name = "rand_core" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c33a3c44ca05fa6f1807d8e6743f3824e8509beca625669633be0acbdf509dc" + [[package]] name = "rand_core" version = "0.6.4" @@ -858,6 +914,35 @@ dependencies = [ "serde", ] +[[package]] +name = "rayon" +version = "1.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b418a60154510ca1a002a752ca9714984e21e4241e804d32555251faf8b78ffa" +dependencies = [ + "either", + "rayon-core", +] + +[[package]] +name = "rayon-core" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1465873a3dfdaa8ae7cb14b4383657caab0b3e8a0aa9ae8e04b044854c8dfce2" +dependencies = [ + "crossbeam-deque", + "crossbeam-utils", +] + +[[package]] +name = "rdrand" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "678054eb77286b51581ba43620cc911abf02758c91f93f479767aed0f90458b2" +dependencies = [ + "rand_core 0.3.1", +] + [[package]] name = "redox_syscall" version = "0.2.16" @@ -880,9 +965,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.10.4" +version = "1.10.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c117dbdfde9c8308975b6a18d71f3f385c89461f7b3fb054288ecf2a2058ba4c" +checksum = "b91213439dad192326a0d7c6ee3955910425f441d7038e0d6933b0aec5c4517f" dependencies = [ "aho-corasick", "memchr", @@ -892,9 +977,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.6" +version = "0.4.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "86b83b8b9847f9bf95ef68afb0b8e6cdb80f498442f5179a29fad448fcc1eaea" +checksum = "38caf58cc5ef2fed281f89292ef23f6365465ed9a41b7a7754eb4e26496c92df" dependencies = [ "aho-corasick", "memchr", @@ -903,9 +988,18 @@ dependencies = [ [[package]] name = "regex-syntax" -version = "0.8.3" +version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "adad44e29e4c806119491a7f06f03de4d1af22c3a680dd47f1e6e179439d1f56" +checksum = "7a66a03ae7c801facd77a29370b4faec201768915ac14a721ba36f20bc9c209b" + +[[package]] +name = "remove_dir_all" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3acd125665422973a33ac9d3dd2df85edad0f4ae9b00dafb1a05e43a9f5ef8e7" +dependencies = [ + "winapi", +] [[package]] name = "ring" @@ -1281,6 +1375,16 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempdir" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15f2b5fb00ccdf689e0149d1b1b3c03fead81c2b37735d812fa8bddbbf41b6d8" +dependencies = [ + "rand 0.4.6", + "remove_dir_all", +] + [[package]] name = "tempfile" version = "3.10.1" @@ -1339,9 +1443,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "toml" -version = "0.8.13" +version = "0.8.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4e43f8cc456c9704c851ae29c67e17ef65d2c30017c17a9765b89c382dc8bba" +checksum = "6f49eb2ab21d2f26bd6db7bf383edc527a7ebaee412d17af4d40fdccd442f335" dependencies = [ "serde", "serde_spanned", @@ -1360,9 +1464,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.22.13" +version = "0.22.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c127785850e8c20836d49732ae6abfa47616e60bf9d9f57c43c250361a9db96c" +checksum = "f21c7aaf97f1bd9ca9d4f9e73b0a6c74bd5afef56f2bc931943a6e1c37e04e38" dependencies = [ "indexmap 2.2.6", "serde", @@ -1464,9 +1568,9 @@ dependencies = [ [[package]] name = "url" -version = "2.5.0" +version = "2.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "31e6302e3bb753d46e83516cae55ae196fc0c309407cf11ab35cc51a4c2a4633" +checksum = "22784dbdf76fdde8af1aeda5622b546b422b6fc585325248a2bf9f5e41e94d6c" dependencies = [ "form_urlencoded", "idna", @@ -1475,9 +1579,9 @@ dependencies = [ [[package]] name = "utf8parse" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" +checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "uuid" @@ -1566,9 +1670,9 @@ checksum = "af190c94f2773fdb3729c55b007a722abb5384da03bc0986df4c289bf5567e96" [[package]] name = "webpki-roots" -version = "0.26.1" +version = "0.26.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b3de34ae270483955a94f4b21bdaaeb83d508bb84a01435f393818edb0012009" +checksum = "bd7c23921eeb1713a4e851530e9b9756e4fb0e89978582942612524cf09f01cd" dependencies = [ "rustls-pki-types", ] @@ -1766,9 +1870,9 @@ checksum = "bec47e5bfd1bff0eeaf6d8b485cc1074891a197ab4225d504cb7a1ab88b02bf0" [[package]] name = "winnow" -version = "0.6.8" +version = "0.6.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3c52e9c97a68071b23e836c9380edae937f17b9c4667bd021973efc689f618d" +checksum = "59b5e5f6c299a3c7890b876a2a587f3115162487e704907d9b6cd29473052ba1" dependencies = [ "memchr", ] diff --git a/hipcheck/Cargo.toml b/hipcheck/Cargo.toml index b15de902..9cd654bd 100644 --- a/hipcheck/Cargo.toml +++ b/hipcheck/Cargo.toml @@ -19,7 +19,7 @@ path = "src/main.rs" content_inspector = "0.2.4" dotenv = "0.15.0" chrono = { version = "0.4.19", features = ["serde"] } -clap = { version = "4.5.4", features = ["derive"] } +clap = { version = "4.5.7", features = ["derive"] } dirs = "5.0.1" duct = "0.13.5" env_logger = { version = "0.11.3" } @@ -35,7 +35,7 @@ ordered-float = { version = "4.2.0", features = ["serde"] } paste = "1.0.7" pathbuf = "1.0.0" petgraph = { version = "0.6.0", features = ["serde-1"] } -regex = "1.5.5" +regex = "1.10.5" # Exactly matching the version of rustls used by ureq rustls = "0.22.4" rustls-native-certs = "0.7.0" @@ -52,17 +52,20 @@ serde_json = "1.0.80" smart-default = "0.7.1" spdx-rs = "0.5.0" termcolor = "1.1.3" -toml = "0.8.13" +toml = "0.8.14" unicode-normalization = "0.1.19" unicode-segmentation = "1.9.0" ureq = { version = "2.9.7", default-features = false, features = [ "json", "tls", ] } -url = "2.2.2" +url = "2.5.1" walkdir = "2.5.0" which = { version = "6.0.1", default-features = false } xml-rs = "0.8.20" +tempdir = "0.3.7" +rayon = "1.10.0" +indexmap = "2.2.6" # Windows-specific dependencies [target.'cfg(windows)'.dependencies] From 8a4e678f5b18b1de022446780e3b33f2013605cd Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 19 Jun 2024 15:20:08 -0400 Subject: [PATCH 3/5] feat(unstable): Add unstable command --- hipcheck/src/cli.rs | 157 ++++++++++++++++++++++++++++++++++++++- hipcheck/src/main.rs | 19 ++++- hipcheck/src/unstable.rs | 64 ++++++++++++++++ xtask/Cargo.toml | 4 +- 4 files changed, 236 insertions(+), 8 deletions(-) create mode 100644 hipcheck/src/unstable.rs diff --git a/hipcheck/src/cli.rs b/hipcheck/src/cli.rs index f77be963..9fbd938f 100644 --- a/hipcheck/src/cli.rs +++ b/hipcheck/src/cli.rs @@ -2,9 +2,12 @@ //! Data structures for Hipcheck's main CLI. +use crate::error::Error; +use crate::hc_error; use crate::report::Format; use crate::session::session::Check; use crate::shell::{ColorChoice, Verbosity}; +use crate::target::TargetType; use crate::CheckKind; use clap::{Parser as _, ValueEnum}; use hipcheck_macros as hc; @@ -361,6 +364,7 @@ fn hc_env_var_value_enum(name: &'static str) -> Option { pub enum FullCommands { Check(CheckArgs), Schema(SchemaArgs), + Unstable(UnstableArgs), Ready, PrintConfig, PrintData, @@ -373,6 +377,7 @@ impl From<&Commands> for FullCommands { Commands::Check(args) => FullCommands::Check(args.clone()), Commands::Schema(args) => FullCommands::Schema(args.clone()), Commands::Ready => FullCommands::Ready, + Commands::Unstable(args) => FullCommands::Unstable(args.clone()), } } } @@ -385,29 +390,100 @@ pub enum Commands { Schema(SchemaArgs), /// Check if Hipcheck is ready to run. Ready, + + /// Unstable command to aid in benchmarking + #[command(hide = true)] + Unstable(UnstableArgs), +} + +#[derive(Debug, Clone, clap::Args)] +pub struct UnstableArgs { + #[clap(subcommand)] + pub benchmark: CheckCommand, } +// If no subcommand matched, default to use of '-t , + + #[arg(short = 't', long = "target")] + pub target_type: Option, + #[arg( + required = true, + help = "The target package, URL, commit, etc. for Hipcheck to analyze. If ambiguous, the -t flag must be set" + )] + pub target: Option, + #[arg(trailing_var_arg(true), hide = true)] + pub trailing_args: Vec, } -#[derive(Debug, Clone, clap::Subcommand)] +impl CheckArgs { + fn target_to_check_command(&self) -> Result { + // Get target str + let Some(target) = self.target.clone() else { + return Err(hc_error!( + "a target must be provided. The CLI should have caught this" + )); + }; + // If a target type was provided use that, otherwise try to resolve from + // the target string + let opt_subcmd = self + .target_type + .clone() + .or_else(|| TargetType::try_resolve_from_target(target.as_str())); + let Some(subcmd) = opt_subcmd else { + return Err(hc_error!( + "could not resolve target '{}' to a target type. please specify with the `-t` flag", + target + )); + }; + // We have resolved the subcommand type. Re-construct a string with all args + // that we can feed back into clap + let binding = "check".to_owned(); + let subcmd_str = subcmd.as_str(); + let mut reconst_args: Vec<&String> = vec![&binding, &subcmd_str, &target]; + reconst_args.extend(self.trailing_args.iter()); + + CheckCommand::try_parse_from(reconst_args).map_err(|e| hc_error!("{}", e)) + } + + pub fn command(&self) -> Result { + if let Some(cmd) = self.command.clone() { + Ok(cmd) + } else { + self.target_to_check_command() + } + } +} + +#[derive(Debug, Clone, clap::Parser)] pub enum CheckCommand { /// Analyze a maven package git repo via package URI + #[command(hide = true)] Maven(CheckMavenArgs), /// Analyze an npm package git repo via package URI or with format [@] + #[command(hide = true)] Npm(CheckNpmArgs), /// Analyze 'git' patches for projects that use a patch-based workflow (not yet implemented) + #[command(hide = true)] Patch(CheckPatchArgs), /// Analyze a PyPI package git repo via package URI or with format [@] + #[command(hide = true)] Pypi(CheckPypiArgs), /// Analyze a repository and output an overall risk assessment + #[command(hide = true)] Repo(CheckRepoArgs), /// Analyze pull/merge request for potential risks + #[command(hide = true)] Request(CheckRequestArgs), /// Analyze packages specified in an SPDX document + #[command(hide = true)] Spdx(CheckSpdxArgs), } @@ -769,4 +845,81 @@ mod tests { assert_eq!(config.data().unwrap(), expected); }); } + + #[test] + fn hc_check_schema_no_args_gives_help() { + let check_args = vec!["hc", "check"]; + let schema_args = vec!["hc", "schema"]; + + let parsed = CliConfig::try_parse_from(check_args.into_iter()); + assert!(parsed.is_err()); + assert_eq!( + parsed.unwrap_err().kind(), + clap::error::ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + ); + + let parsed = CliConfig::try_parse_from(schema_args.into_iter()); + assert!(parsed.is_err()); + assert_eq!( + parsed.unwrap_err().kind(), + clap::error::ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + ); + } + + fn get_check_cmd_from_cli(args: Vec<&str>) -> Result { + let parsed = CliConfig::try_parse_from(args.into_iter()); + assert!(parsed.is_ok()); + let command = parsed.unwrap().command; + let Some(Commands::Check(chck_args)) = command else { + assert!(false); + unreachable!(); + }; + chck_args.command() + } + + #[test] + fn test_deprecated_check_repo() { + let cmd = get_check_cmd_from_cli(vec![ + "hc", + "check", + "repo", + "https://github.com/mitre/hipcheck.git", + ]); + assert!(matches!(cmd, Ok(CheckCommand::Repo(..)))); + } + + #[test] + fn test_deductive_check_no_match() { + let cmd = get_check_cmd_from_cli(vec!["hc", "check", "pkg:unsupportedtype/someurl"]); + assert!(matches!(cmd, Err(..))); + } + + #[test] + fn test_deductive_check_github_url() { + let cmd = + get_check_cmd_from_cli(vec!["hc", "check", "https://github.com/mitre/hipcheck.git"]); + assert!(matches!(cmd, Ok(CheckCommand::Repo(..)))); + } + + #[test] + fn test_deductive_check_maven_pkg() { + let cmd = get_check_cmd_from_cli(vec![ + "hc", + "check", + "pkg:maven/org.apache.xmlgraphics/batik-anim@1.9.1", + ]); + assert!(matches!(cmd, Ok(CheckCommand::Maven(..)))); + } + + #[test] + fn test_check_with_target_flag() { + let cmd = get_check_cmd_from_cli(vec![ + "hc", + "check", + "-t", + "repo", + "https://github.com/mitre/hipcheck.git", + ]); + assert!(matches!(cmd, Ok(CheckCommand::Repo(..)))); + } } diff --git a/hipcheck/src/main.rs b/hipcheck/src/main.rs index 62991021..680aea96 100644 --- a/hipcheck/src/main.rs +++ b/hipcheck/src/main.rs @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 +#[allow(unused)] mod analysis; mod cli; mod command_util; @@ -13,8 +14,10 @@ mod report; mod session; mod shell; mod source; +mod target; #[cfg(test)] mod test_util; +mod unstable; mod util; mod version; @@ -55,6 +58,7 @@ use std::ops::Not as _; use std::path::Path; use std::path::PathBuf; use std::process::ExitCode; +use std::process::Termination; use std::result::Result as StdResult; use util::fs::create_dir_all; @@ -71,6 +75,7 @@ fn main() -> ExitCode { match config.subcommand() { Some(FullCommands::Check(args)) => return cmd_check(&args, &config), Some(FullCommands::Schema(args)) => cmd_schema(&args), + Some(FullCommands::Unstable(args)) => return unstable::main(args, &config).report(), Some(FullCommands::Ready) => cmd_ready(&config), Some(FullCommands::PrintConfig) => cmd_print_config(config.config()), Some(FullCommands::PrintData) => cmd_print_data(config.data()), @@ -83,7 +88,13 @@ fn main() -> ExitCode { /// Run the `check` command. fn cmd_check(args: &CheckArgs, config: &CliConfig) -> ExitCode { - let check = args.command.as_check(); + let check = match args.command() { + Ok(chk) => chk.as_check(), + Err(e) => { + print_error(&e); + return ExitCode::FAILURE; + } + }; if check.kind.target_kind().is_checkable().not() { print_missing(); @@ -576,11 +587,11 @@ fn run_with_shell( let scoring = match score_results(&mut phase, &session) { Ok(scoring) => scoring, - _ => { + Err(x) => { return ( session.end(), - Err(Error::msg("Trouble scoring and analyzing results")), - ) + Err(x), // Error::msg("Trouble scoring and analyzing results")), + ); } }; diff --git a/hipcheck/src/unstable.rs b/hipcheck/src/unstable.rs new file mode 100644 index 00000000..e2a65579 --- /dev/null +++ b/hipcheck/src/unstable.rs @@ -0,0 +1,64 @@ +use crate::{ + cli::{CliConfig, UnstableArgs}, + session::session::Check, + shell::{ColorChoice, Output}, +}; +use std::io::Write; +use std::{io, time::Instant}; +use tempdir::TempDir; + +/// Handle unstable commands. +pub fn main(args: UnstableArgs, config: &CliConfig) -> io::Result<()> { + // Warn the user that they are using unstable stuff. + println!("THIS COMMAND IS UNSTABLE. USE AT YOUR OWN RISK."); + // Get an empty temp dir to force hipcheck to run with no cache. + let temp_dir = TempDir::new("hipcheck-benchmarking-")?; + + // Destructure and convert check arg. + let UnstableArgs { benchmark } = args; + let as_check: Check = benchmark.as_check(); + + // Get hipcheck version. + let raw_version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); + + // Get the start instant. + let start_instant = Instant::now(); + + let (shell, report) = crate::run( + Output::stdout(ColorChoice::Auto), + Output::stderr(ColorChoice::Auto), + config.verbosity(), + as_check, + config.config().map(ToOwned::to_owned), + config.data().map(ToOwned::to_owned), + // Use an empty temp dir for caching -- we do not want previous cache values to mess up benchmarking. + Some(temp_dir.path().to_owned()), + config.format(), + raw_version, + ); + + // Print error if there was one. + if let Err(e) = report { + if shell.error(&e, config.format()).is_err() { + crate::print_error(&e); + return Err(io::Error::other("internal hipcheck error")); + } + } + + // Print the elapsed wall time, in seconds, with microsecond precision. + // Use a stdout lock to wait to do this. + let mut stdout = io::stdout().lock(); + writeln!( + &mut stdout, + "\nDONE. ELAPSED WALL TIME: {:.6} SECONDS.", + (Instant::now() - start_instant).as_secs_f64() + ) + .unwrap(); + // Drop our lock -- free up standard output for someone else. + drop(stdout); + + // Close the temp dir. + temp_dir.close()?; + // Return ok. + Ok(()) +} diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index bd733076..00320808 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -8,14 +8,14 @@ publish = false [dependencies] anyhow = "1.0.86" -clap = { version = "4.5.4", features = ["cargo", "derive"] } +clap = { version = "4.5.7", features = ["cargo", "derive"] } clap-verbosity-flag = "2.2.0" env_logger = "0.11.3" log = "0.4.21" glob = "0.3.0" pathbuf = "1.0.0" serde = { version = "1.0.203", features = ["derive"] } -toml = "0.8.13" +toml = "0.8.14" xshell = "0.2.6" which = "6.0.1" convert_case = "0.6.0" From 4db196356120dee65982ab80d5a85b0cb9197f12 Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 19 Jun 2024 15:21:19 -0400 Subject: [PATCH 4/5] chore: Refactor many uses of `Rc` to `Arc` as groundwork for parallelization --- hipcheck/src/analysis.rs | 1 + hipcheck/src/analysis/analysis.rs | 213 ++++++--------- hipcheck/src/analysis/report_builder.rs | 43 ++- hipcheck/src/analysis/result.rs | 170 ++++++++++++ hipcheck/src/analysis/score.rs | 269 ++++++++++--------- hipcheck/src/data.rs | 38 +-- hipcheck/src/data/es_lint/data.rs | 2 + hipcheck/src/data/git/data.rs | 36 +-- hipcheck/src/data/git/parse.rs | 12 +- hipcheck/src/data/git/query/impls.rs | 102 +++---- hipcheck/src/data/git/query/mod.rs | 40 +-- hipcheck/src/data/github/code_search.rs | 6 +- hipcheck/src/data/github/mod.rs | 4 +- hipcheck/src/data/modules.rs | 1 + hipcheck/src/data/query/dependencies.rs | 20 +- hipcheck/src/data/query/github.rs | 22 +- hipcheck/src/data/query/module.rs | 40 +-- hipcheck/src/data/query/pr_review.rs | 42 +-- hipcheck/src/metric/activity.rs | 6 +- hipcheck/src/metric/affiliation.rs | 20 +- hipcheck/src/metric/binary.rs | 10 +- hipcheck/src/metric/binary_detector/mod.rs | 8 +- hipcheck/src/metric/binary_detector/query.rs | 12 +- hipcheck/src/metric/churn.rs | 18 +- hipcheck/src/metric/commit_trust.rs | 14 +- hipcheck/src/metric/contributor_trust.rs | 30 ++- hipcheck/src/metric/entropy.rs | 18 +- hipcheck/src/metric/fuzz.rs | 6 +- hipcheck/src/metric/identity.rs | 12 +- hipcheck/src/metric/linguist/query.rs | 12 +- hipcheck/src/metric/math.rs | 2 + hipcheck/src/metric/metric.rs | 32 +-- hipcheck/src/metric/module.rs | 8 +- hipcheck/src/metric/module_contributors.rs | 12 +- hipcheck/src/metric/review.rs | 10 +- hipcheck/src/metric/typo.rs | 14 +- hipcheck/src/report.rs | 10 +- hipcheck/src/session/session.rs | 5 +- hipcheck/src/shell.rs | 10 + hipcheck/src/source/query.rs | 40 +-- hipcheck/src/target.rs | 47 ++++ 41 files changed, 820 insertions(+), 597 deletions(-) create mode 100644 hipcheck/src/analysis/result.rs create mode 100644 hipcheck/src/target.rs diff --git a/hipcheck/src/analysis.rs b/hipcheck/src/analysis.rs index 5bb8b64a..a8dcf9c9 100644 --- a/hipcheck/src/analysis.rs +++ b/hipcheck/src/analysis.rs @@ -3,6 +3,7 @@ #[allow(clippy::module_inception)] pub mod analysis; pub mod report_builder; +pub mod result; pub mod score; pub use analysis::AnalysisProvider; diff --git a/hipcheck/src/analysis/analysis.rs b/hipcheck/src/analysis/analysis.rs index c079debc..60265ed0 100644 --- a/hipcheck/src/analysis/analysis.rs +++ b/hipcheck/src/analysis/analysis.rs @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 +use crate::analysis::result::*; use crate::config::AttacksConfigQuery; use crate::config::CommitConfigQuery; use crate::config::FuzzConfigQuery; @@ -12,7 +13,6 @@ use crate::metric::MetricProvider; use crate::report::Concern; use crate::report::PrConcern; use crate::F64; -use chrono::Duration; use std::collections::HashMap; use std::collections::HashSet; use std::default::Default; @@ -20,7 +20,7 @@ use std::fmt; use std::fmt::Display; use std::fmt::Formatter; use std::ops::Not; -use std::rc::Rc; +use std::sync::Arc; /// Queries about analyses #[salsa::query_group(AnalysisProviderStorage)] @@ -33,51 +33,44 @@ pub trait AnalysisProvider: + PracticesConfigQuery { /// Returns result of activity analysis - fn activity_analysis(&self) -> Result>; + fn activity_analysis(&self) -> Arc; /// Returns result of affiliation analysis - fn affiliation_analysis(&self) -> Result>; + fn affiliation_analysis(&self) -> Result>; /// Returns result of binary analysis - fn binary_analysis(&self) -> Result>; + fn binary_analysis(&self) -> Result>; /// Returns result of churn analysis - fn churn_analysis(&self) -> Result>; + fn churn_analysis(&self) -> Result>; /// Returns result of entropy analysis - fn entropy_analysis(&self) -> Result>; + fn entropy_analysis(&self) -> Result>; /// Returns result of identity analysis - fn identity_analysis(&self) -> Result>; + fn identity_analysis(&self) -> Result>; /// Returns result of fuzz analysis - fn fuzz_analysis(&self) -> Result>; + fn fuzz_analysis(&self) -> Result>; /// Returns result of review analysis - fn review_analysis(&self) -> Result>; + fn review_analysis(&self) -> Result>; /// Returns result of typo analysis - fn typo_analysis(&self) -> Result>; + fn typo_analysis(&self) -> Result>; /// Returns result of pull request affiliation analysis - fn pr_affiliation_analysis(&self) -> Result>; + fn pr_affiliation_analysis(&self) -> Result>; /// Returns result of pull request contributor trust analysis - fn pr_contributor_trust_analysis(&self) -> Result>; + fn pr_contributor_trust_analysis(&self) -> Result>; /// Returns result of pull request module contributors analysis - fn pr_module_contributors_analysis(&self) -> Result>; + fn pr_module_contributors_analysis(&self) -> Result>; } #[derive(Debug, Clone, Eq, PartialEq)] pub enum AnalysisReport { - /// Activity analysis result. - Activity { - value: u64, - threshold: u64, - outcome: AnalysisOutcome, - concerns: Vec, - }, /// Affiliation analysis result. Affiliation { value: u64, @@ -186,59 +179,30 @@ impl Display for AnalysisOutcome { } } -pub fn activity_analysis(db: &dyn AnalysisProvider) -> Result> { - if db.activity_active() { - let results = db.activity_metric(); - match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { - outcome: AnalysisOutcome::Error(err), - })), - Ok(results) => { - let value = results.time_since_last_commit.num_weeks(); - let threshold = - Duration::weeks(db.activity_week_count_threshold() as i64).num_weeks(); - let results_score = score_by_threshold(value, threshold); - - let concerns = Vec::new(); - - if results_score == 0 { - let msg = format!( - "{} weeks inactivity <= {} weeks inactivity", - value, threshold - ); - Ok(Rc::new(AnalysisReport::Activity { - value: value as u64, - threshold: threshold as u64, - outcome: AnalysisOutcome::Pass(msg), - concerns, - })) - } else { - let msg = format!( - "{} weeks inactivity > {} weeks inactivity", - value, threshold - ); - Ok(Rc::new(AnalysisReport::Activity { - value: value as u64, - threshold: threshold as u64, - outcome: AnalysisOutcome::Fail(msg), - concerns, - })) - } - } +pub fn activity_analysis(db: &dyn AnalysisProvider) -> Arc { + let results = db.activity_metric(); + match results { + Err(err) => Arc::new(HCAnalysisReport { + outcome: HCAnalysisOutcome::Error(HCAnalysisError::Generic(err)), + concerns: vec![], + }), + Ok(results) => { + let value = results.time_since_last_commit.num_weeks() as u64; + let hc_value = HCBasicValue::from(value); + Arc::new(HCAnalysisReport { + outcome: HCAnalysisOutcome::Completed(HCAnalysisValue::Basic(hc_value)), + concerns: vec![], + }) } - } else { - Ok(Rc::new(AnalysisReport::None { - outcome: AnalysisOutcome::Skipped, - })) } } -pub fn affiliation_analysis(db: &dyn AnalysisProvider) -> Result> { +pub fn affiliation_analysis(db: &dyn AnalysisProvider) -> Result> { if db.affiliation_active() { let results = db.affiliation_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -254,7 +218,8 @@ pub fn affiliation_analysis(db: &dyn AnalysisProvider) -> Result String::from(&commit_view.author.name), @@ -264,7 +229,7 @@ pub fn affiliation_analysis(db: &dyn AnalysisProvider) -> Result Result Result {} affiliated", value, threshold); - Ok(Rc::new(AnalysisReport::Affiliation { + Ok(Arc::new(AnalysisReport::Affiliation { value, threshold, outcome: AnalysisOutcome::Fail(msg), @@ -308,18 +273,18 @@ pub fn affiliation_analysis(db: &dyn AnalysisProvider) -> Result Result> { +pub fn binary_analysis(db: &dyn AnalysisProvider) -> Result> { if db.binary_active() { let results = db.binary_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -341,7 +306,7 @@ pub fn binary_analysis(db: &dyn AnalysisProvider) -> Result> "{} binary files found <= {} binary files found", value, threshold ); - Ok(Rc::new(AnalysisReport::Binary { + Ok(Arc::new(AnalysisReport::Binary { value, threshold, outcome: AnalysisOutcome::Pass(msg), @@ -352,7 +317,7 @@ pub fn binary_analysis(db: &dyn AnalysisProvider) -> Result> "{} binary files found >= {} binary files found", value, threshold ); - Ok(Rc::new(AnalysisReport::Binary { + Ok(Arc::new(AnalysisReport::Binary { value, threshold, outcome: AnalysisOutcome::Fail(msg), @@ -362,18 +327,18 @@ pub fn binary_analysis(db: &dyn AnalysisProvider) -> Result> } } } else { - Ok(Rc::new(AnalysisReport::None { + Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Skipped, })) } } -pub fn churn_analysis(db: &dyn AnalysisProvider) -> Result> { +pub fn churn_analysis(db: &dyn AnalysisProvider) -> Result> { if db.churn_active() { let results = db.churn_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -405,7 +370,7 @@ pub fn churn_analysis(db: &dyn AnalysisProvider) -> Result> { percent_threshold * 100.0 ); // PANIC: percent_flagged and percent_threshold will never be NaN - Ok(Rc::new(AnalysisReport::Churn { + Ok(Arc::new(AnalysisReport::Churn { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -420,7 +385,7 @@ pub fn churn_analysis(db: &dyn AnalysisProvider) -> Result> { percent_threshold * 100.0 ); // PANIC: percent_flagged and percent_threshold will never be NaN - Ok(Rc::new(AnalysisReport::Churn { + Ok(Arc::new(AnalysisReport::Churn { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -432,18 +397,18 @@ pub fn churn_analysis(db: &dyn AnalysisProvider) -> Result> { } } } else { - Ok(Rc::new(AnalysisReport::None { + Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Skipped, })) } } -pub fn entropy_analysis(db: &dyn AnalysisProvider) -> Result> { +pub fn entropy_analysis(db: &dyn AnalysisProvider) -> Result> { if db.entropy_active() { let results = db.entropy_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -462,7 +427,7 @@ pub fn entropy_analysis(db: &dyn AnalysisProvider) -> Result> .iter() .filter(|c| c.entropy.into_inner() > value_threshold) .map(|cf| { - db.get_short_hash(Rc::new(cf.commit.hash.clone())) + db.get_short_hash(Arc::new(cf.commit.hash.clone())) .map(|commit_hash| Concern::Entropy { commit_hash: commit_hash.trim().to_owned(), score: cf.entropy.into_inner(), @@ -478,7 +443,7 @@ pub fn entropy_analysis(db: &dyn AnalysisProvider) -> Result> percent_threshold * 100.0 ); // PANIC: percent_flagged and percent_threshold will never be NaN - Ok(Rc::new(AnalysisReport::Entropy { + Ok(Arc::new(AnalysisReport::Entropy { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -493,7 +458,7 @@ pub fn entropy_analysis(db: &dyn AnalysisProvider) -> Result> percent_threshold * 100.0 ); // PANIC: percent_flagged and percent_threshold will never be NaN - Ok(Rc::new(AnalysisReport::Entropy { + Ok(Arc::new(AnalysisReport::Entropy { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -505,18 +470,18 @@ pub fn entropy_analysis(db: &dyn AnalysisProvider) -> Result> } } } else { - Ok(Rc::new(AnalysisReport::None { + Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Skipped, })) } } -pub fn identity_analysis(db: &dyn AnalysisProvider) -> Result> { +pub fn identity_analysis(db: &dyn AnalysisProvider) -> Result> { if db.identity_active() { let results = db.identity_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -538,7 +503,7 @@ pub fn identity_analysis(db: &dyn AnalysisProvider) -> Result percent_threshold * 100.0 ); // PANIC: percent_flagged and percent_threshold will never be NaN - Ok(Rc::new(AnalysisReport::Identity { + Ok(Arc::new(AnalysisReport::Identity { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -553,7 +518,7 @@ pub fn identity_analysis(db: &dyn AnalysisProvider) -> Result percent_threshold * 100.0 ); // PANIC: percent_flagged and percent_threshold will never be NaN - Ok(Rc::new(AnalysisReport::Identity { + Ok(Arc::new(AnalysisReport::Identity { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -565,18 +530,18 @@ pub fn identity_analysis(db: &dyn AnalysisProvider) -> Result } } } else { - Ok(Rc::new(AnalysisReport::None { + Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Skipped, })) } } -pub fn fuzz_analysis(db: &dyn AnalysisProvider) -> Result> { +pub fn fuzz_analysis(db: &dyn AnalysisProvider) -> Result> { if db.fuzz_active() { let results = db.fuzz_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -592,14 +557,14 @@ pub fn fuzz_analysis(db: &dyn AnalysisProvider) -> Result> { if results_score == 0 { let msg = format!("Is fuzzed: {} results found", exists); - Ok(Rc::new(AnalysisReport::Fuzz { + Ok(Arc::new(AnalysisReport::Fuzz { value: exists, outcome: AnalysisOutcome::Pass(msg), concerns, })) } else { let msg = format!("Is not fuzzed: {} no results found", exists); - Ok(Rc::new(AnalysisReport::Fuzz { + Ok(Arc::new(AnalysisReport::Fuzz { value: exists, outcome: AnalysisOutcome::Fail(msg), concerns, @@ -608,18 +573,18 @@ pub fn fuzz_analysis(db: &dyn AnalysisProvider) -> Result> { } } } else { - Ok(Rc::new(AnalysisReport::None { + Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Skipped, })) } } -pub fn review_analysis(db: &dyn AnalysisProvider) -> Result> { +pub fn review_analysis(db: &dyn AnalysisProvider) -> Result> { if db.review_active() { let results = db.review_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -644,7 +609,7 @@ pub fn review_analysis(db: &dyn AnalysisProvider) -> Result> if results_score == 0 { let msg = format!("{:.2}% pull requests without review <= {:.2}% pull requests without review", percent_flagged * 100.0, percent_threshold * 100.0); // PANIC: percent_flagged and percent_threshold will never be NaN - Ok(Rc::new(AnalysisReport::Review { + Ok(Arc::new(AnalysisReport::Review { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -659,7 +624,7 @@ pub fn review_analysis(db: &dyn AnalysisProvider) -> Result> percent_threshold * 100.0 ); // PANIC: percent_flagged and percent_threshold will never be NaN - Ok(Rc::new(AnalysisReport::Review { + Ok(Arc::new(AnalysisReport::Review { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -671,18 +636,18 @@ pub fn review_analysis(db: &dyn AnalysisProvider) -> Result> } } } else { - Ok(Rc::new(AnalysisReport::None { + Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Skipped, })) } } -pub fn typo_analysis(db: &dyn AnalysisProvider) -> Result> { +pub fn typo_analysis(db: &dyn AnalysisProvider) -> Result> { if db.typo_active() { let results = db.typo_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -705,7 +670,7 @@ pub fn typo_analysis(db: &dyn AnalysisProvider) -> Result> { "{} possible typos <= {} possible typos", num_flagged, count_threshold ); - Ok(Rc::new(AnalysisReport::Typo { + Ok(Arc::new(AnalysisReport::Typo { value: num_flagged, threshold: count_threshold, outcome: AnalysisOutcome::Pass(msg), @@ -716,7 +681,7 @@ pub fn typo_analysis(db: &dyn AnalysisProvider) -> Result> { "{} possible typos > {} possible typos", num_flagged, count_threshold ); - Ok(Rc::new(AnalysisReport::Typo { + Ok(Arc::new(AnalysisReport::Typo { value: num_flagged, threshold: count_threshold, outcome: AnalysisOutcome::Pass(msg), @@ -726,18 +691,18 @@ pub fn typo_analysis(db: &dyn AnalysisProvider) -> Result> { } } } else { - Ok(Rc::new(AnalysisReport::None { + Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Skipped, })) } } -pub fn pr_affiliation_analysis(db: &dyn AnalysisProvider) -> Result> { +pub fn pr_affiliation_analysis(db: &dyn AnalysisProvider) -> Result> { if db.pr_affiliation_active() { let results = db.pr_affiliation_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -754,7 +719,7 @@ pub fn pr_affiliation_analysis(db: &dyn AnalysisProvider) -> Result String::from(&commit_view.author.name), @@ -764,7 +729,7 @@ pub fn pr_affiliation_analysis(db: &dyn AnalysisProvider) -> Result Result Result {} affiliated", value, threshold); - Ok(Rc::new(AnalysisReport::PrAffiliation { + Ok(Arc::new(AnalysisReport::PrAffiliation { value, threshold, outcome: AnalysisOutcome::Fail(msg), @@ -808,18 +773,18 @@ pub fn pr_affiliation_analysis(db: &dyn AnalysisProvider) -> Result Result> { +pub fn pr_contributor_trust_analysis(db: &dyn AnalysisProvider) -> Result> { if db.contributor_trust_active() { let results = db.pr_contributor_trust_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -852,7 +817,7 @@ pub fn pr_contributor_trust_analysis(db: &dyn AnalysisProvider) -> Result Result Result Result> { +pub fn pr_module_contributors_analysis(db: &dyn AnalysisProvider) -> Result> { if db.pr_module_contributors_active() { let results = db.pr_module_contributors_metric(); match results { - Err(err) => Ok(Rc::new(AnalysisReport::None { + Err(err) => Ok(Arc::new(AnalysisReport::None { outcome: AnalysisOutcome::Error(err), })), Ok(results) => { @@ -919,7 +884,7 @@ pub fn pr_module_contributors_analysis(db: &dyn AnalysisProvider) -> Result Result= {:.2}% permitted amount", percent_flagged * 100.0, percent_threshold * 100.0); - Ok(Rc::new(AnalysisReport::PrModuleContributors { + Ok(Arc::new(AnalysisReport::PrModuleContributors { value: F64::new(percent_flagged) .expect("Percent flagged should never be NaN"), threshold: F64::new(percent_threshold) @@ -944,7 +909,7 @@ pub fn pr_module_contributors_analysis(db: &dyn AnalysisProvider) -> Result Result { - let analysis = Analysis::activity(*value, *threshold); - builder.add_analysis(analysis, concerns.clone())?; - } - _ => { - return Err(hc_error!( - "phase name does not match {} analysis", - crate::analysis::score::ACTIVITY_PHASE - )) - } - } - Ok(()) - }, - |builder, error| { + match &scoring.results.activity { + Some((Ok(pred), concerns)) => { + let HCBasicValue::Unsigned(value) = pred.value else { + return Err(hc_error!("activity analysis has a non-u64 value")); + }; + let HCBasicValue::Unsigned(thresh) = pred.threshold else { + return Err(hc_error!("activity analysis has a non-u64 value")); + }; + builder.add_analysis(Analysis::activity(value, thresh), concerns.clone())?; + } + Some((Err(error), _concerns)) => { builder.add_errored_analysis(AnalysisIdent::Activity, error); - Ok(()) - }, - )?; + } + None => (), + }; // Affiliation analysis. diff --git a/hipcheck/src/analysis/result.rs b/hipcheck/src/analysis/result.rs new file mode 100644 index 00000000..855101f8 --- /dev/null +++ b/hipcheck/src/analysis/result.rs @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: Apache-2.0 + +use crate::hc_error; +use crate::report::Concern; +use crate::Result; +use crate::F64; +use std::cmp::Ordering; +use std::fmt::{self, Display}; + +/// Represents the enhanced result of a hipcheck analysis. Contains the actual outcome +/// of the analysis, plus additional meta-information the analysis wants to provide to +/// Hipcheck core, such as raised concerns. +#[derive(Debug, Eq, PartialEq)] +pub struct HCAnalysisReport { + pub outcome: HCAnalysisOutcome, + pub concerns: Vec, +} + +/// Represents the result of a hipcheck analysis. Either the analysis encountered +/// an error, or it completed and returned a value. +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum HCAnalysisOutcome { + Error(HCAnalysisError), + Completed(HCAnalysisValue), +} + +/// Enumeration of potential errors that a Hipcheck analysis might return. The Generic +/// variant enables representing errors that aren't covered by other variants. +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum HCAnalysisError { + Generic(crate::error::Error), +} + +/// A Hipcheck analysis may return a basic or composite value. By splitting the types +/// into two sub-enums under this one, we can eschew a recursive enum definition and +/// ensure composite types only have a depth of one. +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum HCAnalysisValue { + Basic(HCBasicValue), + Composite(HCCompositeValue), +} + +/// Basic Hipcheck analysis return types +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum HCBasicValue { + Integer(i64), + Unsigned(u64), + Float(F64), + Bool(bool), + String(String), +} +impl From for HCBasicValue { + fn from(value: i64) -> Self { + HCBasicValue::Integer(value) + } +} +impl From for HCBasicValue { + fn from(value: u64) -> Self { + HCBasicValue::Unsigned(value) + } +} +impl From for HCBasicValue { + fn from(value: F64) -> Self { + HCBasicValue::Float(value) + } +} +impl TryFrom for HCBasicValue { + type Error = crate::Error; + fn try_from(value: f64) -> Result { + let inner = F64::new(value)?; + Ok(HCBasicValue::Float(inner)) + } +} +impl From for HCBasicValue { + fn from(value: bool) -> Self { + HCBasicValue::Bool(value) + } +} +impl From<&str> for HCBasicValue { + fn from(value: &str) -> Self { + HCBasicValue::String(value.to_owned()) + } +} +impl Display for HCBasicValue { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use HCBasicValue::*; + match self { + Unsigned(u) => u.fmt(f), + Integer(i) => i.fmt(f), + String(s) => s.fmt(f), + Float(fp) => fp.fmt(f), + Bool(b) => b.fmt(f), + } + } +} + +/// Composite Hipcheck analysis return types +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum HCCompositeValue { + List(Vec), + Dict(indexmap::IndexMap), +} + +/// The set of possible predicates for deciding if a source passed an analysis. +pub trait HCPredicate: Display + Clone + Eq + PartialEq { + fn pass(&self) -> Result; +} + +/// This predicate determines analysis pass/fail by whether a returned value was +/// greater than, less than, or equal to a target value. +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct ThresholdPredicate { + pub value: HCBasicValue, + pub threshold: HCBasicValue, + units: String, + pub ordering: Ordering, +} +impl ThresholdPredicate { + pub fn new( + value: HCBasicValue, + threshold: HCBasicValue, + units: Option, + ordering: Ordering, + ) -> Self { + ThresholdPredicate { + value, + threshold, + units: units.unwrap_or("".to_owned()), + ordering, + } + } +} + +fn pass_threshold(a: &T, b: &T, ord: &Ordering) -> bool { + a.cmp(b) == *ord +} + +impl HCPredicate for ThresholdPredicate { + // @FollowUp - would be nice for this match logic to error at compile time if a new + // HCBasicValue type is added, so developer is reminded to add new variant here + fn pass(&self) -> Result { + use HCBasicValue::*; + match (&self.value, &self.threshold) { + (Integer(a), Integer(b)) => Ok(pass_threshold(a, b, &self.ordering)), + (Unsigned(a), Unsigned(b)) => Ok(pass_threshold(a, b, &self.ordering)), + (Float(a), Float(b)) => Ok(pass_threshold(a, b, &self.ordering)), + (Bool(a), Bool(b)) => Ok(pass_threshold(a, b, &self.ordering)), + (String(a), String(b)) => Ok(pass_threshold(a, b, &self.ordering)), + (a, b) => Err(hc_error!( + "threshold and value are of different types: {:?}, {:?}", + a, + b + )), + } + } +} +impl Display for ThresholdPredicate { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use Ordering::*; + // append units. if none, trim() call below will clean up whitespace + let val = format!("{} {}", self.value, &self.units); + let thr = format!("{} {}", self.threshold, &self.units); + let order_str = match &self.ordering { + Less => "<", + Equal => "==", + Greater => ">", + }; + write!(f, "{} {} {}", val.trim(), order_str, thr.trim()) + } +} diff --git a/hipcheck/src/analysis/score.rs b/hipcheck/src/analysis/score.rs index 094cbc59..b05d8e15 100644 --- a/hipcheck/src/analysis/score.rs +++ b/hipcheck/src/analysis/score.rs @@ -2,12 +2,15 @@ use crate::analysis::analysis::AnalysisOutcome; use crate::analysis::analysis::AnalysisReport; +use crate::analysis::result::*; use crate::analysis::AnalysisProvider; use crate::error::Result; use crate::hc_error; +use crate::report::Concern; use crate::shell::Phase; +use std::cmp::Ordering; use std::default::Default; -use std::rc::Rc; +use std::sync::Arc; use petgraph::graph::node_index as n; use petgraph::graph::NodeIndex; @@ -39,23 +42,25 @@ pub struct ScoringResults { pub score: Score, } +#[allow(dead_code)] #[derive(Debug, Default)] pub struct AnalysisResults { - pub activity: Option>>, - pub affiliation: Option>>, - pub binary: Option>>, - pub churn: Option>>, - pub entropy: Option>>, - pub identity: Option>>, - pub fuzz: Option>>, - pub review: Option>>, - pub typo: Option>>, - pub pull_request: Option>>, - pub pr_affiliation: Option>>, - pub pr_contributor_trust: Option>>, - pub pr_module_contributors: Option>>, + pub activity: Option<(Result>, Vec)>, + pub affiliation: Option>>, + pub binary: Option>>, + pub churn: Option>>, + pub entropy: Option>>, + pub identity: Option>>, + pub fuzz: Option>>, + pub review: Option>>, + pub typo: Option>>, + pub pull_request: Option>>, + pub pr_affiliation: Option>>, + pub pr_contributor_trust: Option>>, + pub pr_module_contributors: Option>>, } +#[allow(dead_code)] #[derive(Debug, Default)] pub struct Score { pub total: f64, @@ -77,7 +82,7 @@ pub struct Score { #[salsa::query_group(ScoringProviderStorage)] pub trait ScoringProvider: AnalysisProvider { /// Returns result of phase outcome and scoring - fn phase_outcome(&self, phase_name: Rc) -> Result>; + fn phase_outcome(&self, phase_name: Arc) -> Result>; } #[derive(Debug, Clone, PartialEq, Eq)] @@ -108,6 +113,7 @@ pub struct ScoreTree { //stores the score tree using petgraph //the tree does not need to know what sections it is scoring +#[allow(dead_code)] #[derive(Debug, Clone)] pub struct ScoreTreeNode { pub label: String, @@ -120,45 +126,18 @@ pub struct ScoreTreeNode { pub fn phase_outcome>( db: &dyn ScoringProvider, phase_name: P, -) -> Result> { +) -> Result> { match phase_name.as_ref().as_str() { - ACTIVITY_PHASE => match &db.activity_analysis().unwrap().as_ref() { - AnalysisReport::None { - outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), - AnalysisReport::None { - outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { - count: 0, - score: 0, - outcome: AnalysisOutcome::Error(msg.clone()), - })), - AnalysisReport::Activity { - outcome: AnalysisOutcome::Pass(msg), - .. - } => Ok(Rc::new(ScoreResult { - count: db.activity_weight(), - score: 0, - outcome: AnalysisOutcome::Pass(msg.to_string()), - })), - AnalysisReport::Activity { - outcome: AnalysisOutcome::Fail(msg), - .. - } => Ok(Rc::new(ScoreResult { - count: db.activity_weight(), - score: 1, - outcome: AnalysisOutcome::Fail(msg.to_string()), - })), - _ => Err(hc_error!("phase name does not match analysis")), - }, - + ACTIVITY_PHASE => Err(hc_error!( + "activity analysis does not use this infrastructure" + )), AFFILIATION_PHASE => match &db.affiliation_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -166,7 +145,7 @@ pub fn phase_outcome>( AnalysisReport::Affiliation { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.affiliation_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -174,7 +153,7 @@ pub fn phase_outcome>( AnalysisReport::Affiliation { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.affiliation_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -185,10 +164,10 @@ pub fn phase_outcome>( BINARY_PHASE => match &db.binary_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -196,7 +175,7 @@ pub fn phase_outcome>( AnalysisReport::Binary { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.binary_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -204,7 +183,7 @@ pub fn phase_outcome>( AnalysisReport::Binary { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.binary_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -215,10 +194,10 @@ pub fn phase_outcome>( CHURN_PHASE => match &db.churn_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -226,7 +205,7 @@ pub fn phase_outcome>( AnalysisReport::Churn { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.churn_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -234,7 +213,7 @@ pub fn phase_outcome>( AnalysisReport::Churn { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.churn_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -245,10 +224,10 @@ pub fn phase_outcome>( ENTROPY_PHASE => match &db.entropy_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -256,7 +235,7 @@ pub fn phase_outcome>( AnalysisReport::Entropy { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.entropy_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -264,7 +243,7 @@ pub fn phase_outcome>( AnalysisReport::Entropy { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.entropy_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -275,10 +254,10 @@ pub fn phase_outcome>( IDENTITY_PHASE => match &db.identity_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -286,7 +265,7 @@ pub fn phase_outcome>( AnalysisReport::Identity { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.identity_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -294,7 +273,7 @@ pub fn phase_outcome>( AnalysisReport::Identity { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.identity_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -305,10 +284,10 @@ pub fn phase_outcome>( FUZZ_PHASE => match &db.fuzz_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -316,7 +295,7 @@ pub fn phase_outcome>( AnalysisReport::Fuzz { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.fuzz_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -324,7 +303,7 @@ pub fn phase_outcome>( AnalysisReport::Fuzz { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.fuzz_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -335,10 +314,10 @@ pub fn phase_outcome>( REVIEW_PHASE => match &db.review_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -346,7 +325,7 @@ pub fn phase_outcome>( AnalysisReport::Review { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.review_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -354,7 +333,7 @@ pub fn phase_outcome>( AnalysisReport::Review { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.review_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -365,10 +344,10 @@ pub fn phase_outcome>( TYPO_PHASE => match &db.typo_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -376,7 +355,7 @@ pub fn phase_outcome>( AnalysisReport::Typo { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.typo_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -384,7 +363,7 @@ pub fn phase_outcome>( AnalysisReport::Typo { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.typo_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -395,10 +374,10 @@ pub fn phase_outcome>( PR_AFFILIATION_PHASE => match &db.pr_affiliation_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -406,7 +385,7 @@ pub fn phase_outcome>( AnalysisReport::PrAffiliation { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.pr_affiliation_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -414,7 +393,7 @@ pub fn phase_outcome>( AnalysisReport::PrAffiliation { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.pr_affiliation_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -425,10 +404,10 @@ pub fn phase_outcome>( PR_CONTRIBUTOR_TRUST_PHASE => match &db.pr_contributor_trust_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -436,7 +415,7 @@ pub fn phase_outcome>( AnalysisReport::PrContributorTrust { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.contributor_trust_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -444,7 +423,7 @@ pub fn phase_outcome>( AnalysisReport::PrContributorTrust { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.contributor_trust_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -456,10 +435,10 @@ pub fn phase_outcome>( match &db.pr_module_contributors_analysis().unwrap().as_ref() { AnalysisReport::None { outcome: AnalysisOutcome::Skipped, - } => Ok(Rc::new(ScoreResult::default())), + } => Ok(Arc::new(ScoreResult::default())), AnalysisReport::None { outcome: AnalysisOutcome::Error(msg), - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: 0, score: 0, outcome: AnalysisOutcome::Error(msg.clone()), @@ -467,7 +446,7 @@ pub fn phase_outcome>( AnalysisReport::PrModuleContributors { outcome: AnalysisOutcome::Pass(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.pr_module_contributors_weight(), score: 0, outcome: AnalysisOutcome::Pass(msg.to_string()), @@ -475,7 +454,7 @@ pub fn phase_outcome>( AnalysisReport::PrModuleContributors { outcome: AnalysisOutcome::Fail(msg), .. - } => Ok(Rc::new(ScoreResult { + } => Ok(Arc::new(ScoreResult { count: db.pr_module_contributors_weight(), score: 1, outcome: AnalysisOutcome::Fail(msg.to_string()), @@ -500,13 +479,13 @@ pub fn update_phase(phase: &mut Phase, phase_name: &str) -> Result<()> { //Scores phase and adds nodes and edges to tree pub fn add_node_and_edge_with_score( - score_result: Rc, + score_result: impl AsRef, mut score_tree: ScoreTree, phase: &str, parent_node: NodeIndex, ) -> Result { - let weight = score_result.count as f64; - let score_increment = score_result.score as i64; + let weight = score_result.as_ref().count as f64; + let score_increment = score_result.as_ref().score as i64; //adding nodes/edges to the score tree let (child_node, score_tree_updated) = @@ -590,31 +569,61 @@ pub fn score_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result results.activity = None, - AnalysisReport::None { - outcome: AnalysisOutcome::Error(err), - } => results.activity = Some(Err(err.clone())), - _ => results.activity = Some(Ok(activity_analysis)), - } - - let score_result = db - .phase_outcome(Rc::new(ACTIVITY_PHASE.to_string())) - .unwrap(); - score.activity = score_result.outcome.clone(); - match add_node_and_edge_with_score( - score_result, - score_tree.clone(), - ACTIVITY_PHASE, - practices_node, - ) { - Ok(score_tree_inc) => { - score_tree = score_tree_inc; + if db.activity_active() { + let raw_activity = db.activity_analysis(); + let activity_res = match &raw_activity.as_ref().outcome { + HCAnalysisOutcome::Error(err) => Err(hc_error!("{:?}", err)), + HCAnalysisOutcome::Completed(HCAnalysisValue::Basic(av)) => { + let raw_threshold: u64 = db.activity_week_count_threshold(); + let threshold = HCBasicValue::from(raw_threshold); + let predicate = ThresholdPredicate::new( + av.clone(), + threshold, + Some("weeks inactivity".to_owned()), + Ordering::Less, + ); + Ok(Arc::new(predicate)) + } + HCAnalysisOutcome::Completed(HCAnalysisValue::Composite(_)) => Err(hc_error!( + "activity analysis should return a basic u64 type, not {:?}" + )), + }; + // Scoring based off of predicate + let score_result = Arc::new(match activity_res.as_ref() { + Err(e) => ScoreResult { + count: db.activity_weight(), + score: 1, + outcome: AnalysisOutcome::Error(e.clone()), + }, + Ok(pred) => { + // Derive score from predicate, true --> 0, false --> 1 + let passed = pred.pass()?; + let msg = pred.to_string(); + let outcome = if passed { + AnalysisOutcome::Pass(msg) + } else { + AnalysisOutcome::Fail(msg) + }; + ScoreResult { + count: db.activity_weight(), + score: (!passed) as u64, + outcome, + } + } + }); + results.activity = Some((activity_res, raw_activity.concerns.clone())); + score.activity = score_result.outcome.clone(); + match add_node_and_edge_with_score( + score_result, + score_tree.clone(), + ACTIVITY_PHASE, + practices_node, + ) { + Ok(score_tree_inc) => { + score_tree = score_tree_inc; + } + _ => return Err(hc_error!("failed to complete {} scoring.", ACTIVITY_PHASE)), } - _ => return Err(hc_error!("failed to complete {} scoring.", ACTIVITY_PHASE)), } /*===REVIEW PHASE===*/ @@ -629,7 +638,9 @@ pub fn score_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result results.review = Some(Err(err.clone())), _ => results.review = Some(Ok(review_analysis)), } - let score_result = db.phase_outcome(Rc::new(REVIEW_PHASE.to_string())).unwrap(); + let score_result = db + .phase_outcome(Arc::new(REVIEW_PHASE.to_string())) + .unwrap(); score.review = score_result.outcome.clone(); match add_node_and_edge_with_score(score_result, score_tree, REVIEW_PHASE, practices_node) { Ok(score_tree_inc) => { @@ -650,7 +661,9 @@ pub fn score_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result results.binary = Some(Err(err.clone())), _ => results.binary = Some(Ok(binary_analysis)), } - let score_result = db.phase_outcome(Rc::new(BINARY_PHASE.to_string())).unwrap(); + let score_result = db + .phase_outcome(Arc::new(BINARY_PHASE.to_string())) + .unwrap(); score.binary = score_result.outcome.clone(); match add_node_and_edge_with_score(score_result, score_tree, BINARY_PHASE, practices_node) { Ok(score_tree_inc) => { @@ -673,7 +686,7 @@ pub fn score_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result Result results.fuzz = Some(Ok(fuzz_analysis)), } - let score_result = db.phase_outcome(Rc::new(FUZZ_PHASE.to_string())).unwrap(); + let score_result = db.phase_outcome(Arc::new(FUZZ_PHASE.to_string())).unwrap(); score.fuzz = score_result.outcome.clone(); match add_node_and_edge_with_score(score_result, score_tree, FUZZ_PHASE, practices_node) { Ok(score_tree_inc) => { @@ -738,7 +751,7 @@ pub fn score_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result results.typo = Some(Err(err.clone())), _ => results.typo = Some(Ok(typo_analysis)), } - let score_result = db.phase_outcome(Rc::new(TYPO_PHASE.to_string())).unwrap(); + let score_result = db.phase_outcome(Arc::new(TYPO_PHASE.to_string())).unwrap(); score.typo = score_result.outcome.clone(); match add_node_and_edge_with_score(score_result, score_tree, TYPO_PHASE, attacks_node) { Ok(score_tree_inc) => { @@ -779,7 +792,7 @@ pub fn score_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result results.affiliation = Some(Ok(affiliation_analysis)), } let score_result = db - .phase_outcome(Rc::new(AFFILIATION_PHASE.to_string())) + .phase_outcome(Arc::new(AFFILIATION_PHASE.to_string())) .unwrap(); score.affiliation = score_result.outcome.clone(); match add_node_and_edge_with_score( @@ -811,7 +824,7 @@ pub fn score_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result results.churn = Some(Err(err.clone())), _ => results.churn = Some(Ok(churn_analysis)), } - let score_result = db.phase_outcome(Rc::new(CHURN_PHASE.to_string())).unwrap(); + let score_result = db.phase_outcome(Arc::new(CHURN_PHASE.to_string())).unwrap(); score.churn = score_result.outcome.clone(); match add_node_and_edge_with_score(score_result, score_tree, CHURN_PHASE, commit_node) { Ok(score_tree_inc) => { @@ -834,7 +847,7 @@ pub fn score_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result Result results.pr_affiliation = Some(Ok(pr_affiliation_analysis)), } let score_result = db - .phase_outcome(Rc::new(PR_AFFILIATION_PHASE.to_string())) + .phase_outcome(Arc::new(PR_AFFILIATION_PHASE.to_string())) .unwrap(); score.pr_affiliation = score_result.outcome.clone(); match add_node_and_edge_with_score( @@ -959,7 +972,7 @@ pub fn score_pr_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result results.pr_contributor_trust = Some(Ok(pr_contributor_trust_analysis)), } let score_result = db - .phase_outcome(Rc::new(PR_CONTRIBUTOR_TRUST_PHASE.to_string())) + .phase_outcome(Arc::new(PR_CONTRIBUTOR_TRUST_PHASE.to_string())) .unwrap(); score.pr_contributor_trust = score_result.outcome.clone(); match add_node_and_edge_with_score( @@ -992,7 +1005,7 @@ pub fn score_pr_results(phase: &mut Phase, db: &dyn ScoringProvider) -> Result results.pr_module_contributors = Some(Ok(pr_module_contributors_analysis)), } let score_result = db - .phase_outcome(Rc::new(PR_MODULE_CONTRIBUTORS_PHASE.to_string())) + .phase_outcome(Arc::new(PR_MODULE_CONTRIBUTORS_PHASE.to_string())) .unwrap(); score.pr_module_contributors = score_result.outcome.clone(); match add_node_and_edge_with_score( diff --git a/hipcheck/src/data.rs b/hipcheck/src/data.rs index 29c01e7b..ea101642 100644 --- a/hipcheck/src/data.rs +++ b/hipcheck/src/data.rs @@ -32,12 +32,12 @@ use petgraph::Graph; use serde::Serialize; use std::collections::HashSet; use std::path::Path; -use std::rc::Rc; +use std::sync::Arc; #[derive(Debug, Clone, PartialEq, Eq)] pub struct Dependencies { pub language: Lang, - pub deps: Vec>, + pub deps: Vec>, } impl Dependencies { @@ -46,7 +46,7 @@ impl Dependencies { language @ Lang::JavaScript => { let deps = npm::get_dependencies(repo, version)? .into_iter() - .map(Rc::new) + .map(Arc::new) .collect(); Ok(Dependencies { language, deps }) } @@ -78,7 +78,7 @@ pub struct Fuzz { pub exists: bool, } -pub fn get_fuzz_check(token: &str, repo_uri: Rc) -> Result { +pub fn get_fuzz_check(token: &str, repo_uri: Arc) -> Result { let github = GitHub::new("google", "oss-fuzz", token)?; let github_result = github @@ -102,10 +102,10 @@ pub struct PullRequest { pub struct SinglePullRequest { pub id: u64, pub reviews: u64, - pub commits: Vec>, - pub contributors: Vec>, + pub commits: Vec>, + pub contributors: Vec>, pub commit_contributors: Vec, - pub diffs: Vec>, + pub diffs: Vec>, } pub fn get_pull_request_reviews_from_github( @@ -146,7 +146,7 @@ pub fn get_single_pull_request_review_from_github( .commits .iter() .map(|raw| { - Rc::new(Commit { + Arc::new(Commit { hash: raw.hash.to_owned(), written_on: raw.written_on.to_owned(), committed_on: raw.committed_on.to_owned(), @@ -154,13 +154,13 @@ pub fn get_single_pull_request_review_from_github( }) .collect(); - let mut contributors: Vec> = github_result + let mut contributors: Vec> = github_result .commits .iter() .flat_map(|raw| { [ - Rc::new(raw.author.to_owned()), - Rc::new(raw.committer.to_owned()), + Arc::new(raw.author.to_owned()), + Arc::new(raw.committer.to_owned()), ] }) .collect(); @@ -195,7 +195,7 @@ pub fn get_single_pull_request_review_from_github( let diffs = github_result .diffs .iter() - .map(|diff| Rc::new(diff.to_owned())) + .map(|diff| Arc::new(diff.to_owned())) .collect(); let result = SinglePullRequest { @@ -230,11 +230,11 @@ pub struct ModuleGraph { // For a given ModuleGraph representation of repository files, associate each module with the file's commit hashes pub fn associate_modules_and_commits( repo_path: &Path, - module_graph: Rc, - commits: Rc>>, + module_graph: Arc, + commits: Arc>>, ) -> Result { // Vector containing pairings between module and associated commits - let mut modules_commits: Vec<(Rc, Rc)> = Vec::new(); + let mut modules_commits: Vec<(Arc, Arc)> = Vec::new(); // Graph traversal, depth-first let start = module_graph @@ -256,7 +256,7 @@ pub fn associate_modules_and_commits( .iter() .filter_map(|commit| { if hashes.contains(&commit.hash.as_ref()) { - Some(Rc::clone(commit)) + Some(Arc::clone(commit)) } else { None } @@ -266,13 +266,13 @@ pub fn associate_modules_and_commits( // Add entry to vec for each commit e.g. for commit in commit_vec { modules_commits.push(( - Rc::new(module_graph.connections[visited].clone()), - Rc::clone(&commit), + Arc::new(module_graph.connections[visited].clone()), + Arc::clone(&commit), )); } } - Ok(Rc::new(modules_commits)) + Ok(Arc::new(modules_commits)) } impl ModuleGraph { diff --git a/hipcheck/src/data/es_lint/data.rs b/hipcheck/src/data/es_lint/data.rs index e6ddf498..193565ad 100644 --- a/hipcheck/src/data/es_lint/data.rs +++ b/hipcheck/src/data/es_lint/data.rs @@ -13,6 +13,7 @@ use serde::Deserialize; pub type ESLintReports = Vec; +#[allow(dead_code)] #[derive(Debug, Clone, Deserialize)] pub struct ESLintReport { #[serde(rename = "filePath")] @@ -29,6 +30,7 @@ pub struct ESLintReport { pub source: Option, } +#[allow(dead_code)] #[derive(Debug, Clone, Deserialize)] pub struct ESLintMessage { #[serde(rename = "ruleId")] diff --git a/hipcheck/src/data/git/data.rs b/hipcheck/src/data/git/data.rs index 9c8a58d2..ea425c86 100644 --- a/hipcheck/src/data/git/data.rs +++ b/hipcheck/src/data/git/data.rs @@ -7,7 +7,7 @@ use serde::Serialize; use std::fmt; use std::fmt::Display; use std::fmt::Formatter; -use std::rc::Rc; +use std::sync::Arc; /// Commits as they come directly out of `git log`. #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] @@ -77,52 +77,52 @@ pub struct CommitSigner { /// Temporary data structure for looking up the contributors of a commit #[derive(Debug, Serialize, Clone, PartialEq, Eq)] pub struct CommitContributorView { - pub commit: Rc, - pub author: Rc, - pub committer: Rc, + pub commit: Arc, + pub author: Arc, + pub committer: Arc, } /// Temporary data structure for looking up the commits associated with a contributor #[derive(Debug, Serialize, Clone, PartialEq, Eq)] pub struct ContributorView { - pub contributor: Rc, - pub commits: Vec>, + pub contributor: Arc, + pub commits: Vec>, } /// Temporary data structure for looking up the signer of a commit #[derive(Debug, Serialize, Clone, PartialEq, Eq)] pub struct CommitSignerView { - pub commit: Rc, - pub signer_name: Option>, - pub signer_key: Option>, + pub commit: Arc, + pub signer_name: Option>, + pub signer_key: Option>, } /// Temporary data structure for looking up the commits associated with a signer name #[derive(Debug, Serialize, Clone, PartialEq, Eq)] pub struct SignerNameView { - pub signer_name: Rc, - pub commits: Vec>, + pub signer_name: Arc, + pub commits: Vec>, } /// Temporary data structure for looking up the commits associated with a signer key #[derive(Debug, Serialize, Clone, PartialEq, Eq)] pub struct SignerKeyView { - pub signer_key: Rc, - pub commits: Vec>, + pub signer_key: Arc, + pub commits: Vec>, } /// Temporary data structure for looking up the keys associated with a signer name #[derive(Debug, Serialize, Clone, PartialEq, Eq)] pub struct SignerView { - pub signer_name: Rc, - pub signer_keys: Vec>, + pub signer_name: Arc, + pub signer_keys: Vec>, } /// View into commits and diffs joined together. #[derive(Debug, Serialize, PartialEq, Eq)] pub struct CommitDiff { - pub commit: Rc, - pub diff: Rc, + pub commit: Arc, + pub diff: Arc, } impl Display for CommitDiff { @@ -156,7 +156,7 @@ pub struct Diff { /// A set of changes to a specific file in a commit. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct FileDiff { - pub file_name: Rc, + pub file_name: Arc, pub additions: Option, pub deletions: Option, pub patch: String, diff --git a/hipcheck/src/data/git/parse.rs b/hipcheck/src/data/git/parse.rs index e95f6a36..8836b38d 100644 --- a/hipcheck/src/data/git/parse.rs +++ b/hipcheck/src/data/git/parse.rs @@ -31,8 +31,8 @@ use nom::sequence::terminated; use nom::sequence::tuple; use nom::IResult; use std::iter::Iterator; -use std::rc::Rc; use std::result::Result as StdResult; +use std::sync::Arc; const HEX_CHARS: &str = "0123456789abcdef"; const GIT_HASH_MIN_LEN: usize = 5; @@ -209,7 +209,7 @@ fn diff(input: &str) -> IResult<&str, Diff> { deletions = deletions.map(|d| d + stat.lines_deleted); FileDiff { - file_name: Rc::new(stat.file_name.to_owned()), + file_name: Arc::new(stat.file_name.to_owned()), additions: Some(stat.lines_added), deletions: Some(stat.lines_deleted), patch, @@ -243,7 +243,7 @@ fn gh_diff(input: &str) -> IResult<&str, Diff> { let file_diffs = patches .into_iter() .map(|patch| FileDiff { - file_name: Rc::new(patch.file_name), + file_name: Arc::new(patch.file_name), additions: None, deletions: None, patch: patch.content, @@ -831,7 +831,7 @@ index e7a11a9..4894a2e 100644 deletions: Some(1), file_diffs: vec![ FileDiff { - file_name: Rc::new(String::from(".gitignore")), + file_name: Arc::new(String::from(".gitignore")), additions: Some(10), deletions: Some(0), patch: String::from( @@ -849,7 +849,7 @@ Cargo.lock ), }, FileDiff { - file_name: Rc::new(String::from("Cargo.toml")), + file_name: Arc::new(String::from("Cargo.toml")), additions: Some(4), deletions: Some(0), patch: String::from( @@ -861,7 +861,7 @@ serde_json = "1.0.39" ), }, FileDiff { - file_name: Rc::new(String::from("src/main.rs")), + file_name: Arc::new(String::from("src/main.rs")), additions: Some(127), deletions: Some(1), patch: String::from( diff --git a/hipcheck/src/data/git/query/impls.rs b/hipcheck/src/data/git/query/impls.rs index 6c277689..b924768b 100644 --- a/hipcheck/src/data/git/query/impls.rs +++ b/hipcheck/src/data/git/query/impls.rs @@ -24,17 +24,17 @@ use crate::data::git::SignerView; use crate::error::Error; use crate::error::Result; use chrono::prelude::*; -use std::rc::Rc; +use std::sync::Arc; -pub(crate) fn raw_commits(db: &dyn GitProvider) -> Result>> { - get_commits(db.local().as_ref()).map(Rc::new) +pub(crate) fn raw_commits(db: &dyn GitProvider) -> Result>> { + get_commits(db.local().as_ref()).map(Arc::new) } pub(crate) fn raw_commits_from_date( db: &dyn GitProvider, - date: Rc, -) -> Result>> { - get_commits_from_date(db.local().as_ref(), date.as_str()).map(Rc::new) + date: Arc, +) -> Result>> { + get_commits_from_date(db.local().as_ref(), date.as_str()).map(Arc::new) } pub(crate) fn last_commit_date(db: &dyn GitProvider) -> Result> { @@ -49,24 +49,24 @@ pub(crate) fn last_commit_date(db: &dyn GitProvider) -> Result Result>>> { +pub(crate) fn diffs(db: &dyn GitProvider) -> Result>>> { let diffs = get_diffs(db.local().as_ref())?; - let diffs = diffs.into_iter().map(Rc::new).collect(); + let diffs = diffs.into_iter().map(Arc::new).collect(); - Ok(Rc::new(diffs)) + Ok(Arc::new(diffs)) } //accepts a date parameter in form at 2021-09-10 to filter commits from git pub(crate) fn commits_from_date( db: &dyn GitProvider, - date: Rc, -) -> Result>>> { + date: Arc, +) -> Result>>> { let commits = db - .raw_commits_from_date(Rc::new(date.to_string())) + .raw_commits_from_date(Arc::new(date.to_string())) .context("failed to get raw commits from date")? .iter() .map(|raw| { - Rc::new(Commit { + Arc::new(Commit { hash: raw.hash.to_owned(), written_on: raw.written_on.to_owned(), committed_on: raw.committed_on.to_owned(), @@ -74,16 +74,16 @@ pub(crate) fn commits_from_date( }) .collect(); - Ok(Rc::new(commits)) + Ok(Arc::new(commits)) } -pub(crate) fn commits(db: &dyn GitProvider) -> Result>>> { +pub(crate) fn commits(db: &dyn GitProvider) -> Result>>> { let commits = db .raw_commits() .context("failed to get raw commits")? .iter() .map(|raw| { - Rc::new(Commit { + Arc::new(Commit { hash: raw.hash.to_owned(), written_on: raw.written_on.to_owned(), committed_on: raw.committed_on.to_owned(), @@ -91,18 +91,18 @@ pub(crate) fn commits(db: &dyn GitProvider) -> Result>>> { }) .collect(); - Ok(Rc::new(commits)) + Ok(Arc::new(commits)) } -pub(crate) fn contributors(db: &dyn GitProvider) -> Result>>> { +pub(crate) fn contributors(db: &dyn GitProvider) -> Result>>> { let mut contributors: Vec<_> = db .raw_commits() .context("failed to get raw commits")? .iter() .flat_map(|raw| { [ - Rc::new(raw.author.to_owned()), - Rc::new(raw.committer.to_owned()), + Arc::new(raw.author.to_owned()), + Arc::new(raw.committer.to_owned()), ] }) .collect(); @@ -110,10 +110,10 @@ pub(crate) fn contributors(db: &dyn GitProvider) -> Result Result>> { +pub(crate) fn commit_contributors(db: &dyn GitProvider) -> Result>> { let contributors = db.contributors().context("failed to get contributors")?; let commit_contributors = db @@ -141,38 +141,38 @@ pub(crate) fn commit_contributors(db: &dyn GitProvider) -> Result Result>>>> { +pub(crate) fn signer_names(db: &dyn GitProvider) -> Result>>>> { let mut signer_names: Vec<_> = db .raw_commits() .context("failed to get raw commits")? .iter() - .map(|raw| raw.signer_name.to_owned().map(Rc::new)) + .map(|raw| raw.signer_name.to_owned().map(Arc::new)) .collect(); signer_names.sort(); signer_names.dedup(); - Ok(Rc::new(signer_names)) + Ok(Arc::new(signer_names)) } -pub(crate) fn signer_keys(db: &dyn GitProvider) -> Result>>>> { +pub(crate) fn signer_keys(db: &dyn GitProvider) -> Result>>>> { let mut signer_keys: Vec<_> = db .raw_commits() .context("failed to get raw commits")? .iter() - .map(|raw| raw.signer_key.to_owned().map(Rc::new)) + .map(|raw| raw.signer_key.to_owned().map(Arc::new)) .collect(); signer_keys.sort(); signer_keys.dedup(); - Ok(Rc::new(signer_keys)) + Ok(Arc::new(signer_keys)) } -pub(crate) fn commit_signers(db: &dyn GitProvider) -> Result>> { +pub(crate) fn commit_signers(db: &dyn GitProvider) -> Result>> { let signer_names = db.signer_names().context("failed to get signer names")?; let signer_keys = db.signer_keys().context("failed to get signer keys")?; @@ -202,26 +202,26 @@ pub(crate) fn commit_signers(db: &dyn GitProvider) -> Result Result>> { +pub(crate) fn commit_diffs(db: &dyn GitProvider) -> Result>> { let commits = db.commits().context("failed to get commits")?; let diffs = db.diffs().context("failed to get diffs")?; let commit_diffs = Iterator::zip(commits.iter(), diffs.iter()) .map(|(commit, diff)| CommitDiff { - commit: Rc::clone(commit), - diff: Rc::clone(diff), + commit: Arc::clone(commit), + diff: Arc::clone(diff), }) .collect(); - Ok(Rc::new(commit_diffs)) + Ok(Arc::new(commit_diffs)) } pub(crate) fn commits_for_contributor( db: &dyn GitProvider, - contributor: Rc, + contributor: Arc, ) -> Result { let all_commits = db.commits().context("failed to get commits")?; let contributors = db.contributors().context("failed to get contributors")?; @@ -243,7 +243,7 @@ pub(crate) fn commits_for_contributor( // SAFETY: This index is guaranteed to be valid in // `all_commits` because of how it and `commit_contributors` // are constructed from `db.raw_commits()` - Some(Rc::clone(&all_commits[com_con.commit_id])) + Some(Arc::clone(&all_commits[com_con.commit_id])) } else { None } @@ -258,7 +258,7 @@ pub(crate) fn commits_for_contributor( pub(crate) fn contributors_for_commit( db: &dyn GitProvider, - commit: Rc, + commit: Arc, ) -> Result { let commits = db.commits().context("failed to get commits")?; let contributors = db.contributors().context("failed to get contributors")?; @@ -280,8 +280,8 @@ pub(crate) fn contributors_for_commit( // SAFETY: These indices are guaranteed to be valid in // `contributors` because of how `commit_contributors` is // constructed from it. - let author = Rc::clone(&contributors[com_con.author_id]); - let committer = Rc::clone(&contributors[com_con.committer_id]); + let author = Arc::clone(&contributors[com_con.author_id]); + let committer = Arc::clone(&contributors[com_con.committer_id]); CommitContributorView { commit, @@ -294,7 +294,7 @@ pub(crate) fn contributors_for_commit( pub(crate) fn commits_for_signer_name( db: &dyn GitProvider, - signer_name: Rc, + signer_name: Arc, ) -> Result { let all_commits = db.commits().context("failed to get commits")?; let signer_names = db.signer_names().context("failed to get signer names")?; @@ -314,7 +314,7 @@ pub(crate) fn commits_for_signer_name( // SAFETY: This index is guaranteed to be valid in // `all_commits` because of how it and `commit_signers` // are constructed from `db.raw_commits()`. - Some(Rc::clone(&all_commits[com_sign.commit_id])) + Some(Arc::clone(&all_commits[com_sign.commit_id])) } else { None } @@ -329,7 +329,7 @@ pub(crate) fn commits_for_signer_name( pub(crate) fn commits_for_signer_key( db: &dyn GitProvider, - signer_key: Rc, + signer_key: Arc, ) -> Result { let all_commits = db.commits().context("failed to get commits")?; let signer_keys = db.signer_keys().context("failed to get signer keys")?; @@ -349,7 +349,7 @@ pub(crate) fn commits_for_signer_key( // SAFETY: This index is guaranteed to be valid in // `all_commits` because of how it and `commit_signers` // are constructed from `db.raw_commits()`. - Some(Rc::clone(&all_commits[com_sign.commit_id])) + Some(Arc::clone(&all_commits[com_sign.commit_id])) } else { None } @@ -364,7 +364,7 @@ pub(crate) fn commits_for_signer_key( pub(crate) fn signer_for_commit( db: &dyn GitProvider, - commit: Rc, + commit: Arc, ) -> Result { let commits = db.commits().context("failed to get commits")?; let signer_names = db.signer_names().context("failed to get signer names")?; @@ -397,7 +397,10 @@ pub(crate) fn signer_for_commit( .ok_or_else(|| Error::msg("failed to find contributor info")) } -pub(crate) fn signer_for_name(db: &dyn GitProvider, signer_name: Rc) -> Result { +pub(crate) fn signer_for_name( + db: &dyn GitProvider, + signer_name: Arc, +) -> Result { let signer_names = db.signer_names().context("failed to get signer names")?; let all_signer_keys = db.signer_keys().context("failed to get signer keys")?; let commit_signers = db.commit_signers().context("failed to get join table")?; @@ -430,7 +433,7 @@ pub(crate) fn signer_for_name(db: &dyn GitProvider, signer_name: Rc) -> }) } -pub(crate) fn signer_for_key(db: &dyn GitProvider, signer_key: Rc) -> Result { +pub(crate) fn signer_for_key(db: &dyn GitProvider, signer_key: Arc) -> Result { let signer_names = db.signer_names().context("failed to get signer names")?; let all_signer_keys = db.signer_keys().context("failed to get signer keys")?; let commit_signers = db.commit_signers().context("failed to get join table")?; @@ -459,7 +462,10 @@ pub(crate) fn signer_for_key(db: &dyn GitProvider, signer_key: Rc) -> Re db.signer_for_name(signer_name) } -pub(crate) fn get_short_hash(db: &dyn GitProvider, long_hash: Rc) -> Result { +pub(crate) fn get_short_hash( + db: &dyn GitProvider, + long_hash: impl AsRef, +) -> Result { let repo = db.local(); let repo_path = repo.as_path(); let output = GitCommand::for_repo(repo_path, ["rev-parse", "--short", long_hash.as_ref()])? diff --git a/hipcheck/src/data/git/query/mod.rs b/hipcheck/src/data/git/query/mod.rs index 901f9b9e..b93bdac1 100644 --- a/hipcheck/src/data/git/query/mod.rs +++ b/hipcheck/src/data/git/query/mod.rs @@ -21,18 +21,18 @@ use crate::error::Result; use crate::source::source::SourceQuery; use crate::version::VersionQuery; use chrono::prelude::*; -use std::rc::Rc; +use std::sync::Arc; /// Queries about Git objects #[salsa::query_group(GitProviderStorage)] pub trait GitProvider: SourceQuery + VersionQuery { /// Returns all raw commits extracted from the repository #[salsa::invoke(impls::raw_commits)] - fn raw_commits(&self) -> Result>>; + fn raw_commits(&self) -> Result>>; /// Returns all raw commits extracted from the repository from a certain date #[salsa::invoke(impls::raw_commits_from_date)] - fn raw_commits_from_date(&self, date: Rc) -> Result>>; + fn raw_commits_from_date(&self, date: Arc) -> Result>>; /// Return the date of the most recent commit #[salsa::invoke(impls::last_commit_date)] @@ -40,70 +40,70 @@ pub trait GitProvider: SourceQuery + VersionQuery { /// Returns all diffs extracted from the repository #[salsa::invoke(impls::diffs)] - fn diffs(&self) -> Result>>>; + fn diffs(&self) -> Result>>>; /// Returns all commits extracted from the repository #[salsa::invoke(impls::commits)] - fn commits(&self) -> Result>>>; + fn commits(&self) -> Result>>>; /// Returns all commits extracted from the repository #[salsa::invoke(impls::commits_from_date)] - fn commits_from_date(&self, date: Rc) -> Result>>>; + fn commits_from_date(&self, date: Arc) -> Result>>>; /// Returns all contributors to the repository #[salsa::invoke(impls::contributors)] - fn contributors(&self) -> Result>>>; + fn contributors(&self) -> Result>>>; /// Returns contributors by commit #[salsa::invoke(impls::commit_contributors)] - fn commit_contributors(&self) -> Result>>; + fn commit_contributors(&self) -> Result>>; /// Returns all signer names #[salsa::invoke(impls::signer_names)] - fn signer_names(&self) -> Result>>>>; + fn signer_names(&self) -> Result>>>>; /// Returns all signer keys #[salsa::invoke(impls::signer_keys)] - fn signer_keys(&self) -> Result>>>>; + fn signer_keys(&self) -> Result>>>>; /// Returns all commit signers #[salsa::invoke(impls::commit_signers)] - fn commit_signers(&self) -> Result>>; + fn commit_signers(&self) -> Result>>; /// Returns all commit-diff pairs #[salsa::invoke(impls::commit_diffs)] - fn commit_diffs(&self) -> Result>>; + fn commit_diffs(&self) -> Result>>; /// Returns the commits associated with a given contributor #[salsa::invoke(impls::commits_for_contributor)] - fn commits_for_contributor(&self, contributor: Rc) -> Result; + fn commits_for_contributor(&self, contributor: Arc) -> Result; /// Returns the contributor view for a given commit #[salsa::invoke(impls::contributors_for_commit)] - fn contributors_for_commit(&self, commit: Rc) -> Result; + fn contributors_for_commit(&self, commit: Arc) -> Result; /// Returns the commits associated with a given signer name #[salsa::invoke(impls::commits_for_signer_name)] - fn commits_for_signer_name(&self, signer_name: Rc) -> Result; + fn commits_for_signer_name(&self, signer_name: Arc) -> Result; /// Returns the commits associated with a given signer key #[salsa::invoke(impls::commits_for_signer_key)] - fn commits_for_signer_key(&self, signer_key: Rc) -> Result; + fn commits_for_signer_key(&self, signer_key: Arc) -> Result; /// Returns the signer name and key, if any, associated with a /// given commit #[salsa::invoke(impls::signer_for_commit)] - fn signer_for_commit(&self, commit: Rc) -> Result; + fn signer_for_commit(&self, commit: Arc) -> Result; /// Returns the signer view for a given signer name #[salsa::invoke(impls::signer_for_name)] - fn signer_for_name(&self, signer_name: Rc) -> Result; + fn signer_for_name(&self, signer_name: Arc) -> Result; /// Returns the signer view for a given signer key #[salsa::invoke(impls::signer_for_key)] - fn signer_for_key(&self, signer_key: Rc) -> Result; + fn signer_for_key(&self, signer_key: Arc) -> Result; /// Returns shorter form of a given git hash #[salsa::invoke(impls::get_short_hash)] - fn get_short_hash(&self, long_hash: Rc) -> Result; + fn get_short_hash(&self, long_hash: Arc) -> Result; } diff --git a/hipcheck/src/data/github/code_search.rs b/hipcheck/src/data/github/code_search.rs index 1f2b6d81..45fe0764 100644 --- a/hipcheck/src/data/github/code_search.rs +++ b/hipcheck/src/data/github/code_search.rs @@ -5,12 +5,14 @@ use crate::error::Result; use crate::hc_error; use crate::http::authenticated_agent::AuthenticatedAgent; use serde_json::Value; -use std::rc::Rc; const GH_API_V4_SEARCH: &str = "https://api.github.com/search/code"; /// Make a request to the GitHub Code Search API. -pub fn search_code_request(agent: &AuthenticatedAgent<'_>, repo: Rc) -> Result { +pub fn search_code_request( + agent: &AuthenticatedAgent<'_>, + repo: impl AsRef, +) -> Result { // Example query will look like this: // https://api.github.com/search/code?q=github.com%20assimp%20assimp+in:file+filename:project.yaml+repo:google/oss-fuzz // diff --git a/hipcheck/src/data/github/mod.rs b/hipcheck/src/data/github/mod.rs index 3f98dec9..9b96e0c5 100644 --- a/hipcheck/src/data/github/mod.rs +++ b/hipcheck/src/data/github/mod.rs @@ -13,7 +13,7 @@ use crate::data::github::graphql::get_all_reviews; use crate::data::github::graphql_pr::get_all_pr_reviews; use crate::error::Result; use crate::http::authenticated_agent::AuthenticatedAgent; -use std::rc::Rc; +use std::sync::Arc; pub struct GitHub<'a> { owner: &'a str, @@ -30,7 +30,7 @@ impl<'a> GitHub<'a> { }) } - pub fn fuzz_check(&self, repo_uri: Rc) -> Result { + pub fn fuzz_check(&self, repo_uri: Arc) -> Result { search_code_request(&self.agent, repo_uri).context("unable to search fuzzing information; please check the HC_GITHUB_TOKEN system environment variable") } diff --git a/hipcheck/src/data/modules.rs b/hipcheck/src/data/modules.rs index 8f920cc4..e438f1ed 100644 --- a/hipcheck/src/data/modules.rs +++ b/hipcheck/src/data/modules.rs @@ -54,6 +54,7 @@ fn detect_npm_package_root(pkg_file: &Path) -> Result { } } +#[allow(dead_code)] #[derive(Debug, Deserialize)] pub struct RawModule { pub file: String, diff --git a/hipcheck/src/data/query/dependencies.rs b/hipcheck/src/data/query/dependencies.rs index a858442c..d3b3ea57 100644 --- a/hipcheck/src/data/query/dependencies.rs +++ b/hipcheck/src/data/query/dependencies.rs @@ -8,34 +8,34 @@ use crate::data::Dependencies; use crate::error::Result; use crate::source::source::SourceQuery; use crate::version::VersionQuery; -use std::rc::Rc; +use std::sync::Arc; /// Queries about dependencies #[salsa::query_group(DependenciesProviderStorage)] pub trait DependenciesProvider: SourceQuery + VersionQuery { /// Returns the `Dependencies` struct for the current session - fn dependencies(&self) -> Result>; + fn dependencies(&self) -> Result>; /// The parsed contents of the `package.json` file. - fn package_file(&self) -> Result>; + fn package_file(&self) -> Result>; /// The value of the `main` field in `package.json`. - fn package_file_main(&self) -> Result>; + fn package_file_main(&self) -> Result>; } /// Derived query implementations. Return value is wrapped in an `Rc` /// to keep cloning cheap. -fn dependencies(db: &dyn DependenciesProvider) -> Result> { - Dependencies::resolve(&db.local(), (&db.npm_version().as_ref()).to_string()).map(Rc::new) +fn dependencies(db: &dyn DependenciesProvider) -> Result> { + Dependencies::resolve(&db.local(), (&db.npm_version().as_ref()).to_string()).map(Arc::new) } -fn package_file(db: &dyn DependenciesProvider) -> Result> { - get_package_file(&db.local()).map(Rc::new) +fn package_file(db: &dyn DependenciesProvider) -> Result> { + get_package_file(&db.local()).map(Arc::new) } -fn package_file_main(db: &dyn DependenciesProvider) -> Result> { +fn package_file_main(db: &dyn DependenciesProvider) -> Result> { db.package_file() .map(|package_file| package_file.main.clone()) - .map(Rc::new) + .map(Arc::new) } diff --git a/hipcheck/src/data/query/github.rs b/hipcheck/src/data/query/github.rs index f52ac387..07196d01 100644 --- a/hipcheck/src/data/query/github.rs +++ b/hipcheck/src/data/query/github.rs @@ -6,7 +6,7 @@ use crate::error::Error; use crate::error::Result; use crate::source::source::Remote; use crate::source::source::SourceQuery; -use std::rc::Rc; +use std::sync::Arc; /// Queries about a remote GitHub source #[salsa::query_group(GitHubProviderStorage)] @@ -15,13 +15,13 @@ pub trait GitHubProvider: SourceQuery { /// /// Prefer using the other queries in this group over using /// the `Remote` directly - fn remote_github(&self) -> Result>; + fn remote_github(&self) -> Result>; /// Returns the repository owner - fn owner(&self) -> Result>; + fn owner(&self) -> Result>; /// Returns the repository name - fn repo(&self) -> Result>; + fn repo(&self) -> Result>; /// Returns the pull request number if this is a single pull /// request query. Returns an error otherwise. @@ -31,7 +31,7 @@ pub trait GitHubProvider: SourceQuery { /// Derived query implementations. Return values are wrapped in an /// `Rc` to keep cloning cheap. -fn remote_github(db: &dyn GitHubProvider) -> Result> { +fn remote_github(db: &dyn GitHubProvider) -> Result> { let remote = db .remote() .ok_or_else(|| Error::msg("git source is not remote"))?; @@ -42,24 +42,24 @@ fn remote_github(db: &dyn GitHubProvider) -> Result> { } } -fn owner(db: &dyn GitHubProvider) -> Result> { +fn owner(db: &dyn GitHubProvider) -> Result> { let remote = db.remote_github()?; match remote.as_ref() { - Remote::GitHub { owner, .. } => Ok(Rc::new(owner.to_string())), - Remote::GitHubPr { owner, .. } => Ok(Rc::new(owner.to_string())), + Remote::GitHub { owner, .. } => Ok(Arc::new(owner.to_string())), + Remote::GitHubPr { owner, .. } => Ok(Arc::new(owner.to_string())), _ => Err(Error::msg( "unsupported remote host (supported: github.com)", )), } } -fn repo(db: &dyn GitHubProvider) -> Result> { +fn repo(db: &dyn GitHubProvider) -> Result> { let remote = db.remote_github()?; match remote.as_ref() { - Remote::GitHub { repo, .. } => Ok(Rc::new(repo.to_string())), - Remote::GitHubPr { repo, .. } => Ok(Rc::new(repo.to_string())), + Remote::GitHub { repo, .. } => Ok(Arc::new(repo.to_string())), + Remote::GitHubPr { repo, .. } => Ok(Arc::new(repo.to_string())), _ => Err(Error::msg( "unsupported remote host (supported: github.com)", )), diff --git a/hipcheck/src/data/query/module.rs b/hipcheck/src/data/query/module.rs index da0a2756..fc12a4f8 100644 --- a/hipcheck/src/data/query/module.rs +++ b/hipcheck/src/data/query/module.rs @@ -11,41 +11,41 @@ use crate::error::Error; use crate::error::Result; use pathbuf::pathbuf; use std::path::PathBuf; -use std::rc::Rc; +use std::sync::Arc; /// A module and an associated commit -pub type ModuleCommitMap = Rc, Rc)>>; +pub type ModuleCommitMap = Arc, Arc)>>; /// Queries about modules #[salsa::query_group(ModuleProviderStorage)] pub trait ModuleProvider: GitProvider { /// Returns output of module analysis on the source code. #[salsa::dependencies] - fn get_module_graph(&self) -> Result>; + fn get_module_graph(&self) -> Result>; /// Returns an association list of modules and commits fn commits_for_modules(&self) -> Result; /// Returns the commits associated with a particular module - fn commits_for_module(&self, module: Rc) -> Result>>>; + fn commits_for_module(&self, module: Arc) -> Result>>>; /// Returns the modules associated with a particular commit - fn modules_for_commit(&self, commit: Rc) -> Result>>>; + fn modules_for_commit(&self, commit: Arc) -> Result>>>; /// Returns the directory containing the data files #[salsa::input] - fn data_dir(&self) -> Rc; + fn data_dir(&self) -> Arc; /// Returns the location of the module-deps.js file - fn module_deps(&self) -> Result>; + fn module_deps(&self) -> Result>; } /// Derived query implementations. Return values are wrapped in an /// `Rc` to keep cloning cheap. -fn get_module_graph(db: &dyn ModuleProvider) -> Result> { +fn get_module_graph(db: &dyn ModuleProvider) -> Result> { let module_deps = db.module_deps()?; - ModuleGraph::get_module_graph_from_repo(&db.local(), module_deps.as_ref()).map(Rc::new) + ModuleGraph::get_module_graph_from_repo(&db.local(), module_deps.as_ref()).map(Arc::new) } fn commits_for_modules(db: &dyn ModuleProvider) -> Result { @@ -55,31 +55,37 @@ fn commits_for_modules(db: &dyn ModuleProvider) -> Result { associate_modules_and_commits(repo_path.as_ref(), modules, commits) } -fn commits_for_module(db: &dyn ModuleProvider, module: Rc) -> Result>>> { +fn commits_for_module( + db: &dyn ModuleProvider, + module: impl AsRef, +) -> Result>>> { let commits = db .commits_for_modules()? .iter() - .filter_map(|(m, c)| if m == &module { Some(c.clone()) } else { None }) + .filter_map(|(m, c)| (**m == *module.as_ref()).then(|| Arc::clone(c))) .collect(); - Ok(Rc::new(commits)) + Ok(Arc::new(commits)) } -fn modules_for_commit(db: &dyn ModuleProvider, commit: Rc) -> Result>>> { +fn modules_for_commit( + db: &dyn ModuleProvider, + commit: impl AsRef, +) -> Result>>> { let modules = db .commits_for_modules()? .iter() - .filter_map(|(m, c)| if c == &commit { Some(m.clone()) } else { None }) + .filter_map(|(m, c)| (**c == *commit.as_ref()).then(|| Arc::clone(m))) .collect(); - Ok(Rc::new(modules)) + Ok(Arc::new(modules)) } -fn module_deps(db: &dyn ModuleProvider) -> Result> { +fn module_deps(db: &dyn ModuleProvider) -> Result> { let data_path = db.data_dir(); let module_deps_path = pathbuf![data_path.as_ref(), "module-deps.js"]; if module_deps_path.exists() { - Ok(Rc::new(module_deps_path)) + Ok(Arc::new(module_deps_path)) } else { Err(Error::msg( "module-deps.js missing from Hipcheck data folder", diff --git a/hipcheck/src/data/query/pr_review.rs b/hipcheck/src/data/query/pr_review.rs index c8f18935..bd65dc01 100644 --- a/hipcheck/src/data/query/pr_review.rs +++ b/hipcheck/src/data/query/pr_review.rs @@ -16,50 +16,52 @@ use crate::data::PullRequest; use crate::data::SinglePullRequest; use crate::error::Error; use crate::error::Result; -use std::rc::Rc; +use std::sync::Arc; /// A query that provides GitHub pull request reviews #[salsa::query_group(PullRequestReviewProviderStorage)] pub trait PullRequestReviewProvider: ConfigSource + GitHubProvider { /// Returns the available pull request reviews - fn pull_request_reviews(&self) -> Result>>>; + fn pull_request_reviews(&self) -> Result>>>; /// Returns the available single pull request review - fn single_pull_request_review(&self) -> Result>; + fn single_pull_request_review(&self) -> Result>; /// Returns the commits associated with a contributor for a single pull request fn get_pr_commits_for_contributor( &self, - contributor: Rc, + contributor: Arc, ) -> Result; /// Returns the contributors associed with a commit for a single pull request - fn get_pr_contributors_for_commit(&self, commit: Rc) -> Result; + fn get_pr_contributors_for_commit(&self, commit: Arc) -> Result; /// Returns all commit-diff pairs - fn get_pr_commit_diffs(&self) -> Result>>; + fn get_pr_commit_diffs(&self) -> Result>>; } /// Derived query implementations. The returned `PullRequest` values /// are wrapped in an `Rc` to keep cloning cheap and let other types /// hold references to them. -fn pull_request_reviews(db: &dyn PullRequestReviewProvider) -> Result>>> { +fn pull_request_reviews(db: &dyn PullRequestReviewProvider) -> Result>>> { let token = db.github_api_token().ok_or_else(|| { Error::msg("missing GitHub token in environment variable HC_GITHUB_TOKEN. Please set this system variable.") })?; let prs = get_pull_request_reviews_from_github(&db.owner()?, &db.repo()?, &token)? .into_iter() - .map(Rc::new) + .map(Arc::new) .collect(); - Ok(Rc::new(prs)) + Ok(Arc::new(prs)) } /// Query implementation. The returned `SinglePullRequest` value is /// wrapped in an `Rc` to keep cloning cheap and let other types hold /// references to them. -fn single_pull_request_review(db: &dyn PullRequestReviewProvider) -> Result> { +fn single_pull_request_review( + db: &dyn PullRequestReviewProvider, +) -> Result> { let token = db.github_api_token().ok_or_else(|| { Error::msg("missing GitHub token in environment variable HC_GITHUB_TOKEN. Please set this system variable.") @@ -70,12 +72,12 @@ fn single_pull_request_review(db: &dyn PullRequestReviewProvider) -> Result, + contributor: Arc, ) -> Result { // Get the pull request let pr = db @@ -98,7 +100,7 @@ fn get_pr_commits_for_contributor( // SAFETY: This index is guaranteed to be valid in // `pr.commits` because of how it and `commit_contributors` // are constructed from `get_single_pull_request_review_from_github` - Some(Rc::clone(&pr.commits[com_con.commit_id])) + Some(Arc::clone(&pr.commits[com_con.commit_id])) } else { None } @@ -113,7 +115,7 @@ fn get_pr_commits_for_contributor( fn get_pr_contributors_for_commit( db: &dyn PullRequestReviewProvider, - commit: Rc, + commit: Arc, ) -> Result { // Get the pull request let pr = db @@ -135,8 +137,8 @@ fn get_pr_contributors_for_commit( // SAFETY: These indices are guaranteed to be valid in // `pr.contributors` because of how `commit_contributors` is // constructed from `get_single_pull_request_review_from_github1. - let author = Rc::clone(&pr.contributors[com_con.author_id]); - let committer = Rc::clone(&pr.contributors[com_con.committer_id]); + let author = Arc::clone(&pr.contributors[com_con.author_id]); + let committer = Arc::clone(&pr.contributors[com_con.committer_id]); CommitContributorView { commit, @@ -147,17 +149,17 @@ fn get_pr_contributors_for_commit( .ok_or_else(|| Error::msg("failed to find contributor info")) } -fn get_pr_commit_diffs(db: &dyn PullRequestReviewProvider) -> Result>> { +fn get_pr_commit_diffs(db: &dyn PullRequestReviewProvider) -> Result>> { let pr = db .single_pull_request_review() .context("failed to get pull request")?; let commit_diffs = Iterator::zip(pr.commits.iter(), pr.diffs.iter()) .map(|(commit, diff)| CommitDiff { - commit: Rc::clone(commit), - diff: Rc::clone(diff), + commit: Arc::clone(commit), + diff: Arc::clone(diff), }) .collect(); - Ok(Rc::new(commit_diffs)) + Ok(Arc::new(commit_diffs)) } diff --git a/hipcheck/src/metric/activity.rs b/hipcheck/src/metric/activity.rs index abd12ae5..4878c160 100644 --- a/hipcheck/src/metric/activity.rs +++ b/hipcheck/src/metric/activity.rs @@ -8,8 +8,8 @@ use chrono::Duration; use serde::ser::SerializeStruct; use serde::Serialize; use serde::Serializer; -use std::rc::Rc; use std::result::Result as StdResult; +use std::sync::Arc; #[derive(Debug, Eq, PartialEq)] pub struct ActivityOutput { @@ -38,7 +38,7 @@ impl Serialize for ActivityOutput { } } -pub(crate) fn activity_metric(db: &dyn MetricProvider) -> Result> { +pub(crate) fn activity_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running activity metric"); // Get today's date. @@ -54,7 +54,7 @@ pub(crate) fn activity_metric(db: &dyn MetricProvider) -> Result, + pub commit: Arc, pub affiliated_type: AffiliatedType, } @@ -60,7 +60,7 @@ impl AffiliatedType { } } -pub(crate) fn affiliation_metric(db: &dyn MetricProvider) -> Result> { +pub(crate) fn affiliation_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running affiliation metric"); // Parse the Orgs file and construct an OrgSpec. @@ -81,23 +81,23 @@ pub(crate) fn affiliation_metric(db: &dyn MetricProvider) -> Result Result> { +pub(crate) fn pr_affiliation_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running pull request affiliation metric"); // Parse the Orgs file and construct an OrgSpec. @@ -121,20 +121,20 @@ pub(crate) fn pr_affiliation_metric(db: &dyn MetricProvider) -> Result>, + pub binary_files: Vec>, } /// Determine which files in a repository are of a binary format. @@ -17,16 +17,16 @@ pub struct BinaryOutput { /// Collect the paths to all non-plaintext files, filter out non-code /// binaries (like images or audio, which may be valid parts of a project's /// source), and return the rest to be counted for Hipcheck's report. -pub fn binary_metric(db: &dyn MetricProvider) -> Result> { +pub fn binary_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running binary metric"); let pathbuf_rc = db.local(); let binary_files = detect_binary_files(&pathbuf_rc)? .into_iter() - .try_filter(|f| db.is_likely_binary_file(Rc::clone(f))) + .try_filter(|f| db.is_likely_binary_file(Arc::clone(f))) .collect::>()?; log::info!("completed binary metric"); - Ok(Rc::new(BinaryOutput { binary_files })) + Ok(Arc::new(BinaryOutput { binary_files })) } diff --git a/hipcheck/src/metric/binary_detector/mod.rs b/hipcheck/src/metric/binary_detector/mod.rs index 089afc33..2eef67e8 100644 --- a/hipcheck/src/metric/binary_detector/mod.rs +++ b/hipcheck/src/metric/binary_detector/mod.rs @@ -19,8 +19,8 @@ use std::fs::File; use std::io::prelude::Read; use std::io::BufReader; use std::path::Path; -use std::rc::Rc; use std::result::Result as StdResult; +use std::sync::Arc; use walkdir::DirEntry; use walkdir::WalkDir; @@ -159,9 +159,9 @@ fn fetch_entries(dir: &Path) -> Result> { } /// Searches `dir` for any binary files and records their paths as Strings. -pub fn detect_binary_files(dir: &Path) -> Result>> { +pub fn detect_binary_files(dir: &Path) -> Result>> { let path_entries = fetch_entries(dir)?; - let mut possible_binary: Vec> = Vec::new(); + let mut possible_binary: Vec> = Vec::new(); // Inspect the first 4K of each file for telltale signs of binary data. // Store a String of each Path that leads to a binary file. @@ -178,7 +178,7 @@ pub fn detect_binary_files(dir: &Path) -> Result>> { let _bytes_read = reader.take(SAMPLE_SIZE).read_to_end(&mut contents)?; if inspect(&contents) == ContentType::BINARY { possible_binary.push(match entry.path().strip_prefix(dir)?.to_str() { - Some(p) => Rc::new(String::from(p)), + Some(p) => Arc::new(String::from(p)), None => { return Err(hc_error!( "path was not valid unicode: '{}'", diff --git a/hipcheck/src/metric/binary_detector/query.rs b/hipcheck/src/metric/binary_detector/query.rs index d67b4734..30b8efbe 100644 --- a/hipcheck/src/metric/binary_detector/query.rs +++ b/hipcheck/src/metric/binary_detector/query.rs @@ -6,28 +6,28 @@ use crate::config::PracticesConfigQuery; use crate::context::Context as _; use crate::error::Result; use crate::metric::binary_detector::BinaryFileDetector; -use std::rc::Rc; +use std::sync::Arc; /// Queries related to binary file detection #[salsa::query_group(BinaryFileStorage)] pub trait BinaryFile: PracticesConfigQuery { /// Returns the binary file detector for the current session - fn binary_file_detector(&self) -> Result>; + fn binary_file_detector(&self) -> Result>; /// Returns likely binary file assessment for `file_name` - fn is_likely_binary_file(&self, file_name: Rc) -> Result; + fn is_likely_binary_file(&self, file_name: Arc) -> Result; } /// Derived query implementations -fn binary_file_detector(db: &dyn BinaryFile) -> Result> { +fn binary_file_detector(db: &dyn BinaryFile) -> Result> { let detector = BinaryFileDetector::load(db.binary_formats_file().as_ref()) .context("failed to build a binary file detector from binary format file")?; - Ok(Rc::new(detector)) + Ok(Arc::new(detector)) } -fn is_likely_binary_file(db: &dyn BinaryFile, file_name: Rc) -> Result { +fn is_likely_binary_file(db: &dyn BinaryFile, file_name: Arc) -> Result { let detector = db .binary_file_detector() .context("failed to get binary file detector")?; diff --git a/hipcheck/src/metric/churn.rs b/hipcheck/src/metric/churn.rs index faffe4a5..52cf10fa 100644 --- a/hipcheck/src/metric/churn.rs +++ b/hipcheck/src/metric/churn.rs @@ -12,7 +12,7 @@ use crate::TryFilter; use crate::F64; use serde::Serialize; use std::collections::HashMap; -use std::rc::Rc; +use std::sync::Arc; #[derive(Debug, Eq, PartialEq, Serialize)] pub struct ChurnOutput { @@ -21,11 +21,11 @@ pub struct ChurnOutput { #[derive(Debug, Eq, PartialEq, Serialize)] pub struct CommitChurnFreq { - pub commit: Rc, + pub commit: Arc, pub churn: F64, } -pub fn churn_metric(db: &dyn MetricProvider) -> Result> { +pub fn churn_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running churn metric"); let commit_diffs = db.commit_diffs().context("failed to get commit diffs")?; @@ -36,7 +36,7 @@ pub fn churn_metric(db: &dyn MetricProvider) -> Result> { cd.diff .file_diffs .iter() - .try_any(|fd| db.is_likely_source_file(Rc::clone(&fd.file_name))) + .try_any(|fd| db.is_likely_source_file(Arc::clone(&fd.file_name))) }) .collect::>>()?; @@ -49,7 +49,7 @@ pub fn churn_metric(db: &dyn MetricProvider) -> Result> { .diff .file_diffs .iter() - .try_filter(|file_diff| db.is_likely_source_file(Rc::clone(&file_diff.file_name))) + .try_filter(|file_diff| db.is_likely_source_file(Arc::clone(&file_diff.file_name))) .collect::>>()?; // Update files changed. @@ -69,7 +69,7 @@ pub fn churn_metric(db: &dyn MetricProvider) -> Result> { total_lines_changed += lines_changed; commit_churns.push(CommitChurn { - commit: Rc::clone(&commit_diff.commit), + commit: Arc::clone(&commit_diff.commit), files_changed, lines_changed, }); @@ -118,7 +118,7 @@ pub fn churn_metric(db: &dyn MetricProvider) -> Result> { .unwrap(); CommitChurnFreq { - commit: Rc::clone(&commit_churn.commit), + commit: Arc::clone(&commit_churn.commit), churn, } }) @@ -148,12 +148,12 @@ pub fn churn_metric(db: &dyn MetricProvider) -> Result> { log::info!("completed churn metric"); - Ok(Rc::new(ChurnOutput { commit_churn_freqs })) + Ok(Arc::new(ChurnOutput { commit_churn_freqs })) } #[derive(Debug)] pub struct CommitChurn { - commit: Rc, + commit: Arc, files_changed: i64, lines_changed: i64, } diff --git a/hipcheck/src/metric/commit_trust.rs b/hipcheck/src/metric/commit_trust.rs index 57297e2c..5e4e767e 100644 --- a/hipcheck/src/metric/commit_trust.rs +++ b/hipcheck/src/metric/commit_trust.rs @@ -4,17 +4,17 @@ use crate::context::Context as _; use crate::error::Result; use crate::metric::MetricProvider; use std::collections::HashMap; -use std::rc::Rc; +use std::sync::Arc; pub const TRUST_PHASE: &str = "commit trust"; #[derive(Debug, Eq, PartialEq)] pub struct CommitTrustOutput { - pub commit_counts_in_period: Rc>, + pub commit_counts_in_period: Arc>, } //Add metric to check if a commit is trusted based on whether its author and committer are trusted (depends on trust metric) -pub fn commit_trust_metric(db: &dyn MetricProvider) -> Result> { +pub fn commit_trust_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running {} metric", TRUST_PHASE); let contributor_trust_map = db @@ -34,7 +34,9 @@ pub fn commit_trust_metric(db: &dyn MetricProvider) -> Result Result>, + pub contributor_counts_in_period: Arc>, } #[derive(Debug, Eq, PartialEq, Clone)] @@ -23,14 +23,14 @@ pub struct ContributorTrustResult { //contributor_trust_metric is here mainly for if contributor trust is applied to full repos //it is not currently used -pub fn contributor_trust_metric(db: &dyn MetricProvider) -> Result> { +pub fn contributor_trust_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running {} metric", TRUST_PHASE); let month_range = db.contributor_trust_month_count_threshold().to_string(); let value_threshold = db.contributor_trust_value_threshold() as u32; - let commit_from_date = Rc::new(month_range); + let commit_from_date = Arc::new(month_range); let msg = format!("failed to get commits for {} metric", TRUST_PHASE); @@ -45,7 +45,9 @@ pub fn contributor_trust_metric(db: &dyn MetricProvider) -> Result Result= value_threshold on master //Does not count merges, merges excluded -pub fn pr_contributor_trust_metric(db: &dyn MetricProvider) -> Result> { +pub fn pr_contributor_trust_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running {} metric", PR_TRUST_PHASE); let month_range = db.contributor_trust_month_count_threshold().to_string(); let value_threshold = db.contributor_trust_value_threshold() as u32; - let commit_from_date = Rc::new(month_range); + let commit_from_date = Arc::new(month_range); let msg = format!("failed to get commits for {} metric", PR_TRUST_PHASE); @@ -111,7 +113,9 @@ pub fn pr_contributor_trust_metric(db: &dyn MetricProvider) -> Result Result Result Result> { +pub fn entropy_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running entropy metric"); // Calculate the grapheme frequencies for each commit which contains code. @@ -70,7 +70,7 @@ pub fn entropy_metric(db: &dyn MetricProvider) -> Result> { // Convert to Z-scores and return results. let entropy_output = z_scores(commit_entropies) .map(EntropyOutput::new) - .map(Rc::new)?; + .map(Arc::new)?; log::info!("completed entropy metric"); @@ -96,7 +96,7 @@ impl EntropyOutput { #[derive(Debug, Eq, PartialEq, Serialize)] pub struct CommitEntropy { /// The commit - pub commit: Rc, + pub commit: Arc, /// The entropy score pub entropy: F64, } @@ -105,7 +105,7 @@ pub struct CommitEntropy { #[derive(Debug)] struct CommitGraphemeFreq { /// The commit. - commit: Rc, + commit: Arc, /// The set of grapheme frequencies. grapheme_freqs: Vec, } @@ -145,7 +145,7 @@ fn is_likely_source_file( .diff .file_diffs .iter() - .try_any(|fd| db.is_likely_source_file(Rc::clone(&fd.file_name))) + .try_any(|fd| db.is_likely_source_file(Arc::clone(&fd.file_name))) } /// Calculate grapheme frequencies for each commit. @@ -153,7 +153,7 @@ fn grapheme_freqs(commit_diff: &CommitDiff, db: &dyn MetricProvider) -> Result, ) -> CommitEntropy { - let commit = Rc::clone(&commit_freq.commit); + let commit = Arc::clone(&commit_freq.commit); let entropy = F64::new( commit_freq .grapheme_freqs @@ -245,7 +245,7 @@ impl GraphemeFreqCalculator { let mut cgf = GraphemeFreqCalculator::default(); for file_diff in &diff.file_diffs { - if db.is_likely_source_file(Rc::clone(&file_diff.file_name))? + if db.is_likely_source_file(Arc::clone(&file_diff.file_name))? && file_diff.patch.is_empty().not() { for line in file_diff.patch.lines() { diff --git a/hipcheck/src/metric/fuzz.rs b/hipcheck/src/metric/fuzz.rs index fab473a2..7fd126be 100644 --- a/hipcheck/src/metric/fuzz.rs +++ b/hipcheck/src/metric/fuzz.rs @@ -5,14 +5,14 @@ use crate::data::Fuzz; use crate::error::Result; use crate::metric::MetricProvider; use serde::Serialize; -use std::rc::Rc; +use std::sync::Arc; #[derive(Debug, Eq, PartialEq, Serialize)] pub struct FuzzOutput { pub fuzz_result: Fuzz, } -pub fn fuzz_metric(db: &dyn MetricProvider) -> Result> { +pub fn fuzz_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running fuzz metric"); let fuzz_result = db @@ -21,5 +21,5 @@ pub fn fuzz_metric(db: &dyn MetricProvider) -> Result> { log::info!("completed fuzz metric"); - Ok(Rc::new(FuzzOutput { fuzz_result })) + Ok(Arc::new(FuzzOutput { fuzz_result })) } diff --git a/hipcheck/src/metric/identity.rs b/hipcheck/src/metric/identity.rs index d87911ac..c648a79a 100644 --- a/hipcheck/src/metric/identity.rs +++ b/hipcheck/src/metric/identity.rs @@ -5,7 +5,7 @@ use crate::data::git::Commit; use crate::error::Result; use crate::metric::MetricProvider; use serde::Serialize; -use std::rc::Rc; +use std::sync::Arc; #[derive(Debug, Eq, PartialEq, Serialize)] pub struct IdentityOutput { @@ -14,11 +14,11 @@ pub struct IdentityOutput { #[derive(Debug, Eq, PartialEq, Serialize)] pub struct Match { - pub commit: Rc, + pub commit: Arc, pub identities_match: bool, } -pub fn identity_metric(db: &dyn MetricProvider) -> Result> { +pub fn identity_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running identity metric"); let commits = db.commits().context("failed to get commits")?; @@ -27,18 +27,18 @@ pub fn identity_metric(db: &dyn MetricProvider) -> Result> { for commit in commits.iter() { let commit_view = db - .contributors_for_commit(Rc::clone(commit)) + .contributors_for_commit(Arc::clone(commit)) .context("failed to get commits")?; let identities_match = commit_view.author == commit_view.committer; matches.push(Match { - commit: Rc::clone(commit), + commit: Arc::clone(commit), identities_match, }); } log::info!("completed identity metric"); - Ok(Rc::new(IdentityOutput { matches })) + Ok(Arc::new(IdentityOutput { matches })) } diff --git a/hipcheck/src/metric/linguist/query.rs b/hipcheck/src/metric/linguist/query.rs index 64459403..952e7c28 100644 --- a/hipcheck/src/metric/linguist/query.rs +++ b/hipcheck/src/metric/linguist/query.rs @@ -6,28 +6,28 @@ use crate::config::LanguagesConfigQuery; use crate::context::Context; use crate::error::Result; use crate::metric::linguist::SourceFileDetector; -use std::rc::Rc; +use std::sync::Arc; /// Queries related to source file language detection #[salsa::query_group(LinguistStorage)] pub trait Linguist: LanguagesConfigQuery { /// Returns the code detector for the current session - fn source_file_detector(&self) -> Result>; + fn source_file_detector(&self) -> Result>; /// Returns likely source file assessment for `file_name` - fn is_likely_source_file(&self, file_name: Rc) -> Result; + fn is_likely_source_file(&self, file_name: Arc) -> Result; } /// Derived query implementations -fn source_file_detector(db: &dyn Linguist) -> Result> { +fn source_file_detector(db: &dyn Linguist) -> Result> { let detector = SourceFileDetector::load(db.langs_file().as_ref()) .context("failed to build a source file detector from langs file")?; - Ok(Rc::new(detector)) + Ok(Arc::new(detector)) } -fn is_likely_source_file(db: &dyn Linguist, file_name: Rc) -> Result { +fn is_likely_source_file(db: &dyn Linguist, file_name: Arc) -> Result { let detector = db .source_file_detector() .context("failed to get source file detector")?; diff --git a/hipcheck/src/metric/math.rs b/hipcheck/src/metric/math.rs index 0f75dced..c32bbc58 100644 --- a/hipcheck/src/metric/math.rs +++ b/hipcheck/src/metric/math.rs @@ -3,6 +3,8 @@ /// Calculate the arithmetic mean for a set of floats. Returns an option to account /// for the possibility of dividing by zero. pub fn mean(data: &[f64]) -> Option { + // Do not use rayon's parallel iter/sum here due to the non-associativity of floating point numbers/math. + // See: https://en.wikipedia.org/wiki/Associative_property#Nonassociativity_of_floating_point_calculation. let sum = data.iter().sum::(); let count = data.len(); diff --git a/hipcheck/src/metric/metric.rs b/hipcheck/src/metric/metric.rs index 8fea704e..cfd3534a 100644 --- a/hipcheck/src/metric/metric.rs +++ b/hipcheck/src/metric/metric.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 -use std::rc::Rc; +use std::sync::Arc; use crate::config::AttacksConfigQuery; use crate::config::CommitConfigQuery; @@ -41,61 +41,61 @@ pub trait MetricProvider: { /// Returns result of activity metric #[salsa::invoke(activity::activity_metric)] - fn activity_metric(&self) -> Result>; + fn activity_metric(&self) -> Result>; /// Returns result of affiliation metric #[salsa::invoke(affiliation::affiliation_metric)] - fn affiliation_metric(&self) -> Result>; + fn affiliation_metric(&self) -> Result>; /// Returns result of binary metric #[salsa::invoke(binary::binary_metric)] - fn binary_metric(&self) -> Result>; + fn binary_metric(&self) -> Result>; /// Returns result of churn metric #[salsa::invoke(churn::churn_metric)] - fn churn_metric(&self) -> Result>; + fn churn_metric(&self) -> Result>; /// Returns result of contributor trust metric #[salsa::invoke(commit_trust::commit_trust_metric)] - fn commit_trust_metric(&self) -> Result>; + fn commit_trust_metric(&self) -> Result>; /// Returns result of contributor trust metric #[salsa::invoke(contributor_trust::contributor_trust_metric)] - fn contributor_trust_metric(&self) -> Result>; + fn contributor_trust_metric(&self) -> Result>; /// Returns result of entropy metric #[salsa::invoke(entropy::entropy_metric)] - fn entropy_metric(&self) -> Result>; + fn entropy_metric(&self) -> Result>; /// Returns result of identity metric #[salsa::invoke(identity::identity_metric)] - fn identity_metric(&self) -> Result>; + fn identity_metric(&self) -> Result>; /// Returns result of module analysis. #[salsa::invoke(module::module_analysis)] - fn module_analysis(&self) -> Result>; + fn module_analysis(&self) -> Result>; /// Returns result of fuzz metric #[salsa::invoke(fuzz::fuzz_metric)] - fn fuzz_metric(&self) -> Result>; + fn fuzz_metric(&self) -> Result>; /// Returns result of review metric #[salsa::invoke(review::review_metric)] - fn review_metric(&self) -> Result>; + fn review_metric(&self) -> Result>; /// Returns result of typo metric #[salsa::invoke(typo::typo_metric)] - fn typo_metric(&self) -> Result>; + fn typo_metric(&self) -> Result>; /// Returns result of pull request affiliation metric #[salsa::invoke(affiliation::pr_affiliation_metric)] - fn pr_affiliation_metric(&self) -> Result>; + fn pr_affiliation_metric(&self) -> Result>; /// Returns result of pull request contributor trust metric #[salsa::invoke(contributor_trust::pr_contributor_trust_metric)] - fn pr_contributor_trust_metric(&self) -> Result>; + fn pr_contributor_trust_metric(&self) -> Result>; /// Returns result of pull request module contributors metric #[salsa::invoke(module_contributors::pr_module_contributors_metric)] - fn pr_module_contributors_metric(&self) -> Result>; + fn pr_module_contributors_metric(&self) -> Result>; } diff --git a/hipcheck/src/metric/module.rs b/hipcheck/src/metric/module.rs index d4f934c2..ea4be4bb 100644 --- a/hipcheck/src/metric/module.rs +++ b/hipcheck/src/metric/module.rs @@ -5,15 +5,15 @@ use crate::data::ModuleGraph; use crate::error::Result; use crate::metric::MetricProvider; use serde::Serialize; -use std::rc::Rc; +use std::sync::Arc; #[derive(Debug, Eq, PartialEq, Serialize)] pub struct ModuleOutput { - pub module_graph: Rc, + pub module_graph: Arc, pub is_modular: bool, } -pub fn module_analysis(db: &dyn MetricProvider) -> Result> { +pub fn module_analysis(db: &dyn MetricProvider) -> Result> { log::debug!("running module analysis"); let module_graph = db @@ -29,5 +29,5 @@ pub fn module_analysis(db: &dyn MetricProvider) -> Result> { log::info!("completed module analysis"); - Ok(Rc::new(modules)) + Ok(Arc::new(modules)) } diff --git a/hipcheck/src/metric/module_contributors.rs b/hipcheck/src/metric/module_contributors.rs index e33eb61d..80da1d34 100644 --- a/hipcheck/src/metric/module_contributors.rs +++ b/hipcheck/src/metric/module_contributors.rs @@ -7,22 +7,22 @@ use crate::error::Result; use crate::metric::MetricProvider; use serde::Serialize; use std::collections::HashMap; -use std::rc::Rc; +use std::sync::Arc; #[derive(Debug, Eq, PartialEq, Serialize)] pub struct ModuleContributorsOutput { - pub contributors_map: Rc, Vec>>, + pub contributors_map: Arc, Vec>>, } #[derive(Debug, Eq, PartialEq, Serialize)] pub struct ContributedModule { - pub module: Rc, + pub module: Arc, pub new_contributor: bool, } pub fn pr_module_contributors_metric( db: &dyn MetricProvider, -) -> Result> { +) -> Result> { log::debug!("running pull request module contributors metric"); let pull_request = db @@ -115,7 +115,7 @@ pub fn pr_module_contributors_metric( final_contributors_map.insert(key.to_owned().to_owned(), module_vector); } - Ok(Rc::new(ModuleContributorsOutput { - contributors_map: Rc::new(final_contributors_map), + Ok(Arc::new(ModuleContributorsOutput { + contributors_map: Arc::new(final_contributors_map), })) } diff --git a/hipcheck/src/metric/review.rs b/hipcheck/src/metric/review.rs index dc8e1901..7bf00eaa 100644 --- a/hipcheck/src/metric/review.rs +++ b/hipcheck/src/metric/review.rs @@ -5,7 +5,7 @@ use crate::data::PullRequest; use crate::error::Result; use crate::metric::MetricProvider; use serde::Serialize; -use std::rc::Rc; +use std::sync::Arc; #[derive(Debug, Eq, PartialEq, Serialize)] pub struct ReviewOutput { @@ -14,11 +14,11 @@ pub struct ReviewOutput { #[derive(Debug, Eq, PartialEq, Serialize)] pub struct PullReview { - pub pull_request: Rc, + pub pull_request: Arc, pub has_review: bool, } -pub fn review_metric(db: &dyn MetricProvider) -> Result> { +pub fn review_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running review metric"); let pull_requests = db @@ -32,12 +32,12 @@ pub fn review_metric(db: &dyn MetricProvider) -> Result> { for pull_request in pull_requests.as_ref() { let has_review = pull_request.reviews > 0; pull_reviews.push(PullReview { - pull_request: pull_request.clone(), + pull_request: Arc::clone(pull_request), has_review, }); } log::info!("completed review metric"); - Ok(Rc::new(ReviewOutput { pull_reviews })) + Ok(Arc::new(ReviewOutput { pull_reviews })) } diff --git a/hipcheck/src/metric/typo.rs b/hipcheck/src/metric/typo.rs index 094513f3..dd8d2ece 100644 --- a/hipcheck/src/metric/typo.rs +++ b/hipcheck/src/metric/typo.rs @@ -15,8 +15,8 @@ use std::convert::AsRef; use std::fmt; use std::fmt::Display; use std::path::Path; -use std::rc::Rc; use std::str; +use std::sync::Arc; #[derive(Debug, Eq, PartialEq, Serialize)] pub struct TypoOutput { @@ -25,11 +25,11 @@ pub struct TypoOutput { #[derive(Debug, Eq, PartialEq, Serialize)] pub struct TypoDep { - pub dependency: Rc, + pub dependency: Arc, pub typo: Typo, } -pub fn typo_metric(db: &dyn MetricProvider) -> Result> { +pub fn typo_metric(db: &dyn MetricProvider) -> Result> { log::debug!("running typo metric"); let typo_file = TypoFile::load_from(&db.typo_file()).context("failed to load typo file")?; @@ -50,8 +50,8 @@ pub fn typo_metric(db: &dyn MetricProvider) -> Result> { fn typos_for_javascript( typo_file: &TypoFile, - dependencies: Rc, -) -> Result> { + dependencies: Arc, +) -> Result> { let mut typos = Vec::new(); for legit_name in &typo_file.languages.javascript { @@ -60,14 +60,14 @@ fn typos_for_javascript( for dependency in &dependencies.deps { for typo in fuzzer.fuzz(dependency) { typos.push(TypoDep { - dependency: dependency.clone(), + dependency: Arc::clone(&dependency), typo: typo.clone(), }) } } } - Ok(Rc::new(TypoOutput { typos })) + Ok(Arc::new(TypoOutput { typos })) } #[derive(Debug, Deserialize)] diff --git a/hipcheck/src/report.rs b/hipcheck/src/report.rs index c371f292..c4bd1228 100644 --- a/hipcheck/src/report.rs +++ b/hipcheck/src/report.rs @@ -26,8 +26,8 @@ use std::hash::Hash; use std::hash::Hasher; use std::iter::Iterator; use std::ops::Not as _; -use std::rc::Rc; use std::result::Result as StdResult; +use std::sync::Arc; /// The format to report results in. #[derive(Debug, Default, Clone, Copy, clap::ValueEnum)] @@ -60,10 +60,10 @@ pub enum AnyReport { #[schemars(crate = "schemars")] pub struct Report { /// The name of the repository being analyzed. - pub repo_name: Rc, + pub repo_name: Arc, /// The HEAD commit hash of the repository during analysis. - pub repo_head: Rc, + pub repo_head: Arc, /// The version of Hipcheck used to analyze the repo. pub hipcheck_version: String, @@ -792,10 +792,10 @@ impl Eq for Concern {} #[schemars(crate = "schemars")] pub struct PrReport { /// The URI of the pull request being analyzed. - pub pr_uri: Rc, + pub pr_uri: Arc, /// The HEAD commit hash of the repository during analysis. - pub repo_head: Rc, + pub repo_head: Arc, /// The version of Hipcheck used to analyze the repo. pub hipcheck_version: String, diff --git a/hipcheck/src/session/session.rs b/hipcheck/src/session/session.rs index 25926989..cea15c54 100644 --- a/hipcheck/src/session/session.rs +++ b/hipcheck/src/session/session.rs @@ -56,6 +56,7 @@ use std::path::Path; use std::path::PathBuf; use std::rc::Rc; use std::result::Result as StdResult; +use std::sync::Arc; /// Immutable configuration and base data for a run of Hipcheck. #[salsa::database( @@ -178,7 +179,7 @@ impl Session { session.set_config_dir(Rc::new(config_dir)); // Set data folder location for module analysis - session.set_data_dir(Rc::new(data_dir)); + session.set_data_dir(Arc::new(data_dir)); // Set github token in salsa session.set_github_api_token(Some(Rc::new(hc_github_token))); @@ -205,7 +206,7 @@ impl Session { Err(err) => return Err((session.shell, err)), }; - session.set_source(Rc::new(source)); + session.set_source(Arc::new(source)); /*=================================================================== * Resolving the Hipcheck version. diff --git a/hipcheck/src/shell.rs b/hipcheck/src/shell.rs index 11c4fad7..a10dd05e 100644 --- a/hipcheck/src/shell.rs +++ b/hipcheck/src/shell.rs @@ -95,6 +95,7 @@ use std::cell::Cell; use std::fmt; use std::fmt::Debug; use std::fmt::Formatter; +use std::io; use std::io::stderr; use std::io::stdout; use std::io::IsTerminal as _; @@ -806,6 +807,15 @@ impl Output { is_atty, } } + + /// Create a new [`Output`] that prints/writes to the void using [io::Sink]. + #[allow(dead_code)] + pub fn sink() -> Output { + Output { + out: Box::new(io::sink()), + is_atty: false, + } + } } impl Debug for Output { diff --git a/hipcheck/src/source/query.rs b/hipcheck/src/source/query.rs index befce935..963340fa 100644 --- a/hipcheck/src/source/query.rs +++ b/hipcheck/src/source/query.rs @@ -7,7 +7,7 @@ use crate::source::source::Remote; use crate::source::source::Source; use pathbuf::pathbuf; use std::path::PathBuf; -use std::rc::Rc; +use std::sync::Arc; /// Queries for accessing info about a Git source #[salsa::query_group(SourceQueryStorage)] @@ -15,20 +15,20 @@ pub trait SourceQuery: salsa::Database { /// Returns the input `Source` struct #[salsa::input] #[salsa::query_type(SourceTypeQuery)] - fn source(&self) -> Rc; + fn source(&self) -> Arc; /// Returns the local path to the repository - fn local(&self) -> Rc; + fn local(&self) -> Arc; /// Returns remote source information, if any - fn remote(&self) -> Option>; + fn remote(&self) -> Option>; /// Returns the repository HEAD commit - fn head(&self) -> Rc; + fn head(&self) -> Arc; /// Returns the repository name - fn name(&self) -> Rc; + fn name(&self) -> Arc; /// Returns the repository url - fn url(&self) -> Option>; + fn url(&self) -> Option>; /// Returns a filesystem-usable storage path for the source - fn storage_path(&self) -> Rc; + fn storage_path(&self) -> Arc; } /// Derived query implementations @@ -40,34 +40,34 @@ pub trait SourceQuery: salsa::Database { // PANIC: It is safe to unwrap in these functions, because a valid // `Source` will always contain a `SourceRepo`, so `get_repo()` will // not return an error here. -fn local(db: &dyn SourceQuery) -> Rc { +fn local(db: &dyn SourceQuery) -> Arc { let source = db.source(); - Rc::new(source.get_repo().local().to_path_buf()) + Arc::new(source.get_repo().local().to_path_buf()) } -fn remote(db: &dyn SourceQuery) -> Option> { +fn remote(db: &dyn SourceQuery) -> Option> { db.source() .get_remote() .as_ref() - .map(|remote| Rc::new(remote.clone())) + .map(|remote| Arc::new(remote.clone())) } -fn head(db: &dyn SourceQuery) -> Rc { +fn head(db: &dyn SourceQuery) -> Arc { let source = db.source(); - Rc::new(source.get_repo().head().to_string()) + Arc::new(source.get_repo().head().to_string()) } -fn name(db: &dyn SourceQuery) -> Rc { +fn name(db: &dyn SourceQuery) -> Arc { let source = db.source(); - Rc::new(source.get_repo().name().to_string()) + Arc::new(source.get_repo().name().to_string()) } -fn url(db: &dyn SourceQuery) -> Option> { - Some(Rc::new(db.remote()?.url().to_string())) +fn url(db: &dyn SourceQuery) -> Option> { + Some(Arc::new(db.remote()?.url().to_string())) } // Computes the appropriate path based on the remote or local info -fn storage_path(db: &dyn SourceQuery) -> Rc { +fn storage_path(db: &dyn SourceQuery) -> Arc { use crate::source::source::Remote::*; let path_buf = match db.remote() { @@ -91,5 +91,5 @@ fn storage_path(db: &dyn SourceQuery) -> Rc { }, }; - Rc::new(path_buf) + Arc::new(path_buf) } diff --git a/hipcheck/src/target.rs b/hipcheck/src/target.rs new file mode 100644 index 00000000..a5b2f792 --- /dev/null +++ b/hipcheck/src/target.rs @@ -0,0 +1,47 @@ +use clap::ValueEnum; +use serde::Serialize; + +#[derive(Debug, Clone, ValueEnum, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum TargetType { + Maven, + Npm, + Patch, + Pypi, + Repo, + Request, + Spdx, +} + +impl TargetType { + pub fn try_resolve_from_target(tgt: &str) -> Option { + use TargetType::*; + if let Some(pkg) = tgt.strip_prefix("pkg:") { + if let Some((pkg_type, _)) = pkg.split_once('/') { + // Match on purl package type + match pkg_type { + "github" => Some(Repo), + "npm" => Some(Npm), + "maven" => Some(Maven), + "pypi" => Some(Pypi), + _ => None, + } + } else { + None + } + } else if tgt.starts_with("https://github.com/") { + Some(Repo) + } else if tgt.ends_with(".spdx") { + Some(Spdx) + } else { + None + } + } + pub fn as_str(&self) -> String { + use serde_json::{to_value, Value}; + let Ok(Value::String(out)) = to_value(self) else { + unreachable!(); + }; + out + } +} From 30922de183d8f5f4f50beb1268f807c7d7af30d5 Mon Sep 17 00:00:00 2001 From: Venus Xeon-Blonde Date: Wed, 19 Jun 2024 15:40:13 -0400 Subject: [PATCH 5/5] chore: Satisfy `clippy` by requiring `Send + Sync` in several places and replacing `filter_map` with `filter` and `map` in two places. --- hipcheck/src/context.rs | 2 +- hipcheck/src/data/query/module.rs | 6 ++++-- hipcheck/src/error.rs | 22 +++++++++++----------- hipcheck/src/metric/typo.rs | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/hipcheck/src/context.rs b/hipcheck/src/context.rs index ea838973..5900bffc 100644 --- a/hipcheck/src/context.rs +++ b/hipcheck/src/context.rs @@ -49,7 +49,7 @@ impl Context for Result { impl Context for Result where - E: StdError + 'static, + E: StdError + Send + Sync + 'static, { fn context(self, context: C) -> Result where diff --git a/hipcheck/src/data/query/module.rs b/hipcheck/src/data/query/module.rs index fc12a4f8..5810d8b2 100644 --- a/hipcheck/src/data/query/module.rs +++ b/hipcheck/src/data/query/module.rs @@ -62,7 +62,8 @@ fn commits_for_module( let commits = db .commits_for_modules()? .iter() - .filter_map(|(m, c)| (**m == *module.as_ref()).then(|| Arc::clone(c))) + .filter(|(m, _)| **m == *module.as_ref()) + .map(|(_, c)| Arc::clone(c)) .collect(); Ok(Arc::new(commits)) @@ -75,7 +76,8 @@ fn modules_for_commit( let modules = db .commits_for_modules()? .iter() - .filter_map(|(m, c)| (**c == *commit.as_ref()).then(|| Arc::clone(m))) + .filter(|(_, c)| **c == *commit.as_ref()) + .map(|(m, _)| Arc::clone(m)) .collect(); Ok(Arc::new(modules)) diff --git a/hipcheck/src/error.rs b/hipcheck/src/error.rs index 097f80a3..bb441e0f 100644 --- a/hipcheck/src/error.rs +++ b/hipcheck/src/error.rs @@ -15,7 +15,7 @@ use std::error::Error as StdError; use std::fmt; use std::fmt::Debug; use std::fmt::Display; -use std::rc::Rc; +use std::sync::Arc; pub type Result = std::result::Result; @@ -29,7 +29,7 @@ impl>> Introspect for T {} /// An error type compatible with Salsa. pub struct Error { /// The start of the error linked list. - head: Rc, + head: Arc, } impl Error { @@ -45,11 +45,11 @@ impl Error { /// Create a new `Error` from a source error. pub fn new(error: M) -> Self where - M: StdError + 'static, + M: StdError + Send + Sync + 'static, { Error { - head: Rc::new(ErrorNode { - current: Rc::new(error), + head: Arc::new(ErrorNode { + current: Arc::new(error), next: None, }), } @@ -69,8 +69,8 @@ impl Error { ); Error { - head: Rc::new(ErrorNode { - current: Rc::new(Message(message)), + head: Arc::new(ErrorNode { + current: Arc::new(Message(message)), next: Some(self.head), }), } @@ -85,7 +85,7 @@ impl Error { /// Allows use of `?` operator on query system entry. impl From for Error where - T: StdError + 'static, + T: StdError + Send + Sync + 'static, { fn from(std_error: T) -> Error { Error::new(std_error) @@ -95,7 +95,7 @@ where impl Clone for Error { fn clone(&self) -> Error { Error { - head: Rc::clone(&self.head), + head: Arc::clone(&self.head), } } } @@ -178,10 +178,10 @@ impl StdError for ErrorNode { } /// A reference-counted fat pointer to a standard error type. -type ErrorObj = Rc; +type ErrorObj = Arc; /// A link in the linked list. -type ErrorLink = Rc; +type ErrorLink = Arc; /// A string-only error message, which can either be a static string /// slice, or an owned string. diff --git a/hipcheck/src/metric/typo.rs b/hipcheck/src/metric/typo.rs index dd8d2ece..37e9e0df 100644 --- a/hipcheck/src/metric/typo.rs +++ b/hipcheck/src/metric/typo.rs @@ -60,7 +60,7 @@ fn typos_for_javascript( for dependency in &dependencies.deps { for typo in fuzzer.fuzz(dependency) { typos.push(TypoDep { - dependency: Arc::clone(&dependency), + dependency: Arc::clone(dependency), typo: typo.clone(), }) }