Merge pull request #135 from jmcconnell26/ISSUE-16-RemoveCargoCoreTra…

…versal

ISSUE-16 - Remove Cargo Core from Traversal Module:

cargo-geiger/src/args.rs

@@ -55,16 +55,16 @@ OPTIONS:
     -V, --version                 Prints version information.
 ";
 
+#[derive(Default)]
 pub struct Args {
     pub all: bool,
     pub all_deps: bool,
-    pub all_features: bool,
     pub all_targets: bool,
     pub build_deps: bool,
     pub charset: Charset,
     pub color: Option<String>,
     pub dev_deps: bool,
-    pub features: Option<String>,
+    pub features_args: FeaturesArgs,
     pub forbid_only: bool,
     pub format: String,
     pub frozen: bool,
@@ -73,7 +73,6 @@ pub struct Args {
     pub invert: bool,
     pub locked: bool,
     pub manifest_path: Option<PathBuf>,
-    pub no_default_features: bool,
     pub no_indent: bool,
     pub offline: bool,
     pub package: Option<String>,
@@ -93,15 +92,20 @@ impl Args {
         let args = Args {
             all: raw_args.contains(["-a", "--all"]),
             all_deps: raw_args.contains("--all-dependencies"),
-            all_features: raw_args.contains("--all-features"),
             all_targets: raw_args.contains("--all-targets"),
             build_deps: raw_args.contains("--build-dependencies"),
             charset: raw_args
                 .opt_value_from_str("--charset")?
                 .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_args: FeaturesArgs {
+                all_features: raw_args.contains("--all-features"),
+                features: parse_features(
+                    raw_args.opt_value_from_str("--features")?,
+                ),
+                no_default_features: raw_args.contains("--no-default-features"),
+            },
             forbid_only: raw_args.contains(["-f", "--forbid-only"]),
             format: raw_args
                 .opt_value_from_str("--format")?
@@ -112,7 +116,6 @@ impl Args {
             invert: raw_args.contains(["-i", "--invert"]),
             locked: raw_args.contains("--locked"),
             manifest_path: raw_args.opt_value_from_str("--manifest-path")?,
-            no_default_features: raw_args.contains("--no-default-features"),
             no_indent: raw_args.contains("--no-indent"),
             offline: raw_args.contains("--offline"),
             package: raw_args.opt_value_from_str("--manifest-path")?,
@@ -142,6 +145,24 @@ impl Args {
     }
 }
 
+#[derive(Default)]
+pub struct FeaturesArgs {
+    pub all_features: bool,
+    pub features: Vec<String>,
+    pub no_default_features: bool,
+}
+
+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 +223,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

@@ -8,6 +8,7 @@
 // TODO: Investigate how cargo-clippy is implemented. Is it using syn?  Is is
 // using rustc? Is it implementing a compiler plugin?
 
+use crate::args::FeaturesArgs;
 use crate::Args;
 
 // TODO: Consider making this a lib.rs (again) and expose a full API, excluding
@@ -37,25 +38,18 @@ pub fn get_cargo_metadata(
     let mut metadata_command = MetadataCommand::new();
     metadata_command.manifest_path(root_manifest_path);
 
-    if args.all_features {
+    if args.features_args.all_features {
         metadata_command.features(CargoOpt::AllFeatures);
     }
 
-    if args.no_default_features {
+    if args.features_args.no_default_features {
         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_args.features.is_empty() {
+        metadata_command.features(CargoOpt::SomeFeatures(
+            args.features_args.features.clone(),
+        ));
     }
 
     Ok(metadata_command.exec()?)
@@ -113,19 +107,17 @@ pub fn get_workspace(
 }
 
 pub fn resolve<'a, 'cfg>(
+    args: &FeaturesArgs,
     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 +142,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 +185,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 +234,16 @@ mod cli_tests {
 
     #[rstest]
     fn resolve_test() {
+        let args = FeaturesArgs::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::*;