Merge pull request #197 from jmcconnell26/NOISSUE-RefactorMappingModu…

…leToUseTraits

NOISSUE - refactor mapping module to use traits

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,32 +16,30 @@ 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)
                     }
                 }
                 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))?

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<PackageId, cargo_metadata::DependencyKind>,
+    pub graph: petgraph::Graph<PackageId, DependencyKind>,
     pub nodes: HashMap<PackageId, NodeIndex>,
 }
 
@@ -130,7 +112,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)) => {
@@ -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,

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
+        );
+    }
+}