ISSUE-16 - Remove Cargo Core from Traversal Module:

* Switch Args struct to use Default
* Refactor rs_file module to under scan
* Remove Node struct from Graph struct, instead only use PackageId
* Pull out function to parse features from args
* Pull out cargo core usage from traversal module

Signed-off-by: joshmc <josh-mcc@tiscali.co.uk>

cargo-geiger/src/args.rs

@@ -55,6 +55,7 @@ OPTIONS:
     -V, --version                 Prints version information.
 ";
 
+#[derive(Default)]
 pub struct Args {
     pub all: bool,
     pub all_deps: bool,
@@ -64,7 +65,7 @@ pub struct Args {
     pub charset: Charset,
     pub color: Option<String>,
     pub dev_deps: bool,
-    pub features: Option<String>,
+    pub features: Vec<String>,
     pub forbid_only: bool,
     pub format: String,
     pub frozen: bool,
@@ -101,7 +102,9 @@ impl Args {
                 .unwrap_or(Charset::Utf8),
             color: raw_args.opt_value_from_str("--color")?,
             dev_deps: raw_args.contains("--dev-dependencies"),
-            features: raw_args.opt_value_from_str("--features")?,
+            features: parse_features(
+                raw_args.opt_value_from_str("--features")?,
+            ),
             forbid_only: raw_args.contains(["-f", "--forbid-only"]),
             format: raw_args
                 .opt_value_from_str("--format")?
@@ -142,6 +145,17 @@ impl Args {
     }
 }
 
+fn parse_features(raw_features: Option<String>) -> Vec<String> {
+    raw_features
+        .as_ref()
+        .cloned()
+        .unwrap_or_else(String::new)
+        .split(' ')
+        .map(str::to_owned)
+        .filter(|f| f != "")
+        .collect::<Vec<String>>()
+}
+
 #[cfg(test)]
 pub mod args_tests {
     use super::*;
@@ -202,4 +216,35 @@ pub mod args_tests {
         assert_eq!(args.charset, expected_charset);
         assert_eq!(args.verbose, expected_verbose)
     }
+
+    #[rstest(
+        input_raw_features,
+        expected_features,
+        case(
+            Some(String::from("test some features")),
+            vec![
+                String::from("test"),
+                String::from("some"),
+                String::from("features")
+            ]
+        ),
+        case(
+            Some(String::from("test")),
+            vec![String::from("test")]
+        ),
+        case(
+            Some(String::from("")),
+            vec![]
+        ),
+        case(
+            None,
+            vec![]
+        )
+    )]
+    fn parse_features_test(
+        input_raw_features: Option<String>,
+        expected_features: Vec<String>,
+    ) {
+        assert_eq!(parse_features(input_raw_features), expected_features);
+    }
 }

cargo-geiger/src/cli.rs

@@ -45,17 +45,9 @@ pub fn get_cargo_metadata(
         metadata_command.features(CargoOpt::NoDefaultFeatures);
     }
 
-    if args.features.is_some() {
-        let features = args
-            .features
-            .as_ref()
-            .cloned()
-            .unwrap_or_else(String::new)
-            .split(' ')
-            .map(str::to_owned)
-            .collect::<Vec<String>>();
-
-        metadata_command.features(CargoOpt::SomeFeatures(features));
+    if !args.features.is_empty() {
+        metadata_command
+            .features(CargoOpt::SomeFeatures(args.features.clone()));
     }
 
     Ok(metadata_command.exec()?)
@@ -113,19 +105,17 @@ pub fn get_workspace(
 }
 
 pub fn resolve<'a, 'cfg>(
+    args: &Args,
     package_id: PackageId,
     registry: &mut PackageRegistry<'cfg>,
     workspace: &'a Workspace<'cfg>,
-    features: &[String],
-    all_features: bool,
-    no_default_features: bool,
 ) -> CargoResult<(PackageSet<'a>, Resolve)> {
     let dev_deps = true; // TODO: Review this.
-    let uses_default_features = !no_default_features;
+    let uses_default_features = !args.no_default_features;
     let opts = ResolveOpts::new(
         dev_deps,
-        features,
-        all_features,
+        &args.features.clone(),
+        args.all_features,
         uses_default_features,
     );
     let prev = ops::load_pkg_lockfile(workspace)?;
@@ -150,12 +140,11 @@ pub fn resolve<'a, 'cfg>(
 #[cfg(test)]
 mod cli_tests {
     use super::*;
-    use crate::format::Charset;
     use rstest::*;
 
     #[rstest]
     fn get_cargo_metadata_test() {
-        let args = create_args();
+        let args = Args::default();
         let config = Config::default().unwrap();
 
         let cargo_metadata_result = get_cargo_metadata(&args, &config);
@@ -194,7 +183,7 @@ mod cli_tests {
 
     #[rstest]
     fn get_krates_test() {
-        let args = create_args();
+        let args = Args::default();
         let config = Config::default().unwrap();
         let cargo_metadata = get_cargo_metadata(&args, &config).unwrap();
 
@@ -243,58 +232,16 @@ mod cli_tests {
 
     #[rstest]
     fn resolve_test() {
+        let args = Args::default();
         let config = Config::default().unwrap();
         let manifest_path: Option<PathBuf> = None;
         let workspace = get_workspace(&config, manifest_path).unwrap();
         let package = workspace.current().unwrap();
         let mut registry = get_registry(&config, &package).unwrap();
 
-        let features: Vec<String> = vec![];
-        let all_features = false;
-        let no_default_features = false;
-
-        let resolve_cargo_result = resolve(
-            package.package_id(),
-            &mut registry,
-            &workspace,
-            &features,
-            all_features,
-            no_default_features,
-        );
+        let resolve_cargo_result =
+            resolve(&args, package.package_id(), &mut registry, &workspace);
 
         assert!(resolve_cargo_result.is_ok());
     }
-
-    fn create_args() -> Args {
-        Args {
-            all: false,
-            all_deps: false,
-            all_features: false,
-            all_targets: false,
-            build_deps: false,
-            charset: Charset::Ascii,
-            color: None,
-            dev_deps: false,
-            features: None,
-            forbid_only: false,
-            format: "".to_string(),
-            frozen: false,
-            help: false,
-            include_tests: false,
-            invert: false,
-            locked: false,
-            manifest_path: None,
-            no_default_features: false,
-            no_indent: false,
-            offline: false,
-            package: None,
-            prefix_depth: false,
-            quiet: false,
-            target: None,
-            unstable_flags: vec![],
-            verbose: 0,
-            version: false,
-            output_format: None,
-        }
-    }
 }

cargo-geiger/src/format.rs

@@ -17,6 +17,12 @@ pub enum Charset {
     Utf8,
 }
 
+impl Default for Charset {
+    fn default() -> Self {
+        Charset::Ascii
+    }
+}
+
 #[derive(Debug, PartialEq)]
 pub enum Chunk {
     License,

cargo-geiger/src/format/print_config.rs

@@ -122,7 +122,7 @@ mod print_config_tests {
         input_invert_bool: bool,
         expected_edge_direction: EdgeDirection,
     ) {
-        let mut args = create_args();
+        let mut args = Args::default();
         args.invert = input_invert_bool;
 
         let print_config_result = PrintConfig::new(&args);
@@ -144,7 +144,7 @@ mod print_config_tests {
         input_include_tests_bool: bool,
         expected_include_tests: IncludeTests,
     ) {
-        let mut args = create_args();
+        let mut args = Args::default();
         args.include_tests = input_include_tests_bool;
 
         let print_config_result = PrintConfig::new(&args);
@@ -170,7 +170,7 @@ mod print_config_tests {
         input_no_indent_bool: bool,
         expected_output_prefix: Prefix,
     ) {
-        let mut args = create_args();
+        let mut args = Args::default();
         args.prefix_depth = input_prefix_depth_bool;
         args.no_indent = input_no_indent_bool;
 
@@ -191,7 +191,7 @@ mod print_config_tests {
         input_verbosity_u32: u32,
         expected_verbosity: Verbosity,
     ) {
-        let mut args = create_args();
+        let mut args = Args::default();
         args.verbose = input_verbosity_u32;
 
         let print_config_result = PrintConfig::new(&args);
@@ -227,37 +227,4 @@ mod print_config_tests {
             expected_colorized_string
         );
     }
-
-    fn create_args() -> Args {
-        Args {
-            all: false,
-            all_deps: false,
-            all_features: false,
-            all_targets: false,
-            build_deps: false,
-            charset: Charset::Ascii,
-            color: None,
-            dev_deps: false,
-            features: None,
-            forbid_only: false,
-            format: "".to_string(),
-            frozen: false,
-            help: false,
-            include_tests: false,
-            invert: false,
-            locked: false,
-            manifest_path: None,
-            no_default_features: false,
-            no_indent: false,
-            offline: false,
-            package: None,
-            prefix_depth: false,
-            quiet: false,
-            target: None,
-            unstable_flags: vec![],
-            verbose: 0,
-            version: false,
-            output_format: None,
-        }
-    }
 }

cargo-geiger/src/format/table.rs

@@ -146,8 +146,7 @@ fn table_row_empty() -> String {
 mod table_tests {
     use super::*;
 
-    use crate::rs_file::RsFileMetricsWrapper;
-    use crate::scan::{unsafe_stats, PackageMetrics};
+    use crate::scan::{unsafe_stats, PackageMetrics, RsFileMetricsWrapper};
 
     use geiger::RsFileMetrics;
     use rstest::*;