Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(refactor): distinguish TargetSeed and TargetSeedKind #243

Merged
merged 1 commit into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 42 additions & 37 deletions hipcheck/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use crate::session::pm;
use crate::shell::{color_choice::ColorChoice, verbosity::Verbosity};
use crate::source::source;
use crate::target::{
LocalGitRepo, MavenPackage, Package, PackageHost, Sbom, SbomStandard, TargetSeed, TargetType,
ToTargetSeed,
LocalGitRepo, MavenPackage, Package, PackageHost, Sbom, SbomStandard, TargetSeed,
TargetSeedKind, TargetType, ToTargetSeed, ToTargetSeedKind,
};
use clap::{Parser as _, ValueEnum};
use hipcheck_macros as hc;
Expand Down Expand Up @@ -428,6 +428,10 @@ pub enum Commands {
#[command(subcommand_negates_reqs = true)]
#[command(arg_required_else_help = true)]
pub struct CheckArgs {
/// The ref of the target to analyze
#[clap(long = "ref")]
pub refspec: Option<String>,

#[clap(subcommand)]
command: Option<CheckCommand>,

Expand Down Expand Up @@ -505,6 +509,15 @@ impl CheckArgs {
}
}
}
impl ToTargetSeed for CheckArgs {
fn to_target_seed(&self) -> Result<TargetSeed> {
let kind = self.command()?.to_target_seed_kind()?;
Ok(TargetSeed {
kind,
refspec: self.refspec.clone(),
})
}
}

#[derive(Debug, Clone, clap::Parser)]
pub enum CheckCommand {
Expand All @@ -525,14 +538,14 @@ pub enum CheckCommand {
Sbom(CheckSbomArgs),
}

impl ToTargetSeed for CheckCommand {
fn to_target_seed(&self) -> Result<TargetSeed> {
impl ToTargetSeedKind for CheckCommand {
fn to_target_seed_kind(&self) -> Result<TargetSeedKind> {
match self {
CheckCommand::Maven(args) => args.to_target_seed(),
CheckCommand::Npm(args) => args.to_target_seed(),
CheckCommand::Pypi(args) => args.to_target_seed(),
CheckCommand::Repo(args) => args.to_target_seed(),
CheckCommand::Sbom(args) => args.to_target_seed(),
CheckCommand::Maven(args) => args.to_target_seed_kind(),
CheckCommand::Npm(args) => args.to_target_seed_kind(),
CheckCommand::Pypi(args) => args.to_target_seed_kind(),
CheckCommand::Repo(args) => args.to_target_seed_kind(),
CheckCommand::Sbom(args) => args.to_target_seed_kind(),
}
}
}
Expand All @@ -543,13 +556,13 @@ pub struct CheckMavenArgs {
pub package: String,
}

impl ToTargetSeed for CheckMavenArgs {
fn to_target_seed(&self) -> Result<TargetSeed> {
impl ToTargetSeedKind for CheckMavenArgs {
fn to_target_seed_kind(&self) -> Result<TargetSeedKind> {
let arg = &self.package;
// Confirm that the provided URL is valid.
let url = Url::parse(arg)
.map_err(|e| hc_error!("The provided Maven URL {} is not a valid URL. {}", arg, e))?;
Ok(TargetSeed::MavenPackage(MavenPackage { url }))
Ok(TargetSeedKind::MavenPackage(MavenPackage { url }))
}
}

Expand All @@ -559,8 +572,8 @@ pub struct CheckNpmArgs {
pub package: String,
}

impl ToTargetSeed for CheckNpmArgs {
fn to_target_seed(&self) -> Result<TargetSeed> {
impl ToTargetSeedKind for CheckNpmArgs {
fn to_target_seed_kind(&self) -> Result<TargetSeedKind> {
let raw_package = &self.package;

let (name, version) = match Url::parse(raw_package) {
Expand All @@ -575,7 +588,7 @@ impl ToTargetSeed for CheckNpmArgs {
})
.unwrap();

Ok(TargetSeed::Package(Package {
Ok(TargetSeedKind::Package(Package {
purl,
name,
version,
Expand All @@ -590,8 +603,8 @@ pub struct CheckPypiArgs {
pub package: String,
}

impl ToTargetSeed for CheckPypiArgs {
fn to_target_seed(&self) -> Result<TargetSeed> {
impl ToTargetSeedKind for CheckPypiArgs {
fn to_target_seed_kind(&self) -> Result<TargetSeedKind> {
let raw_package = &self.package;

let (name, version) = match Url::parse(raw_package) {
Expand All @@ -605,7 +618,7 @@ impl ToTargetSeed for CheckPypiArgs {
})
.unwrap();

Ok(TargetSeed::Package(Package {
Ok(TargetSeedKind::Package(Package {
purl,
name,
version,
Expand All @@ -618,28 +631,20 @@ impl ToTargetSeed for CheckPypiArgs {
pub struct CheckRepoArgs {
/// Repository to analyze; can be a local path or a URI
pub source: String,
/// The ref of the repo to analyze
#[clap(long = "ref")]
pub refspec: Option<String>,
}

impl ToTargetSeed for CheckRepoArgs {
fn to_target_seed(&self) -> Result<TargetSeed> {
impl ToTargetSeedKind for CheckRepoArgs {
fn to_target_seed_kind(&self) -> Result<TargetSeedKind> {
if let Ok(url) = Url::parse(&self.source) {
let remote_repo = source::get_remote_repo_from_url(url)?;
Ok(TargetSeed::RemoteRepo {
target: remote_repo,
refspec: self.refspec.clone(),
})
Ok(TargetSeedKind::RemoteRepo(remote_repo))
} else {
let path = Path::new(&self.source).canonicalize()?;
let git_ref = match &self.refspec {
Some(r) => r.clone(),
None => source::get_head_commit(path.as_path())
.context("can't get head commit for local source")?,
};
if path.exists() {
Ok(TargetSeed::LocalRepo(LocalGitRepo { path, git_ref }))
Ok(TargetSeedKind::LocalRepo(LocalGitRepo {
path,
git_ref: "".to_owned(),
}))
} else {
Err(hc_error!("Provided target repository could not be identified as either a remote url or path to a local file"))
}
Expand All @@ -653,12 +658,12 @@ pub struct CheckSbomArgs {
pub path: String,
}

impl ToTargetSeed for CheckSbomArgs {
fn to_target_seed(&self) -> Result<TargetSeed> {
impl ToTargetSeedKind for CheckSbomArgs {
fn to_target_seed_kind(&self) -> Result<TargetSeedKind> {
let path = PathBuf::from(&self.path);
if path.exists() {
if self.path.ends_with(".spdx") {
Ok(TargetSeed::Sbom(Sbom {
Ok(TargetSeedKind::Sbom(Sbom {
path,
standard: SbomStandard::Spdx,
}))
Expand All @@ -667,7 +672,7 @@ impl ToTargetSeed for CheckSbomArgs {
|| self.path.ends_with("bom.xml")
|| self.path.ends_with(".cdx.xml")
{
Ok(TargetSeed::Sbom(Sbom {
Ok(TargetSeedKind::Sbom(Sbom {
path,
standard: SbomStandard::CycloneDX,
}))
Expand Down
18 changes: 6 additions & 12 deletions hipcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ use std::process::Command;
use std::process::ExitCode;
use std::result::Result as StdResult;
use std::time::Duration;
use target::{RemoteGitRepo, TargetSeed, ToTargetSeed};
use target::{RemoteGitRepo, TargetSeed, TargetSeedKind, ToTargetSeed};
use util::fs::create_dir_all;
use which::which;

Expand Down Expand Up @@ -159,14 +159,8 @@ fn main() -> ExitCode {

/// Run the `check` command.
fn cmd_check(args: &CheckArgs, config: &CliConfig) -> ExitCode {
let target = match args.command() {
Ok(chk) => match chk.to_target_seed() {
Ok(target) => target,
Err(e) => {
Shell::print_error(&e, Format::Human);
return ExitCode::FAILURE;
}
},
let target = match args.to_target_seed() {
Ok(target) => target,
Err(e) => {
Shell::print_error(&e, Format::Human);
return ExitCode::FAILURE;
Expand Down Expand Up @@ -219,11 +213,11 @@ fn cmd_print_weights(config: &CliConfig) -> Result<()> {
// Create a dummy session to query the salsa database for a weight graph for printing.
let session = Session::new(
// Use the hipcheck repo as a dummy url until checking is de-coupled from `Session`.
&TargetSeed::RemoteRepo {
target: RemoteGitRepo {
&TargetSeed {
kind: TargetSeedKind::RemoteRepo(RemoteGitRepo {
url: url::Url::parse("https://github.com/mitre/hipcheck.git").unwrap(),
known_remote: None,
},
}),
refspec: Some("HEAD".to_owned()),
},
config.config().map(ToOwned::to_owned),
Expand Down
58 changes: 29 additions & 29 deletions hipcheck/src/session/pm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ fn is_none_or_empty(s: Option<&str>) -> bool {
mod tests {
use crate::{
cli::{CheckNpmArgs, CheckPypiArgs},
target::{TargetSeed, ToTargetSeed},
target::{TargetSeedKind, ToTargetSeedKind},
};

// Note this useful idiom: importing names from outer (for mod tests) scope.
Expand All @@ -565,9 +565,9 @@ mod tests {
let target_seed = CheckPypiArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -593,9 +593,9 @@ mod tests {
let target_seed = CheckPypiArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -621,9 +621,9 @@ mod tests {
let target_seed = CheckPypiArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -649,9 +649,9 @@ mod tests {
let target_seed = CheckPypiArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -677,9 +677,9 @@ mod tests {
let target_seed = CheckPypiArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -705,9 +705,9 @@ mod tests {
let target_seed = CheckPypiArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -734,9 +734,9 @@ mod tests {
let target_seed = CheckPypiArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand Down Expand Up @@ -766,9 +766,9 @@ mod tests {
let target_seed = CheckNpmArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -794,9 +794,9 @@ mod tests {
let target_seed = CheckNpmArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -822,9 +822,9 @@ mod tests {
let target_seed = CheckNpmArgs {
package: link.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -850,9 +850,9 @@ mod tests {
let target_seed = CheckNpmArgs {
package: npm_package.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -878,9 +878,9 @@ mod tests {
let target_seed = CheckNpmArgs {
package: npm_package.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -906,9 +906,9 @@ mod tests {
let target_seed = CheckNpmArgs {
package: npm_package.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand All @@ -934,9 +934,9 @@ mod tests {
let target_seed = CheckNpmArgs {
package: npm_package.to_string(),
}
.to_target_seed()
.to_target_seed_kind()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
if let TargetSeedKind::Package(package) = target_seed {
assert_eq!(
package,
Package {
Expand Down
Loading