From 9010d7435c151ea56d8e58d4f4ead2ddf8ec2809 Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Tue, 28 May 2024 16:00:18 -0700 Subject: [PATCH 1/8] chore: Clean up CLI implementation. This commit is a pretty substantial internal refactoring of the CLI implementation, but is purposefully designed not to break any backwards compatibility. A number of existing flags have been deprecated and removed from the CLI help text, but in fact are still present and work as they did before. This commit also introduces `hipcheck-macros`, a procedural macro helper crate for Hipcheck. Signed-off-by: Andrew Lilley Brinker --- Cargo.lock | 11 +- Cargo.toml | 4 +- hipcheck-macros/Cargo.toml | 13 + hipcheck-macros/README.md | 12 + hipcheck-macros/src/lib.rs | 21 + hipcheck-macros/src/update.rs | 110 +++++ hipcheck/Cargo.toml | 4 +- hipcheck/src/analysis/session/mod.rs | 229 +++------- hipcheck/src/analysis/session/spdx.rs | 12 +- hipcheck/src/cli.rs | 522 ++++++++++++++++----- hipcheck/src/data/source/mod.rs | 11 +- hipcheck/src/main.rs | 628 ++++++++------------------ hipcheck/src/report.rs | 4 +- hipcheck/src/shell.rs | 19 +- 14 files changed, 857 insertions(+), 743 deletions(-) create mode 100644 hipcheck-macros/Cargo.toml create mode 100644 hipcheck-macros/README.md create mode 100644 hipcheck-macros/src/lib.rs create mode 100644 hipcheck-macros/src/update.rs diff --git a/Cargo.lock b/Cargo.lock index e6c5370e..aa8d6d31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -485,13 +485,13 @@ dependencies = [ "anyhow", "chrono", "clap", - "clap-verbosity-flag", "content_inspector", "dirs", "dotenv", "duct", "env_logger", "graphql_client", + "hipcheck-macros", "lazy_static", "libc", "log", @@ -526,6 +526,15 @@ dependencies = [ "xml-rs", ] +[[package]] +name = "hipcheck-macros" +version = "0.1.0" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.66", +] + [[package]] name = "home" version = "0.5.9" diff --git a/Cargo.toml b/Cargo.toml index 895358d3..06012287 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,8 +10,8 @@ # will default to the old resolver. resolver = "2" -# Members of the workspace, just Hipcheck and our custom task runner. -members = ["hipcheck", "xtask"] +# Members of the workspace. +members = ["hipcheck", "hipcheck-macros", "xtask"] # Make sure Hipcheck is run with `cargo run`. # diff --git a/hipcheck-macros/Cargo.toml b/hipcheck-macros/Cargo.toml new file mode 100644 index 00000000..de15402a --- /dev/null +++ b/hipcheck-macros/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "hipcheck-macros" +version = "0.1.0" +edition = "2021" +license = "Apache-2.0" + +[lib] +proc-macro = true + +[dependencies] +proc-macro2 = "1.0.84" +quote = "1.0.36" +syn = "2.0.66" diff --git a/hipcheck-macros/README.md b/hipcheck-macros/README.md new file mode 100644 index 00000000..8105ac74 --- /dev/null +++ b/hipcheck-macros/README.md @@ -0,0 +1,12 @@ + +# `hipcheck-macros` + +`hipcheck-macros` is a helper crate for [Hipcheck] which provides procedural +macros. It's not intended for use by anyone else, and generally involves +private APIs. + +## License + +`hipcheck-macros` is Apache-2.0 licensed. + +[Hipcheck]: https://github.com/mitre/hipcheck diff --git a/hipcheck-macros/src/lib.rs b/hipcheck-macros/src/lib.rs new file mode 100644 index 00000000..27532de6 --- /dev/null +++ b/hipcheck-macros/src/lib.rs @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Use the `README.md` as the crate docs. +#![doc = include_str!("../README.md")] + +mod update; + +use proc_macro::TokenStream; +use syn::{parse_macro_input, DeriveInput, Error}; + +/// Derive an implementation of the `Update` trait for the type. +#[proc_macro_derive(Update)] +pub fn derive_update(input: TokenStream) -> TokenStream { + // Parse the input. + let input = parse_macro_input!(input as DeriveInput); + + // Generate the token stream. + update::derive_update(input) + .unwrap_or_else(Error::into_compile_error) + .into() +} diff --git a/hipcheck-macros/src/update.rs b/hipcheck-macros/src/update.rs new file mode 100644 index 00000000..8131c4b3 --- /dev/null +++ b/hipcheck-macros/src/update.rs @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Apache-2.0 + +use proc_macro2::TokenStream; +use quote::quote; +use std::ops::Not as _; +use syn::{ + spanned::Spanned, Data, DataStruct, DeriveInput, Error, Field, Fields, FieldsNamed, Ident, + Result, +}; + +/// Convenience macro for producing new `syn::Error`s. +macro_rules! err { + ( $span:expr, $msg:literal ) => { + Err(Error::new($span, $msg)) + }; +} + +/// A `proc_macro2`-flavor implementation of the derive macro. +pub fn derive_update(input: DeriveInput) -> Result { + let ident = &input.ident; + let fields = extract_field_names(&input)?; + + Ok(TokenStream::from(quote! { + impl crate::cli::Update for #ident { + fn update(&mut self, other: &Self) { + #( self.#fields.update(&other.#fields ); )* + } + } + })) +} + +/// Extract field names from derive input. +fn extract_field_names(input: &DeriveInput) -> Result> { + let strukt = extract_struct(input)?; + let fields = extract_named_fields(&strukt)?; + let names = extract_field_names_from_fields(&fields[..])?; + Ok(names) +} + +/// Validate the input type has no generic parameters. +fn validate_no_generics(input: &DeriveInput) -> Result<()> { + // We don't support generic types. + let generics = input.generics.params.iter().collect::>(); + if generics.is_empty().not() { + return err!( + input.span(), + "#[derive(Update)] does not support generic types" + ); + } + + Ok(()) +} + +/// Extract a struct from the input. +fn extract_struct(input: &DeriveInput) -> Result<&DataStruct> { + validate_no_generics(input)?; + + match &input.data { + Data::Struct(struct_data) => Ok(struct_data), + Data::Enum(_) => return err!(input.span(), "#[derive(Update)] does not support enums"), + Data::Union(_) => return err!(input.span(), "#[derive(Update)] does not support unions"), + } +} + +/// Extract named fields from a struct. +fn extract_named_fields(input: &DataStruct) -> Result> { + match &input.fields { + Fields::Named(fields) => Ok(collect_named_fields(fields)), + field @ Fields::Unnamed(_) => err!( + field.span(), + "#[derive(Update)] does not support unnamed fields" + ), + field @ Fields::Unit => err!( + field.span(), + "#[derive(Update)] does not support unit structs" + ), + } +} + +/// Validate all fields are named. +fn validate_named_fields(fields: &[&Field]) -> Result<()> { + for field in fields { + if field.ident.is_none() { + return err!( + field.span(), + "#[derive(Update)] does not support unnamed fields" + ); + } + } + + Ok(()) +} + +/// Collect named fields into a convenient `Vec`. +fn collect_named_fields(fields: &FieldsNamed) -> Vec<&Field> { + fields.named.iter().collect::>() +} + +/// Extract field names from a bunch of named fields. +fn extract_field_names_from_fields(fields: &[&Field]) -> Result> { + // SAFETY: We confirm the `ident` is present, so the `unwrap` is fine. + validate_named_fields(fields)?; + + let names = fields + .iter() + .map(|field| field.ident.as_ref().unwrap().to_owned()) + .collect(); + + Ok(names) +} diff --git a/hipcheck/Cargo.toml b/hipcheck/Cargo.toml index 3c23a6a7..b15de902 100644 --- a/hipcheck/Cargo.toml +++ b/hipcheck/Cargo.toml @@ -19,12 +19,12 @@ 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", default-features = false, features = ["string", "std", "derive"] } -clap-verbosity-flag = "2.2.0" +clap = { version = "4.5.4", features = ["derive"] } dirs = "5.0.1" duct = "0.13.5" env_logger = { version = "0.11.3" } graphql_client = "0.14.0" +hipcheck-macros = { path = "../hipcheck-macros" } lazy_static = "1.4.0" libc = "0.2.155" log = "0.4.16" diff --git a/hipcheck/src/analysis/session/mod.rs b/hipcheck/src/analysis/session/mod.rs index 9209bae9..af54f54d 100644 --- a/hipcheck/src/analysis/session/mod.rs +++ b/hipcheck/src/analysis/session/mod.rs @@ -47,13 +47,13 @@ use crate::util::fs::create_dir_all; use crate::version::get_version; use crate::version::VersionQuery; use crate::version::VersionQueryStorage; +use crate::CheckKind; use crate::HIPCHECK_TOML_FILE; use chrono::prelude::*; use dotenv::var; use pathbuf::pathbuf; -use std::ffi::OsStr; -use std::ffi::OsString; use std::fmt; +use std::ops::Not as _; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; @@ -124,7 +124,7 @@ impl Session { pub fn new( shell: Shell, source_type: &Check, - source: &OsStr, + source: &str, config_path: Option, data_path: Option, home_dir: Option, @@ -146,7 +146,7 @@ impl Session { * Printing the prelude. *-----------------------------------------------------------------*/ - if let Err(err) = session.shell.prelude(source.to_string_lossy().as_ref()) { + if let Err(err) = session.shell.prelude(source) { return Err((session.shell, err)); }; @@ -282,23 +282,23 @@ fn load_config_and_data( fn load_home(_shell: &mut Shell, home_dir: Option<&Path>) -> Result { // If no env or dotenv vars set, return error as Hipcheck can not run without config set - let home = resolve_home(home_dir)?; + let home = resolve_cache(home_dir)?; Ok(home) } fn load_source( shell: &mut Shell, - source: &OsStr, + source: &str, source_type: &Check, home: &Path, ) -> Result { // Resolve the source specifier into an actual source. - let phase_desc = match source_type.check_type { - CheckType::RepoSource => "resolving git repository source", - CheckType::PrUri => "resolving git pull request source", - CheckType::PackageVersion => "resolving package source", - CheckType::SpdxDocument => "parsing SPDX document", + let phase_desc = match source_type.kind.target_kind() { + TargetKind::RepoSource => "resolving git repository source", + TargetKind::PrUri => "resolving git pull request source", + TargetKind::PackageVersion => "resolving package source", + TargetKind::SpdxDocument => "parsing SPDX document", _ => return Err(hc_error!("source specified was not a valid source")), }; @@ -317,141 +317,50 @@ fn resolve_token() -> Result { } } -/// Resolves a home location for Hipcheck to cache data. -pub fn resolve_home(home_flag: Option<&Path>) -> Result { - // 1. Prefer --home flag if it is set use home_dir parameter - // 2. Prefer HC_HOME if it is set in env or .env file. - // 3. Otherwise, use cross platform cache directory as a default. - // `$XDG_CACHE_HOME` or `$HOME/.cache` on Linux, - // `$HOME/Library/Caches` on macOS, - // `{FOLDERID_LocalAppData}` on Windows - // (See https://docs.rs/dirs/3.0.2/dirs/fn.cache_dir.html) - - if let Some(home_dir) = home_flag { - if home_dir.exists() { - return Ok(home_dir.to_owned()); - } - - return Err(hc_error!( - "home directory {} (from --home) does not exist", - home_dir.display() - )); - } - - if let Ok(home_dir) = dotenv::var("HC_HOME").map(PathBuf::from) { - if home_dir.exists() { - return Ok(home_dir); - } - - return Err(hc_error!( - "home directory {} (from HC_HOME) does not exist", - home_dir.display() - )); - } - - if let Some(cache_dir) = dirs::cache_dir() { - // We should always be fine to create the cache directory if it doesn't exist already. - let home_dir = pathbuf![&cache_dir, "hipcheck"]; - - create_dir_all(&home_dir).context("failed to create Hipcheck home directory")?; - - return Ok(home_dir); +/// Resolves a cache location for Hipcheck to cache data. +pub fn resolve_cache(cache_flag: Option<&Path>) -> Result { + let path = cache_flag.ok_or_else(|| { + hc_error!("can't find cache folder (try setting the `--cache` flag or `HC_CACHE` environment variable)") + })?; + + if path.exists().not() { + // Try to create the cache directory if it doesn't exist. + create_dir_all(path).context(format!( + "failed to create cache folder '{}'", + path.display() + ))?; } - Err(hc_error!("can't find Hipcheck home (try setting the `HC_HOME` environment variable or `--home ` flag)")) + Ok(path.to_owned()) } /// Resolves a config folder location for Hipcheck to to find config files in +#[allow(clippy::size_of_ref)] pub fn resolve_config(config_flag: Option<&Path>) -> Result { - // 1. Prefer --config flag parameter if it exists as path - // 2. Prefer HC_CONFIG if it is set in env or .env file. - // 3. Otherwise, use cross platform cache directory as a default. - // `$XDG_CONFIG_HOME` or `$HOME/.config` on Linux, - // `$HOME/Library/Application Support` on macOS, - // `{FOLDERID_RoamingAppData}` on Windows - // (See https://docs.rs/dirs/3.0.2/dirs/fn.cache_dir.html) - - if let Some(config_path) = config_flag { - let full_config_path = pathbuf![config_path, HIPCHECK_TOML_FILE]; - if full_config_path.exists() { - return Ok(full_config_path); - } + let path = config_flag.ok_or_else(|| { + hc_error!("can't find config file (try setting the `--config` flag or `HC_CONFIG` environment variable)") + })?; - return Err(hc_error!( - "config file {} (from --config) does not exist", - full_config_path.display() - )); - } + let path = pathbuf![&path, HIPCHECK_TOML_FILE]; - if let Ok(config_path) = dotenv::var("HC_CONFIG").map(PathBuf::from) { - let full_config_path = pathbuf![&config_path, HIPCHECK_TOML_FILE]; - if full_config_path.exists() { - return Ok(full_config_path); - } - - return Err(hc_error!( - "config file {} (from HC_CONFIG) does not exist", - full_config_path.display() - )); + if path.exists() { + return Ok(path); } - if let Some(config_dir) = dirs::config_dir() { - if config_dir.exists() { - let config_path = pathbuf![&config_dir, "hipcheck", HIPCHECK_TOML_FILE]; - - if config_path.exists() { - return Ok(config_path); - } - } - } - - Err(hc_error!("can't find config (try setting the `HC_CONFIG` environment variable or `--config ` flag)")) + Err(hc_error!("config file '{}' does not exist", path.display())) } /// Resolves a data folder location for Hipcheck to to find data files in pub fn resolve_data(data_flag: Option<&Path>) -> Result { - // 1. Prefer --data flag parameter if it exists as path - // 2. Prefer HC_DATA if it is set in env or .env file. - // 3. Otherwise, use cross platform data directory as a default. - // `$XDG_DATA_HOME` or `$HOME/.local/share` on Linux, - // `$HOME`/Library/Application Support` on macOS, - // `{FOLDERID_RoamingAppData}` on Windows - // (See https://docs.rs/dirs/3.0.2/dirs/fn.cache_dir.html) - - if let Some(data_path) = data_flag { - if data_path.exists() { - return Ok(data_path.to_owned()); - } + let path = data_flag.ok_or_else(|| { + hc_error!("can't find data folder (try setting the `--data` flag or `HC_DATA` environment variable)") + })?; - return Err(hc_error!( - "data file {} (from --data) does not exist", - data_path.display() - )); + if path.exists() { + return Ok(path.to_owned()); } - if let Ok(data_path) = dotenv::var("HC_DATA").map(PathBuf::from) { - if data_path.exists() { - return Ok(data_path); - } - - return Err(hc_error!( - "data file {} (from HC_DATA) does not exist", - data_path.display() - )); - } - - if let Some(data_dir) = dirs::data_dir() { - if data_dir.exists() { - let data_path = pathbuf![&data_dir, "hipcheck"]; - if data_path.exists() { - return Ok(data_path); - } - } - } - - Err(hc_error!( - "can't find data (try setting the `HC_DATA` environment variable or `--data ` flag)" - )) + Err(hc_error!("data folder '{}' does not exist", path.display())) } /// Resolves the source specifier into an actual source. @@ -459,32 +368,34 @@ fn resolve_source( source_type: &Check, phase: &mut Phase, home: &Path, - source: &OsStr, + source: &str, ) -> Result { - match source_type.check_type { - CheckType::RepoSource => SourceRepo::resolve_repo(phase, home, source).map(|repo| Source { - kind: SourceKind::Repo(repo), - }), - CheckType::PackageVersion => { - let package = source.to_str().unwrap(); + match source_type.kind.target_kind() { + TargetKind::RepoSource => { + SourceRepo::resolve_repo(phase, home, source).map(|repo| Source { + kind: SourceKind::Repo(repo), + }) + } + TargetKind::PackageVersion => { + let package = source; - let command = &source_type.to_owned().parent_command_value; + let command = &source_type.to_owned().kind; - let package_git_repo_url = pm::detect_and_extract(package, command.to_owned()) + let package_git_repo_url = pm::detect_and_extract(package, command.name().to_owned()) .context("Could not get git repo URL for package")?; - SourceRepo::resolve_repo(phase, home, OsStr::new(package_git_repo_url.as_str())).map( - |repo| Source { + SourceRepo::resolve_repo(phase, home, package_git_repo_url.as_str()).map(|repo| { + Source { kind: SourceKind::Repo(repo), - }, - ) + } + }) } - CheckType::PrUri => { + TargetKind::PrUri => { SourceChangeRequest::resolve_change_request(phase, home, source).map(|cr| Source { kind: SourceKind::ChangeRequest(cr), }) } - CheckType::SpdxDocument => { + TargetKind::SpdxDocument => { let download_url = spdx::extract_download_url(source)?; SourceRepo::resolve_repo(phase, home, &download_url).map(|repo| Source { kind: SourceKind::Repo(repo), @@ -495,22 +406,30 @@ fn resolve_source( } pub struct Check { - pub check_type: CheckType, - pub check_value: OsString, - - //hc check 'parent_command_value', where parent_command_value request, repo, npm, maven, pypi etc - pub parent_command_value: String, - //pub check_url: Url, this does not seem to be used anywhere and was causing a ci error, so turning off + pub target: String, + pub kind: CheckKind, } #[derive(Debug, PartialEq, Eq)] -pub enum CheckType { +pub enum TargetKind { RepoSource, PrUri, PatchUri, PackageVersion, SpdxDocument, } + +impl TargetKind { + pub fn is_checkable(&self) -> bool { + use TargetKind::*; + + match self { + RepoSource | PrUri | PackageVersion | SpdxDocument => true, + PatchUri => false, + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -543,7 +462,7 @@ mod tests { ], || { let home_dir = None; - let result = resolve_home(home_dir).unwrap(); + let result = resolve_cache(home_dir).unwrap(); let path = result.to_str().unwrap(); if cfg!(target_os = "linux") { @@ -581,7 +500,7 @@ mod tests { // Passing in config path that does not exist let manual_flag_path = &tempdir_path; let home_flag = pathbuf![manual_flag_path]; - let result = resolve_home(Some(&home_flag)).unwrap(); + let result = resolve_cache(Some(&home_flag)).unwrap(); let path = result.to_str().unwrap(); if cfg!(target_os = "linux") @@ -614,7 +533,7 @@ mod tests { ("HC_HOME", None), ], || { - let result = resolve_home(None).unwrap(); + let result = resolve_cache(None).unwrap(); let path = result.to_str().unwrap(); if cfg!(target_os = "linux") { @@ -650,7 +569,7 @@ mod tests { ("HC_HOME", Some(&tempdir_path)), ], || { - let result = resolve_home(None).unwrap(); + let result = resolve_cache(None).unwrap(); let path = result.to_str().unwrap(); // Skip test if we cannot identify the OS assert_eq!(path, &tempdir_path); diff --git a/hipcheck/src/analysis/session/spdx.rs b/hipcheck/src/analysis/session/spdx.rs index fdd215e8..4d132a3b 100644 --- a/hipcheck/src/analysis/session/spdx.rs +++ b/hipcheck/src/analysis/session/spdx.rs @@ -6,8 +6,6 @@ use crate::context::Context as _; use crate::error::Result; use crate::hc_error; use spdx_rs::models::SPDX; -use std::ffi::OsStr; -use std::ffi::OsString; use url::Url; // The package download location field tag @@ -30,7 +28,7 @@ const SCM_HTTPS: &str = "https"; /// Extract the first compatible package download location from an /// SPDX document -pub fn extract_download_url(filepath: &OsStr) -> Result { +pub fn extract_download_url(filepath: &str) -> Result { let contents = std::fs::read_to_string(filepath)?; if contents.contains(DLOAD_LOCN_TAG) { @@ -44,7 +42,7 @@ pub fn extract_download_url(filepath: &OsStr) -> Result { // Extract the first compatible package download location from an SPDX // object obtained from a JSON file -fn extract_download_url_json(spdx: SPDX) -> Result { +fn extract_download_url_json(spdx: SPDX) -> Result { for package in spdx.package_information { if let Ok(url) = parse_download_url(&package.package_download_location) { return Ok(url); @@ -56,7 +54,7 @@ fn extract_download_url_json(spdx: SPDX) -> Result { // Extract the first compatible package download location from an SPDX // text document -fn extract_download_url_text(contents: &str) -> Result { +fn extract_download_url_text(contents: &str) -> Result { for line in contents.lines() { if let Some((DLOAD_LOCN_TAG, value)) = line.split_once(DELIMITER) { match value.trim() { @@ -74,7 +72,7 @@ fn extract_download_url_text(contents: &str) -> Result { } // Select and prepare compatible URIs and VCS locations for use -fn parse_download_url(locn: &str) -> Result { +fn parse_download_url(locn: &str) -> Result { let mut url = match locn.strip_prefix(SCM_GIT_PLUS) { Some(rest) => { // In this case, any secondary scheme is compatible @@ -93,5 +91,5 @@ fn parse_download_url(locn: &str) -> Result { // Remove element identifiers url.set_fragment(None); - Ok(url.as_str().into()) + Ok(url.as_str().to_owned()) } diff --git a/hipcheck/src/cli.rs b/hipcheck/src/cli.rs index 973766dd..04aa179f 100644 --- a/hipcheck/src/cli.rs +++ b/hipcheck/src/cli.rs @@ -2,220 +2,500 @@ //! Data structures for Hipcheck's main CLI. -/// Automatically assess and score git repositories for risk -#[derive(Debug, clap::Parser)] -#[command(about, disable_help_flag=true, disable_version_flag=true, long_about=None)] -pub struct Args { - /// print help text - #[arg(name = "extra-help", short = 'h', long = "help")] - pub extra_help: bool, - - /// print version information - #[arg(short = 'V', long, global = true)] - pub version: bool, - - /// silences progress reporting - #[arg(short = 'q', long = "quiet", global = true)] - pub verbosity: bool, - - /// set output coloring ('always', 'never', or 'auto') +use crate::analysis::session::Check; +use crate::report::Format; +use crate::shell::{ColorChoice, Verbosity}; +use crate::CheckKind; +use clap::{Parser as _, ValueEnum}; +use hipcheck_macros as hc; +use pathbuf::pathbuf; +use std::env::var_os; +use std::ffi::OsString; +use std::path::{Path, PathBuf}; + +/// Automatated supply chain risk assessment of software packages. +#[derive(Debug, Default, clap::Parser, hc::Update)] +#[command(name = "Hipcheck", about, version, long_about=None)] +pub struct CliConfig { + #[command(subcommand)] + command: Option, + + /// Arguments configuring the CLI output. + #[clap(flatten)] + output_args: OutputArgs, + + /// Arguments setting paths which Hipcheck will use. + #[clap(flatten)] + path_args: PathArgs, + + /// Soft-deprecated flags. + /// + /// The following are flags which still work, but are hidden from help text. + /// The goal in the future would be to remove these with a version break. + #[clap(flatten)] + deprecated_args: DeprecatedArgs, +} + +/// Arguments configuring Hipcheck's output. +#[derive(Debug, Default, clap::Args, hc::Update)] +struct OutputArgs { + /// How verbose to be. + #[arg( + short = 'v', + long = "verbosity", + global = true, + help_heading = "Output Flags", + long_help = "How verbose to be. Can also be set with the `HC_VERBOSITY` environment variable" + )] + verbosity: Option, + + /// When to use color. #[arg( short = 'k', - long, - default_value = "auto", - value_name = "COLOR", - global = true + long = "color", + global = true, + help_heading = "Output Flags", + long_help = "When to use color. Can also be set with the `HC_COLOR` environment variable" )] - pub color: Option, + color: Option, - /// path to the configuration file - #[arg(short, long, value_name = "FILE", global = true)] - pub config: Option, + /// What format to use. + #[arg( + short = 'f', + long = "format", + global = true, + help_heading = "Output Flags", + long_help = "What format to use. Can also be set with the `HC_FORMAT` environment variable" + )] + format: Option, +} - /// path to the data folder - #[arg(short, long, value_name = "FILE", global = true)] - pub data: Option, +/// Arguments configuring paths for Hipcheck to use. +#[derive(Debug, Default, clap::Args, hc::Update)] +struct PathArgs { + /// Path to the configuration folder. + #[arg( + short = 'c', + long = "config", + global = true, + help_heading = "Path Flags", + long_help = "Path to the configuration folder. Can also be set with the `HC_CONFIG` environment variable" + )] + config: Option, - /// path to the hipcheck home - #[arg(short = 'H', long, value_name = "FILE", global = true)] - pub home: Option, + /// Path to the data folder. + #[arg( + short = 'd', + long = "data", + global = true, + help_heading = "Path Flags", + long_help = "Path to the data folder. Can also be set with the `HC_DATA` environment variable" + )] + data: Option, - /// output results in JSON format - #[arg(short, long, global = true)] - pub json: bool, + /// Path to the cache folder. + #[arg( + short = 'C', + long = "cache", + global = true, + help_heading = "Path Flags", + long_help = "Path to the cache folder. Can also be set with the `HC_CACHE` environment variable" + )] + cache: Option, +} - #[command(subcommand)] - pub command: Option, +/// Soft-deprecated arguments, to be removed in a future version. +#[derive(Debug, Default, clap::Args, hc::Update)] +struct DeprecatedArgs { + /// Set quiet output. + #[arg(short = 'q', long = "quiet", hide = true, global = true)] + quiet: Option, + + /// Use JSON output format. + #[arg(short = 'j', long = "json", hide = true, global = true)] + json: Option, + + /// Print the home directory for Hipcheck. + #[arg(long = "print-home", hide = true, global = true)] + print_home: Option, + + /// Print the config file path for Hipcheck. + #[arg(long = "print-config", hide = true, global = true)] + print_config: Option, + + /// Print the data folder path for Hipcheck. + #[arg(long = "print-data", hide = true, global = true)] + print_data: Option, + + /// Path to the Hipcheck home folder. + #[arg(short = 'H', long = "home", hide = true, global = true)] + home: Option, } -// impl Default for Args { -// fn default() -> Self { -// command::None -// } -// } +impl CliConfig { + /// Load CLI configuration. + /// + /// This loads values in increasing order of precedence: + /// + /// - Platform-specific defaults + /// - Environment variables, if set + /// - CLI flags, if set. + /// - Final defaults, if still unset. + pub fn load() -> CliConfig { + let mut config = CliConfig::empty(); + config.update(&CliConfig::from_platform()); + config.update(&CliConfig::from_env()); + config.update(&CliConfig::from_cli()); + config + } -#[derive(Debug, clap::Subcommand)] -pub enum Commands { - /// Analyzes a repository or pull/merge request - #[command(disable_help_subcommand = true)] + /// Get the selected subcommand, if any. + pub fn subcommand(&self) -> Option { + if self.print_data() { + return Some(FullCommands::PrintData); + } + + if self.print_home() { + return Some(FullCommands::PrintCache); + } + + if self.print_config() { + return Some(FullCommands::PrintConfig); + } + + self.command.as_ref().map(FullCommands::from) + } + + /// Get the configured verbosity. + pub fn verbosity(&self) -> Verbosity { + match (self.output_args.verbosity, self.deprecated_args.quiet) { + (None, None) => Verbosity::default(), + (None, Some(quiet)) => Verbosity::use_quiet(quiet), + (Some(verbosity), None) => verbosity, + (Some(verbosity), Some(_quiet)) => { + log::warn!("verbosity specified with both -v/--verbosity and -q/--quiet; prefer -v/--verbosity"); + verbosity + } + } + } + + /// Get the configured color. + pub fn color(&self) -> ColorChoice { + self.output_args.color.unwrap_or_default() + } + + /// Get the configured format. + pub fn format(&self) -> Format { + match (self.output_args.format, self.deprecated_args.json) { + (None, None) => Format::default(), + (None, Some(json)) => Format::use_json(json), + (Some(format), None) => format, + (Some(format), Some(_json)) => { + log::warn!( + "format specified with both -f/--format and -j/--json; prefer -f/--format" + ); + format + } + } + } + + /// Get the path to the configuration directory. + pub fn config(&self) -> Option<&Path> { + self.path_args.config.as_deref() + } + + /// Get the path to the data directory. + pub fn data(&self) -> Option<&Path> { + self.path_args.data.as_deref() + } + + /// Get the path to the cache directory. + pub fn cache(&self) -> Option<&Path> { + match (&self.path_args.cache, &self.deprecated_args.home) { + (None, None) => None, + (None, Some(home)) => Some(home), + (Some(cache), None) => Some(cache), + (Some(cache), Some(_home)) => { + log::warn!("cache directory specified with both -C/--cache and -H/--home; prefer -C/--cache"); + Some(cache) + } + } + } + + /// Check if the `--print-home` flag was used. + pub fn print_home(&self) -> bool { + self.deprecated_args.print_home.unwrap_or(false) + } + + /// Check if the `--print-config` flag was used. + pub fn print_config(&self) -> bool { + self.deprecated_args.print_config.unwrap_or(false) + } + + /// Check if the `--print-data` flag was used. + pub fn print_data(&self) -> bool { + self.deprecated_args.print_data.unwrap_or(false) + } + + /// Get an empty configuration object with nothing set. + /// + /// This is just an alias for `default()`. + fn empty() -> CliConfig { + CliConfig::default() + } + + /// Load configuration from CLI flags and positional arguments. + /// + /// This is just an alias for `parse()`. + fn from_cli() -> CliConfig { + CliConfig::parse() + } + + /// Load config from environment variables. + /// + /// Note that this only loads _some_ config items from the environment. + fn from_env() -> CliConfig { + CliConfig { + output_args: OutputArgs { + verbosity: hc_env_var_value_enum("verbosity"), + color: hc_env_var_value_enum("color"), + format: hc_env_var_value_enum("format"), + }, + path_args: PathArgs { + config: hc_env_var("config"), + data: hc_env_var("data"), + cache: hc_env_var("cache"), + }, + deprecated_args: DeprecatedArgs { + home: hc_env_var("home"), + ..Default::default() + }, + ..Default::default() + } + } + + /// Load config from platform-specific information. + /// + /// Note that this only loads _some_ config items based on platform-specific + /// information. + fn from_platform() -> CliConfig { + CliConfig { + path_args: PathArgs { + config: dirs::config_dir().map(|dir| pathbuf![&dir, "hipcheck"]), + data: dirs::data_dir().map(|dir| pathbuf![&dir, "hipcheck"]), + cache: dirs::cache_dir().map(|dir| pathbuf![&dir, "hipcheck"]), + }, + ..Default::default() + } + } +} + +/// Get a Hipcheck configuration environment variable. +/// +/// This is generic in the return type, to automatically handle +/// converting from any type that can be derived from an [`OsString`]. +fn hc_env_var>(name: &'static str) -> Option { + let name = format!("HC_{}", name.to_uppercase()); + let val = var_os(name)?; + Some(O::from(val)) +} + +/// Get a Hipcheck configuration environment variable and parse it into a [`ValueEnum`] type. +fn hc_env_var_value_enum(name: &'static str) -> Option { + let ostr: OsString = hc_env_var(name)?; + + // We don't ignore case; must be fully uppercase. + let ignore_case = false; + + match ostr.into_string() { + Ok(s) => E::from_str(&s, ignore_case).ok(), + Err(_e) => { + log::warn!( + "environment variable HC_{} is not UTF-8 and will be ignored", + name.to_uppercase() + ); + None + } + } +} + +/// All commands, both subcommands and flag-like commands. +pub enum FullCommands { Check(CheckArgs), - /// Check if Hipcheck is ready to execute and reports status to user - #[command(disable_help_subcommand = true)] - Ready, - /// Print help information, optionally for a given subcommand - #[command(disable_help_subcommand = true)] - Help(HelpArgs), - /// Print the schema for JSON-format output for a specified subtarget - #[command(disable_help_subcommand = true)] Schema(SchemaArgs), + Ready, + PrintConfig, + PrintData, + PrintCache, } -impl Default for Commands { - fn default() -> Commands { - Commands::Help(HelpArgs { command: None }) +impl From<&Commands> for FullCommands { + fn from(command: &Commands) -> Self { + match command { + Commands::Check(args) => FullCommands::Check(args.clone()), + Commands::Schema(args) => FullCommands::Schema(args.clone()), + Commands::Ready => FullCommands::Ready, + } } } -#[derive(Debug, clap::Args)] -pub struct CheckArgs { - /// print help text - #[arg(short = 'h', long = "help", global = true)] - pub extra_help: bool, +#[derive(Debug, Clone, clap::Subcommand)] +pub enum Commands { + /// Analyze a package, source repository, SBOM, or pull request. + Check(CheckArgs), + /// Print the JSON schema for output of a specific `check` command. + Schema(SchemaArgs), + /// Check if Hipcheck is ready to run. + Ready, +} +#[derive(Debug, Clone, clap::Args)] +pub struct CheckArgs { #[clap(subcommand)] pub command: Option, } -#[derive(Debug, clap::Subcommand)] +#[derive(Debug, Clone, clap::Subcommand)] pub enum CheckCommand { /// Analyze a maven package git repo via package URI - #[command(disable_help_subcommand = true)] Maven(CheckMavenArgs), /// Analyze an npm package git repo via package URI or with format [@] - #[command(disable_help_subcommand = true)] Npm(CheckNpmArgs), /// Analyze 'git' patches for projects that use a patch-based workflow (not yet implemented) - #[command(disable_help_subcommand = true)] Patch(CheckPatchArgs), /// Analyze a PyPI package git repo via package URI or with format [@] - #[command(disable_help_subcommand = true)] Pypi(CheckPypiArgs), /// Analyze a repository and output an overall risk assessment - #[command(disable_help_subcommand = true)] Repo(CheckRepoArgs), /// Analyze pull/merge request for potential risks - #[command(disable_help_subcommand = true)] Request(CheckRequestArgs), /// Analyze packages specified in an SPDX document - #[command(disable_help_subcommand = true)] Spdx(CheckSpdxArgs), } -#[derive(Debug, clap::Args)] +impl CheckCommand { + pub fn as_check(&self) -> Check { + match self { + CheckCommand::Maven(args) => Check { + target: args.package.clone(), + kind: CheckKind::Maven, + }, + CheckCommand::Npm(args) => Check { + target: args.package.clone(), + kind: CheckKind::Npm, + }, + CheckCommand::Patch(args) => Check { + target: args.patch_file_uri.clone(), + kind: CheckKind::Patch, + }, + CheckCommand::Pypi(args) => Check { + target: args.package.clone(), + kind: CheckKind::Pypi, + }, + CheckCommand::Repo(args) => Check { + target: args.source.clone(), + kind: CheckKind::Repo, + }, + CheckCommand::Request(args) => Check { + target: args.pr_mr_uri.clone(), + kind: CheckKind::Request, + }, + CheckCommand::Spdx(args) => Check { + target: args.path.clone(), + kind: CheckKind::Spdx, + }, + } + } +} + +#[derive(Debug, Clone, clap::Args)] pub struct CheckMavenArgs { /// Maven package URI to analyze - #[arg(value_name = "PACKAGE", index = 1)] pub package: String, } -#[derive(Debug, clap::Args)] +#[derive(Debug, Clone, clap::Args)] pub struct CheckNpmArgs { /// NPM package URI or package[@] to analyze - #[arg(value_name = "PACKAGE", index = 1)] pub package: String, } -#[derive(Debug, clap::Args)] +#[derive(Debug, Clone, clap::Args)] pub struct CheckPatchArgs { - /// URI of git patch to analyze - #[arg(value_name = "PATCH FILE URI", index = 1)] + /// Path to Git patch file to analyze + #[arg(value_name = "PATCH FILE URI")] pub patch_file_uri: String, } -#[derive(Debug, clap::Args)] +#[derive(Debug, Clone, clap::Args)] pub struct CheckPypiArgs { /// PyPI package URI or package[@] to analyze" - #[arg(value_name = "PACKAGE", index = 1)] pub package: String, } -#[derive(Debug, clap::Args)] +#[derive(Debug, Clone, clap::Args)] pub struct CheckRepoArgs { /// Repository to analyze; can be a local path or a URI - #[arg(value_name = "SOURCE", index = 1)] pub source: String, } -#[derive(Debug, clap::Args)] +#[derive(Debug, Clone, clap::Args)] pub struct CheckRequestArgs { /// URI of pull/merge request to analyze - #[arg(value_name = "PR/MR URI", index = 1)] + #[arg(value_name = "PR/MR URI")] pub pr_mr_uri: String, } -#[derive(Debug, clap::Args)] +#[derive(Debug, Clone, clap::Args)] pub struct CheckSpdxArgs { /// SPDX document to analyze - #[arg(value_name = "FILEPATH", index = 1)] - pub filepath: String, + pub path: String, } -#[derive(Debug, clap::Args)] -pub struct HelpArgs { - #[clap(subcommand)] - pub command: Option, -} - -#[derive(Debug, clap::Subcommand)] -pub enum HelpCommand { - /// Print help information for the check subcommand - #[command(disable_help_subcommand = true)] - Check, - /// Print help information for the schema subcommand - #[command(disable_help_subcommand = true)] - Schema, -} - -#[derive(Debug, clap::Args)] +#[derive(Debug, Clone, clap::Args)] pub struct SchemaArgs { - /// print help text - #[arg(short = 'h', long = "help", global = true)] - pub extra_help: bool, - #[clap(subcommand)] pub command: Option, } -#[derive(Debug, clap::Subcommand)] +#[derive(Debug, Clone, clap::Subcommand)] pub enum SchemaCommand { - /// Print the schema for JSON-format output for running Hipcheck against a Maven package - #[command(disable_help_subcommand = true)] + /// Print the JSONN schema for running Hipcheck against a Maven package Maven, - /// Print the schema for JSON-format output for running Hipcheck against a NPM package - #[command(disable_help_subcommand = true)] + /// Print the JSON schema for running Hipcheck against a NPM package Npm, - /// Print the schema for JSON-format output for running Hipcheck against a patch - #[command(disable_help_subcommand = true)] + /// Print the JSON schema for running Hipcheck against a Git patchfile Patch, - /// Print the schema for JSON-format output for running Hipcheck against a PyPI package - #[command(disable_help_subcommand = true)] + /// Print the JSON schema for running Hipcheck against a PyPI package Pypi, - /// Print the schema for JSON-format output for running Hipcheck against a repository - #[command(disable_help_subcommand = true)] + /// Print the JSON schema for running Hipcheck against a source repository Repo, - /// Print the schema for JSON-format output for running Hipcheck against a pull/merge request - #[command(disable_help_subcommand = true)] + /// Print the JSON schema for running Hipcheck against a pull request Request, } /// Test CLI commands #[cfg(test)] mod tests { - use super::Args; + use super::CliConfig; use clap::CommandFactory; #[test] fn verify_cli() { - Args::command().debug_assert() + CliConfig::command().debug_assert() + } +} + +/// A type that can copy non-`None` values from other instances of itself. +pub trait Update { + /// Update self with the value from other, if present. + fn update(&mut self, other: &Self); +} + +impl Update for Option { + fn update(&mut self, other: &Option) { + if other.is_some() { + self.clone_from(&other); + } } } diff --git a/hipcheck/src/data/source/mod.rs b/hipcheck/src/data/source/mod.rs index c7080839..3ccbfd01 100644 --- a/hipcheck/src/data/source/mod.rs +++ b/hipcheck/src/data/source/mod.rs @@ -12,7 +12,6 @@ use crate::hc_error; use crate::shell::Phase; use log::debug; use pathbuf::pathbuf; -use std::ffi::OsStr; use std::fmt; use std::fmt::Debug; use std::fmt::Display; @@ -57,11 +56,8 @@ impl SourceRepo { /// make sure future operations are all done relative to the HEAD, and that any /// cached data records what the HEAD was at the time of caching, to enable /// cache invalidation. - pub fn resolve_repo(phase: &mut Phase, root: &Path, raw: &OsStr) -> Result { + pub fn resolve_repo(phase: &mut Phase, root: &Path, raw: &str) -> Result { let local = PathBuf::from(raw); - let raw = raw - .to_str() - .ok_or_else(|| Error::msg("source isn't UTF-8 encoded"))?; let source = match (local.exists(), local.is_dir()) { // It's a local file, not a dir. @@ -252,12 +248,9 @@ impl SourceChangeRequest { pub fn resolve_change_request( phase: &mut Phase, root: &Path, - raw: &OsStr, + raw: &str, ) -> Result { let local = PathBuf::from(raw); - let raw = raw - .to_str() - .ok_or_else(|| Error::msg("source isn't UTF-8 encoded"))?; let source = match (local.exists(), local.is_dir()) { // It's a local file, not a dir. diff --git a/hipcheck/src/main.rs b/hipcheck/src/main.rs index 0765fce3..272e70af 100644 --- a/hipcheck/src/main.rs +++ b/hipcheck/src/main.rs @@ -24,444 +24,128 @@ use crate::analysis::report_builder::PrReport; use crate::analysis::report_builder::Report; use crate::analysis::score::score_pr_results; use crate::analysis::score::score_results; +use crate::analysis::session::resolve_cache; use crate::analysis::session::resolve_config; use crate::analysis::session::resolve_data; -use crate::analysis::session::resolve_home; use crate::analysis::session::Check; -use crate::analysis::session::CheckType; use crate::analysis::session::Session; -use crate::cli::Commands; -use crate::command_util::DependentProgram; +use crate::analysis::session::TargetKind; use crate::context::Context as _; use crate::error::Error; use crate::error::Result; -use crate::shell::ColorChoice; use crate::shell::Output; use crate::shell::Shell; use crate::shell::Verbosity; use crate::util::iter::TryAny; use crate::util::iter::TryFilter; -use clap::Parser as _; -use cli::CheckCommand; -use cli::HelpCommand; +use cli::CheckArgs; +use cli::CliConfig; +use cli::FullCommands; +use cli::SchemaArgs; use cli::SchemaCommand; -use dotenv::var; -use env_logger::Builder; +use command_util::DependentProgram; +use env_logger::Builder as EnvLoggerBuilder; use env_logger::Env; use schemars::schema_for; use std::env; -use std::ffi::OsString; +use std::ops::Not as _; use std::path::Path; use std::path::PathBuf; -use std::process::exit; -use std::str::FromStr; +use std::process::ExitCode; -/// Entry point for Hipcheck. -/// -/// Sets up logging and makes sure error codes are output correctly. -fn main() { - init_log(); - exit(go().exit_code()) +fn init_logging() { + EnvLoggerBuilder::from_env(Env::new().filter("HC_LOG").write_style("HC_LOG_STYLE")).init(); } -/// The environment variable for configuring logging output. -static LOG_NAME: &str = "HC_LOG"; - -/// The environment variable for configuring logging style. -static LOG_STYLE: &str = "HC_LOG_STYLE"; - -const MAVEN: &str = CheckKind::Maven.name(); -const NPM: &str = CheckKind::Npm.name(); -const PATCH: &str = CheckKind::Patch.name(); -const PYPI: &str = CheckKind::Pypi.name(); -const REPO: &str = CheckKind::Repo.name(); -const REQUEST: &str = CheckKind::Request.name(); -const SPDX: &str = CheckKind::Spdx.name(); +/// Entry point for Hipcheck. +fn main() -> ExitCode { + init_logging(); + + let config = CliConfig::load(); + + match config.subcommand() { + Some(FullCommands::Check(args)) => return cmd_check(&args, &config), + Some(FullCommands::Schema(args)) => cmd_schema(&args), + Some(FullCommands::Ready) => cmd_ready(&config), + Some(FullCommands::PrintConfig) => cmd_print_config(config.config()), + Some(FullCommands::PrintData) => cmd_print_data(config.data()), + Some(FullCommands::PrintCache) => cmd_print_home(config.cache()), + None => print_error(&hc_error!("missing subcommand")), + } -/// Initialize the logger. -fn init_log() { - let env = Env::new().filter(LOG_NAME).write_style(LOG_STYLE); - Builder::from_env(env).init(); + ExitCode::SUCCESS } -fn go() -> Outcome { - // Get the source specifier and output directory from the user. - let args = match CliArgs::from_env().context("argument parsing failed") { - Ok(args) => args, - Err(e) => { - print_error(&e); - return Outcome::Err; +/// Run the `check` command. +fn cmd_check(args: &CheckArgs, config: &CliConfig) -> ExitCode { + let check = match &args.command { + Some(command) => command.as_check(), + None => { + print_error(&hc_error!("unknown check type")); + return ExitCode::FAILURE; } }; - let raw_version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); - - if args.check.check_type == CheckType::RepoSource - || args.check.check_type == CheckType::PrUri - || args.check.check_type == CheckType::PackageVersion - || args.check.check_type == CheckType::SpdxDocument - { - let (shell, report) = run( - Output::stdout(args.color_choice), - Output::stderr(args.color_choice), - args.verbosity, - args.check, - args.config_path, - args.data_path, - args.home_dir, - args.format, - raw_version, - ); - - let mut stdout = Output::stdout(args.color_choice); - let result = match report { - Ok(AnyReport::Report(report)) => shell.report(&mut stdout, report, args.format), - Ok(AnyReport::PrReport(pr_report)) => { - shell.pr_report(&mut stdout, pr_report, args.format) - } - Err(e) => Err(e), - }; - - match result { - Ok(_) => Outcome::Ok, - Err(e) => { - if shell.error(&e, args.format).is_err() { - print_error(&e); - } - Outcome::Err - } - } - } else { + if check.kind.target_kind().is_checkable().not() { print_missing(); } -} - -/// The arguments passed to the program from the CLI. -struct CliArgs { - /// The path to the configuration file. - config_path: Option, - - /// The path to the data folder. - data_path: Option, - - /// The path to the home/root directory. - home_dir: Option, - - /// The source specifier (local or remote). - check: Check, - - /// How verbose the output should be. - verbosity: Verbosity, - /// Whether the output should use color. - color_choice: ColorChoice, - - /// The format to use when reporting results. - format: Format, -} + let raw_version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); -impl CliArgs { - /// Pull arguments from the environment, potentially exiting with a help or version message. - fn from_env() -> Result { - let args = cli::Args::parse(); + let (shell, report) = run( + Output::stdout(config.color()), + Output::stderr(config.color()), + config.verbosity(), + check, + config.config().map(ToOwned::to_owned), + config.data().map(ToOwned::to_owned), + config.cache().map(ToOwned::to_owned), + config.format(), + raw_version, + ); - if args.extra_help { - print_help(); + match report { + Ok(AnyReport::Report(report)) => { + let _ = shell.report(&mut Output::stdout(config.color()), report, config.format()); + ExitCode::SUCCESS } - - if args.version { - print_version(); + Ok(AnyReport::PrReport(pr_report)) => { + let _ = shell.pr_report( + &mut Output::stdout(config.color()), + pr_report, + config.format(), + ); + ExitCode::SUCCESS } - - let verbosity = Verbosity::from(args.verbosity); - - let home_dir = { - let path: Option<&String> = args.home.as_ref(); - path.map(PathBuf::from) - }; - - // PANIC: Optional but has a default value, so unwrap() should never panic. - let color_choice = { - let color: &String = &args.color.unwrap(); - ColorChoice::from_str(color).unwrap() - }; - - let config_path = { - let config: Option<&String> = args.config.as_ref(); - config.map(PathBuf::from) - }; - - let data_path = { - let data: Option<&String> = args.data.as_ref(); - data.map(PathBuf::from) - }; - - let format = Format::use_json(args.json); - - // initialized later when the "check" subcommand is called - let check; - - match args.command { - Some(Commands::Help(args)) => match args.command { - None => print_help(), - Some(HelpCommand::Check) => print_check_help(), - Some(HelpCommand::Schema) => print_schema_help(), - }, - Some(Commands::Ready) => { - print_readiness( - home_dir.as_deref(), - config_path.as_deref(), - data_path.as_deref(), - ); - } - Some(Commands::Check(args)) => { - if args.extra_help { - print_check_help(); - } - match args.command { - Some(CheckCommand::Maven(args)) => { - check = Check { - check_type: CheckType::PackageVersion, - check_value: OsString::from(args.package), - parent_command_value: MAVEN.to_string(), - } - } - Some(CheckCommand::Npm(args)) => { - check = Check { - check_type: CheckType::PackageVersion, - check_value: OsString::from(args.package), - parent_command_value: NPM.to_string(), - } - } - Some(CheckCommand::Patch(args)) => { - check = Check { - check_type: CheckType::PatchUri, - check_value: OsString::from(args.patch_file_uri), - parent_command_value: PATCH.to_string(), - } - } - Some(CheckCommand::Pypi(args)) => { - check = Check { - check_type: CheckType::PackageVersion, - check_value: OsString::from(args.package), - parent_command_value: PYPI.to_string(), - } - } - Some(CheckCommand::Repo(args)) => { - check = Check { - check_type: CheckType::RepoSource, - check_value: OsString::from(args.source), - parent_command_value: REPO.to_string(), - } - } - Some(CheckCommand::Request(args)) => { - check = Check { - check_type: CheckType::PrUri, - check_value: OsString::from(args.pr_mr_uri), - parent_command_value: REQUEST.to_string(), - } - } - Some(CheckCommand::Spdx(args)) => { - check = Check { - check_type: CheckType::SpdxDocument, - check_value: OsString::from(args.filepath), - parent_command_value: SPDX.to_string(), - } - } - None => print_check_help(), - } - } - Some(Commands::Schema(args)) => { - if args.extra_help { - print_schema_help(); - } - match args.command { - Some(SchemaCommand::Maven) => print_maven_schema(), - Some(SchemaCommand::Npm) => print_npm_schema(), - Some(SchemaCommand::Patch) => print_patch_schema(), - Some(SchemaCommand::Pypi) => print_pypi_schema(), - Some(SchemaCommand::Repo) => print_report_schema(), - Some(SchemaCommand::Request) => print_request_schema(), - None => print_schema_help(), - } + Err(e) => { + if shell.error(&e, config.format()).is_err() { + print_error(&e); } - None => print_help(), + ExitCode::FAILURE } - - Ok(CliArgs { - config_path, - data_path, - home_dir, - check, - verbosity, - color_choice, - format, - }) } } -/// Global flags and options that are repeated in different help text. -const GLOBAL_HELP: &str = "\ -FLAGS: - -V, --version print version information - -OPTIONS (CONFIGURATION): - -c, --config path to the configuration file [default: ./Hipcheck.toml] - -d, --data path to the data folder, which includes the custom module_deps.js - -H, --home set hipcheck home via command flag - -OPTIONS (OUTPUT): - -j, --json output results in JSON format - -k, --color set output coloring ('always', 'never', or 'auto') [default: auto] - -q, --quiet silence progress reporting -"; - -/// Print Hipcheck's help text. -fn print_help() -> ! { - let raw_version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); - - let help_text = format!( - "\ -{} {} -{} - -USAGE: -{} [FLAGS] [OPTIONS] [] - -TASKS: - check analyzes a repository or pull/merge request - ready print a report of whether or not Hipcheck is ready to run (experimental) - schema print the schema for JSON-format output for a specified subtarget - help [] print help information, optionally for a given subcommand - -{}", - env!("CARGO_PKG_NAME"), - version::get_version(raw_version).unwrap(), - env!("CARGO_PKG_DESCRIPTION"), - env!("CARGO_BIN_NAME"), - GLOBAL_HELP - ); - - println!("{}", help_text); - exit(Outcome::Err.exit_code()); -} - -/// Print the help text for Hipcheck's schema subcommand. -fn print_schema_help() -> ! { - let raw_version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); - - let help_text = format!( - "\ -{} {} -Print the schema for JSON-format output for a specified subtarget - -USAGE: -{} [FLAGS] [OPTIONS] schema - -SUBTASKS: - repo print the schema for JSON-format output for running Hipcheck against a repository - request print the schema for JSON-format output for running Hipcheck against a pull/merge request - patch print the schema for JSON-format output for running Hipcheck against a patch - -{}", - env!("CARGO_PKG_NAME"), - version::get_version(raw_version).unwrap(), - env!("CARGO_BIN_NAME"), - GLOBAL_HELP - ); - - println!("{}", help_text); - exit(Outcome::Err.exit_code()); -} - -/// Print the help text for Hipcheck's check subcommand. -fn print_check_help() -> ! { - let raw_version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); - - let help_text = format!( - "\ -{} {} -Analyze a repository, pull/merge request, or 'git' patch - -USAGE: - {} [FLAGS] [OPTIONS] check - -SUBTASKS: - maven analyze a maven package git repo via package uri - npm analyze an npm package git repo via package uri or with format [@] - patch analyze 'git' patches for projects that use a patch-based workflow (not yet implemented) - pypi analyze a pypi package git repo via package uri or with format [@] - repo analyze a repository and output an overall risk assessment - request analyze pull/merge request for potential risks - spdx analyze packages specified in an SPDX document (not yet implemented) - -{}", - env!("CARGO_PKG_NAME"), - version::get_version(raw_version).unwrap(), - env!("CARGO_BIN_NAME"), - GLOBAL_HELP - ); - - println!("{}", help_text); - exit(Outcome::Err.exit_code()); -} - -/// Print the current version of Hipcheck. -fn print_version() -> ! { - let raw_version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); - - let version_text = format!( - "{} {}", - env!("CARGO_PKG_NAME"), - version::get_version(raw_version).unwrap() - ); - println!("{}", version_text); - exit(Outcome::Err.exit_code()); -} - -/// Print the JSON schema of the report. -fn print_report_schema() -> ! { - let schema = schema_for!(Report); - let report_text = serde_json::to_string_pretty(&schema).unwrap(); - println!("{}", report_text); - exit(Outcome::Err.exit_code()); -} - -/// Print the JSON schema of the pull/merge request. -fn print_request_schema() -> ! { - let schema = schema_for!(PrReport); - let report_text = serde_json::to_string_pretty(&schema).unwrap(); - println!("{}", report_text); - exit(Outcome::Err.exit_code()); -} - -/// Print the JSON schem a of the maven package -fn print_maven_schema() -> ! { - print_missing() -} - -/// Print the JSON schem a of the npm package -fn print_npm_schema() -> ! { - print_missing() -} - -/// Print the JSON schem a of the patch. -fn print_patch_schema() -> ! { - print_missing() +/// Run the `schema` command. +fn cmd_schema(args: &SchemaArgs) { + match args.command { + Some(SchemaCommand::Maven) => print_maven_schema(), + Some(SchemaCommand::Npm) => print_npm_schema(), + Some(SchemaCommand::Patch) => print_patch_schema(), + Some(SchemaCommand::Pypi) => print_pypi_schema(), + Some(SchemaCommand::Repo) => print_report_schema(), + Some(SchemaCommand::Request) => print_request_schema(), + None => { + print_error(&hc_error!("unknown schema type")); + } + } } -/// Print the JSON schem a of the pypi package -fn print_pypi_schema() -> ! { - print_missing() -} +fn cmd_ready(config: &CliConfig) { + let cache_path = config.cache(); + let config_path = config.config(); + let data_path = config.data(); -/// Prints a "readiness report" for Hipcheck, indicating if any dependent programs are missing or our of date -/// or if any necessary folders or files are missing. It then returns a final readiness status. -fn print_readiness( - home_dir: Option<&Path>, - config_path: Option<&Path>, - data_path: Option<&Path>, -) -> ! { let mut failed = false; // Print Hipcheck version @@ -476,8 +160,7 @@ fn print_readiness( // Check that git is installed and that its version is up to date // Print the version number either way - let git_check = data::git::get_git_version(); - match git_check { + match data::git::get_git_version() { Ok(git_version) => match DependentProgram::Git.check_version(&git_version) { // No need to check Boolean value, because currentl check_version() only returns Ok(true) or Err() Ok(_) => print!("Found installed {}", git_version), @@ -494,8 +177,7 @@ fn print_readiness( // Check that git is installed and that its version is up to date // Print the version number either way - let npm_check = data::npm::get_npm_version(); - match npm_check { + match data::npm::get_npm_version() { Ok(npm_version) => match DependentProgram::Npm.check_version(&npm_version) { // No need to check Boolean value, because currently check_version() only returns Ok(true) or Err() Ok(_) => print!("Found installed NPM version {}", npm_version), @@ -511,8 +193,7 @@ fn print_readiness( } // Check that the Hipcheck home folder is findable - let hipcheck_home = resolve_home(home_dir); - match hipcheck_home { + match resolve_cache(cache_path) { Ok(path_buffer) => println!("Hipcheck home directory: {}", path_buffer.display()), Err(err) => { failed = true; @@ -521,8 +202,7 @@ fn print_readiness( } // Check that the Hipcheck config TOML exists in the designated location - let hipcheck_config = resolve_config(config_path); - match hipcheck_config { + match resolve_config(config_path) { Ok(path_buffer) => println!("Hipcheck config file: {}", path_buffer.display()), Err(err) => { failed = true; @@ -531,8 +211,7 @@ fn print_readiness( } // Check that Hipcheck data folder is findable - let hipcheck_data = resolve_data(data_path); - match hipcheck_data { + match resolve_data(data_path) { Ok(path_buffer) => println!("Hipcheck data directory: {}", path_buffer.display()), Err(err) => { failed = true; @@ -544,26 +223,103 @@ fn print_readiness( // This does not check if the token is valid or not // The absence of a token does not trigger the failure state for the readiness check, because // Hipcheck *can* run without a token, but some analyses will not. - match var("HC_GITHUB_TOKEN") { - Ok(_) => println!("HC_GITHUB_TOKEN system environment variable found."), - Err(_) => println!("Missing HC_GITHUB_TOKEN system environment variable. Some analyses will not run without this token set."), + if std::env::var("HC_GITHUB_TOKEN").is_ok() { + println!("HC_GITHUB_TOKEN system environment variable found."); + } else { + println!("Missing HC_GITHUB_TOKEN system environment variable. Some analyses will not run without this token set."); } if failed { println!("One or more dependencies or configuration settings are missing. Hipcheck is not ready to run."); - } else { - println!("Hipcheck is ready to run!"); + return; + } + + println!("Hipcheck is ready to run!"); +} + +/// Print the current home directory for Hipcheck. +/// +/// Exits `Ok` if home directory is specified, `Err` otherwise. +fn cmd_print_home(path: Option<&Path>) { + let cache = resolve_cache(path); + + match cache { + Ok(path_buffer) => { + println!("{}", path_buffer.display()); + } + Err(err) => { + print_error(&err); + } } +} - let exit_code = Outcome::Err.exit_code(); - exit(exit_code); +/// Print the current config path for Hipcheck. +/// +/// Exits `Ok` if config path is specified, `Err` otherwise. +fn cmd_print_config(config_path: Option<&Path>) { + let config = resolve_config(config_path); + match config { + Ok(path_buffer) => { + println!("{}", path_buffer.display()); + } + Err(err) => { + print_error(&err); + } + } +} + +/// Print the current data folder path for Hipcheck. +/// +/// Exits `Ok` if config path is specified, `Err` otherwise. +fn cmd_print_data(data_path: Option<&Path>) { + let hipcheck_data = resolve_data(data_path); + match hipcheck_data { + Ok(path_buffer) => { + println!("{}", path_buffer.display()); + } + Err(err) => { + print_error(&err); + } + } +} + +/// Print the JSON schema of the report. +fn print_report_schema() { + let schema = schema_for!(Report); + let report_text = serde_json::to_string_pretty(&schema).unwrap(); + println!("{}", report_text); +} + +/// Print the JSON schema of the pull/merge request. +fn print_request_schema() { + let schema = schema_for!(PrReport); + let report_text = serde_json::to_string_pretty(&schema).unwrap(); + println!("{}", report_text); +} + +/// Print the JSON schema of the maven package +fn print_maven_schema() { + print_missing() +} + +/// Print the JSON schema of the npm package +fn print_npm_schema() { + print_missing() +} + +/// Print the JSON schema of the patch. +fn print_patch_schema() { + print_missing() +} + +/// Print the JSON schema of the pypi package +fn print_pypi_schema() { + print_missing() } /// Prints a message telling the user that this functionality has not been implemented yet -fn print_missing() -> ! { +fn print_missing() { println!("This feature is not implemented yet."); - let exit_code = Outcome::Ok.exit_code(); - exit(exit_code) } /// An `f64` that is never `NaN`. @@ -580,7 +336,7 @@ const HIPCHECK_TOML_FILE: &str = "Hipcheck.toml"; /// Indicates the program failed. const EXIT_FAILURE: i32 = 1; -//used in hc_session::pm and main.rs, global variables for hc check CheckKindHere node-ipc@9.2.1 +#[derive(Debug, Copy, Clone, PartialEq, Eq)] enum CheckKind { Repo, Request, @@ -592,6 +348,7 @@ enum CheckKind { } impl CheckKind { + /// Get the name of the check. const fn name(&self) -> &'static str { match self { CheckKind::Repo => "repo", @@ -603,6 +360,19 @@ impl CheckKind { CheckKind::Spdx => "spdx", } } + + /// Get the kind of target implied by the object being checked. + const fn target_kind(&self) -> TargetKind { + match self { + CheckKind::Repo => TargetKind::RepoSource, + CheckKind::Request => TargetKind::PrUri, + CheckKind::Patch => TargetKind::PatchUri, + CheckKind::Maven => TargetKind::PackageVersion, + CheckKind::Npm => TargetKind::PackageVersion, + CheckKind::Pypi => TargetKind::PackageVersion, + CheckKind::Spdx => TargetKind::SpdxDocument, + } + } } /// Run Hipcheck. @@ -652,7 +422,7 @@ fn run_with_shell( let session = match Session::new( shell, &check, - &check.check_value, + &check.target, config_path, data_path, home_dir, @@ -663,8 +433,8 @@ fn run_with_shell( Err((shell, err)) => return (shell, Err(err)), }; - match check.check_type { - CheckType::RepoSource | CheckType::SpdxDocument => { + match check.kind.target_kind() { + TargetKind::RepoSource | TargetKind::SpdxDocument => { // Run analyses against a repo and score the results (score calls analyses that call metrics). let mut phase = match session.shell.phase("analyzing and scoring results") { Ok(phase) => phase, @@ -695,7 +465,7 @@ fn run_with_shell( (session.end(), Ok(AnyReport::Report(report))) } - CheckType::PackageVersion => { + TargetKind::PackageVersion => { // Run analyses against a repo and score the results (score calls analyses that call metrics). let mut phase = match session.shell.phase("analyzing and scoring results") { Ok(phase) => phase, @@ -726,7 +496,7 @@ fn run_with_shell( (session.end(), Ok(AnyReport::Report(report))) } - CheckType::PrUri => { + TargetKind::PrUri => { // Run analyses against a pull request and score the results (score calls analyses that call metrics). let mut phase = match session.shell.phase("scoring and analyzing results") { Ok(phase) => phase, @@ -777,17 +547,3 @@ fn print_error(err: &Error) { eprintln!(" {}", err); } } - -enum Outcome { - Ok, - Err, -} - -impl Outcome { - fn exit_code(&self) -> i32 { - match self { - Outcome::Ok => 0, - Outcome::Err => 1, - } - } -} diff --git a/hipcheck/src/report.rs b/hipcheck/src/report.rs index 6b8d9487..c371f292 100644 --- a/hipcheck/src/report.rs +++ b/hipcheck/src/report.rs @@ -30,16 +30,16 @@ use std::rc::Rc; use std::result::Result as StdResult; /// The format to report results in. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Default, Clone, Copy, clap::ValueEnum)] pub enum Format { /// JSON format. Json, /// Human-readable format. + #[default] Human, } impl Format { - /// Set if the format is JSON. pub fn use_json(json: bool) -> Format { if json { Format::Json diff --git a/hipcheck/src/shell.rs b/hipcheck/src/shell.rs index 32c0c746..11c4fad7 100644 --- a/hipcheck/src/shell.rs +++ b/hipcheck/src/shell.rs @@ -951,20 +951,22 @@ impl PrintMode { } /// How verbose CLI output should be. -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Debug, Default, Copy, Clone, PartialEq, clap::ValueEnum)] pub enum Verbosity { - /// Don't output anything except results. + /// Output results, not progress indicators. Quiet, - /// Output the normal amount of things. + /// Output results and progress indicators. + #[default] Normal, // This one is only used in testing. - /// Don't output anything, including results. + /// Do not output anything. + #[value(hide = true)] Silent, } -impl From for Verbosity { - fn from(b: bool) -> Verbosity { - if b { +impl Verbosity { + pub fn use_quiet(quiet: bool) -> Verbosity { + if quiet { Verbosity::Quiet } else { Verbosity::Normal @@ -973,13 +975,14 @@ impl From for Verbosity { } /// Selection of whether the CLI output should use color. -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Debug, Default, Copy, Clone, PartialEq, clap::ValueEnum)] pub enum ColorChoice { /// Always use color output Always, /// Never use color output Never, /// Guess whether to use color output + #[default] Auto, } From 37b5fb693824d9cc5c6b2aae4e8dbf2a979877b6 Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Thu, 30 May 2024 11:03:47 -0700 Subject: [PATCH 2/8] chore: Refactored `ready` command. This commit refactors the ready command to separate the different ready-checks from the result printing. The goal here is to give a better user experience with all results being reported together. Signed-off-by: Andrew Lilley Brinker --- hipcheck/src/main.rs | 270 +++++++++++++++++++++++++++++++------------ 1 file changed, 197 insertions(+), 73 deletions(-) diff --git a/hipcheck/src/main.rs b/hipcheck/src/main.rs index 272e70af..5b6d95f6 100644 --- a/hipcheck/src/main.rs +++ b/hipcheck/src/main.rs @@ -44,14 +44,18 @@ use cli::FullCommands; use cli::SchemaArgs; use cli::SchemaCommand; use command_util::DependentProgram; +use core::fmt; use env_logger::Builder as EnvLoggerBuilder; use env_logger::Env; use schemars::schema_for; use std::env; +use std::fmt::Display; +use std::fmt::Formatter; use std::ops::Not as _; use std::path::Path; use std::path::PathBuf; use std::process::ExitCode; +use std::result::Result as StdResult; fn init_logging() { EnvLoggerBuilder::from_env(Env::new().filter("HC_LOG").write_style("HC_LOG_STYLE")).init(); @@ -141,100 +145,220 @@ fn cmd_schema(args: &SchemaArgs) { } } -fn cmd_ready(config: &CliConfig) { - let cache_path = config.cache(); - let config_path = config.config(); - let data_path = config.data(); +#[derive(Debug)] +struct ReadyChecks { + hipcheck_version_check: StdResult, + git_version_check: StdResult, + npm_version_check: StdResult, + config_path_check: StdResult, + data_path_check: StdResult, + cache_path_check: StdResult, + github_token_check: StdResult<(), EnvVarCheckError>, +} - let mut failed = false; +impl ReadyChecks { + /// Check if Hipcheck is ready to run. + /// + /// We don't check `github_token_check`, because it's allowed to fail. + fn is_ready(&self) -> bool { + self.hipcheck_version_check.is_ok() + && self.git_version_check.is_ok() + && self.npm_version_check.is_ok() + && self.config_path_check.is_ok() + && self.data_path_check.is_ok() + && self.cache_path_check.is_ok() + } +} - // Print Hipcheck version - let raw_version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); +#[derive(Debug)] +struct VersionCheckError { + cmd_name: &'static str, + kind: VersionCheckErrorKind, +} - let version_text = format!( - "{} {}", - env!("CARGO_PKG_NAME"), - version::get_version(raw_version).unwrap() - ); - println!("{}", version_text); - - // Check that git is installed and that its version is up to date - // Print the version number either way - match data::git::get_git_version() { - Ok(git_version) => match DependentProgram::Git.check_version(&git_version) { - // No need to check Boolean value, because currentl check_version() only returns Ok(true) or Err() - Ok(_) => print!("Found installed {}", git_version), - Err(err) => { - print_error(&err); - failed = true; +impl Display for VersionCheckError { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + match &self.kind { + VersionCheckErrorKind::CmdNotFound => { + write!(f, "command '{}' not found", self.cmd_name) } - }, - Err(err) => { - print_error(&err); - failed = true; + VersionCheckErrorKind::VersionTooOld { expected, found } => write!( + f, + "command '{}' version is too old; found {}, must be at least {}", + self.cmd_name, found, expected + ), } } +} - // Check that git is installed and that its version is up to date - // Print the version number either way - match data::npm::get_npm_version() { - Ok(npm_version) => match DependentProgram::Npm.check_version(&npm_version) { - // No need to check Boolean value, because currently check_version() only returns Ok(true) or Err() - Ok(_) => print!("Found installed NPM version {}", npm_version), - Err(err) => { - print_error(&err); - failed = true; - } - }, - Err(err) => { - print_error(&err); - failed = true; +#[derive(Debug)] +enum VersionCheckErrorKind { + CmdNotFound, + VersionTooOld { expected: String, found: String }, +} + +#[derive(Debug)] +enum PathCheckError { + PathNotFound, +} + +impl Display for PathCheckError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + PathCheckError::PathNotFound => write!(f, "path not found"), } } +} - // Check that the Hipcheck home folder is findable - match resolve_cache(cache_path) { - Ok(path_buffer) => println!("Hipcheck home directory: {}", path_buffer.display()), - Err(err) => { - failed = true; - print_error(&err); +#[derive(Debug)] +struct EnvVarCheckError { + name: &'static str, + kind: EnvVarCheckErrorKind, +} + +impl Display for EnvVarCheckError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match &self.kind { + EnvVarCheckErrorKind::VarNotFound => { + write!(f, "environment variable '{}' was not found", self.name) + } } } +} - // Check that the Hipcheck config TOML exists in the designated location - match resolve_config(config_path) { - Ok(path_buffer) => println!("Hipcheck config file: {}", path_buffer.display()), - Err(err) => { - failed = true; - print_error(&err); - } +#[derive(Debug)] +enum EnvVarCheckErrorKind { + VarNotFound, +} + +fn check_hipcheck_version() -> StdResult { + let pkg_name = env!("CARGO_PKG_NAME", "can't find Hipcheck package name"); + + let version = env!("CARGO_PKG_VERSION", "can't find Hipcheck package version"); + let version = version::get_version(version).map_err(|_| VersionCheckError { + cmd_name: "hc", + kind: VersionCheckErrorKind::CmdNotFound, + })?; + + Ok(format!("{} {}", pkg_name, version)) +} + +fn check_git_version() -> StdResult { + let version = data::git::get_git_version().map_err(|_| VersionCheckError { + cmd_name: "git", + kind: VersionCheckErrorKind::CmdNotFound, + })?; + + DependentProgram::Git + .check_version(&version) + .map(|_| version.trim().to_owned()) + .map_err(|_| VersionCheckError { + cmd_name: "git", + kind: VersionCheckErrorKind::VersionTooOld { + expected: DependentProgram::Git.min_version().unwrap().to_string(), + found: version, + }, + }) +} + +fn check_npm_version() -> StdResult { + let version = data::npm::get_npm_version() + .map(|version| version.trim().to_owned()) + .map_err(|_| VersionCheckError { + cmd_name: "npm", + kind: VersionCheckErrorKind::CmdNotFound, + })?; + + DependentProgram::Npm + .check_version(&version) + .map(|_| version.clone()) + .map_err(|_| VersionCheckError { + cmd_name: "npm", + kind: VersionCheckErrorKind::VersionTooOld { + expected: DependentProgram::Npm.min_version().unwrap().to_string(), + found: version, + }, + }) +} + +fn check_config_path(config: &CliConfig) -> StdResult { + resolve_config(config.config()).map_err(|_| PathCheckError::PathNotFound) +} + +fn check_cache_path(config: &CliConfig) -> StdResult { + resolve_cache(config.cache()).map_err(|_| PathCheckError::PathNotFound) +} + +fn check_data_path(config: &CliConfig) -> StdResult { + resolve_data(config.data()).map_err(|_| PathCheckError::PathNotFound) +} + +/// Check that a GitHub token has been provided as an environment variable +/// This does not check if the token is valid or not +/// The absence of a token does not trigger the failure state for the readiness check, because +/// Hipcheck *can* run without a token, but some analyses will not. +fn check_github_token() -> StdResult<(), EnvVarCheckError> { + let name = "HC_GITHUB_TOKEN"; + + std::env::var(name) + .map(|_| ()) + .map_err(|_| EnvVarCheckError { + name, + kind: EnvVarCheckErrorKind::VarNotFound, + }) +} + +fn cmd_ready(config: &CliConfig) { + let ready = ReadyChecks { + hipcheck_version_check: check_hipcheck_version(), + git_version_check: check_git_version(), + npm_version_check: check_npm_version(), + config_path_check: check_config_path(config), + data_path_check: check_data_path(config), + cache_path_check: check_cache_path(config), + github_token_check: check_github_token(), + }; + + match &ready.hipcheck_version_check { + Ok(version) => println!("{:<17} {}", "Hipcheck Version:", version), + Err(e) => println!("{:<17} {}", "Hipcheck Version:", e), } - // Check that Hipcheck data folder is findable - match resolve_data(data_path) { - Ok(path_buffer) => println!("Hipcheck data directory: {}", path_buffer.display()), - Err(err) => { - failed = true; - print_error(&err); - } + match &ready.git_version_check { + Ok(version) => println!("{:<17} {}", "Git Version:", version), + Err(e) => println!("{:<17} {}", "Git Version:", e), } - // Check that a GitHub token has been provided as an environment variable - // This does not check if the token is valid or not - // The absence of a token does not trigger the failure state for the readiness check, because - // Hipcheck *can* run without a token, but some analyses will not. - if std::env::var("HC_GITHUB_TOKEN").is_ok() { - println!("HC_GITHUB_TOKEN system environment variable found."); - } else { - println!("Missing HC_GITHUB_TOKEN system environment variable. Some analyses will not run without this token set."); + match &ready.npm_version_check { + Ok(version) => println!("{:<17} {}", "NPM Version:", version), + Err(e) => println!("{:<17} {}", "NPM Version:", e), } - if failed { - println!("One or more dependencies or configuration settings are missing. Hipcheck is not ready to run."); - return; + match &ready.cache_path_check { + Ok(path) => println!("{:<17} {}", "Cache Path:", path.display()), + Err(e) => println!("{:<17} {}", "Cache Path:", e), } - println!("Hipcheck is ready to run!"); + match &ready.config_path_check { + Ok(path) => println!("{:<17} {}", "Config Path:", path.display()), + Err(e) => println!("{:<17} {}", "Config Path:", e), + } + + match &ready.data_path_check { + Ok(path) => println!("{:<17} {}", "Data Path:", path.display()), + Err(e) => println!("{:<17} {}", "Data Path:", e), + } + + match &ready.github_token_check { + Ok(_) => println!("{:<17} {}", "GitHub Token:", "Found!"), + Err(e) => println!("{:<17} {}", "GitHub Token:", e), + } + + if ready.is_ready() { + println!("Hipcheck is ready to run!"); + } else { + println!("Hipheck is NOT ready to run"); + } } /// Print the current home directory for Hipcheck. From 36909d7e24a4ca7f3668370b6717c09da91cadee Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Thu, 30 May 2024 13:56:56 -0700 Subject: [PATCH 3/8] fix: Fixing broken CLI tests. Signed-off-by: Andrew Lilley Brinker --- hipcheck/src/analysis/session/mod.rs | 375 +++++++++++---------------- hipcheck/src/cli.rs | 33 +-- 2 files changed, 164 insertions(+), 244 deletions(-) diff --git a/hipcheck/src/analysis/session/mod.rs b/hipcheck/src/analysis/session/mod.rs index af54f54d..bb1e3461 100644 --- a/hipcheck/src/analysis/session/mod.rs +++ b/hipcheck/src/analysis/session/mod.rs @@ -433,286 +433,205 @@ impl TargetKind { #[cfg(test)] mod tests { use super::*; + use crate::cli::CliConfig; use crate::test_util::with_env_vars; use tempfile::TempDir; - const TEMPDIR_PREFIX: &str = "hc_test"; + const TEMPDIR_PREFIX: &str = "hipcheck"; #[test] fn resolve_token_test() { - with_env_vars(vec![("HC_GITHUB_TOKEN", Some("test"))], || { - let result = resolve_token().unwrap(); - println!("result token {}", result); - let empty = "".to_string(); - assert_ne!(result, empty); - assert_eq!(result, "test"); - }); + let vars = vec![("HC_GITHUB_TOKEN", Some("test"))]; + with_env_vars(vars, || assert_eq!(resolve_token().unwrap(), "test")); } + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] #[test] fn resolve_home_with_home_env_var() { let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir_path = tempdir.path().to_string_lossy().into_owned(); - - with_env_vars( - vec![ - ("HOME", Some(&tempdir_path)), - ("XDG_CACHE_HOME", None), - ("HC_HOME", None), - ], - || { - let home_dir = None; - let result = resolve_cache(home_dir).unwrap(); - let path = result.to_str().unwrap(); - - if cfg!(target_os = "linux") { - let expected = pathbuf![&tempdir_path, ".cache", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else if cfg!(target_os = "macos") { - let expected = pathbuf![&tempdir_path, "Library", "Caches", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else if cfg!(target_os = "windows") { - let expected = - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Local", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else { - // Skip test if we cannot identify the OS - let _path = path; - } - }, - ); - tempdir.close().unwrap(); + let vars = vec![ + ("HOME", Some(tempdir.path().to_str().unwrap())), + ("XDG_CACHE_HOME", None), + ("HC_CACHE", None), + ]; + + with_env_vars(vars, || { + let config = CliConfig::load(); + let path = resolve_cache(config.cache()).unwrap(); + + let expected = if cfg!(target_os = "linux") { + pathbuf![&tempdir.path(), ".cache", "hipcheck"] + } else if cfg!(target_os = "macos") { + pathbuf![&tempdir.path(), "Library", "Caches", "hipcheck"] + } else { + // Windows + pathbuf![&dirs::home_dir().unwrap(), "AppData", "Local", "hipcheck"] + }; + + assert_eq!(path, expected); + }); } + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] #[test] fn resolve_home_with_home_flag() { let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir_path = tempdir.path().to_string_lossy().into_owned(); - - with_env_vars( - vec![ - ("HOME", None), - ("XDG_CACHE_HOME", None), - ("HC_HOME", Some(&tempdir_path)), - ], - || { - // Passing in config path that does not exist - let manual_flag_path = &tempdir_path; - let home_flag = pathbuf![manual_flag_path]; - let result = resolve_cache(Some(&home_flag)).unwrap(); - let path = result.to_str().unwrap(); - - if cfg!(target_os = "linux") - || cfg!(target_os = "macos") - || cfg!(target_os = "windows") - { - assert_eq!(path, manual_flag_path); - } else { - // Skip test if we cannot identify the OS - let _path = path; - } - }, - ); - tempdir.close().unwrap(); + let vars = vec![ + ("HC_CACHE", None), + ("XDG_CACHE_HOME", None), + ("HC_CACHE", Some(tempdir.path().to_str().unwrap())), + ]; + + with_env_vars(vars, || { + let path = resolve_cache(Some(tempdir.path())).unwrap(); + assert_eq!(path, tempdir.path()); + }); } + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] #[test] fn resolve_home_with_xdg_cache_preferred() { let tempdir1 = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir1_path = tempdir1.path().to_string_lossy().into_owned(); - let tempdir2 = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir2_path = tempdir2.path().to_string_lossy().into_owned(); - - with_env_vars( - vec![ - ("HOME", Some(&tempdir1_path)), - ("XDG_CACHE_HOME", Some(&tempdir2_path)), - ("HC_HOME", None), - ], - || { - let result = resolve_cache(None).unwrap(); - let path = result.to_str().unwrap(); - - if cfg!(target_os = "linux") { - let expected = pathbuf![&tempdir2_path, "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else if cfg!(target_os = "macos") { - let expected = pathbuf![&tempdir1_path, "Library", "Caches", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else if cfg!(target_os = "windows") { - let expected = - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Local", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else { - // Skip test if we cannot identify the OS - let _path = path; - } - }, - ); - tempdir1.close().unwrap(); - tempdir2.close().unwrap(); + let vars = vec![ + ("HC_CACHE", Some(tempdir1.path().to_str().unwrap())), + ("XDG_CACHE_HOME", Some(tempdir2.path().to_str().unwrap())), + ("HC_CACHE", None), + ]; + + with_env_vars(vars, || { + let config = CliConfig::load(); + let path = resolve_cache(config.cache()).unwrap(); + + let expected = if cfg!(target_os = "linux") { + pathbuf![tempdir2.path(), "hipcheck"] + } else if cfg!(target_os = "macos") { + pathbuf![tempdir1.path(), "Library", "Caches", "hipcheck"] + } else { + // Windows + pathbuf![&dirs::home_dir().unwrap(), "AppData", "Local", "hipcheck"] + }; + + assert_eq!(path, expected); + }); } #[test] - fn resolve_home_with_hc_home_preferred() { + fn resolve_home_with_hc_cache_preferred() { let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir_path = tempdir.path().to_string_lossy().into_owned(); - - with_env_vars( - vec![ - ("HOME", Some("/users/foo")), - ("XDG_CACHE_HOME", Some("/xdg_cache_home")), - ("HC_HOME", Some(&tempdir_path)), - ], - || { - let result = resolve_cache(None).unwrap(); - let path = result.to_str().unwrap(); - // Skip test if we cannot identify the OS - assert_eq!(path, &tempdir_path); - }, - ); - - tempdir.close().unwrap(); + + let vars = vec![ + ("HOME", Some("/users/foo")), + ("XDG_CACHE_HOME", Some("/xdg_cache_home")), + ("HC_CACHE", Some(tempdir.path().to_str().unwrap())), + ]; + + with_env_vars(vars, || { + let config = CliConfig::load(); + assert_eq!(&resolve_cache(config.cache()).unwrap(), &tempdir.path()) + }); } + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] #[test] fn resolve_data_with_data_env_var() { let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir_path = tempdir.path().to_string_lossy().into_owned(); - - with_env_vars( - vec![ - ("HOME", Some(&tempdir_path)), - ("XDG_DATA_HOME", None), - ("HC_DATA", None), - ], - || { - let data_dir = None; - let data_path = pathbuf![&dirs::data_dir().unwrap(), "hipcheck"]; - create_dir_all(data_path.as_path()).unwrap(); - let result = resolve_data(data_dir).unwrap(); - let path = result.to_str().unwrap(); - - if cfg!(target_os = "linux") { - let expected = pathbuf![&tempdir_path, ".local", "share", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else if cfg!(target_os = "macos") { - let expected = - pathbuf![&tempdir_path, "Library", "Application Support", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else if cfg!(target_os = "windows") { - let expected = - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else { - // Skip test if we cannot identify the OS - let _path = path; - } - }, - ); - tempdir.close().unwrap(); + let vars = vec![ + ("HOME", Some(tempdir.path().to_str().unwrap())), + ("XDG_DATA_HOME", None), + ("HC_DATA", Some(tempdir.path().to_str().unwrap())), + ]; + + with_env_vars(vars, || { + let config = CliConfig::load(); + let path = resolve_data(config.data()).unwrap(); + + let expected = if cfg!(target_os = "linux") { + pathbuf![tempdir.path(), ".local", "share", "hipcheck"] + } else if cfg!(target_os = "macos") { + pathbuf![tempdir.path(), "Library", "Application Support", "hipcheck"] + } else { + // Windows + pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"] + }; + + assert_eq!(path, expected); + }); } + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] #[test] fn resolve_data_with_data_flag() { let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir_path = tempdir.path().to_string_lossy().into_owned(); - - with_env_vars( - vec![ - ("HOME", None), - ("XDG_DATA_HOME", None), - ("HC_DATA", Some(&tempdir_path)), - ], - || { - // Passing in config path that does not exist - let manual_flag_path = &tempdir_path; - let data_flag = pathbuf![manual_flag_path]; - let result = resolve_data(Some(&data_flag)).unwrap(); - let path = result.to_str().unwrap(); - - if cfg!(target_os = "linux") - || cfg!(target_os = "macos") - || cfg!(target_os = "windows") - { - assert_eq!(path, manual_flag_path); - } else { - // Skip test if we cannot identify the OS - let _path = path; - } - }, - ); - tempdir.close().unwrap(); + let vars = vec![ + ("HC_CACHE", None), + ("XDG_DATA_HOME", None), + ("HC_DATA", Some(tempdir.path().to_str().unwrap())), + ]; + + with_env_vars(vars, || { + let path = resolve_data(Some(tempdir.path())).unwrap(); + assert_eq!(path, tempdir.path()); + }); } + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] #[test] fn resolve_data_with_xdg_cache_preferred() { let tempdir1 = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir1_path = tempdir1.path().to_string_lossy().into_owned(); - let tempdir2 = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir2_path = tempdir2.path().to_string_lossy().into_owned(); - - with_env_vars( - vec![ - ("HOME", Some(&tempdir1_path)), - ("XDG_DATA_HOME", Some(&tempdir2_path)), - ("HC_HOME", None), - ("HC_DATA", None), - ], - || { - let data_path = pathbuf![&dirs::data_dir().unwrap(), "hipcheck"]; - create_dir_all(data_path.as_path()).unwrap(); - let result = resolve_data(None).unwrap(); - let path = result.to_str().unwrap(); - - if cfg!(target_os = "linux") { - let expected = pathbuf![&tempdir2_path, "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else if cfg!(target_os = "macos") { - let expected = - pathbuf![&tempdir1_path, "Library", "Application Support", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else if cfg!(target_os = "windows") { - let expected = - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"]; - assert_eq!(path, expected.to_str().unwrap()); - } else { - // Skip test if we cannot identify the OS - let _path = path; - } - }, - ); - tempdir1.close().unwrap(); - tempdir2.close().unwrap(); + let vars = vec![ + ("HOME", Some(tempdir1.path().to_str().unwrap())), + ("XDG_DATA_HOME", Some(tempdir2.path().to_str().unwrap())), + ("HC_CACHE", None), + ("HC_DATA", None), + ]; + + with_env_vars(vars, || { + // Create the data path + let data_path = pathbuf![&dirs::data_dir().unwrap(), "hipcheck"]; + create_dir_all(data_path.as_path()).unwrap(); + + let config = CliConfig::load(); + let path = resolve_data(config.data()).unwrap(); + + let expected = if cfg!(target_os = "linux") { + pathbuf![tempdir2.path(), "hipcheck"] + } else if cfg!(target_os = "macos") { + pathbuf![ + tempdir1.path(), + "Library", + "Application Support", + "hipcheck" + ] + } else { + pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"] + }; + + assert_eq!(path, expected); + }); } #[test] fn resolve_data_with_hc_data_preferred() { let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir_path = tempdir.path().to_string_lossy().into_owned(); - - with_env_vars( - vec![ - ("HOME", Some("/users/foo")), - ("XDG_DATA_HOME", Some("/xdg_cache_home")), - ("HC_DATA", Some(&tempdir_path)), - ], - || { - let result = resolve_data(None).unwrap(); - let path = result.to_str().unwrap(); - // This should work on all platforms - assert_eq!(path, &tempdir_path); - }, - ); - - tempdir.close().unwrap(); + + let vars = vec![ + ("HC_CACHE", Some("/users/foo")), + ("XDG_DATA_HOME", Some("/xdg_cache_home")), + ("HC_DATA", Some(tempdir.path().to_str().unwrap())), + ]; + + with_env_vars(vars, || { + let config = CliConfig::load(); + let path = resolve_data(config.data()).unwrap(); + assert_eq!(path, tempdir.path()); + }); } } diff --git a/hipcheck/src/cli.rs b/hipcheck/src/cli.rs index 04aa179f..c3219cf1 100644 --- a/hipcheck/src/cli.rs +++ b/hipcheck/src/cli.rs @@ -9,8 +9,6 @@ use crate::CheckKind; use clap::{Parser as _, ValueEnum}; use hipcheck_macros as hc; use pathbuf::pathbuf; -use std::env::var_os; -use std::ffi::OsString; use std::path::{Path, PathBuf}; /// Automatated supply chain risk assessment of software packages. @@ -143,6 +141,7 @@ impl CliConfig { /// - Final defaults, if still unset. pub fn load() -> CliConfig { let mut config = CliConfig::empty(); + config.update(&CliConfig::backups()); config.update(&CliConfig::from_platform()); config.update(&CliConfig::from_env()); config.update(&CliConfig::from_cli()); @@ -288,35 +287,37 @@ impl CliConfig { ..Default::default() } } + + /// Set configuration backups for paths. + fn backups() -> CliConfig { + CliConfig { + path_args: PathArgs { + config: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "config"]), + data: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "data"]), + cache: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "cache"]), + }, + ..Default::default() + } + } } /// Get a Hipcheck configuration environment variable. /// /// This is generic in the return type, to automatically handle /// converting from any type that can be derived from an [`OsString`]. -fn hc_env_var>(name: &'static str) -> Option { +fn hc_env_var>(name: &'static str) -> Option { let name = format!("HC_{}", name.to_uppercase()); - let val = var_os(name)?; + let val = dotenv::var(name).ok()?; Some(O::from(val)) } /// Get a Hipcheck configuration environment variable and parse it into a [`ValueEnum`] type. fn hc_env_var_value_enum(name: &'static str) -> Option { - let ostr: OsString = hc_env_var(name)?; + let s: String = hc_env_var(name)?; // We don't ignore case; must be fully uppercase. let ignore_case = false; - - match ostr.into_string() { - Ok(s) => E::from_str(&s, ignore_case).ok(), - Err(_e) => { - log::warn!( - "environment variable HC_{} is not UTF-8 and will be ignored", - name.to_uppercase() - ); - None - } - } + E::from_str(&s, ignore_case).ok() } /// All commands, both subcommands and flag-like commands. From 7eb9e529acd619cd36996040f87af227f81e9489 Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Sat, 1 Jun 2024 11:50:38 -0700 Subject: [PATCH 4/8] fix: Fixed broken CLI tests. This commit updates the tests for the CLI to use the new `CliConfig` API. This also involves deprecation of the old `resolve_{cache, config, data}` functions in the `session` module, as they're no longer doing anything especially meaningful. Signed-off-by: Andrew Lilley Brinker --- hipcheck/src/analysis/session/mod.rs | 266 +----------------------- hipcheck/src/cli.rs | 294 +++++++++++++++++++++++++-- hipcheck/src/main.rs | 42 ++-- 3 files changed, 321 insertions(+), 281 deletions(-) diff --git a/hipcheck/src/analysis/session/mod.rs b/hipcheck/src/analysis/session/mod.rs index bb1e3461..44ec2a03 100644 --- a/hipcheck/src/analysis/session/mod.rs +++ b/hipcheck/src/analysis/session/mod.rs @@ -43,17 +43,13 @@ use crate::report::ReportParams; use crate::report::ReportParamsStorage; use crate::shell::Phase; use crate::shell::Shell; -use crate::util::fs::create_dir_all; use crate::version::get_version; use crate::version::VersionQuery; use crate::version::VersionQueryStorage; use crate::CheckKind; -use crate::HIPCHECK_TOML_FILE; use chrono::prelude::*; use dotenv::var; -use pathbuf::pathbuf; use std::fmt; -use std::ops::Not as _; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; @@ -189,7 +185,11 @@ impl Session { * Resolving the Hipcheck home. *-----------------------------------------------------------------*/ - let home = match load_home(&mut session.shell, home_dir.as_deref()) { + let home = match home_dir + .as_deref() + .map(ToOwned::to_owned) + .ok_or_else(|| hc_error!("can't find cache directory")) + { Ok(results) => results, Err(err) => return Err((session.shell, err)), }; @@ -255,8 +255,8 @@ fn load_config_and_data( let phase = shell.phase("loading configuration and data files")?; // Resolve the path to the config file. - let valid_config_path = resolve_config(config_path) - .context("Failed to load configuration. Please make sure the path set by the hc_config env variable exists.")?; + let valid_config_path = config_path + .ok_or_else(|| hc_error!("Failed to load configuration. Please make sure the path set by the hc_config env variable exists."))?; // Get the directory the config file is in. let config_dir = valid_config_path @@ -269,8 +269,9 @@ fn load_config_and_data( .context("Failed to load configuration. Please make sure the config files are in the config directory.")?; // Get the directory the data file is in. - let data_dir = resolve_data(data_path) - .context("Failed to load data files. Please make sure the path set by the hc_data env variable exists.")?; + let data_dir = data_path + .ok_or_else(|| hc_error!("Failed to load data files. Please make sure the path set by the hc_data env variable exists."))? + .to_owned(); // Resolve the github token file. let hc_github_token = resolve_token()?; @@ -280,13 +281,6 @@ fn load_config_and_data( Ok((config, config_dir, data_dir, hc_github_token)) } -fn load_home(_shell: &mut Shell, home_dir: Option<&Path>) -> Result { - // If no env or dotenv vars set, return error as Hipcheck can not run without config set - let home = resolve_cache(home_dir)?; - - Ok(home) -} - fn load_source( shell: &mut Shell, source: &str, @@ -317,52 +311,6 @@ fn resolve_token() -> Result { } } -/// Resolves a cache location for Hipcheck to cache data. -pub fn resolve_cache(cache_flag: Option<&Path>) -> Result { - let path = cache_flag.ok_or_else(|| { - hc_error!("can't find cache folder (try setting the `--cache` flag or `HC_CACHE` environment variable)") - })?; - - if path.exists().not() { - // Try to create the cache directory if it doesn't exist. - create_dir_all(path).context(format!( - "failed to create cache folder '{}'", - path.display() - ))?; - } - - Ok(path.to_owned()) -} - -/// Resolves a config folder location for Hipcheck to to find config files in -#[allow(clippy::size_of_ref)] -pub fn resolve_config(config_flag: Option<&Path>) -> Result { - let path = config_flag.ok_or_else(|| { - hc_error!("can't find config file (try setting the `--config` flag or `HC_CONFIG` environment variable)") - })?; - - let path = pathbuf![&path, HIPCHECK_TOML_FILE]; - - if path.exists() { - return Ok(path); - } - - Err(hc_error!("config file '{}' does not exist", path.display())) -} - -/// Resolves a data folder location for Hipcheck to to find data files in -pub fn resolve_data(data_flag: Option<&Path>) -> Result { - let path = data_flag.ok_or_else(|| { - hc_error!("can't find data folder (try setting the `--data` flag or `HC_DATA` environment variable)") - })?; - - if path.exists() { - return Ok(path.to_owned()); - } - - Err(hc_error!("data folder '{}' does not exist", path.display())) -} - /// Resolves the source specifier into an actual source. fn resolve_source( source_type: &Check, @@ -433,205 +381,11 @@ impl TargetKind { #[cfg(test)] mod tests { use super::*; - use crate::cli::CliConfig; use crate::test_util::with_env_vars; - use tempfile::TempDir; - - const TEMPDIR_PREFIX: &str = "hipcheck"; #[test] fn resolve_token_test() { let vars = vec![("HC_GITHUB_TOKEN", Some("test"))]; with_env_vars(vars, || assert_eq!(resolve_token().unwrap(), "test")); } - - #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] - #[test] - fn resolve_home_with_home_env_var() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HOME", Some(tempdir.path().to_str().unwrap())), - ("XDG_CACHE_HOME", None), - ("HC_CACHE", None), - ]; - - with_env_vars(vars, || { - let config = CliConfig::load(); - let path = resolve_cache(config.cache()).unwrap(); - - let expected = if cfg!(target_os = "linux") { - pathbuf![&tempdir.path(), ".cache", "hipcheck"] - } else if cfg!(target_os = "macos") { - pathbuf![&tempdir.path(), "Library", "Caches", "hipcheck"] - } else { - // Windows - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Local", "hipcheck"] - }; - - assert_eq!(path, expected); - }); - } - - #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] - #[test] - fn resolve_home_with_home_flag() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HC_CACHE", None), - ("XDG_CACHE_HOME", None), - ("HC_CACHE", Some(tempdir.path().to_str().unwrap())), - ]; - - with_env_vars(vars, || { - let path = resolve_cache(Some(tempdir.path())).unwrap(); - assert_eq!(path, tempdir.path()); - }); - } - - #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] - #[test] - fn resolve_home_with_xdg_cache_preferred() { - let tempdir1 = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir2 = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HC_CACHE", Some(tempdir1.path().to_str().unwrap())), - ("XDG_CACHE_HOME", Some(tempdir2.path().to_str().unwrap())), - ("HC_CACHE", None), - ]; - - with_env_vars(vars, || { - let config = CliConfig::load(); - let path = resolve_cache(config.cache()).unwrap(); - - let expected = if cfg!(target_os = "linux") { - pathbuf![tempdir2.path(), "hipcheck"] - } else if cfg!(target_os = "macos") { - pathbuf![tempdir1.path(), "Library", "Caches", "hipcheck"] - } else { - // Windows - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Local", "hipcheck"] - }; - - assert_eq!(path, expected); - }); - } - - #[test] - fn resolve_home_with_hc_cache_preferred() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HOME", Some("/users/foo")), - ("XDG_CACHE_HOME", Some("/xdg_cache_home")), - ("HC_CACHE", Some(tempdir.path().to_str().unwrap())), - ]; - - with_env_vars(vars, || { - let config = CliConfig::load(); - assert_eq!(&resolve_cache(config.cache()).unwrap(), &tempdir.path()) - }); - } - - #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] - #[test] - fn resolve_data_with_data_env_var() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HOME", Some(tempdir.path().to_str().unwrap())), - ("XDG_DATA_HOME", None), - ("HC_DATA", Some(tempdir.path().to_str().unwrap())), - ]; - - with_env_vars(vars, || { - let config = CliConfig::load(); - let path = resolve_data(config.data()).unwrap(); - - let expected = if cfg!(target_os = "linux") { - pathbuf![tempdir.path(), ".local", "share", "hipcheck"] - } else if cfg!(target_os = "macos") { - pathbuf![tempdir.path(), "Library", "Application Support", "hipcheck"] - } else { - // Windows - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"] - }; - - assert_eq!(path, expected); - }); - } - - #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] - #[test] - fn resolve_data_with_data_flag() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HC_CACHE", None), - ("XDG_DATA_HOME", None), - ("HC_DATA", Some(tempdir.path().to_str().unwrap())), - ]; - - with_env_vars(vars, || { - let path = resolve_data(Some(tempdir.path())).unwrap(); - assert_eq!(path, tempdir.path()); - }); - } - - #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] - #[test] - fn resolve_data_with_xdg_cache_preferred() { - let tempdir1 = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - let tempdir2 = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HOME", Some(tempdir1.path().to_str().unwrap())), - ("XDG_DATA_HOME", Some(tempdir2.path().to_str().unwrap())), - ("HC_CACHE", None), - ("HC_DATA", None), - ]; - - with_env_vars(vars, || { - // Create the data path - let data_path = pathbuf![&dirs::data_dir().unwrap(), "hipcheck"]; - create_dir_all(data_path.as_path()).unwrap(); - - let config = CliConfig::load(); - let path = resolve_data(config.data()).unwrap(); - - let expected = if cfg!(target_os = "linux") { - pathbuf![tempdir2.path(), "hipcheck"] - } else if cfg!(target_os = "macos") { - pathbuf![ - tempdir1.path(), - "Library", - "Application Support", - "hipcheck" - ] - } else { - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"] - }; - - assert_eq!(path, expected); - }); - } - - #[test] - fn resolve_data_with_hc_data_preferred() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HC_CACHE", Some("/users/foo")), - ("XDG_DATA_HOME", Some("/xdg_cache_home")), - ("HC_DATA", Some(tempdir.path().to_str().unwrap())), - ]; - - with_env_vars(vars, || { - let config = CliConfig::load(); - let path = resolve_data(config.data()).unwrap(); - assert_eq!(path, tempdir.path()); - }); - } } diff --git a/hipcheck/src/cli.rs b/hipcheck/src/cli.rs index c3219cf1..d659b424 100644 --- a/hipcheck/src/cli.rs +++ b/hipcheck/src/cli.rs @@ -475,18 +475,6 @@ pub enum SchemaCommand { Request, } -/// Test CLI commands -#[cfg(test)] -mod tests { - use super::CliConfig; - use clap::CommandFactory; - - #[test] - fn verify_cli() { - CliConfig::command().debug_assert() - } -} - /// A type that can copy non-`None` values from other instances of itself. pub trait Update { /// Update self with the value from other, if present. @@ -500,3 +488,285 @@ impl Update for Option { } } } + +/// Test CLI commands +#[cfg(test)] +mod tests { + use super::*; + use crate::cli::CliConfig; + use crate::test_util::with_env_vars; + use clap::CommandFactory; + use tempfile::TempDir; + + const TEMPDIR_PREFIX: &str = "hipcheck"; + + #[test] + fn verify_cli() { + CliConfig::command().debug_assert() + } + + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] + #[test] + fn resolve_cache_with_platform() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", Some(tempdir.path().to_str().unwrap())), + ("XDG_CACHE_HOME", None), + ("HC_CACHE", None), + ]; + + with_env_vars(vars, || { + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp + }; + + let expected = if cfg!(target_os = "linux") { + pathbuf![&tempdir.path(), ".cache", "hipcheck"] + } else if cfg!(target_os = "macos") { + pathbuf![&tempdir.path(), "Library", "Caches", "hipcheck"] + } else { + // Windows + pathbuf![&dirs::home_dir().unwrap(), "AppData", "Local", "hipcheck"] + }; + + assert_eq!(config.cache().unwrap(), expected); + }); + } + + #[test] + fn resolve_cache_with_env_var() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", None), + ("XDG_CACHE_HOME", None), + ("HC_CACHE", Some(tempdir.path().to_str().unwrap())), + ]; + + with_env_vars(vars, || { + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp + }; + + assert_eq!(config.cache().unwrap(), tempdir.path()); + }); + } + + #[test] + fn resolve_cache_with_flag() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", Some(tempdir.path().to_str().unwrap())), + ("XDG_CACHE_HOME", None), + ("HC_CACHE", None), + ]; + + with_env_vars(vars, || { + let expected = pathbuf![tempdir.path(), "hipcheck"]; + + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp.update(&CliConfig { + path_args: PathArgs { + cache: Some(expected.clone()), + ..Default::default() + }, + ..Default::default() + }); + temp + }; + + assert_eq!(config.cache().unwrap(), expected); + }); + } + + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] + #[test] + fn resolve_config_with_platform() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", Some(tempdir.path().to_str().unwrap())), + ("XDG_CONFIG_HOME", None), + ("HC_CONFIG", None), + ]; + + with_env_vars(vars, || { + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp + }; + + let expected = if cfg!(target_os = "linux") { + pathbuf![&tempdir.path(), ".config", "hipcheck"] + } else if cfg!(target_os = "macos") { + pathbuf![ + &tempdir.path(), + "Library", + "Application Support", + "hipcheck" + ] + } else { + // Windows + pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"] + }; + + assert_eq!(config.config().unwrap(), expected); + }); + } + + #[test] + fn resolve_config_with_env_var() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", None), + ("XDG_CONFIG_HOME", None), + ("HC_CONFIG", Some(tempdir.path().to_str().unwrap())), + ]; + + with_env_vars(vars, || { + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp + }; + + assert_eq!(config.config().unwrap(), tempdir.path()); + }); + } + + #[test] + fn resolve_config_with_flag() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", Some(tempdir.path().to_str().unwrap())), + ("XDG_CONFIG_HOME", None), + ("HC_CONFIG", None), + ]; + + with_env_vars(vars, || { + let expected = pathbuf![tempdir.path(), "hipcheck"]; + + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp.update(&CliConfig { + path_args: PathArgs { + config: Some(expected.clone()), + ..Default::default() + }, + ..Default::default() + }); + temp + }; + + assert_eq!(config.config().unwrap(), expected); + }); + } + + #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] + #[test] + fn resolve_data_with_platform() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", Some(tempdir.path().to_str().unwrap())), + ("XDG_DATA_HOME", None), + ("HC_DATA", None), + ]; + + with_env_vars(vars, || { + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp + }; + + let expected = if cfg!(target_os = "linux") { + pathbuf![&tempdir.path(), ".local", "share", "hipcheck"] + } else if cfg!(target_os = "macos") { + pathbuf![ + &tempdir.path(), + "Library", + "Application Support", + "hipcheck" + ] + } else { + // Windows + pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"] + }; + + assert_eq!(config.data().unwrap(), expected); + }); + } + + #[test] + fn resolve_data_with_env_var() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", None), + ("XDG_DATA_HOME", None), + ("HC_DATA", Some(tempdir.path().to_str().unwrap())), + ]; + + with_env_vars(vars, || { + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp + }; + + assert_eq!(config.data().unwrap(), tempdir.path()); + }); + } + + #[test] + fn resolve_data_with_flag() { + let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); + + let vars = vec![ + ("HOME", Some(tempdir.path().to_str().unwrap())), + ("XDG_DATA_HOME", None), + ("HC_DATA", None), + ]; + + with_env_vars(vars, || { + let expected = pathbuf![tempdir.path(), "hipcheck"]; + + let config = { + let mut temp = CliConfig::empty(); + temp.update(&CliConfig::from_platform()); + temp.update(&CliConfig::from_env()); + temp.update(&CliConfig { + path_args: PathArgs { + data: Some(expected.clone()), + ..Default::default() + }, + ..Default::default() + }); + temp + }; + + assert_eq!(config.data().unwrap(), expected); + }); + } +} diff --git a/hipcheck/src/main.rs b/hipcheck/src/main.rs index 5b6d95f6..316f5123 100644 --- a/hipcheck/src/main.rs +++ b/hipcheck/src/main.rs @@ -24,9 +24,6 @@ use crate::analysis::report_builder::PrReport; use crate::analysis::report_builder::Report; use crate::analysis::score::score_pr_results; use crate::analysis::score::score_results; -use crate::analysis::session::resolve_cache; -use crate::analysis::session::resolve_config; -use crate::analysis::session::resolve_data; use crate::analysis::session::Check; use crate::analysis::session::Session; use crate::analysis::session::TargetKind; @@ -47,6 +44,7 @@ use command_util::DependentProgram; use core::fmt; use env_logger::Builder as EnvLoggerBuilder; use env_logger::Env; +use pathbuf::pathbuf; use schemars::schema_for; use std::env; use std::fmt::Display; @@ -56,6 +54,7 @@ use std::path::Path; use std::path::PathBuf; use std::process::ExitCode; use std::result::Result as StdResult; +use util::fs::create_dir_all; fn init_logging() { EnvLoggerBuilder::from_env(Env::new().filter("HC_LOG").write_style("HC_LOG_STYLE")).init(); @@ -282,15 +281,36 @@ fn check_npm_version() -> StdResult { } fn check_config_path(config: &CliConfig) -> StdResult { - resolve_config(config.config()).map_err(|_| PathCheckError::PathNotFound) + let path = config.config().ok_or(PathCheckError::PathNotFound)?; + + let path = pathbuf![path, HIPCHECK_TOML_FILE]; + + if path.exists().not() { + return Err(PathCheckError::PathNotFound); + } + + Ok(path) } fn check_cache_path(config: &CliConfig) -> StdResult { - resolve_cache(config.cache()).map_err(|_| PathCheckError::PathNotFound) + let path = config.cache().ok_or(PathCheckError::PathNotFound)?; + + // Try to create the cache directory if it doesn't exist. + if path.exists().not() { + create_dir_all(path).map_err(|_| PathCheckError::PathNotFound)?; + } + + Ok(path.to_owned()) } fn check_data_path(config: &CliConfig) -> StdResult { - resolve_data(config.data()).map_err(|_| PathCheckError::PathNotFound) + let path = config.data().ok_or(PathCheckError::PathNotFound)?; + + if path.exists().not() { + return Err(PathCheckError::PathNotFound); + } + + Ok(path.to_owned()) } /// Check that a GitHub token has been provided as an environment variable @@ -365,9 +385,7 @@ fn cmd_ready(config: &CliConfig) { /// /// Exits `Ok` if home directory is specified, `Err` otherwise. fn cmd_print_home(path: Option<&Path>) { - let cache = resolve_cache(path); - - match cache { + match path.ok_or_else(|| hc_error!("can't find cache directory")) { Ok(path_buffer) => { println!("{}", path_buffer.display()); } @@ -381,8 +399,7 @@ fn cmd_print_home(path: Option<&Path>) { /// /// Exits `Ok` if config path is specified, `Err` otherwise. fn cmd_print_config(config_path: Option<&Path>) { - let config = resolve_config(config_path); - match config { + match config_path.ok_or_else(|| hc_error!("can't find config directory")) { Ok(path_buffer) => { println!("{}", path_buffer.display()); } @@ -396,8 +413,7 @@ fn cmd_print_config(config_path: Option<&Path>) { /// /// Exits `Ok` if config path is specified, `Err` otherwise. fn cmd_print_data(data_path: Option<&Path>) { - let hipcheck_data = resolve_data(data_path); - match hipcheck_data { + match data_path.ok_or_else(|| hc_error!("can't find data directory")) { Ok(path_buffer) => { println!("{}", path_buffer.display()); } From 8aab39314afa609ae83abed9d3382a8d4e62a59f Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Sat, 1 Jun 2024 11:53:56 -0700 Subject: [PATCH 5/8] fix: Applied `cargo clippy` fixes Signed-off-by: Andrew Lilley Brinker --- hipcheck/src/analysis/session/mod.rs | 2 +- hipcheck/src/cli.rs | 2 +- hipcheck/src/main.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hipcheck/src/analysis/session/mod.rs b/hipcheck/src/analysis/session/mod.rs index 44ec2a03..dc65b7d9 100644 --- a/hipcheck/src/analysis/session/mod.rs +++ b/hipcheck/src/analysis/session/mod.rs @@ -265,7 +265,7 @@ fn load_config_and_data( .ok_or_else(|| hc_error!("can't identify directory of config file"))?; // Load the configuration file. - let config = Config::load_from(&valid_config_path) + let config = Config::load_from(valid_config_path) .context("Failed to load configuration. Please make sure the config files are in the config directory.")?; // Get the directory the data file is in. diff --git a/hipcheck/src/cli.rs b/hipcheck/src/cli.rs index d659b424..b4b0b1ea 100644 --- a/hipcheck/src/cli.rs +++ b/hipcheck/src/cli.rs @@ -484,7 +484,7 @@ pub trait Update { impl Update for Option { fn update(&mut self, other: &Option) { if other.is_some() { - self.clone_from(&other); + self.clone_from(other); } } } diff --git a/hipcheck/src/main.rs b/hipcheck/src/main.rs index 316f5123..3c56bb56 100644 --- a/hipcheck/src/main.rs +++ b/hipcheck/src/main.rs @@ -370,7 +370,7 @@ fn cmd_ready(config: &CliConfig) { } match &ready.github_token_check { - Ok(_) => println!("{:<17} {}", "GitHub Token:", "Found!"), + Ok(_) => println!("{:<17} Found!", "GitHub Token:"), Err(e) => println!("{:<17} {}", "GitHub Token:", e), } From b466f7b5b538b3038108f1e401f027c9b717a9c8 Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Sat, 1 Jun 2024 11:57:30 -0700 Subject: [PATCH 6/8] fix: More `cargo clippy` fixes. Signed-off-by: Andrew Lilley Brinker --- hipcheck-macros/src/update.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hipcheck-macros/src/update.rs b/hipcheck-macros/src/update.rs index 8131c4b3..67128067 100644 --- a/hipcheck-macros/src/update.rs +++ b/hipcheck-macros/src/update.rs @@ -20,19 +20,19 @@ pub fn derive_update(input: DeriveInput) -> Result { let ident = &input.ident; let fields = extract_field_names(&input)?; - Ok(TokenStream::from(quote! { + Ok(quote! { impl crate::cli::Update for #ident { fn update(&mut self, other: &Self) { #( self.#fields.update(&other.#fields ); )* } } - })) + }) } /// Extract field names from derive input. fn extract_field_names(input: &DeriveInput) -> Result> { let strukt = extract_struct(input)?; - let fields = extract_named_fields(&strukt)?; + let fields = extract_named_fields(strukt)?; let names = extract_field_names_from_fields(&fields[..])?; Ok(names) } @@ -57,8 +57,8 @@ fn extract_struct(input: &DeriveInput) -> Result<&DataStruct> { match &input.data { Data::Struct(struct_data) => Ok(struct_data), - Data::Enum(_) => return err!(input.span(), "#[derive(Update)] does not support enums"), - Data::Union(_) => return err!(input.span(), "#[derive(Update)] does not support unions"), + Data::Enum(_) => err!(input.span(), "#[derive(Update)] does not support enums"), + Data::Union(_) => err!(input.span(), "#[derive(Update)] does not support unions"), } } From 84f30a1b3a6509aa7f551b54e84a5051b216234d Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Sat, 1 Jun 2024 12:07:48 -0700 Subject: [PATCH 7/8] feat: Show help text on no args. Signed-off-by: Andrew Lilley Brinker --- hipcheck/src/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hipcheck/src/cli.rs b/hipcheck/src/cli.rs index b4b0b1ea..557e9bd3 100644 --- a/hipcheck/src/cli.rs +++ b/hipcheck/src/cli.rs @@ -13,7 +13,7 @@ use std::path::{Path, PathBuf}; /// Automatated supply chain risk assessment of software packages. #[derive(Debug, Default, clap::Parser, hc::Update)] -#[command(name = "Hipcheck", about, version, long_about=None)] +#[command(name = "Hipcheck", about, version, long_about=None, arg_required_else_help = true)] pub struct CliConfig { #[command(subcommand)] command: Option, From f510305efc44a3dee6ff51e3e43402af2760cc25 Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Sat, 1 Jun 2024 12:19:24 -0700 Subject: [PATCH 8/8] feat: Differentiate `config` and `data` on MacOS and Windows. The `dirs` crate doesn't different config and data paths on MacOS and Windows, so we differentiate them ourselves to make sure we're not stuffing configuration and scripts into the same place. Signed-off-by: Andrew Lilley Brinker --- hipcheck/src/cli.rs | 86 ++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/hipcheck/src/cli.rs b/hipcheck/src/cli.rs index 557e9bd3..d6678aa8 100644 --- a/hipcheck/src/cli.rs +++ b/hipcheck/src/cli.rs @@ -280,9 +280,9 @@ impl CliConfig { fn from_platform() -> CliConfig { CliConfig { path_args: PathArgs { - config: dirs::config_dir().map(|dir| pathbuf![&dir, "hipcheck"]), - data: dirs::data_dir().map(|dir| pathbuf![&dir, "hipcheck"]), - cache: dirs::cache_dir().map(|dir| pathbuf![&dir, "hipcheck"]), + cache: platform_cache(), + config: platform_config(), + data: platform_data(), }, ..Default::default() } @@ -301,6 +301,43 @@ impl CliConfig { } } +/// Get the platform cache directory. +/// +/// See: https://docs.rs/dirs/latest/dirs/fn.cache_dir.html +fn platform_cache() -> Option { + dirs::cache_dir().map(|dir| pathbuf![&dir, "hipcheck"]) +} + +/// Get the platform config directory. +/// +/// See: https://docs.rs/dirs/latest/dirs/fn.config_dir.html +fn platform_config() -> Option { + let base = dirs::config_dir().map(|dir| pathbuf![&dir, "hipcheck"]); + + // Config and data paths aren't differentiated on MacOS or Windows, + // so on those platforms we differentiate them ourselves. + if cfg!(target_os = "macos") || cfg!(target_os = "windows") { + base.map(|dir| pathbuf![&dir, "config"]) + } else { + base + } +} + +/// Get the platform data directory. +/// +/// See: https://docs.rs/dirs/latest/dirs/fn.data_dir.html +fn platform_data() -> Option { + let base = dirs::data_dir().map(|dir| pathbuf![&dir, "hipcheck"]); + + // Config and data paths aren't differentiated on MacOS or Windows, + // so on those platforms we differentiate them ourselves. + if cfg!(target_os = "macos") || cfg!(target_os = "windows") { + base.map(|dir| pathbuf![&dir, "data"]) + } else { + base + } +} + /// Get a Hipcheck configuration environment variable. /// /// This is generic in the return type, to automatically handle @@ -524,16 +561,7 @@ mod tests { temp }; - let expected = if cfg!(target_os = "linux") { - pathbuf![&tempdir.path(), ".cache", "hipcheck"] - } else if cfg!(target_os = "macos") { - pathbuf![&tempdir.path(), "Library", "Caches", "hipcheck"] - } else { - // Windows - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Local", "hipcheck"] - }; - - assert_eq!(config.cache().unwrap(), expected); + assert_eq!(config.cache().unwrap(), platform_cache().unwrap()); }); } @@ -609,21 +637,7 @@ mod tests { temp }; - let expected = if cfg!(target_os = "linux") { - pathbuf![&tempdir.path(), ".config", "hipcheck"] - } else if cfg!(target_os = "macos") { - pathbuf![ - &tempdir.path(), - "Library", - "Application Support", - "hipcheck" - ] - } else { - // Windows - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"] - }; - - assert_eq!(config.config().unwrap(), expected); + assert_eq!(config.config().unwrap(), platform_config().unwrap()); }); } @@ -699,21 +713,7 @@ mod tests { temp }; - let expected = if cfg!(target_os = "linux") { - pathbuf![&tempdir.path(), ".local", "share", "hipcheck"] - } else if cfg!(target_os = "macos") { - pathbuf![ - &tempdir.path(), - "Library", - "Application Support", - "hipcheck" - ] - } else { - // Windows - pathbuf![&dirs::home_dir().unwrap(), "AppData", "Roaming", "hipcheck"] - }; - - assert_eq!(config.data().unwrap(), expected); + assert_eq!(config.data().unwrap(), platform_data().unwrap()); }); }