From 8b1d64d44485cc531c895e5118dfd867ef7a948f Mon Sep 17 00:00:00 2001 From: joshmc Date: Sat, 27 Feb 2021 13:17:38 +0000 Subject: [PATCH 1/3] NOISSUE - Add Mocking --- cargo-geiger/src/graph.rs | 2 +- cargo-geiger/src/mapping.rs | 145 ++++++++++++++---- cargo-geiger/src/mapping/geiger.rs | 9 +- cargo-geiger/src/mapping/krates.rs | 16 +- cargo-geiger/src/mapping/metadata.rs | 125 +++++++-------- .../src/mapping/metadata/dependency.rs | 22 +++ cargo-geiger/src/mapping/metadata/package.rs | 32 ++++ .../src/mapping/metadata/package_id.rs | 33 ++++ cargo-geiger/src/scan/find.rs | 2 +- 9 files changed, 274 insertions(+), 112 deletions(-) create mode 100644 cargo-geiger/src/mapping/metadata/dependency.rs create mode 100644 cargo-geiger/src/mapping/metadata/package.rs create mode 100644 cargo-geiger/src/mapping/metadata/package_id.rs diff --git a/cargo-geiger/src/graph.rs b/cargo-geiger/src/graph.rs index b09d541b..4667e9e7 100644 --- a/cargo-geiger/src/graph.rs +++ b/cargo-geiger/src/graph.rs @@ -130,7 +130,7 @@ fn add_package_dependencies_to_graph( let dep_not_replaced_option = cargo_metadata_parameters .metadata - .deps_not_replaced(package_id.clone()); + .deps_not_replaced(&package_id); match (krates_node_option, dep_not_replaced_option) { (Some(krates_node), Some(dependencies)) => { diff --git a/cargo-geiger/src/mapping.rs b/cargo-geiger/src/mapping.rs index c47e0d1f..5943b4fc 100644 --- a/cargo-geiger/src/mapping.rs +++ b/cargo-geiger/src/mapping.rs @@ -2,12 +2,26 @@ mod geiger; mod krates; mod metadata; +use metadata::package_id::ToCargoMetadataPackage; +use metadata::package::{GetPackageName, GetPackageParent, GetPackageVersion}; + use ::krates::Krates; use cargo::core::dependency::DepKind; use cargo_metadata::Metadata; use std::collections::HashSet; +use std::fmt::Display; use std::path::PathBuf; +use cargo_metadata::Dependency as CargoMetadataDependency; +use cargo_metadata::PackageId as CargoMetadataPackageId; +use cargo_metadata::Version as CargoMetadataVersion; + +use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; +use cargo_geiger_serde::PackageId as CargoGeigerSerdePackageId; +use cargo_geiger_serde::Source as CargoGeigerSerdeSource; +use crate::mapping::metadata::GetMetadataPackages; +use crate::mapping::metadata::dependency::{GetDependencyName, GetDependencyRequirement}; + /// Holds a pointer to both a `Krates` graph, and the `Metadata` struct /// which are often required together pub struct CargoMetadataParameters<'a> { @@ -16,13 +30,13 @@ pub struct CargoMetadataParameters<'a> { } pub trait DepsNotReplaced { - fn deps_not_replaced( + fn deps_not_replaced( &self, - package_id: cargo_metadata::PackageId, + package_id: &T, ) -> Option< Vec<( - cargo_metadata::PackageId, - HashSet, + CargoMetadataPackageId, + HashSet, )>, >; } @@ -30,38 +44,50 @@ pub trait DepsNotReplaced { pub trait GetLicenceFromCargoMetadataPackageId { fn get_licence_from_cargo_metadata_package_id( &self, - package_id: &cargo_metadata::PackageId, + package_id: &CargoMetadataPackageId, ) -> Option; } + +pub trait GetPackageRoot : GetPackageName + GetPackageParent + GetPackageVersion { + fn get_root(&self) -> Option { + match self.get_package_parent() { + Some(path) => Some(path.to_path_buf()), + None => { + eprintln!( + "Failed to get root for: {} {:?}", + self.get_package_name(), self.get_package_version() + ); + None + } + } + } +} + pub trait GetPackageNameAndVersionFromCargoMetadataPackageId { fn get_package_name_and_version_from_cargo_metadata_package_id( &self, - package_id: &cargo_metadata::PackageId, - ) -> Option<(String, cargo_metadata::Version)>; + package_id: &CargoMetadataPackageId, + ) -> Option<(String, CargoMetadataVersion)>; } pub trait GetRepositoryFromCargoMetadataPackageId { fn get_repository_from_cargo_metadata_package_id( &self, - package_id: &cargo_metadata::PackageId, + package_id: &CargoMetadataPackageId, ) -> Option; } -pub trait GetRoot { - fn get_root(&self) -> Option; -} - pub trait MatchesIgnoringSource { fn matches_ignoring_source( &self, krates: &Krates, - package_id: cargo_metadata::PackageId, + package_id: CargoMetadataPackageId, ) -> Option; } pub trait QueryResolve { - fn query_resolve(&self, query: &str) -> Option; + fn query_resolve(&self, query: &str) -> Option; } pub trait ToCargoCoreDepKind { @@ -71,33 +97,100 @@ pub trait ToCargoCoreDepKind { pub trait ToCargoGeigerDependencyKind { fn to_cargo_geiger_dependency_kind( &self, - ) -> Option; + ) -> Option; } pub trait ToCargoGeigerPackageId { fn to_cargo_geiger_package_id( &self, metadata: &Metadata, - ) -> Option; + ) -> Option; } pub trait ToCargoGeigerSource { fn to_cargo_geiger_source( &self, metadata: &Metadata, - ) -> cargo_geiger_serde::Source; + ) -> CargoGeigerSerdeSource; } -pub trait ToCargoMetadataPackage { - fn to_cargo_metadata_package( + +pub trait ToCargoMetadataPackageId: GetDependencyName + GetDependencyRequirement { + fn to_cargo_metadata_package_id( &self, - metadata: &Metadata, - ) -> Option; + metadata: &T, + ) -> Option { + metadata.get_metadata_packages() + .filter(|p| p.name == self.get_dependency_name() && self.get_dependency_requirement().matches(&p.version)) + .map(|p| p.id.clone()) + .collect::>() + .pop() + } } -pub trait ToCargoMetadataPackageId { - fn to_cargo_metadata_package_id( - &self, - metadata: &Metadata, - ) -> Option; +#[cfg(test)] +mod mapping_tests { + use super::*; + + use std::path::Path; + use rstest::*; + + struct MockPackage<'a> { + mock_package_name: String, + mock_package_parent: Option<&'a Path>, + mock_package_version: CargoMetadataVersion, + } + + impl GetPackageName for MockPackage<'_> { + fn get_package_name(&self) -> String { + self.mock_package_name.clone() + } + } + + impl GetPackageParent for MockPackage<'_> { + fn get_package_parent(&self) -> Option<&Path> { + self.mock_package_parent + } + } + + impl GetPackageVersion for MockPackage<'_> { + fn get_package_version(&self) -> CargoMetadataVersion { + self.mock_package_version.clone() + } + } + + impl GetPackageRoot for MockPackage<'_> {} + + #[rstest( + input_package_path_option, + expected_package_path_buf_option, + case( + Some(Path::new("/path/to/file")), + Some(PathBuf::from("/path/to/file")) + ), + case( + None, + None + ) + )] + fn get_package_root_test( + input_package_path_option: Option<&Path>, + expected_package_path_buf_option: Option + ) { + let _mock_package_parent = match input_package_path_option { + Some(path) => Some(path), + None => None + }; + + let mock_package = MockPackage { + mock_package_name: String::from("package_name"), + mock_package_parent: input_package_path_option, + mock_package_version: CargoMetadataVersion::new(1,1,1) + }; + + assert_eq!( + mock_package.get_root(), + expected_package_path_buf_option + ) + } } diff --git a/cargo-geiger/src/mapping/geiger.rs b/cargo-geiger/src/mapping/geiger.rs index 85b760d3..70135531 100644 --- a/cargo-geiger/src/mapping/geiger.rs +++ b/cargo-geiger/src/mapping/geiger.rs @@ -1,4 +1,5 @@ -use super::{ToCargoGeigerSource, ToCargoMetadataPackage}; +use super::{ToCargoGeigerSource}; +use super::metadata::package_id::{GetPackageIdRepr, ToCargoMetadataPackage}; use cargo_metadata::Metadata; use url::Url; @@ -56,10 +57,10 @@ fn handle_source_repr(source_repr: &str) -> cargo_geiger_serde::Source { } } -fn handle_path_source( - package_id: &cargo_metadata::PackageId, +fn handle_path_source( + package_id: &T, ) -> cargo_geiger_serde::Source { - let raw_repr = package_id.repr.clone(); + let raw_repr = package_id.get_package_id_repr(); let raw_path_repr = raw_repr[1..raw_repr.len() - 1] .split("+file://") .skip(1) diff --git a/cargo-geiger/src/mapping/krates.rs b/cargo-geiger/src/mapping/krates.rs index 4f3b1880..00eef710 100644 --- a/cargo-geiger/src/mapping/krates.rs +++ b/cargo-geiger/src/mapping/krates.rs @@ -7,10 +7,13 @@ use crate::mapping::{ use krates::{Krates, PkgSpec}; use std::str::FromStr; +use cargo_metadata::PackageId as CargoMetadataPackageId; +use cargo_metadata::Version as CargoMetadataVersion; + impl GetLicenceFromCargoMetadataPackageId for Krates { fn get_licence_from_cargo_metadata_package_id( &self, - package_id: &cargo_metadata::PackageId, + package_id: &CargoMetadataPackageId, ) -> Option { self.node_for_kid(package_id) .and_then(|package| package.krate.clone().license) @@ -20,8 +23,8 @@ impl GetLicenceFromCargoMetadataPackageId for Krates { impl GetPackageNameAndVersionFromCargoMetadataPackageId for Krates { fn get_package_name_and_version_from_cargo_metadata_package_id( &self, - package_id: &cargo_metadata::PackageId, - ) -> Option<(String, cargo_metadata::Version)> { + package_id: &CargoMetadataPackageId, + ) -> Option<(String, CargoMetadataVersion)> { self.node_for_kid(package_id).map(|package| { (package.krate.clone().name, package.krate.clone().version) }) @@ -31,7 +34,7 @@ impl GetPackageNameAndVersionFromCargoMetadataPackageId for Krates { impl GetRepositoryFromCargoMetadataPackageId for Krates { fn get_repository_from_cargo_metadata_package_id( &self, - package_id: &cargo_metadata::PackageId, + package_id: &CargoMetadataPackageId, ) -> Option { self.node_for_kid(package_id) .and_then(|package| package.krate.clone().repository) @@ -39,13 +42,13 @@ impl GetRepositoryFromCargoMetadataPackageId for Krates { } impl QueryResolve for Krates { - fn query_resolve(&self, query: &str) -> Option { + fn query_resolve(&self, query: &str) -> Option { match PkgSpec::from_str(query) { Ok(package_spec) => self .krates_by_name(package_spec.name.as_str()) .filter(|(_, node)| package_spec.matches(&node.krate)) .map(|(_, node)| node.krate.clone().id) - .collect::>() + .collect::>() .pop(), _ => { eprintln!("Failed to construct PkgSpec from string: {}", query); @@ -154,7 +157,6 @@ mod krates_tests { .unwrap(); assert_eq!(package_name, expected_package_name); - assert_eq!(package_version, expected_package_version); } diff --git a/cargo-geiger/src/mapping/metadata.rs b/cargo-geiger/src/mapping/metadata.rs index 9c202d7d..1815becb 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -1,30 +1,41 @@ +pub mod dependency; +pub mod package_id; +pub mod package; + use super::{ DepsNotReplaced, GetPackageNameAndVersionFromCargoMetadataPackageId, - GetRoot, MatchesIgnoringSource, ToCargoGeigerPackageId, + MatchesIgnoringSource, ToCargoGeigerPackageId, ToCargoMetadataPackageId, }; +use package_id::ToCargoMetadataPackage; -use crate::mapping::{ - ToCargoGeigerDependencyKind, ToCargoGeigerSource, ToCargoMetadataPackage, -}; +use crate::mapping::{ToCargoGeigerDependencyKind, ToCargoGeigerSource, GetPackageRoot}; -use cargo_metadata::{DependencyKind, Metadata, PackageId}; +use cargo_metadata::Metadata; use krates::Krates; use std::collections::HashSet; -use std::path::PathBuf; +use std::fmt::Display; +use std::slice::Iter; + +use cargo_metadata::DependencyKind as CargoMetadataDependencyKind; +use cargo_metadata::Dependency as CargoMetadataDependency; +use cargo_metadata::PackageId as CargoMetadataPackageId; +use cargo_metadata::Package as CargoMetadataPackage; -impl DepsNotReplaced for cargo_metadata::Metadata { - fn deps_not_replaced( +use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; + +impl DepsNotReplaced for Metadata { + fn deps_not_replaced( &self, - package_id: cargo_metadata::PackageId, + package_id: &T, ) -> Option< Vec<( - cargo_metadata::PackageId, - HashSet, + CargoMetadataPackageId, + HashSet, )>, > { let mut cargo_metadata_deps_not_replaced = vec![]; - let mut package_id_hashset = HashSet::::new(); + let mut package_id_hashset = HashSet::::new(); match package_id.to_cargo_metadata_package(self) { Some(package) => { @@ -35,7 +46,7 @@ impl DepsNotReplaced for cargo_metadata::Metadata { if !package_id_hashset.contains(&package_id) { cargo_metadata_deps_not_replaced.push(( package_id.clone(), - HashSet::::new(), + HashSet::::new(), )); package_id_hashset.insert(package_id); } @@ -51,26 +62,13 @@ impl DepsNotReplaced for cargo_metadata::Metadata { } } -impl GetRoot for cargo_metadata::Package { - fn get_root(&self) -> Option { - match self.manifest_path.parent() { - Some(path) => Some(path.to_path_buf()), - None => { - eprintln!( - "Failed to get root for: {} {:?}", - self.name, self.version - ); - None - } - } - } -} +impl GetPackageRoot for CargoMetadataPackage { } -impl MatchesIgnoringSource for cargo_metadata::Dependency { +impl MatchesIgnoringSource for CargoMetadataDependency { fn matches_ignoring_source( &self, krates: &Krates, - package_id: cargo_metadata::PackageId, + package_id: CargoMetadataPackageId, ) -> Option { match krates .get_package_name_and_version_from_cargo_metadata_package_id( @@ -91,19 +89,19 @@ impl MatchesIgnoringSource for cargo_metadata::Dependency { } } -impl ToCargoGeigerDependencyKind for cargo_metadata::DependencyKind { +impl ToCargoGeigerDependencyKind for CargoMetadataDependencyKind { fn to_cargo_geiger_dependency_kind( &self, - ) -> Option { + ) -> Option { match self { - DependencyKind::Build => { - Some(cargo_geiger_serde::DependencyKind::Build) + CargoMetadataDependencyKind::Build => { + Some(CargoGeigerSerdeDependencyKind::Build) } - DependencyKind::Development => { - Some(cargo_geiger_serde::DependencyKind::Development) + CargoMetadataDependencyKind::Development => { + Some(CargoGeigerSerdeDependencyKind::Development) } - DependencyKind::Normal => { - Some(cargo_geiger_serde::DependencyKind::Normal) + CargoMetadataDependencyKind::Normal => { + Some(CargoGeigerSerdeDependencyKind::Normal) } _ => { eprintln!("Unrecognised Dependency Kind"); @@ -113,7 +111,7 @@ impl ToCargoGeigerDependencyKind for cargo_metadata::DependencyKind { } } -impl ToCargoGeigerPackageId for cargo_metadata::PackageId { +impl ToCargoGeigerPackageId for CargoMetadataPackageId { fn to_cargo_geiger_package_id( &self, metadata: &Metadata, @@ -121,7 +119,6 @@ impl ToCargoGeigerPackageId for cargo_metadata::PackageId { match self.to_cargo_metadata_package(metadata) { Some(package) => { let metadata_source = self.to_cargo_geiger_source(metadata); - Some(cargo_geiger_serde::PackageId { name: package.name, version: package.version, @@ -136,33 +133,15 @@ impl ToCargoGeigerPackageId for cargo_metadata::PackageId { } } -impl ToCargoMetadataPackageId for cargo_metadata::Dependency { - fn to_cargo_metadata_package_id( - &self, - metadata: &Metadata, - ) -> Option { - metadata - .packages - .iter() - .filter(|p| p.name == self.name && self.req.matches(&p.version)) - .map(|p| p.id.clone()) - .collect::>() - .pop() - } +impl ToCargoMetadataPackageId for CargoMetadataDependency { } + +pub trait GetMetadataPackages { + fn get_metadata_packages(&self) -> Iter; } -impl ToCargoMetadataPackage for cargo_metadata::PackageId { - fn to_cargo_metadata_package( - &self, - metadata: &Metadata, - ) -> Option { - metadata - .packages - .iter() - .filter(|p| p.id == *self) - .cloned() - .collect::>() - .pop() +impl GetMetadataPackages for Metadata { + fn get_metadata_packages(&self) -> Iter { + self.packages.iter() } } @@ -208,7 +187,7 @@ mod metadata_tests { let deps_not_replaced = resolve.deps_not_replaced(package.package_id()); let cargo_metadata_deps_not_replaced = metadata - .deps_not_replaced(cargo_metadata_package_id) + .deps_not_replaced(&cargo_metadata_package_id) .unwrap(); let mut cargo_core_package_names = deps_not_replaced @@ -278,12 +257,12 @@ mod metadata_tests { #[rstest( input_dependency_kind, expected_dep_kind, - case(DependencyKind::Build, DepKind::Build), - case(DependencyKind::Development, DepKind::Development), - case(DependencyKind::Normal, DepKind::Normal) + case(CargoMetadataDependencyKind::Build, DepKind::Build), + case(CargoMetadataDependencyKind::Development, DepKind::Development), + case(CargoMetadataDependencyKind::Normal, DepKind::Normal) )] fn to_cargo_core_dep_kind( - input_dependency_kind: DependencyKind, + input_dependency_kind: CargoMetadataDependencyKind, expected_dep_kind: DepKind, ) { assert_eq!( @@ -384,12 +363,12 @@ mod metadata_tests { Ok((packages, resolve)) } - impl ToCargoCoreDepKind for DependencyKind { + impl ToCargoCoreDepKind for CargoMetadataDependencyKind { fn to_cargo_core_dep_kind(&self) -> DepKind { match self { - DependencyKind::Build => DepKind::Build, - DependencyKind::Development => DepKind::Development, - DependencyKind::Normal => DepKind::Normal, + CargoMetadataDependencyKind::Build => DepKind::Build, + CargoMetadataDependencyKind::Development => DepKind::Development, + CargoMetadataDependencyKind::Normal => DepKind::Normal, _ => panic!("Unknown dependency kind"), } } diff --git a/cargo-geiger/src/mapping/metadata/dependency.rs b/cargo-geiger/src/mapping/metadata/dependency.rs new file mode 100644 index 00000000..3bbad418 --- /dev/null +++ b/cargo-geiger/src/mapping/metadata/dependency.rs @@ -0,0 +1,22 @@ +use cargo_metadata::Dependency; +use krates::semver::VersionReq; + +pub trait GetDependencyName { + fn get_dependency_name(&self) -> String; +} + +impl GetDependencyName for Dependency { + fn get_dependency_name(&self) -> String { + self.name.clone() + } +} + +pub trait GetDependencyRequirement { + fn get_dependency_requirement(&self) -> VersionReq; +} + +impl GetDependencyRequirement for Dependency { + fn get_dependency_requirement(&self) -> VersionReq { + self.req.clone() + } +} \ No newline at end of file diff --git a/cargo-geiger/src/mapping/metadata/package.rs b/cargo-geiger/src/mapping/metadata/package.rs new file mode 100644 index 00000000..bd63e1cc --- /dev/null +++ b/cargo-geiger/src/mapping/metadata/package.rs @@ -0,0 +1,32 @@ +use cargo_metadata::{Package, Version}; +use std::path::Path; + +pub trait GetPackageName { + fn get_package_name(&self) -> String; +} + +pub trait GetPackageParent { + fn get_package_parent(&self) -> Option<&Path>; +} + +pub trait GetPackageVersion { + fn get_package_version(&self) -> Version; +} + +impl GetPackageName for Package { + fn get_package_name(&self) -> String { + self.name.clone() + } +} + +impl GetPackageParent for Package { + fn get_package_parent(&self) -> Option<&Path> { + self.manifest_path.parent() + } +} + +impl GetPackageVersion for Package { + fn get_package_version(&self) -> Version { + self.version.clone() + } +} \ No newline at end of file diff --git a/cargo-geiger/src/mapping/metadata/package_id.rs b/cargo-geiger/src/mapping/metadata/package_id.rs new file mode 100644 index 00000000..d45eb950 --- /dev/null +++ b/cargo-geiger/src/mapping/metadata/package_id.rs @@ -0,0 +1,33 @@ +use cargo_metadata::{Metadata, PackageId, Package}; + +pub trait GetPackageIdRepr { + fn get_package_id_repr(&self) -> String; +} + +impl GetPackageIdRepr for PackageId { + fn get_package_id_repr(&self) -> String { + self.repr.clone() + } +} + +pub trait ToCargoMetadataPackage { + fn to_cargo_metadata_package( + &self, + metadata: &Metadata, + ) -> Option; +} + +impl ToCargoMetadataPackage for PackageId { + fn to_cargo_metadata_package( + &self, + metadata: &Metadata, + ) -> Option { + metadata + .packages + .iter() + .filter(|p| p.id == *self) + .cloned() + .collect::>() + .pop() + } +} \ No newline at end of file diff --git a/cargo-geiger/src/scan/find.rs b/cargo-geiger/src/scan/find.rs index 83ffa776..d360f997 100644 --- a/cargo-geiger/src/scan/find.rs +++ b/cargo-geiger/src/scan/find.rs @@ -1,5 +1,5 @@ use crate::format::print_config::PrintConfig; -use crate::mapping::{CargoMetadataParameters, GetRoot}; +use crate::mapping::{CargoMetadataParameters, GetPackageRoot}; use crate::scan::rs_file::{ into_is_entry_point_and_path_buf, into_rs_code_file, into_target_kind, is_file_with_ext, RsFile, RsFileMetricsWrapper, From 0d7f27a1d0cffc82cf1c2a825f08b12271500d69 Mon Sep 17 00:00:00 2001 From: joshmc Date: Thu, 4 Mar 2021 17:47:43 +0000 Subject: [PATCH 2/3] NOISSUE-AddMocking --- cargo-geiger/src/format/display.rs | 30 +++--- cargo-geiger/src/graph.rs | 56 ++---------- cargo-geiger/src/graph/extra_deps.rs | 55 +++++++++++ cargo-geiger/src/mapping.rs | 91 ++++++++----------- cargo-geiger/src/mapping/geiger.rs | 2 +- cargo-geiger/src/mapping/krates.rs | 77 +++++----------- cargo-geiger/src/mapping/metadata.rs | 85 +++++++---------- .../src/mapping/metadata/dependency.rs | 16 +--- cargo-geiger/src/mapping/metadata/package.rs | 18 ++-- .../src/mapping/metadata/package_id.rs | 41 +++++++-- 10 files changed, 218 insertions(+), 253 deletions(-) create mode 100644 cargo-geiger/src/graph/extra_deps.rs diff --git a/cargo-geiger/src/format/display.rs b/cargo-geiger/src/format/display.rs index 052f61a6..996dbd94 100644 --- a/cargo-geiger/src/format/display.rs +++ b/cargo-geiger/src/format/display.rs @@ -1,10 +1,6 @@ use crate::format::pattern::Pattern; use crate::format::Chunk; -use crate::mapping::{ - CargoMetadataParameters, GetLicenceFromCargoMetadataPackageId, - GetPackageNameAndVersionFromCargoMetadataPackageId, - GetRepositoryFromCargoMetadataPackageId, -}; +use crate::mapping::{CargoMetadataParameters, GetPackageIdInformation}; use cargo_metadata::PackageId; use std::fmt; @@ -20,20 +16,20 @@ impl<'a> fmt::Display for Display<'a> { for chunk in &self.pattern.0 { match *chunk { Chunk::License => { - if let Some(ref license) = self - .cargo_metadata_parameters - .krates - .get_licence_from_cargo_metadata_package_id( - self.package, + if let Some(ref license) = + self.package.get_package_id_licence( + self.cargo_metadata_parameters.krates, ) { (write!(fmt, "{}", license))? } } Chunk::Package => { - if let Some((package_name, package_version)) = self.cargo_metadata_parameters - .krates - .get_package_name_and_version_from_cargo_metadata_package_id(self.package) { + if let Some((package_name, package_version)) = + self.package.get_package_id_name_and_version( + self.cargo_metadata_parameters.krates, + ) + { (write!(fmt, "{} {}", package_name, package_version))? } else { eprintln!("Failed to format Package: {}", self.package) @@ -41,11 +37,9 @@ impl<'a> fmt::Display for Display<'a> { } Chunk::Raw(ref s) => (fmt.write_str(s))?, Chunk::Repository => { - if let Some(ref repository) = self - .cargo_metadata_parameters - .krates - .get_repository_from_cargo_metadata_package_id( - self.package, + if let Some(ref repository) = + self.package.get_package_id_repository( + self.cargo_metadata_parameters.krates, ) { (write!(fmt, "{}", repository))? diff --git a/cargo-geiger/src/graph.rs b/cargo-geiger/src/graph.rs index 4667e9e7..8621fe91 100644 --- a/cargo-geiger/src/graph.rs +++ b/cargo-geiger/src/graph.rs @@ -1,3 +1,7 @@ +pub mod extra_deps; + +use extra_deps::ExtraDeps; + use crate::args::{Args, DepsArgs, TargetArgs}; use crate::cli::get_cfgs; use crate::mapping::{ @@ -14,31 +18,9 @@ use petgraph::graph::NodeIndex; use std::collections::hash_map::Entry; use std::collections::HashMap; -#[derive(Debug, PartialEq)] -pub enum ExtraDeps { - All, - Build, - Dev, - NoMore, -} - -impl ExtraDeps { - // This clippy recommendation is valid, but makes this function much harder to read - #[allow(clippy::match_like_matches_macro)] - pub fn allows(&self, dep: DependencyKind) -> bool { - match (self, dep) { - (_, DependencyKind::Normal) => true, - (ExtraDeps::All, _) => true, - (ExtraDeps::Build, DependencyKind::Build) => true, - (ExtraDeps::Dev, DependencyKind::Development) => true, - _ => false, - } - } -} - /// Representation of the package dependency graph pub struct Graph { - pub graph: petgraph::Graph, + pub graph: petgraph::Graph, pub nodes: HashMap, } @@ -197,7 +179,7 @@ fn filter_dependencies<'a>( .filter(|d| { d.matches_ignoring_source( cargo_metadata_parameters.krates, - dependency_package_id.clone(), + dependency_package_id, ) .unwrap_or(false) }) @@ -223,32 +205,6 @@ mod graph_tests { use super::*; use rstest::*; - #[rstest( - input_extra_deps, - input_dependency_kind, - expected_allows, - case(ExtraDeps::All, DependencyKind::Normal, true), - case(ExtraDeps::Build, DependencyKind::Normal, true), - case(ExtraDeps::Dev, DependencyKind::Normal, true), - case(ExtraDeps::NoMore, DependencyKind::Normal, true), - case(ExtraDeps::All, DependencyKind::Build, true), - case(ExtraDeps::All, DependencyKind::Development, true), - case(ExtraDeps::Build, DependencyKind::Build, true), - case(ExtraDeps::Build, DependencyKind::Development, false), - case(ExtraDeps::Dev, DependencyKind::Build, false), - case(ExtraDeps::Dev, DependencyKind::Development, true) - )] - fn extra_deps_allows_test( - input_extra_deps: ExtraDeps, - input_dependency_kind: DependencyKind, - expected_allows: bool, - ) { - assert_eq!( - input_extra_deps.allows(input_dependency_kind), - expected_allows - ); - } - #[rstest( input_deps_args, expected_extra_deps, diff --git a/cargo-geiger/src/graph/extra_deps.rs b/cargo-geiger/src/graph/extra_deps.rs new file mode 100644 index 00000000..4d599f10 --- /dev/null +++ b/cargo-geiger/src/graph/extra_deps.rs @@ -0,0 +1,55 @@ +use cargo_metadata::DependencyKind; + +#[derive(Debug, PartialEq)] +pub enum ExtraDeps { + All, + Build, + Dev, + NoMore, +} + +impl ExtraDeps { + // This clippy recommendation is valid, but makes this function much harder to read + #[allow(clippy::match_like_matches_macro)] + pub fn allows(&self, dependency_kind: DependencyKind) -> bool { + match (self, dependency_kind) { + (_, DependencyKind::Normal) => true, + (ExtraDeps::All, _) => true, + (ExtraDeps::Build, DependencyKind::Build) => true, + (ExtraDeps::Dev, DependencyKind::Development) => true, + _ => false, + } + } +} + +#[cfg(test)] +mod extra_deps_tests { + use super::*; + use rstest::*; + + #[rstest( + input_extra_deps, + input_dependency_kind, + expected_allows, + case(ExtraDeps::All, DependencyKind::Normal, true), + case(ExtraDeps::Build, DependencyKind::Normal, true), + case(ExtraDeps::Dev, DependencyKind::Normal, true), + case(ExtraDeps::NoMore, DependencyKind::Normal, true), + case(ExtraDeps::All, DependencyKind::Build, true), + case(ExtraDeps::All, DependencyKind::Development, true), + case(ExtraDeps::Build, DependencyKind::Build, true), + case(ExtraDeps::Build, DependencyKind::Development, false), + case(ExtraDeps::Dev, DependencyKind::Build, false), + case(ExtraDeps::Dev, DependencyKind::Development, true) + )] + fn extra_deps_allows_test( + input_extra_deps: ExtraDeps, + input_dependency_kind: DependencyKind, + expected_allows: bool, + ) { + assert_eq!( + input_extra_deps.allows(input_dependency_kind), + expected_allows + ); + } +} \ No newline at end of file diff --git a/cargo-geiger/src/mapping.rs b/cargo-geiger/src/mapping.rs index 5943b4fc..de57f2b7 100644 --- a/cargo-geiger/src/mapping.rs +++ b/cargo-geiger/src/mapping.rs @@ -3,7 +3,6 @@ mod krates; mod metadata; use metadata::package_id::ToCargoMetadataPackage; -use metadata::package::{GetPackageName, GetPackageParent, GetPackageVersion}; use ::krates::Krates; use cargo::core::dependency::DepKind; @@ -16,11 +15,13 @@ use cargo_metadata::Dependency as CargoMetadataDependency; use cargo_metadata::PackageId as CargoMetadataPackageId; use cargo_metadata::Version as CargoMetadataVersion; +use crate::mapping::krates::GetNodeForKid; +use crate::mapping::metadata::dependency::GetDependencyInformation; +use crate::mapping::metadata::package::GetPackageInformation; +use crate::mapping::metadata::GetMetadataPackages; use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; use cargo_geiger_serde::PackageId as CargoGeigerSerdePackageId; use cargo_geiger_serde::Source as CargoGeigerSerdeSource; -use crate::mapping::metadata::GetMetadataPackages; -use crate::mapping::metadata::dependency::{GetDependencyName, GetDependencyRequirement}; /// Holds a pointer to both a `Krates` graph, and the `Metadata` struct /// which are often required together @@ -33,30 +34,35 @@ pub trait DepsNotReplaced { fn deps_not_replaced( &self, package_id: &T, - ) -> Option< - Vec<( - CargoMetadataPackageId, - HashSet, - )>, - >; + ) -> Option)>>; } -pub trait GetLicenceFromCargoMetadataPackageId { - fn get_licence_from_cargo_metadata_package_id( +pub trait GetPackageIdInformation { + fn get_package_id_licence( &self, - package_id: &CargoMetadataPackageId, + krates: &T, ) -> Option; -} + fn get_package_id_name_and_version( + &self, + krates: &T, + ) -> Option<(String, CargoMetadataVersion)>; + + fn get_package_id_repository( + &self, + krates: &T, + ) -> Option; +} -pub trait GetPackageRoot : GetPackageName + GetPackageParent + GetPackageVersion { +pub trait GetPackageRoot: GetPackageInformation { fn get_root(&self) -> Option { match self.get_package_parent() { Some(path) => Some(path.to_path_buf()), None => { eprintln!( "Failed to get root for: {} {:?}", - self.get_package_name(), self.get_package_version() + self.get_package_name(), + self.get_package_version() ); None } @@ -64,25 +70,11 @@ pub trait GetPackageRoot : GetPackageName + GetPackageParent + GetPackageVersion } } -pub trait GetPackageNameAndVersionFromCargoMetadataPackageId { - fn get_package_name_and_version_from_cargo_metadata_package_id( - &self, - package_id: &CargoMetadataPackageId, - ) -> Option<(String, CargoMetadataVersion)>; -} - -pub trait GetRepositoryFromCargoMetadataPackageId { - fn get_repository_from_cargo_metadata_package_id( - &self, - package_id: &CargoMetadataPackageId, - ) -> Option; -} - pub trait MatchesIgnoringSource { - fn matches_ignoring_source( + fn matches_ignoring_source( &self, - krates: &Krates, - package_id: CargoMetadataPackageId, + krates: &T, + package_id: &U, ) -> Option; } @@ -114,14 +106,17 @@ pub trait ToCargoGeigerSource { ) -> CargoGeigerSerdeSource; } - -pub trait ToCargoMetadataPackageId: GetDependencyName + GetDependencyRequirement { +pub trait ToCargoMetadataPackageId: GetDependencyInformation { fn to_cargo_metadata_package_id( &self, metadata: &T, ) -> Option { - metadata.get_metadata_packages() - .filter(|p| p.name == self.get_dependency_name() && self.get_dependency_requirement().matches(&p.version)) + metadata + .get_metadata_packages() + .filter(|p| { + p.name == self.get_dependency_name() + && self.get_dependency_version_req().matches(&p.version) + }) .map(|p| p.id.clone()) .collect::>() .pop() @@ -132,8 +127,8 @@ pub trait ToCargoMetadataPackageId: GetDependencyName + GetDependencyRequirement mod mapping_tests { use super::*; - use std::path::Path; use rstest::*; + use std::path::Path; struct MockPackage<'a> { mock_package_name: String, @@ -141,19 +136,15 @@ mod mapping_tests { mock_package_version: CargoMetadataVersion, } - impl GetPackageName for MockPackage<'_> { + impl GetPackageInformation for MockPackage<'_> { fn get_package_name(&self) -> String { self.mock_package_name.clone() } - } - impl GetPackageParent for MockPackage<'_> { fn get_package_parent(&self) -> Option<&Path> { self.mock_package_parent } - } - impl GetPackageVersion for MockPackage<'_> { fn get_package_version(&self) -> CargoMetadataVersion { self.mock_package_version.clone() } @@ -168,29 +159,23 @@ mod mapping_tests { Some(Path::new("/path/to/file")), Some(PathBuf::from("/path/to/file")) ), - case( - None, - None - ) + case(None, None) )] fn get_package_root_test( input_package_path_option: Option<&Path>, - expected_package_path_buf_option: Option + expected_package_path_buf_option: Option, ) { let _mock_package_parent = match input_package_path_option { Some(path) => Some(path), - None => None + None => None, }; let mock_package = MockPackage { mock_package_name: String::from("package_name"), mock_package_parent: input_package_path_option, - mock_package_version: CargoMetadataVersion::new(1,1,1) + mock_package_version: CargoMetadataVersion::new(1, 1, 1), }; - assert_eq!( - mock_package.get_root(), - expected_package_path_buf_option - ) + assert_eq!(mock_package.get_root(), expected_package_path_buf_option) } } diff --git a/cargo-geiger/src/mapping/geiger.rs b/cargo-geiger/src/mapping/geiger.rs index 70135531..d0712001 100644 --- a/cargo-geiger/src/mapping/geiger.rs +++ b/cargo-geiger/src/mapping/geiger.rs @@ -1,5 +1,5 @@ -use super::{ToCargoGeigerSource}; use super::metadata::package_id::{GetPackageIdRepr, ToCargoMetadataPackage}; +use super::ToCargoGeigerSource; use cargo_metadata::Metadata; use url::Url; diff --git a/cargo-geiger/src/mapping/krates.rs b/cargo-geiger/src/mapping/krates.rs index 00eef710..a5c418a9 100644 --- a/cargo-geiger/src/mapping/krates.rs +++ b/cargo-geiger/src/mapping/krates.rs @@ -1,43 +1,23 @@ -use crate::mapping::{ - GetLicenceFromCargoMetadataPackageId, - GetPackageNameAndVersionFromCargoMetadataPackageId, - GetRepositoryFromCargoMetadataPackageId, QueryResolve, -}; +use crate::mapping::QueryResolve; -use krates::{Krates, PkgSpec}; +use krates::{Krates, Node, PkgSpec}; use std::str::FromStr; -use cargo_metadata::PackageId as CargoMetadataPackageId; -use cargo_metadata::Version as CargoMetadataVersion; +use cargo_metadata::{Package, PackageId as CargoMetadataPackageId}; -impl GetLicenceFromCargoMetadataPackageId for Krates { - fn get_licence_from_cargo_metadata_package_id( +pub trait GetNodeForKid { + fn get_node_for_kid( &self, package_id: &CargoMetadataPackageId, - ) -> Option { - self.node_for_kid(package_id) - .and_then(|package| package.krate.clone().license) - } + ) -> Option<&Node>; } -impl GetPackageNameAndVersionFromCargoMetadataPackageId for Krates { - fn get_package_name_and_version_from_cargo_metadata_package_id( +impl GetNodeForKid for Krates { + fn get_node_for_kid( &self, package_id: &CargoMetadataPackageId, - ) -> Option<(String, CargoMetadataVersion)> { - self.node_for_kid(package_id).map(|package| { - (package.krate.clone().name, package.krate.clone().version) - }) - } -} - -impl GetRepositoryFromCargoMetadataPackageId for Krates { - fn get_repository_from_cargo_metadata_package_id( - &self, - package_id: &CargoMetadataPackageId, - ) -> Option { + ) -> Option<&Node> { self.node_for_kid(package_id) - .and_then(|package| package.krate.clone().repository) } } @@ -62,6 +42,8 @@ impl QueryResolve for Krates { mod krates_tests { use super::*; + use crate::mapping::GetPackageIdInformation; + use cargo_metadata::{CargoOpt, Metadata, MetadataCommand, Version}; use krates::Builder as KratesBuilder; use rstest::*; @@ -70,22 +52,18 @@ mod krates_tests { fn get_licence_from_cargo_metadata_package_id_test() { let (krates, metadata) = construct_krates_and_metadata(); let package = metadata.root_package().unwrap(); - let licence_option = - krates.get_licence_from_cargo_metadata_package_id(&package.id); - assert!(licence_option.is_some()); - let licence = licence_option.unwrap(); - assert_eq!(licence, String::from("Apache-2.0/MIT")) + let licence_option = &package.id.get_package_id_licence(&krates); + assert!(licence_option.as_ref().is_some()); + let licence = licence_option.as_ref().unwrap(); + assert_eq!(licence, &String::from("Apache-2.0/MIT")) } #[rstest] fn get_package_name_from_cargo_metadata_package_id_test() { let (krates, metadata) = construct_krates_and_metadata(); let package = metadata.root_package().unwrap(); - let (package_name, _) = krates - .get_package_name_and_version_from_cargo_metadata_package_id( - &package.id, - ) - .unwrap(); + let (package_name, _) = + package.id.get_package_id_name_and_version(&krates).unwrap(); assert_eq!(package_name, package.name); } @@ -93,11 +71,8 @@ mod krates_tests { fn get_package_version_from_cargo_metadata_package_id_test() { let (krates, metadata) = construct_krates_and_metadata(); let package = metadata.root_package().unwrap(); - let (_, package_version) = krates - .get_package_name_and_version_from_cargo_metadata_package_id( - &package.id, - ) - .unwrap(); + let (_, package_version) = + package.id.get_package_id_name_and_version(&krates).unwrap(); assert_eq!(package_version, package.version); } @@ -105,13 +80,12 @@ mod krates_tests { fn get_repository_from_cargo_metadata_package_id_test() { let (krates, metadata) = construct_krates_and_metadata(); let package = metadata.root_package().unwrap(); - let repository_option = - krates.get_repository_from_cargo_metadata_package_id(&package.id); + let repository_option = &package.id.get_package_id_repository(&krates); assert!(repository_option.is_some()); - let repository = repository_option.unwrap(); + let repository = repository_option.as_ref().unwrap(); assert_eq!( repository, - String::from("https://github.com/rust-secure-code/cargo-geiger") + &String::from("https://github.com/rust-secure-code/cargo-geiger") ); } @@ -150,11 +124,8 @@ mod krates_tests { let (krates, _) = construct_krates_and_metadata(); let package_id = krates.query_resolve(input_query_string).unwrap(); - let (package_name, package_version) = krates - .get_package_name_and_version_from_cargo_metadata_package_id( - &package_id, - ) - .unwrap(); + let (package_name, package_version) = + package_id.get_package_id_name_and_version(&krates).unwrap(); assert_eq!(package_name, expected_package_name); assert_eq!(package_version, expected_package_version); diff --git a/cargo-geiger/src/mapping/metadata.rs b/cargo-geiger/src/mapping/metadata.rs index 1815becb..17da77bd 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -1,39 +1,34 @@ pub mod dependency; -pub mod package_id; pub mod package; +pub mod package_id; use super::{ - DepsNotReplaced, GetPackageNameAndVersionFromCargoMetadataPackageId, - MatchesIgnoringSource, ToCargoGeigerPackageId, - ToCargoMetadataPackageId, + DepsNotReplaced, GetPackageIdInformation, MatchesIgnoringSource, + ToCargoGeigerPackageId, ToCargoMetadataPackageId, }; use package_id::ToCargoMetadataPackage; -use crate::mapping::{ToCargoGeigerDependencyKind, ToCargoGeigerSource, GetPackageRoot}; +use crate::mapping::{ToCargoGeigerDependencyKind, ToCargoGeigerSource}; use cargo_metadata::Metadata; -use krates::Krates; use std::collections::HashSet; use std::fmt::Display; use std::slice::Iter; -use cargo_metadata::DependencyKind as CargoMetadataDependencyKind; use cargo_metadata::Dependency as CargoMetadataDependency; -use cargo_metadata::PackageId as CargoMetadataPackageId; +use cargo_metadata::DependencyKind as CargoMetadataDependencyKind; use cargo_metadata::Package as CargoMetadataPackage; +use cargo_metadata::PackageId as CargoMetadataPackageId; use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; +use crate::mapping::krates::GetNodeForKid; impl DepsNotReplaced for Metadata { fn deps_not_replaced( &self, package_id: &T, - ) -> Option< - Vec<( - CargoMetadataPackageId, - HashSet, - )>, - > { + ) -> Option)>> + { let mut cargo_metadata_deps_not_replaced = vec![]; let mut package_id_hashset = HashSet::::new(); @@ -62,18 +57,13 @@ impl DepsNotReplaced for Metadata { } } -impl GetPackageRoot for CargoMetadataPackage { } - impl MatchesIgnoringSource for CargoMetadataDependency { - fn matches_ignoring_source( + fn matches_ignoring_source( &self, - krates: &Krates, - package_id: CargoMetadataPackageId, + krates: &T, + package_id: &U, ) -> Option { - match krates - .get_package_name_and_version_from_cargo_metadata_package_id( - &package_id, - ) { + match package_id.get_package_id_name_and_version(krates) { Some((name, version)) => { Some(name == self.name && self.req.matches(&version)) } @@ -133,7 +123,7 @@ impl ToCargoGeigerPackageId for CargoMetadataPackageId { } } -impl ToCargoMetadataPackageId for CargoMetadataDependency { } +impl ToCargoMetadataPackageId for CargoMetadataDependency {} pub trait GetMetadataPackages { fn get_metadata_packages(&self) -> Iter; @@ -149,13 +139,14 @@ impl GetMetadataPackages for Metadata { mod metadata_tests { use super::*; - use super::super::{ - GetPackageNameAndVersionFromCargoMetadataPackageId, ToCargoCoreDepKind, - }; + use super::super::{GetPackageIdInformation, ToCargoCoreDepKind}; use crate::args::FeaturesArgs; use crate::cli::get_workspace; + use crate::mapping::metadata::dependency::GetDependencyInformation; + use crate::mapping::GetPackageRoot; + use cargo::core::dependency::DepKind; use cargo::core::registry::PackageRegistry; use cargo::core::resolver::ResolveOpts; @@ -164,6 +155,8 @@ mod metadata_tests { }; use cargo::{ops, CargoResult, Config}; use cargo_metadata::{CargoOpt, Metadata, MetadataCommand}; + use krates::{Krates}; + use krates::semver::VersionReq; use krates::Builder as KratesBuilder; use rstest::*; use std::path::PathBuf; @@ -197,9 +190,8 @@ mod metadata_tests { let mut cargo_metadata_package_names = cargo_metadata_deps_not_replaced .iter() .map(|(p, _)| { - let (name, _) = krates - .get_package_name_and_version_from_cargo_metadata_package_id(p) - .unwrap(); + let (name, _) = + p.get_package_id_name_and_version(&krates).unwrap(); name }) .collect::>(); @@ -230,7 +222,7 @@ mod metadata_tests { assert_eq!( dependency - .matches_ignoring_source(&krates, package.clone().id) + .matches_ignoring_source(&krates, &package.clone().id) .unwrap(), false ); @@ -248,7 +240,7 @@ mod metadata_tests { assert!( dependency - .matches_ignoring_source(&krates, dependency_package_id) + .matches_ignoring_source(&krates, &dependency_package_id) .unwrap(), true ); @@ -367,30 +359,23 @@ mod metadata_tests { fn to_cargo_core_dep_kind(&self) -> DepKind { match self { CargoMetadataDependencyKind::Build => DepKind::Build, - CargoMetadataDependencyKind::Development => DepKind::Development, + CargoMetadataDependencyKind::Development => { + DepKind::Development + } CargoMetadataDependencyKind::Normal => DepKind::Normal, _ => panic!("Unknown dependency kind"), } } } - impl ToCargoMetadataPackageId for PackageId { - fn to_cargo_metadata_package_id( - &self, - metadata: &Metadata, - ) -> Option { - metadata - .packages - .iter() - .filter(|p| { - p.name == self.name().to_string() - && p.version.major == self.version().major - && p.version.minor == self.version().minor - && p.version.patch == self.version().patch - }) - .map(|p| p.id.clone()) - .collect::>() - .pop() + impl ToCargoMetadataPackageId for PackageId {} + + impl GetDependencyInformation for PackageId { + fn get_dependency_name(&self) -> String { + self.name().clone().to_string() + } + fn get_dependency_version_req(&self) -> VersionReq { + VersionReq::parse(&self.version().clone().to_string()).unwrap() } } } diff --git a/cargo-geiger/src/mapping/metadata/dependency.rs b/cargo-geiger/src/mapping/metadata/dependency.rs index 3bbad418..31d89853 100644 --- a/cargo-geiger/src/mapping/metadata/dependency.rs +++ b/cargo-geiger/src/mapping/metadata/dependency.rs @@ -1,22 +1,16 @@ use cargo_metadata::Dependency; use krates::semver::VersionReq; -pub trait GetDependencyName { +pub trait GetDependencyInformation { fn get_dependency_name(&self) -> String; + fn get_dependency_version_req(&self) -> VersionReq; } -impl GetDependencyName for Dependency { +impl GetDependencyInformation for Dependency { fn get_dependency_name(&self) -> String { self.name.clone() } -} - -pub trait GetDependencyRequirement { - fn get_dependency_requirement(&self) -> VersionReq; -} - -impl GetDependencyRequirement for Dependency { - fn get_dependency_requirement(&self) -> VersionReq { + fn get_dependency_version_req(&self) -> VersionReq { self.req.clone() } -} \ No newline at end of file +} diff --git a/cargo-geiger/src/mapping/metadata/package.rs b/cargo-geiger/src/mapping/metadata/package.rs index bd63e1cc..0f13a037 100644 --- a/cargo-geiger/src/mapping/metadata/package.rs +++ b/cargo-geiger/src/mapping/metadata/package.rs @@ -1,32 +1,28 @@ use cargo_metadata::{Package, Version}; use std::path::Path; -pub trait GetPackageName { +use crate::mapping::GetPackageRoot; + +pub trait GetPackageInformation { fn get_package_name(&self) -> String; -} -pub trait GetPackageParent { fn get_package_parent(&self) -> Option<&Path>; -} -pub trait GetPackageVersion { fn get_package_version(&self) -> Version; } -impl GetPackageName for Package { +impl GetPackageInformation for Package { fn get_package_name(&self) -> String { self.name.clone() } -} -impl GetPackageParent for Package { fn get_package_parent(&self) -> Option<&Path> { self.manifest_path.parent() } -} -impl GetPackageVersion for Package { fn get_package_version(&self) -> Version { self.version.clone() } -} \ No newline at end of file +} + +impl GetPackageRoot for Package {} diff --git a/cargo-geiger/src/mapping/metadata/package_id.rs b/cargo-geiger/src/mapping/metadata/package_id.rs index d45eb950..36880e10 100644 --- a/cargo-geiger/src/mapping/metadata/package_id.rs +++ b/cargo-geiger/src/mapping/metadata/package_id.rs @@ -1,4 +1,35 @@ -use cargo_metadata::{Metadata, PackageId, Package}; +use crate::mapping::krates::GetNodeForKid; +use crate::mapping::GetPackageIdInformation; +use cargo_metadata::{Metadata, Package, PackageId, Version}; + +impl GetPackageIdInformation for PackageId { + fn get_package_id_licence( + &self, + krates: &T, + ) -> Option { + krates + .get_node_for_kid(self) + .and_then(|package| package.krate.clone().license) + } + + fn get_package_id_name_and_version( + &self, + krates: &T, + ) -> Option<(String, Version)> { + krates.get_node_for_kid(self).map(|package| { + (package.krate.clone().name, package.krate.clone().version) + }) + } + + fn get_package_id_repository( + &self, + krates: &T, + ) -> Option { + krates + .get_node_for_kid(self) + .and_then(|package| package.krate.clone().repository) + } +} pub trait GetPackageIdRepr { fn get_package_id_repr(&self) -> String; @@ -11,10 +42,8 @@ impl GetPackageIdRepr for PackageId { } pub trait ToCargoMetadataPackage { - fn to_cargo_metadata_package( - &self, - metadata: &Metadata, - ) -> Option; + fn to_cargo_metadata_package(&self, metadata: &Metadata) + -> Option; } impl ToCargoMetadataPackage for PackageId { @@ -30,4 +59,4 @@ impl ToCargoMetadataPackage for PackageId { .collect::>() .pop() } -} \ No newline at end of file +} From 071c023ad1cede584c899a673921d6113dbc726a Mon Sep 17 00:00:00 2001 From: joshmc Date: Thu, 4 Mar 2021 18:20:23 +0000 Subject: [PATCH 3/3] NOISSUE - Refactor the mapping module to accept traits, with a view to be able to mock methods in unit tests --- Cargo.lock | 1 - cargo-geiger/Cargo.toml | 1 - cargo-geiger/src/args.rs | 2 +- cargo-geiger/src/format/print_config.rs | 48 ++-- .../src/format/table/handle_text_tree_line.rs | 79 +++++- cargo-geiger/src/graph/extra_deps.rs | 2 +- cargo-geiger/src/lib.rs | 21 ++ cargo-geiger/src/main.rs | 1 - cargo-geiger/src/mapping.rs | 1 + cargo-geiger/src/mapping/geiger.rs | 27 +- cargo-geiger/src/mapping/krates.rs | 19 +- cargo-geiger/src/mapping/metadata.rs | 20 +- cargo-geiger/src/scan.rs | 245 +++++++++++++++--- cargo-geiger/src/scan/rs_file.rs | 2 +- 14 files changed, 363 insertions(+), 106 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d66c2d1e..6cb8eef7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -197,7 +197,6 @@ dependencies = [ "cargo_metadata", "colored", "console 0.11.3", - "env_logger", "fs_extra", "geiger", "insta", diff --git a/cargo-geiger/Cargo.toml b/cargo-geiger/Cargo.toml index 6a5cb2a3..0ac5de20 100644 --- a/cargo-geiger/Cargo.toml +++ b/cargo-geiger/Cargo.toml @@ -21,7 +21,6 @@ cargo_metadata = "0.12.3" cargo-platform = "0.1.1" colored = "2.0.0" console = "0.11.3" -env_logger = "0.8.2" geiger = { path = "../geiger", version = "0.4.6" } krates = "0.5.0" petgraph = "0.5.1" diff --git a/cargo-geiger/src/args.rs b/cargo-geiger/src/args.rs index 44131faa..31f692f3 100644 --- a/cargo-geiger/src/args.rs +++ b/cargo-geiger/src/args.rs @@ -127,7 +127,7 @@ impl Args { manifest_path: raw_args.opt_value_from_str("--manifest-path")?, no_indent: raw_args.contains("--no-indent"), offline: raw_args.contains("--offline"), - package: raw_args.opt_value_from_str("--manifest-path")?, + package: raw_args.opt_value_from_str(["-p", "--package"])?, prefix_depth: raw_args.contains("--prefix-depth"), quiet: raw_args.contains(["-q", "--quiet"]), readme_args: ReadmeArgs { diff --git a/cargo-geiger/src/format/print_config.rs b/cargo-geiger/src/format/print_config.rs index 27bf89bc..002e8fd5 100644 --- a/cargo-geiger/src/format/print_config.rs +++ b/cargo-geiger/src/format/print_config.rs @@ -6,7 +6,7 @@ use cargo::core::shell::Verbosity; use cargo::util::errors::CliError; use colored::{ColoredString, Colorize}; use geiger::IncludeTests; -use petgraph::EdgeDirection; +use petgraph::{Direction, EdgeDirection}; use strum_macros::EnumString; #[derive(Clone, Copy, Debug, PartialEq)] @@ -54,10 +54,9 @@ impl PrintConfig { // TODO: Add command line flag for this and make it default to false? let allow_partial_results = true; - let direction = if args.invert { - EdgeDirection::Incoming - } else { - EdgeDirection::Outgoing + let direction = match args.invert { + true => EdgeDirection::Incoming, + false => EdgeDirection::Outgoing, }; let format = Pattern::try_build(&args.format).map_err(|e| { @@ -70,24 +69,20 @@ impl PrintConfig { ) })?; - let include_tests = if args.include_tests { - IncludeTests::Yes - } else { - IncludeTests::No + let include_tests = match args.include_tests { + true => IncludeTests::Yes, + false => IncludeTests::No, }; - let prefix = if args.prefix_depth { - Prefix::Depth - } else if args.no_indent { - Prefix::None - } else { - Prefix::Indent + let prefix = match (args.prefix_depth, args.no_indent) { + (true, _) => Prefix::Depth, + (false, true) => Prefix::None, + (false, false) => Prefix::Indent, }; - let verbosity = if args.verbose == 0 { - Verbosity::Normal - } else { - Verbosity::Verbose + let verbosity = match args.verbose { + 0 => Verbosity::Normal, + _ => Verbosity::Verbose, }; Ok(PrintConfig { @@ -103,6 +98,21 @@ impl PrintConfig { } } +impl Default for PrintConfig { + fn default() -> Self { + PrintConfig { + all: false, + allow_partial_results: false, + direction: Direction::Outgoing, + format: Pattern::try_build("p").unwrap(), + include_tests: IncludeTests::Yes, + prefix: Prefix::Depth, + output_format: Default::default(), + verbosity: Verbosity::Verbose, + } + } +} + pub fn colorize( crate_detection_status: &CrateDetectionStatus, output_format: OutputFormat, diff --git a/cargo-geiger/src/format/table/handle_text_tree_line.rs b/cargo-geiger/src/format/table/handle_text_tree_line.rs index 754faede..edcb259a 100644 --- a/cargo-geiger/src/format/table/handle_text_tree_line.rs +++ b/cargo-geiger/src/format/table/handle_text_tree_line.rs @@ -9,7 +9,9 @@ use super::TableParameters; use super::{table_row, table_row_empty}; use cargo_metadata::{DependencyKind, PackageId}; +use colored::ColoredString; use std::collections::HashSet; +use std::fmt::Display; pub struct HandlePackageParameters<'a> { pub total_package_counts: &'a mut TotalPackageCounts, @@ -112,6 +114,28 @@ pub fn handle_text_tree_line_package( ), ); + let line = construct_package_text_tree_line( + crate_detection_status, + emoji_symbols, + icon, + package_name, + table_parameters, + tree_vines, + unsafe_info, + ); + + table_lines.push(line); +} + +fn construct_package_text_tree_line( + crate_detection_status: CrateDetectionStatus, + emoji_symbols: &EmojiSymbols, + icon: Box, + package_name: ColoredString, + table_parameters: &TableParameters, + tree_vines: String, + unsafe_info: ColoredString, +) -> String { let shift_chars = unsafe_info.chars().count() + 4; let mut line = String::new(); @@ -139,7 +163,7 @@ pub fn handle_text_tree_line_package( line.push(' '); } - table_lines.push(format!("{} {}{}", line, tree_vines, package_name)); + format!("{} {}{}", line, tree_vines, package_name) } fn get_crate_detection_status_and_update_package_counts( @@ -174,6 +198,8 @@ fn get_crate_detection_status_and_update_package_counts( mod handle_text_tree_line_tests { use super::*; + use crate::format::print_config::PrintConfig; + use colored::Colorize; use rstest::*; #[rstest( @@ -219,6 +245,57 @@ mod handle_text_tree_line_tests { } } + #[rstest( + input_crate_detection_status, + input_output_format, + input_symbol_kind, + expected_package_text_tree_line, + case( + CrateDetectionStatus::NoneDetectedForbidsUnsafe, + OutputFormat::GitHubMarkdown, + SymbolKind::Lock, + String::from("unsafe_info 🔒 tree_vinespackage_name") + ), + case( + CrateDetectionStatus::UnsafeDetected, + OutputFormat::GitHubMarkdown, + SymbolKind::Rads, + String::from("unsafe_info ☢\u{fe0f} tree_vinespackage_name") + ) + )] + fn construct_package_text_tree_line_test( + input_crate_detection_status: CrateDetectionStatus, + input_output_format: OutputFormat, + input_symbol_kind: SymbolKind, + expected_package_text_tree_line: String, + ) { + let emoji_symbols = EmojiSymbols::new(input_output_format); + let icon = emoji_symbols.emoji(input_symbol_kind); + let package_name = String::from("package_name").normal(); + let table_parameters = TableParameters { + geiger_context: &Default::default(), + print_config: &PrintConfig { + output_format: input_output_format, + ..Default::default() + }, + rs_files_used: &Default::default(), + }; + let tree_vines = String::from("tree_vines"); + let unsafe_info = ColoredString::from("unsafe_info").normal(); + + let package_text_tree_line = construct_package_text_tree_line( + input_crate_detection_status, + &emoji_symbols, + icon, + package_name, + &table_parameters, + tree_vines, + unsafe_info, + ); + + assert_eq!(package_text_tree_line, expected_package_text_tree_line); + } + #[rstest( input_crate_forbids_unsafe, input_total_inc, diff --git a/cargo-geiger/src/graph/extra_deps.rs b/cargo-geiger/src/graph/extra_deps.rs index 4d599f10..4a1d124e 100644 --- a/cargo-geiger/src/graph/extra_deps.rs +++ b/cargo-geiger/src/graph/extra_deps.rs @@ -52,4 +52,4 @@ mod extra_deps_tests { expected_allows ); } -} \ No newline at end of file +} diff --git a/cargo-geiger/src/lib.rs b/cargo-geiger/src/lib.rs index 62767c69..cc081460 100644 --- a/cargo-geiger/src/lib.rs +++ b/cargo-geiger/src/lib.rs @@ -24,3 +24,24 @@ pub mod scan; mod format; /// Tree construction mod tree; + +#[cfg(test)] +mod lib_tests { + use cargo_metadata::{CargoOpt, Metadata, MetadataCommand}; + use krates::Builder as KratesBuilder; + use krates::Krates; + + pub fn construct_krates_and_metadata() -> (Krates, Metadata) { + let metadata = MetadataCommand::new() + .manifest_path("./Cargo.toml") + .features(CargoOpt::AllFeatures) + .exec() + .unwrap(); + + let krates = KratesBuilder::new() + .build_with_metadata(metadata.clone(), |_| ()) + .unwrap(); + + (krates, metadata) + } +} diff --git a/cargo-geiger/src/main.rs b/cargo-geiger/src/main.rs index 830f7a50..c82dbeb1 100644 --- a/cargo-geiger/src/main.rs +++ b/cargo-geiger/src/main.rs @@ -114,7 +114,6 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult { } fn main() { - env_logger::init(); let mut config = match Config::default() { Ok(cfg) => cfg, Err(e) => { diff --git a/cargo-geiger/src/mapping.rs b/cargo-geiger/src/mapping.rs index de57f2b7..17018e7b 100644 --- a/cargo-geiger/src/mapping.rs +++ b/cargo-geiger/src/mapping.rs @@ -19,6 +19,7 @@ use crate::mapping::krates::GetNodeForKid; use crate::mapping::metadata::dependency::GetDependencyInformation; use crate::mapping::metadata::package::GetPackageInformation; use crate::mapping::metadata::GetMetadataPackages; + use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; use cargo_geiger_serde::PackageId as CargoGeigerSerdePackageId; use cargo_geiger_serde::Source as CargoGeigerSerdeSource; diff --git a/cargo-geiger/src/mapping/geiger.rs b/cargo-geiger/src/mapping/geiger.rs index d0712001..3349361a 100644 --- a/cargo-geiger/src/mapping/geiger.rs +++ b/cargo-geiger/src/mapping/geiger.rs @@ -4,11 +4,14 @@ use super::ToCargoGeigerSource; use cargo_metadata::Metadata; use url::Url; -impl ToCargoGeigerSource for cargo_metadata::PackageId { +use cargo_geiger_serde::Source as CargoGeigerSerdeSource; +use cargo_metadata::PackageId as CargoMetadataPackageId; + +impl ToCargoGeigerSource for CargoMetadataPackageId { fn to_cargo_geiger_source( &self, metadata: &Metadata, - ) -> cargo_geiger_serde::Source { + ) -> CargoGeigerSerdeSource { let package = self.to_cargo_metadata_package(metadata).unwrap(); match package.source { @@ -18,14 +21,14 @@ impl ToCargoGeigerSource for cargo_metadata::PackageId { } } -fn handle_source_repr(source_repr: &str) -> cargo_geiger_serde::Source { +fn handle_source_repr(source_repr: &str) -> CargoGeigerSerdeSource { let mut source_repr_vec = source_repr.split('+').collect::>(); let source_type = source_repr_vec[0]; match source_type { "registry" => { - cargo_geiger_serde::Source::Registry { + CargoGeigerSerdeSource::Registry { // It looks like cargo metadata drops this information name: String::from("crates.io"), url: Url::parse(source_repr_vec.pop().unwrap()).unwrap(), @@ -46,7 +49,7 @@ fn handle_source_repr(source_repr: &str) -> cargo_geiger_serde::Source { .unwrap() .1; - cargo_geiger_serde::Source::Git { + CargoGeigerSerdeSource::Git { url: Url::parse(&git_url_without_query).unwrap(), rev: String::from(revision), } @@ -59,7 +62,7 @@ fn handle_source_repr(source_repr: &str) -> cargo_geiger_serde::Source { fn handle_path_source( package_id: &T, -) -> cargo_geiger_serde::Source { +) -> CargoGeigerSerdeSource { let raw_repr = package_id.get_package_id_repr(); let raw_path_repr = raw_repr[1..raw_repr.len() - 1] .split("+file://") @@ -75,7 +78,7 @@ fn handle_path_source( source_url = Url::from_file_path(raw_path_repr).unwrap(); } - cargo_geiger_serde::Source::Path(source_url) + CargoGeigerSerdeSource::Path(source_url) } #[cfg(test)] @@ -90,14 +93,14 @@ mod geiger_tests { expected_source, case( "registry+https://github.com/rust-lang/crates.io-index", - cargo_geiger_serde::Source::Registry { + CargoGeigerSerdeSource::Registry { name: String::from("crates.io"), url: Url::parse("https://github.com/rust-lang/crates.io-index").unwrap() } ), case( "git+https://github.com/rust-itertools/itertools.git?rev=8761fbefb3b209", - cargo_geiger_serde::Source::Git { + CargoGeigerSerdeSource::Git { url: Url::parse("https://github.com/rust-itertools/itertools.git").unwrap(), rev: String::from("8761fbefb3b209") } @@ -105,7 +108,7 @@ mod geiger_tests { )] fn handle_source_repr_test( input_source_repr: &str, - expected_source: cargo_geiger_serde::Source, + expected_source: CargoGeigerSerdeSource, ) { let source = handle_source_repr(input_source_repr); assert_eq!(source, expected_source); @@ -114,11 +117,11 @@ mod geiger_tests { #[rstest] fn handle_path_source_test() { if !cfg!(windows) { - let package_id = cargo_metadata::PackageId { + let package_id = CargoMetadataPackageId { repr: String::from("(path+file:///cargo_geiger/test_crates/test1_package_with_no_deps)"), }; - let expected_source = cargo_geiger_serde::Source::Path( + let expected_source = CargoGeigerSerdeSource::Path( Url::from_file_path( "/cargo_geiger/test_crates/test1_package_with_no_deps", ) diff --git a/cargo-geiger/src/mapping/krates.rs b/cargo-geiger/src/mapping/krates.rs index a5c418a9..1d21ee03 100644 --- a/cargo-geiger/src/mapping/krates.rs +++ b/cargo-geiger/src/mapping/krates.rs @@ -42,10 +42,11 @@ impl QueryResolve for Krates { mod krates_tests { use super::*; + use crate::lib_tests::construct_krates_and_metadata; use crate::mapping::GetPackageIdInformation; - use cargo_metadata::{CargoOpt, Metadata, MetadataCommand, Version}; - use krates::Builder as KratesBuilder; + use cargo_metadata::Version; + use rstest::*; #[rstest] @@ -130,18 +131,4 @@ mod krates_tests { assert_eq!(package_name, expected_package_name); assert_eq!(package_version, expected_package_version); } - - fn construct_krates_and_metadata() -> (Krates, Metadata) { - let metadata = MetadataCommand::new() - .manifest_path("./Cargo.toml") - .features(CargoOpt::AllFeatures) - .exec() - .unwrap(); - - let krates = KratesBuilder::new() - .build_with_metadata(metadata.clone(), |_| ()) - .unwrap(); - - (krates, metadata) - } } diff --git a/cargo-geiger/src/mapping/metadata.rs b/cargo-geiger/src/mapping/metadata.rs index 17da77bd..f659ce1c 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -20,8 +20,8 @@ use cargo_metadata::DependencyKind as CargoMetadataDependencyKind; use cargo_metadata::Package as CargoMetadataPackage; use cargo_metadata::PackageId as CargoMetadataPackageId; -use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; use crate::mapping::krates::GetNodeForKid; +use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; impl DepsNotReplaced for Metadata { fn deps_not_replaced( @@ -143,6 +143,7 @@ mod metadata_tests { use crate::args::FeaturesArgs; use crate::cli::get_workspace; + use crate::lib_tests::construct_krates_and_metadata; use crate::mapping::metadata::dependency::GetDependencyInformation; use crate::mapping::GetPackageRoot; @@ -154,10 +155,7 @@ mod metadata_tests { Package, PackageId, PackageIdSpec, PackageSet, Resolve, Workspace, }; use cargo::{ops, CargoResult, Config}; - use cargo_metadata::{CargoOpt, Metadata, MetadataCommand}; - use krates::{Krates}; use krates::semver::VersionReq; - use krates::Builder as KratesBuilder; use rstest::*; use std::path::PathBuf; @@ -290,20 +288,6 @@ mod metadata_tests { ); } - fn construct_krates_and_metadata() -> (Krates, Metadata) { - let metadata = MetadataCommand::new() - .manifest_path("./Cargo.toml") - .features(CargoOpt::AllFeatures) - .exec() - .unwrap(); - - let krates = KratesBuilder::new() - .build_with_metadata(metadata.clone(), |_| ()) - .unwrap(); - - (krates, metadata) - } - fn construct_package_registry_workspace_tuple( config: &Config, ) -> (Package, PackageRegistry, Workspace) { diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index c839d404..1949c712 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -18,8 +18,12 @@ use forbid::scan_forbid_unsafe; use cargo::core::Workspace; use cargo::{CliError, Config}; -use cargo_geiger_serde::{CounterBlock, PackageInfo, UnsafeInfo}; +use cargo_geiger_serde::{ + CounterBlock, DependencyKind, PackageInfo, UnsafeInfo, +}; use cargo_metadata::PackageId; +use krates::NodeId; +use petgraph::prelude::NodeIndex; use petgraph::visit::EdgeRef; use std::collections::{HashMap, HashSet}; use std::error::Error; @@ -33,10 +37,9 @@ pub struct FoundWarningsError { impl Error for FoundWarningsError {} -/// Forward Display to Debug. impl fmt::Display for FoundWarningsError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Debug::fmt(self, f) + write!(f, "Found {} warnings", self.warning_count) } } @@ -47,6 +50,7 @@ pub struct ScanResult { /// Provides a more terse and searchable name for the wrapped generic /// collection. +#[derive(Default)] pub struct GeigerContext { pub package_id_to_metrics: HashMap, } @@ -172,8 +176,11 @@ fn list_files_used_but_not_scanned( let scanned_files = geiger_context .package_id_to_metrics .iter() - .flat_map(|(_, v)| v.rs_path_to_metrics.keys()) + .flat_map(|(_, package_metrics)| { + package_metrics.rs_path_to_metrics.keys() + }) .collect::>(); + rs_files_used .iter() .cloned() @@ -203,39 +210,19 @@ fn package_metrics( for edge in graph.graph.edges(index) { let dep_index = edge.target(); - if visited.insert(dep_index) { - indices.push(dep_index); - } - - let dependency_package_id_option = graph.graph[dep_index] - .to_cargo_geiger_package_id( - cargo_metadata_parameters.metadata, - ); let dependency_kind_option = edge.weight().to_cargo_geiger_dependency_kind(); - match (dependency_package_id_option, dependency_kind_option) { - (Some(dependency_package_id), Some(dependency_kind)) => { - package_info.add_dependency( - dependency_package_id, - dependency_kind, - ); - } - (Some(dependency_package_id), None) => { - eprintln!( - "Failed to add dependency for: {} {:?}", - dependency_package_id.name, - dependency_package_id.version - ) - } - _ => { - eprintln!( - "Error converting: {} to Cargo Geiger Package Id", - graph.graph[dep_index] - ) - } - } + add_dependency_to_package_info( + cargo_metadata_parameters, + dep_index, + dependency_kind_option, + graph, + &mut indices, + &mut package_info, + &mut visited, + ); } match geiger_context.package_id_to_metrics.get(&package_id) { @@ -256,6 +243,41 @@ fn package_metrics( package_metrics } +fn add_dependency_to_package_info( + cargo_metadata_parameters: &CargoMetadataParameters, + dependency_index: NodeId, + dependency_kind_option: Option, + graph: &Graph, + indices: &mut Vec, + package_info: &mut PackageInfo, + visited: &mut HashSet, +) { + if visited.insert(dependency_index) { + indices.push(dependency_index); + } + + let dependency_package_id_option = graph.graph[dependency_index] + .to_cargo_geiger_package_id(cargo_metadata_parameters.metadata); + + match (dependency_package_id_option, dependency_kind_option) { + (Some(dependency_package_id), Some(dependency_kind)) => { + package_info.add_dependency(dependency_package_id, dependency_kind); + } + (Some(dependency_package_id), None) => { + eprintln!( + "Failed to add dependency for: {} {:?}", + dependency_package_id.name, dependency_package_id.version + ) + } + _ => { + eprintln!( + "Error converting: {} to Cargo Geiger Package Id", + graph.graph[dependency_index] + ) + } + } +} + #[cfg(test)] mod scan_tests { use super::*; @@ -263,9 +285,80 @@ mod scan_tests { use crate::scan::PackageMetrics; use rs_file::RsFileMetricsWrapper; - use cargo_geiger_serde::{Count, UnsafeInfo}; + use crate::lib_tests::construct_krates_and_metadata; + use cargo_geiger_serde::{Count, Source, UnsafeInfo}; use rstest::*; + use semver::Version; use std::{collections::HashSet, path::PathBuf}; + use url::Url; + + #[rstest( + input_dependency_kind_option, + expected_package_info_dependency_length, + case(Some(DependencyKind::Normal), 1,), + case(None, 0) + )] + fn add_dependency_to_package_info_test( + input_dependency_kind_option: Option, + expected_package_info_dependency_length: usize, + ) { + let (krates, metadata) = construct_krates_and_metadata(); + let package_id = metadata.root_package().unwrap().id.clone(); + + let cargo_metadata_parameters = CargoMetadataParameters { + krates: &krates, + metadata: &metadata, + }; + + let mut graph = Graph { + graph: Default::default(), + nodes: Default::default(), + }; + graph.graph.add_node(package_id); + + let mut package_info = PackageInfo { + id: cargo_geiger_serde::PackageId { + name: String::from("package_id"), + version: Version { + major: 0, + minor: 0, + patch: 0, + pre: vec![], + build: vec![], + }, + source: Source::Path( + Url::parse( + "https://github.com/rust-secure-code/cargo-geiger", + ) + .unwrap(), + ), + }, + dependencies: Default::default(), + dev_dependencies: Default::default(), + build_dependencies: Default::default(), + }; + + let mut indices = vec![]; + let mut visited = HashSet::new(); + + let dependency_index = NodeIndex::new(0); + + add_dependency_to_package_info( + &cargo_metadata_parameters, + NodeIndex::new(0), + input_dependency_kind_option, + &graph, + &mut indices, + &mut package_info, + &mut visited, + ); + + assert_eq!(visited, vec![dependency_index].iter().cloned().collect()); + assert_eq!( + package_info.dependencies.len(), + expected_package_info_dependency_length + ) + } #[rstest] fn construct_rs_files_used_lines_test() { @@ -287,6 +380,90 @@ mod scan_tests { ); } + #[rstest( + input_rs_path_to_metrics_vec, + input_rs_files_used_vec, + expected_files_used_but_not_scanned, + case( + vec![( + PathBuf::from("third/file/path.rs"), + RsFileMetricsWrapper { + metrics: Default::default(), + is_crate_entry_point: false, + }, + )], + vec![ + PathBuf::from("first/file/path.rs"), + PathBuf::from("second/file/path.rs"), + PathBuf::from("third/file/path.rs"), + ], + vec![ + PathBuf::from("first/file/path.rs"), + PathBuf::from("second/file/path.rs") + ] + ), + case( + vec![( + PathBuf::from("first/file/path.rs"), + RsFileMetricsWrapper { + metrics: Default::default(), + is_crate_entry_point: false, + }), + ( + PathBuf::from("second/file/path.rs"), + RsFileMetricsWrapper { + metrics: Default::default(), + is_crate_entry_point: false, + }), + (PathBuf::from("third/file/path.rs"), + RsFileMetricsWrapper { + metrics: Default::default(), + is_crate_entry_point: false, + } + )], + vec![ + PathBuf::from("first/file/path.rs"), + PathBuf::from("second/file/path.rs"), + PathBuf::from("third/file/path.rs"), + ], + vec![ + ] + ) + )] + fn list_files_used_but_not_scanned_test( + input_rs_path_to_metrics_vec: Vec<(PathBuf, RsFileMetricsWrapper)>, + input_rs_files_used_vec: Vec, + expected_files_used_but_not_scanned: Vec, + ) { + let (_, metadata) = construct_krates_and_metadata(); + let package_id = metadata.root_package().unwrap().id.clone(); + + let rs_path_to_metrics: HashMap = + input_rs_path_to_metrics_vec.iter().cloned().collect(); + + let geiger_context = GeigerContext { + package_id_to_metrics: vec![( + package_id, + PackageMetrics { rs_path_to_metrics }, + )] + .iter() + .cloned() + .collect(), + }; + + let rs_files_used = input_rs_files_used_vec.iter().cloned().collect(); + + let mut files_used_but_not_scanned = + list_files_used_but_not_scanned(&geiger_context, &rs_files_used); + + files_used_but_not_scanned.sort(); + + assert_eq!( + files_used_but_not_scanned, + expected_files_used_but_not_scanned + ) + } + #[rstest] fn unsafe_stats_from_nothing_are_empty() { let stats = unsafe_stats(&Default::default(), &Default::default()); diff --git a/cargo-geiger/src/scan/rs_file.rs b/cargo-geiger/src/scan/rs_file.rs index 13acc3b9..47236e67 100644 --- a/cargo-geiger/src/scan/rs_file.rs +++ b/cargo-geiger/src/scan/rs_file.rs @@ -222,7 +222,7 @@ fn add_dir_entries_to_path_buf_hash_set( })?; let canonical_paths = dependencies .into_iter() - .flat_map(|t| t.1) + .flat_map(|(_, dependency_files)| dependency_files) .map(PathBuf::from) .map(|pb| workspace_root.join(pb)) .map(|pb| pb.canonicalize().map_err(|e| RsResolveError::Io(e, pb)));