From cfe186f54933991b68a9feb802133e98ee7a7046 Mon Sep 17 00:00:00 2001 From: joshmc Date: Fri, 16 Oct 2020 18:33:55 +0100 Subject: [PATCH] ISSUE-16 - Add further testing: * Update existing testing to use rstest * Add cases for tests where appropriate * Pulling out sub-module from format/table * Improve general test coverage --- Cargo.lock | 71 +++- cargo-geiger/Cargo.toml | 2 + cargo-geiger/src/args.rs | 130 ++++++-- cargo-geiger/src/cli.rs | 12 +- cargo-geiger/src/format.rs | 13 +- cargo-geiger/src/format/display.rs | 84 ++++- cargo-geiger/src/format/parse.rs | 39 +++ cargo-geiger/src/format/pattern.rs | 1 + cargo-geiger/src/format/print.rs | 136 -------- cargo-geiger/src/format/print_config.rs | 278 ++++++++++++++++ cargo-geiger/src/format/table.rs | 263 +++++---------- .../src/format/table/handle_text_tree_line.rs | 302 ++++++++++++++++++ .../src/format/table/total_package_counts.rs | 37 +++ cargo-geiger/src/graph.rs | 100 +++++- cargo-geiger/src/main.rs | 2 +- cargo-geiger/src/rs_file.rs | 94 +++--- cargo-geiger/src/scan.rs | 27 +- cargo-geiger/src/scan/default.rs | 8 +- cargo-geiger/src/scan/default/table.rs | 16 +- cargo-geiger/src/scan/find.rs | 4 +- cargo-geiger/src/scan/forbid.rs | 2 +- cargo-geiger/src/scan/forbid/table.rs | 9 +- cargo-geiger/src/tree.rs | 64 ++-- cargo-geiger/src/tree/traversal.rs | 2 +- geiger/src/lib.rs | 7 +- 25 files changed, 1218 insertions(+), 485 deletions(-) delete mode 100644 cargo-geiger/src/format/print.rs create mode 100644 cargo-geiger/src/format/print_config.rs create mode 100644 cargo-geiger/src/format/table/handle_text_tree_line.rs create mode 100644 cargo-geiger/src/format/table/total_package_counts.rs diff --git a/Cargo.lock b/Cargo.lock index efbed76b..83374569 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -196,6 +196,7 @@ dependencies = [ "colored", "console 0.11.3", "env_logger", + "fake", "geiger", "insta", "petgraph", @@ -247,7 +248,7 @@ dependencies = [ "ansi_term", "atty", "bitflags", - "strsim", + "strsim 0.8.0", "textwrap", "unicode-width", "vec_map", @@ -420,6 +421,41 @@ dependencies = [ "winapi", ] +[[package]] +name = "darling" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d706e75d87e35569db781a9b5e2416cff1236a47ed380831f959382ccd5f858" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0c960ae2da4de88a91b2d920c2a7233b400bc33cb28453a2987822d8392519b" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim 0.9.3", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9b5a2f4ac4969822c62224815d069952656cadc7084fdca9751e6d959189b72" +dependencies = [ + "darling_core", + "quote", + "syn", +] + [[package]] name = "difference" version = "2.0.0" @@ -438,6 +474,17 @@ version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "134951f4028bdadb9b84baf4232681efbf277da25144b9b0ad65df75946c422b" +[[package]] +name = "dummy" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09e6c397d4775a916085e9da36f228cef1fc9a58210096aacfc9ca858bb59c14" +dependencies = [ + "darling", + "quote", + "syn", +] + [[package]] name = "encode_unicode" version = "0.3.6" @@ -457,6 +504,16 @@ dependencies = [ "termcolor", ] +[[package]] +name = "fake" +version = "2.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0254e0f9710a2ad0c2daf44c276d2a678260ab8a0a5833ffa948e217ae7d5346" +dependencies = [ + "dummy", + "rand", +] + [[package]] name = "filetime" version = "0.2.10" @@ -654,6 +711,12 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3c1ad908cc71012b7bea4d0c53ba96a8cba9962f048fa68d143376143d863b7a" +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "idna" version = "0.2.0" @@ -1276,6 +1339,12 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" +[[package]] +name = "strsim" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6446ced80d6c486436db5c078dde11a9f73d42b57fb273121e160b84f63d894c" + [[package]] name = "strum" version = "0.19.2" diff --git a/cargo-geiger/Cargo.toml b/cargo-geiger/Cargo.toml index 68f9a171..80dc5957 100644 --- a/cargo-geiger/Cargo.toml +++ b/cargo-geiger/Cargo.toml @@ -19,9 +19,11 @@ cargo-platform = "0.1.1" colored = "2.0.0" console = "0.11.3" env_logger = "0.7.1" +fake = { version = "2.2", features=["derive"] } geiger = { path = "../geiger", version = "0.4.5" } petgraph = "0.5.1" pico-args = "0.3.3" +rand = "0.7.3" serde = { version = "1.0.116", features = ["derive"] } serde_json = "1.0.57" strum = "0.19.2" diff --git a/cargo-geiger/src/args.rs b/cargo-geiger/src/args.rs index a1062db3..dd5eea64 100644 --- a/cargo-geiger/src/args.rs +++ b/cargo-geiger/src/args.rs @@ -1,6 +1,8 @@ -use crate::format::print::OutputFormat; +use crate::format::print_config::OutputFormat; use crate::format::Charset; +use fake::{Dummy, Fake}; +use pico_args::Arguments; use std::path::PathBuf; pub const HELP: &str = @@ -54,6 +56,7 @@ OPTIONS: -V, --version Prints version information. "; +#[derive(Debug, Dummy)] pub struct Args { pub all: bool, pub all_deps: bool, @@ -86,51 +89,52 @@ pub struct Args { } impl Args { - pub fn parse_args() -> Result> { - let mut args = pico_args::Arguments::from_env(); + pub fn parse_args( + mut raw_args: Arguments, + ) -> Result> { let args = Args { - all: args.contains(["-a", "--all"]), - all_deps: args.contains("--all-dependencies"), - all_features: args.contains("--all-features"), - all_targets: args.contains("--all-targets"), - build_deps: args.contains("--build-dependencies"), - charset: 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: args.opt_value_from_str("--color")?, - dev_deps: args.contains("--dev-dependencies"), - features: args.opt_value_from_str("--features")?, - forbid_only: args.contains(["-f", "--forbid-only"]), - format: args + color: raw_args.opt_value_from_str("--color")?, + dev_deps: raw_args.contains("--dev-dependencies"), + features: raw_args.opt_value_from_str("--features")?, + forbid_only: raw_args.contains(["-f", "--forbid-only"]), + format: raw_args .opt_value_from_str("--format")? .unwrap_or_else(|| "{p}".to_string()), - frozen: args.contains("--frozen"), - help: args.contains(["-h", "--help"]), - include_tests: args.contains("--include-tests"), - invert: args.contains(["-i", "--invert"]), - locked: args.contains("--locked"), - manifest_path: args.opt_value_from_str("--manifest-path")?, - no_default_features: args.contains("--no-default-features"), - no_indent: args.contains("--no-indent"), - offline: args.contains("--offline"), - package: args.opt_value_from_str("--manifest-path")?, - prefix_depth: args.contains("--prefix-depth"), - quiet: args.contains(["-q", "--quiet"]), - target: args.opt_value_from_str("--target")?, - unstable_flags: args + frozen: raw_args.contains("--frozen"), + help: raw_args.contains(["-h", "--help"]), + include_tests: raw_args.contains("--include-tests"), + 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")?, + prefix_depth: raw_args.contains("--prefix-depth"), + quiet: raw_args.contains(["-q", "--quiet"]), + target: raw_args.opt_value_from_str("--target")?, + unstable_flags: raw_args .opt_value_from_str("-Z")? .map(|s: String| s.split(' ').map(|s| s.to_owned()).collect()) .unwrap_or_else(Vec::new), verbose: match ( - args.contains("-vv"), - args.contains(["-v", "--verbose"]), + raw_args.contains("-vv"), + raw_args.contains(["-v", "--verbose"]), ) { (false, false) => 0, (false, true) => 1, (true, _) => 2, }, - version: args.contains(["-V", "--version"]), - output_format: if args.contains("--json") { + version: raw_args.contains(["-V", "--version"]), + output_format: if raw_args.contains("--json") { Some(OutputFormat::Json) } else { None @@ -139,3 +143,65 @@ impl Args { Ok(args) } } + +#[cfg(test)] +pub mod args_tests { + use super::*; + + use rstest::*; + use std::ffi::OsString; + + #[rstest( + input_argument_vector, + expected_all, + expected_charset, + expected_verbose, + case( + vec![], + false, + Charset::Utf8, + 0 + ), + case( + vec![OsString::from("--all")], + true, + Charset::Utf8, + 0, + ), + case( + vec![OsString::from("--charset"), OsString::from("ascii")], + false, + Charset::Ascii, + 0 + ), + case( + vec![OsString::from("-v")], + false, + Charset::Utf8, + 1 + ), + case( + vec![OsString::from("-vv")], + false, + Charset::Utf8, + 2 + ) + )] + fn parse_args_test( + input_argument_vector: Vec, + expected_all: bool, + expected_charset: Charset, + expected_verbose: u32, + ) { + let args_result = + Args::parse_args(Arguments::from_vec(input_argument_vector)); + + assert!(args_result.is_ok()); + + let args = args_result.unwrap(); + + assert_eq!(args.all, expected_all); + assert_eq!(args.charset, expected_charset); + assert_eq!(args.verbose, expected_verbose) + } +} diff --git a/cargo-geiger/src/cli.rs b/cargo-geiger/src/cli.rs index 151359b0..8e85e01f 100644 --- a/cargo-geiger/src/cli.rs +++ b/cargo-geiger/src/cli.rs @@ -108,7 +108,9 @@ pub fn resolve<'a, 'cfg>( mod cli_tests { use super::*; - #[test] + use rstest::*; + + #[rstest] fn get_cfgs_test() { let config = Config::default().unwrap(); @@ -135,11 +137,11 @@ mod cli_tests { .filter(|cfg| matches!(cfg, Cfg::KeyPair(_, _))) .collect(); - assert!(names.len() > 0); - assert!(key_pairs.len() > 0); + assert!(!names.is_empty()); + assert!(!key_pairs.is_empty()); } - #[test] + #[rstest] fn get_registry_test() { let config = Config::default().unwrap(); let workspace = Workspace::new( @@ -162,7 +164,7 @@ mod cli_tests { assert_eq!(package_set.sources().len(), 1); } - #[test] + #[rstest] fn get_workspace_test() { let config = Config::default().unwrap(); let manifest_path: Option = None; diff --git a/cargo-geiger/src/format.rs b/cargo-geiger/src/format.rs index c8cebcab..26f57330 100644 --- a/cargo-geiger/src/format.rs +++ b/cargo-geiger/src/format.rs @@ -1,22 +1,24 @@ pub mod emoji_symbols; pub mod pattern; -pub mod print; +pub mod print_config; pub mod table; mod display; mod parse; use cargo::core::dependency::DepKind; +use fake::Dummy; use std::fmt; use std::str::{self, FromStr}; use strum_macros::EnumIter; -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, Dummy, PartialEq)] pub enum Charset { Ascii, Utf8, } +#[derive(Debug, PartialEq)] pub enum Chunk { License, Package, @@ -43,6 +45,7 @@ pub enum CrateDetectionStatus { UnsafeDetected, } +#[derive(Debug, PartialEq)] pub enum RawChunk<'a> { Argument(&'a str), Error(&'static str), @@ -83,14 +86,16 @@ pub fn get_kind_group_name(dep_kind: DepKind) -> Option<&'static str> { mod format_tests { use super::*; - #[test] + use rstest::*; + + #[rstest] fn charset_from_str_test() { assert_eq!(Charset::from_str("ascii"), Ok(Charset::Ascii)); assert_eq!(Charset::from_str("utf8"), Ok(Charset::Utf8)); assert_eq!(Charset::from_str("invalid_str"), Err("invalid charset")); } - #[test] + #[rstest] fn get_kind_group_name_test() { assert_eq!( get_kind_group_name(DepKind::Build), diff --git a/cargo-geiger/src/format/display.rs b/cargo-geiger/src/format/display.rs index 4afecb14..b2461937 100644 --- a/cargo-geiger/src/format/display.rs +++ b/cargo-geiger/src/format/display.rs @@ -15,7 +15,11 @@ impl<'a> fmt::Display for Display<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { for chunk in &self.pattern.0 { match *chunk { - Chunk::Raw(ref s) => (fmt.write_str(s))?, + Chunk::License => { + if let Some(ref license) = self.metadata.license { + (write!(fmt, "{}", license))? + } + } Chunk::Package => { (write!( fmt, @@ -24,11 +28,7 @@ impl<'a> fmt::Display for Display<'a> { self.package.version() ))? } - Chunk::License => { - if let Some(ref license) = self.metadata.license { - (write!(fmt, "{}", license))? - } - } + Chunk::Raw(ref s) => (fmt.write_str(s))?, Chunk::Repository => { if let Some(ref repository) = self.metadata.repository { (write!(fmt, "{}", repository))? @@ -36,7 +36,77 @@ impl<'a> fmt::Display for Display<'a> { } } } - Ok(()) } } + +#[cfg(test)] +pub mod display_tests { + use super::*; + + use crate::format::pattern::Pattern; + use crate::format::Chunk; + + use cargo::core::manifest::ManifestMetadata; + use cargo::core::{PackageId, SourceId}; + use cargo::util::ToSemver; + use rstest::*; + + #[rstest( + input_pattern, + expected_formatted_string, + case( + Pattern(vec![Chunk::License]), + "licence_string" + ), + case( + Pattern(vec![Chunk::Package]), + "package_name 1.2.3" + ), + case( + Pattern(vec![Chunk::Raw(String::from("chunk_value"))]), + "chunk_value" + ), + case( + Pattern(vec![Chunk::Repository]), + "repository_string" + ) + )] + fn display_format_fmt_test( + input_pattern: Pattern, + expected_formatted_string: &str, + ) { + let package_id = PackageId::new( + "package_name", + "1.2.3".to_semver().unwrap(), + SourceId::from_url( + "git+https://github.com/rust-secure-code/cargo-geiger", + ) + .unwrap(), + ) + .unwrap(); + + let manifest_metadata = ManifestMetadata { + authors: vec![], + keywords: vec![], + categories: vec![], + license: Some(String::from("licence_string")), + license_file: None, + description: None, + readme: None, + homepage: None, + repository: Some(String::from("repository_string")), + documentation: None, + badges: Default::default(), + links: None, + }; + + let display = Display { + pattern: &input_pattern, + package: &package_id, + metadata: &manifest_metadata, + }; + + assert_eq!(format!("{}", display), expected_formatted_string); + } +} diff --git a/cargo-geiger/src/format/parse.rs b/cargo-geiger/src/format/parse.rs index 9d2f1d86..6238e52d 100644 --- a/cargo-geiger/src/format/parse.rs +++ b/cargo-geiger/src/format/parse.rs @@ -90,3 +90,42 @@ impl<'a> Iterator for Parser<'a> { } } } + +#[cfg(test)] +pub mod parse_tests { + use super::*; + + use rstest::*; + + #[rstest] + fn parser_new_test() { + let parser_s = "parser_string"; + let parser = Parser::new(parser_s); + assert_eq!(parser.s, parser_s); + } + + #[rstest] + fn parser_argument_test() { + let mut parser = Parser::new("parser 1.2.3"); + let raw_chunk = parser.argument(); + assert_eq!(raw_chunk, RawChunk::Argument("parser")); + } + + #[rstest( + input_s_string, + expected_name_string, + case("parser 1.2.3", "parser"), + case("1.2.3 parser", "") + )] + fn parser_name_test(input_s_string: &str, expected_name_string: &str) { + let mut parser = Parser::new(input_s_string); + assert_eq!(parser.name(), expected_name_string) + } + + #[rstest] + fn parser_text_test() { + let parser_s = "parser 1.2.3"; + let mut parser = Parser::new(parser_s); + assert_eq!(parser.text(0), RawChunk::Text(parser_s)); + } +} diff --git a/cargo-geiger/src/format/pattern.rs b/cargo-geiger/src/format/pattern.rs index 8e5591c5..98380130 100644 --- a/cargo-geiger/src/format/pattern.rs +++ b/cargo-geiger/src/format/pattern.rs @@ -7,6 +7,7 @@ use cargo::core::manifest::ManifestMetadata; use cargo::core::PackageId; use std::error::Error; +#[derive(Debug, PartialEq)] pub struct Pattern(pub Vec); impl Pattern { diff --git a/cargo-geiger/src/format/print.rs b/cargo-geiger/src/format/print.rs deleted file mode 100644 index 6f8a2265..00000000 --- a/cargo-geiger/src/format/print.rs +++ /dev/null @@ -1,136 +0,0 @@ -use crate::args::Args; -use crate::format::pattern::Pattern; -use crate::format::{Charset, CrateDetectionStatus, FormatError}; - -use cargo::core::shell::Verbosity; -use cargo::util::errors::CliError; -use colored::Colorize; -use geiger::IncludeTests; -use petgraph::EdgeDirection; - -#[derive(Clone, Copy)] -pub enum Prefix { - Depth, - Indent, - None, -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum OutputFormat { - Json, -} - -pub struct PrintConfig { - /// Don't truncate dependencies that have already been displayed. - pub all: bool, - - pub allow_partial_results: bool, - pub charset: Charset, - pub direction: EdgeDirection, - - // Is anyone using this? This is a carry-over from cargo-tree. - // TODO: Open a github issue to discuss deprecation. - pub format: Pattern, - - pub include_tests: IncludeTests, - pub prefix: Prefix, - pub output_format: Option, - pub verbosity: Verbosity, -} - -impl PrintConfig { - pub fn new(args: &Args) -> Result { - // 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 format = Pattern::try_build(&args.format).map_err(|e| { - CliError::new( - (FormatError { - message: e.to_string(), - }) - .into(), - 1, - ) - })?; - - let include_tests = if args.include_tests { - IncludeTests::Yes - } else { - IncludeTests::No - }; - - let prefix = if args.prefix_depth { - Prefix::Depth - } else if args.no_indent { - Prefix::None - } else { - Prefix::Indent - }; - - let verbosity = if args.verbose == 0 { - Verbosity::Normal - } else { - Verbosity::Verbose - }; - - Ok(PrintConfig { - all: args.all, - allow_partial_results, - charset: args.charset, - direction, - format, - include_tests, - output_format: args.output_format, - prefix, - verbosity, - }) - } -} - -pub fn colorize( - string: String, - crate_detection_status: &CrateDetectionStatus, -) -> colored::ColoredString { - match crate_detection_status { - CrateDetectionStatus::NoneDetectedForbidsUnsafe => string.green(), - CrateDetectionStatus::NoneDetectedAllowsUnsafe => string.normal(), - CrateDetectionStatus::UnsafeDetected => string.red().bold(), - } -} - -#[cfg(test)] -mod print_tests { - use super::*; - - #[test] - fn colorize_test() { - let string = String::from("string_value"); - - assert_eq!( - string.clone().green(), - colorize( - string.clone(), - &CrateDetectionStatus::NoneDetectedForbidsUnsafe - ) - ); - - assert_eq!( - string.clone().normal(), - colorize( - string.clone(), - &CrateDetectionStatus::NoneDetectedAllowsUnsafe - ) - ); - - assert_eq!( - string.clone().red().bold(), - colorize(string.clone(), &CrateDetectionStatus::UnsafeDetected) - ); - } -} diff --git a/cargo-geiger/src/format/print_config.rs b/cargo-geiger/src/format/print_config.rs new file mode 100644 index 00000000..98eb8e03 --- /dev/null +++ b/cargo-geiger/src/format/print_config.rs @@ -0,0 +1,278 @@ +use crate::args::Args; +use crate::format::pattern::Pattern; +use crate::format::{Charset, CrateDetectionStatus, FormatError}; + +use cargo::core::shell::Verbosity; +use cargo::util::errors::CliError; +use colored::Colorize; +use fake::Dummy; +use geiger::IncludeTests; +use petgraph::EdgeDirection; + +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum Prefix { + Depth, + Indent, + None, +} + +#[derive(Clone, Copy, Debug, Dummy, Eq, PartialEq)] +pub enum OutputFormat { + Json, +} + +#[derive(Debug, PartialEq)] +pub struct PrintConfig { + /// Don't truncate dependencies that have already been displayed. + pub all: bool, + + pub allow_partial_results: bool, + pub charset: Charset, + pub direction: EdgeDirection, + + // Is anyone using this? This is a carry-over from cargo-tree. + // TODO: Open a github issue to discuss deprecation. + pub format: Pattern, + + pub include_tests: IncludeTests, + pub prefix: Prefix, + pub output_format: Option, + pub verbosity: Verbosity, +} + +impl PrintConfig { + pub fn new(args: &Args) -> Result { + // 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 format = Pattern::try_build(&args.format).map_err(|e| { + CliError::new( + (FormatError { + message: e.to_string(), + }) + .into(), + 1, + ) + })?; + + let include_tests = if args.include_tests { + IncludeTests::Yes + } else { + IncludeTests::No + }; + + let prefix = if args.prefix_depth { + Prefix::Depth + } else if args.no_indent { + Prefix::None + } else { + Prefix::Indent + }; + + let verbosity = if args.verbose == 0 { + Verbosity::Normal + } else { + Verbosity::Verbose + }; + + Ok(PrintConfig { + all: args.all, + allow_partial_results, + charset: args.charset, + direction, + format, + include_tests, + output_format: args.output_format, + prefix, + verbosity, + }) + } +} + +pub fn colorize( + string: String, + crate_detection_status: &CrateDetectionStatus, +) -> colored::ColoredString { + match crate_detection_status { + CrateDetectionStatus::NoneDetectedForbidsUnsafe => string.green(), + CrateDetectionStatus::NoneDetectedAllowsUnsafe => string.normal(), + CrateDetectionStatus::UnsafeDetected => string.red().bold(), + } +} + +#[cfg(test)] +mod print_config_tests { + use super::*; + + use colored::ColoredString; + use fake::{Fake, Faker}; + use rstest::*; + + #[rstest( + input_invert_bool, + expected_edge_direction, + case( + true, + EdgeDirection::Incoming + ), + case( + false, + EdgeDirection::Outgoing + ) + )] + fn print_config_new_test_invert( + input_invert_bool: bool, + expected_edge_direction: EdgeDirection + ) { + let mut args: Args = Faker.fake(); + args.invert = input_invert_bool; + + let print_config_result = PrintConfig::new(&args); + + assert!(print_config_result.is_ok()); + assert_eq!( + print_config_result.unwrap().direction, + expected_edge_direction + ); + } + + #[rstest( + input_include_tests_bool, + expected_include_tests, + case( + true, + IncludeTests::Yes + ), + case( + false, + IncludeTests::No + ), + )] + fn print_config_new_test_include_tests( + input_include_tests_bool: bool, + expected_include_tests: IncludeTests + ) { + let mut args: Args = Faker.fake(); + args.include_tests = input_include_tests_bool; + + let print_config_result = PrintConfig::new(&args); + + assert!(print_config_result.is_ok()); + assert_eq!( + print_config_result.unwrap().include_tests, + expected_include_tests + ); + } + + #[rstest( + input_prefix_depth_bool, + input_no_indent_bool, + expected_output_prefix, + case( + true, + false, + Prefix::Depth, + ), + case( + true, + false, + Prefix::Depth, + ), + case( + false, + true, + Prefix::None, + ), + case( + false, + false, + Prefix::Indent, + ), + )] + fn print_config_new_test_prefix( + input_prefix_depth_bool: bool, + input_no_indent_bool: bool, + expected_output_prefix: Prefix + ) { + let mut args: Args = Faker.fake(); + args.prefix_depth = input_prefix_depth_bool; + args.no_indent = input_no_indent_bool; + + let print_config_result = PrintConfig::new(&args); + + assert!(print_config_result.is_ok()); + assert_eq!( + print_config_result.unwrap().prefix, + expected_output_prefix + ); + } + + #[rstest( + input_verbosity_u32, + expected_verbosity, + case( + 0, + Verbosity::Normal + ), + case( + 1, + Verbosity::Verbose + ), + case( + 1, + Verbosity::Verbose + ) + )] + fn print_config_new_test_verbosity( + input_verbosity_u32: u32, + expected_verbosity: Verbosity, + ) { + let mut args: Args = Faker.fake(); + args.verbose = input_verbosity_u32; + + let print_config_result = PrintConfig::new(&args); + + assert!(print_config_result.is_ok()); + assert_eq!( + print_config_result.unwrap().verbosity, + expected_verbosity + ); + } + + #[rstest( + input_crate_detection_status, + expected_colorized_string, + case( + CrateDetectionStatus::NoneDetectedForbidsUnsafe, + String::from("string_value").green() + ), + case( + CrateDetectionStatus::NoneDetectedAllowsUnsafe, + String::from("string_value").normal() + ), + case( + CrateDetectionStatus::UnsafeDetected, + String::from("string_value").red().bold() + ) + )] + fn colorize_test( + input_crate_detection_status: CrateDetectionStatus, + expected_colorized_string: ColoredString, + ) { + let string_value = String::from("string_value"); + + assert_eq!( + colorize( + string_value, + &input_crate_detection_status + ), + expected_colorized_string + ); + } +} diff --git a/cargo-geiger/src/format/table.rs b/cargo-geiger/src/format/table.rs index b9109fb5..7aa06732 100644 --- a/cargo-geiger/src/format/table.rs +++ b/cargo-geiger/src/format/table.rs @@ -1,9 +1,18 @@ +mod handle_text_tree_line; +mod total_package_counts; + use crate::format::emoji_symbols::EmojiSymbols; -use crate::format::print::{colorize, PrintConfig}; -use crate::format::{get_kind_group_name, CrateDetectionStatus, SymbolKind}; -use crate::scan::{unsafe_stats, GeigerContext}; +use crate::format::print_config::{colorize, PrintConfig}; +use crate::format::CrateDetectionStatus; +use crate::scan::GeigerContext; use crate::tree::TextTreeLine; +use handle_text_tree_line::{ + handle_text_tree_line_extra_deps_group, handle_text_tree_line_package, + HandlePackageParameters, +}; +use total_package_counts::TotalPackageCounts; + use cargo::core::package::PackageSet; use geiger::{Count, CounterBlock}; use std::collections::HashSet; @@ -22,128 +31,44 @@ pub const UNSAFE_COUNTERS_HEADER: [&str; 6] = [ ]; pub fn create_table_from_text_tree_lines( - geiger_context: &GeigerContext, package_set: &PackageSet, - print_config: &PrintConfig, - rs_files_used: &HashSet, + table_parameters: &TableParameters, text_tree_lines: Vec, ) -> (Vec, u64) { let mut table_lines = Vec::::new(); let mut total_package_counts = TotalPackageCounts::new(); let mut warning_count = 0; - let mut visited = HashSet::new(); - let emoji_symbols = EmojiSymbols::new(print_config.charset); + let mut visited_package_ids = HashSet::new(); + let emoji_symbols = + EmojiSymbols::new(table_parameters.print_config.charset); + let mut handle_package_parameters = HandlePackageParameters { + total_package_counts: &mut total_package_counts, + visited_package_ids: &mut visited_package_ids, + warning_count: &mut warning_count, + }; + for text_tree_line in text_tree_lines { match text_tree_line { - TextTreeLine::Package { id, tree_vines } => { - let package_is_new = visited.insert(id); - let pack = package_set.get_one(id).unwrap_or_else(|_| { - // TODO: Avoid panic, return Result. - panic!("Expected to find package by id: {}", id); - }); - let pack_metrics = - match geiger_context.pack_id_to_metrics.get(&id) { - Some(m) => m, - None => { - warning_count += package_is_new as u64; - eprintln!( - "WARNING: No metrics found for package: {}", - id - ); - continue; - } - }; - let unsafety = unsafe_stats(pack_metrics, rs_files_used); - if package_is_new { - total_package_counts.total_counter_block += - unsafety.used.clone(); - total_package_counts.total_unused_counter_block += - unsafety.unused.clone(); - } - let unsafe_found = unsafety.used.has_unsafe(); - let crate_forbids_unsafe = unsafety.forbids_unsafe; - let total_inc = package_is_new as i32; - let crate_detection_status = - match (unsafe_found, crate_forbids_unsafe) { - (false, true) => { - total_package_counts - .none_detected_forbids_unsafe += total_inc; - CrateDetectionStatus::NoneDetectedForbidsUnsafe - } - (false, false) => { - total_package_counts.none_detected_allows_unsafe += - total_inc; - CrateDetectionStatus::NoneDetectedAllowsUnsafe - } - (true, _) => { - total_package_counts.unsafe_detected += total_inc; - CrateDetectionStatus::UnsafeDetected - } - }; - - let icon = match crate_detection_status { - CrateDetectionStatus::NoneDetectedForbidsUnsafe => { - emoji_symbols.emoji(SymbolKind::Lock) - } - CrateDetectionStatus::NoneDetectedAllowsUnsafe => { - emoji_symbols.emoji(SymbolKind::QuestionMark) - } - CrateDetectionStatus::UnsafeDetected => { - emoji_symbols.emoji(SymbolKind::Rads) - } - }; - - let package_name = colorize( - format!( - "{}", - print_config - .format - .display(&id, pack.manifest().metadata()) - ), - &crate_detection_status, - ); - let unsafe_info = colorize( - table_row(&unsafety.used, &unsafety.unused), - &crate_detection_status, - ); - - let shift_chars = unsafe_info.chars().count() + 4; - - let mut line = String::new(); - line.push_str( - format!("{} {: <2}", unsafe_info, icon).as_str(), - ); - - // Here comes some special control characters to position the cursor - // properly for printing the last column containing the tree vines, after - // the emoji icon. This is a workaround for a potential bug where the - // radiation emoji will visually cover two characters in width but only - // count as a single character if using the column formatting provided by - // Rust. This could be unrelated to Rust and a quirk of this particular - // symbol or something in the Terminal app on macOS. - if emoji_symbols.will_output_emoji() { - line.push_str("\r"); // Return the cursor to the start of the line. - line.push_str(format!("\x1B[{}C", shift_chars).as_str()); // Move the cursor to the right so that it points to the icon character. - } - - table_lines - .push(format!("{} {}{}", line, tree_vines, package_name)) - } - TextTreeLine::ExtraDepsGroup { kind, tree_vines } => { - let name = get_kind_group_name(kind); - if name.is_none() { - continue; - } - let name = name.unwrap(); - - // TODO: Fix the alignment on macOS (others too?) - table_lines.push(format!( - "{}{}{}", - table_row_empty(), - tree_vines, - name - )) - } + TextTreeLine::ExtraDepsGroup { + kind: dep_kind, + tree_vines, + } => handle_text_tree_line_extra_deps_group( + dep_kind, + &mut table_lines, + tree_vines, + ), + TextTreeLine::Package { + id: package_id, + tree_vines, + } => handle_text_tree_line_package( + &emoji_symbols, + &mut handle_package_parameters, + package_id, + package_set, + &mut table_lines, + table_parameters, + tree_vines, + ), } } @@ -165,38 +90,10 @@ pub fn create_table_from_text_tree_lines( (table_lines, warning_count) } -struct TotalPackageCounts { - none_detected_forbids_unsafe: i32, - none_detected_allows_unsafe: i32, - unsafe_detected: i32, - total_counter_block: CounterBlock, - total_unused_counter_block: CounterBlock, -} - -impl TotalPackageCounts { - fn new() -> TotalPackageCounts { - TotalPackageCounts { - none_detected_forbids_unsafe: 0, - none_detected_allows_unsafe: 0, - unsafe_detected: 0, - total_counter_block: CounterBlock::default(), - total_unused_counter_block: CounterBlock::default(), - } - } - - fn get_total_detection_status(&self) -> CrateDetectionStatus { - match ( - self.none_detected_forbids_unsafe > 0, - self.none_detected_allows_unsafe > 0, - self.unsafe_detected > 0, - ) { - (_, _, true) => CrateDetectionStatus::UnsafeDetected, - (true, false, false) => { - CrateDetectionStatus::NoneDetectedForbidsUnsafe - } - _ => CrateDetectionStatus::NoneDetectedAllowsUnsafe, - } - } +pub struct TableParameters<'a> { + pub geiger_context: &'a GeigerContext, + pub print_config: &'a PrintConfig, + pub rs_files_used: &'a HashSet, } fn table_footer( @@ -250,14 +147,15 @@ mod table_tests { use super::*; use crate::rs_file::RsFileMetricsWrapper; - use crate::scan::PackageMetrics; + use crate::scan::{unsafe_stats, PackageMetrics}; use geiger::RsFileMetrics; + use rstest::*; use std::collections::HashMap; use std::path::Path; use strum::IntoEnumIterator; - #[test] + #[rstest] fn table_footer_test() { let used_counter_block = create_counter_block(); let not_used_counter_block = create_counter_block(); @@ -279,7 +177,7 @@ mod table_tests { } } - #[test] + #[rstest] fn table_row_test() { let mut rs_path_to_metrics = HashMap::::new(); @@ -313,55 +211,38 @@ mod table_tests { assert_eq!(table_row, "4/6 8/12 12/18 16/24 20/30 "); } - #[test] + #[rstest] fn table_row_empty_test() { let empty_table_row = table_row_empty(); assert_eq!(empty_table_row.len(), 51); } - #[test] - fn total_package_counts_get_total_detection_status_tests() { - let total_package_counts_unsafe_detected = TotalPackageCounts { - none_detected_forbids_unsafe: 0, - none_detected_allows_unsafe: 0, - unsafe_detected: 1, + #[rstest( + input_none_detected_forbids_unsafe, + input_none_detected_allows_unsafe, + input_unsafe_detected, + expected_crate_detection_status, + case(0, 0, 1, CrateDetectionStatus::UnsafeDetected), + case(1, 0, 0, CrateDetectionStatus::NoneDetectedForbidsUnsafe), + case(4, 1, 0, CrateDetectionStatus::NoneDetectedAllowsUnsafe) + )] + fn total_package_counts_get_total_detection_status_tests( + input_none_detected_forbids_unsafe: i32, + input_none_detected_allows_unsafe: i32, + input_unsafe_detected: i32, + expected_crate_detection_status: CrateDetectionStatus, + ) { + let total_detection_status = TotalPackageCounts { + none_detected_forbids_unsafe: input_none_detected_forbids_unsafe, + none_detected_allows_unsafe: input_none_detected_allows_unsafe, + unsafe_detected: input_unsafe_detected, total_counter_block: CounterBlock::default(), total_unused_counter_block: CounterBlock::default(), }; assert_eq!( - total_package_counts_unsafe_detected.get_total_detection_status(), - CrateDetectionStatus::UnsafeDetected - ); - - let total_package_counts_none_detected_forbids_unsafe = - TotalPackageCounts { - none_detected_forbids_unsafe: 1, - none_detected_allows_unsafe: 0, - unsafe_detected: 0, - total_counter_block: CounterBlock::default(), - total_unused_counter_block: CounterBlock::default(), - }; - - assert_eq!( - total_package_counts_none_detected_forbids_unsafe - .get_total_detection_status(), - CrateDetectionStatus::NoneDetectedForbidsUnsafe - ); - - let total_package_counts_none_detected_allows_unsafe = - TotalPackageCounts { - none_detected_forbids_unsafe: 4, - none_detected_allows_unsafe: 1, - unsafe_detected: 0, - total_counter_block: CounterBlock::default(), - total_unused_counter_block: CounterBlock::default(), - }; - - assert_eq!( - total_package_counts_none_detected_allows_unsafe - .get_total_detection_status(), - CrateDetectionStatus::NoneDetectedAllowsUnsafe + total_detection_status.get_total_detection_status(), + expected_crate_detection_status ); } diff --git a/cargo-geiger/src/format/table/handle_text_tree_line.rs b/cargo-geiger/src/format/table/handle_text_tree_line.rs new file mode 100644 index 00000000..c94a7b26 --- /dev/null +++ b/cargo-geiger/src/format/table/handle_text_tree_line.rs @@ -0,0 +1,302 @@ +use crate::format::print_config::colorize; +use crate::format::{get_kind_group_name, CrateDetectionStatus, SymbolKind}; +use crate::scan::unsafe_stats; + +use super::total_package_counts::TotalPackageCounts; +use super::TableParameters; +use super::{table_row, table_row_empty}; + +use crate::format::emoji_symbols::EmojiSymbols; +use cargo::core::dependency::DepKind; +use cargo::core::package::PackageSet; +use cargo::core::PackageId; +use std::collections::HashSet; + +pub struct HandlePackageParameters<'a> { + pub total_package_counts: &'a mut TotalPackageCounts, + pub visited_package_ids: &'a mut HashSet, + pub warning_count: &'a mut u64, +} + +pub fn handle_text_tree_line_extra_deps_group( + dep_kind: DepKind, + table_lines: &mut Vec, + tree_vines: String, +) { + let name = get_kind_group_name(dep_kind); + if name.is_none() { + return; + } + let name = name.unwrap(); + + // TODO: Fix the alignment on macOS (others too?) + table_lines.push(format!("{}{}{}", table_row_empty(), tree_vines, name)); +} + +pub fn handle_text_tree_line_package( + emoji_symbols: &EmojiSymbols, + handle_package_parameters: &mut HandlePackageParameters, + package_id: PackageId, + package_set: &PackageSet, + table_lines: &mut Vec, + table_parameters: &TableParameters, + tree_vines: String, +) { + let package_is_new = handle_package_parameters + .visited_package_ids + .insert(package_id); + let package = package_set.get_one(package_id).unwrap_or_else(|_| { + // TODO: Avoid panic, return Result. + panic!("Expected to find package by id: {}", package_id); + }); + let package_metrics = match table_parameters + .geiger_context + .package_id_to_metrics + .get(&package_id) + { + Some(m) => m, + None => { + *handle_package_parameters.warning_count += package_is_new as u64; + eprintln!("WARNING: No metrics found for package: {}", package_id); + return; + } + }; + let unsafe_info = + unsafe_stats(package_metrics, table_parameters.rs_files_used); + if package_is_new { + handle_package_parameters + .total_package_counts + .total_counter_block += unsafe_info.used.clone(); + handle_package_parameters + .total_package_counts + .total_unused_counter_block += unsafe_info.unused.clone(); + } + let unsafe_found = unsafe_info.used.has_unsafe(); + let crate_forbids_unsafe = unsafe_info.forbids_unsafe; + let total_inc = package_is_new as i32; + let crate_detection_status = + get_crate_detection_status_and_update_package_counts( + crate_forbids_unsafe, + handle_package_parameters, + total_inc, + unsafe_found, + ); + + let icon = match crate_detection_status { + CrateDetectionStatus::NoneDetectedForbidsUnsafe => { + emoji_symbols.emoji(SymbolKind::Lock) + } + CrateDetectionStatus::NoneDetectedAllowsUnsafe => { + emoji_symbols.emoji(SymbolKind::QuestionMark) + } + CrateDetectionStatus::UnsafeDetected => { + emoji_symbols.emoji(SymbolKind::Rads) + } + }; + + let package_name = colorize( + format!( + "{}", + table_parameters + .print_config + .format + .display(&package_id, package.manifest().metadata()) + ), + &crate_detection_status, + ); + let unsafe_info = colorize( + table_row(&unsafe_info.used, &unsafe_info.unused), + &crate_detection_status, + ); + + let shift_chars = unsafe_info.chars().count() + 4; + + let mut line = String::new(); + line.push_str(format!("{} {: <2}", unsafe_info, icon).as_str()); + + // Here comes some special control characters to position the cursor + // properly for printing the last column containing the tree vines, after + // the emoji icon. This is a workaround for a potential bug where the + // radiation emoji will visually cover two characters in width but only + // count as a single character if using the column formatting provided by + // Rust. This could be unrelated to Rust and a quirk of this particular + // symbol or something in the Terminal app on macOS. + if emoji_symbols.will_output_emoji() { + line.push('\r'); // Return the cursor to the start of the line. + line.push_str(format!("\x1B[{}C", shift_chars).as_str()); // Move the cursor to the right so that it points to the icon character. + } + + table_lines.push(format!("{} {}{}", line, tree_vines, package_name)); +} + +fn get_crate_detection_status_and_update_package_counts( + crate_forbids_unsafe: bool, + handle_package_parameters: &mut HandlePackageParameters, + total_inc: i32, + unsafe_found: bool, +) -> CrateDetectionStatus { + match (crate_forbids_unsafe, unsafe_found) { + (true, false) => { + handle_package_parameters + .total_package_counts + .none_detected_forbids_unsafe += total_inc; + CrateDetectionStatus::NoneDetectedForbidsUnsafe + } + (false, false) => { + handle_package_parameters + .total_package_counts + .none_detected_allows_unsafe += total_inc; + CrateDetectionStatus::NoneDetectedAllowsUnsafe + } + (_, true) => { + handle_package_parameters + .total_package_counts + .unsafe_detected += total_inc; + CrateDetectionStatus::UnsafeDetected + } + } +} + +#[cfg(test)] +mod handle_text_tree_line_tests { + use super::*; + + use rstest::*; + + #[rstest( + input_dep_kind, + expected_kind_group_name, + case(DepKind::Build, Some(String::from("[build-dependencies]"))), + case(DepKind::Development, Some(String::from("[dev-dependencies]"))), + case(DepKind::Normal, None) + )] + fn handle_text_tree_line_extra_deps_group_test( + input_dep_kind: DepKind, + expected_kind_group_name: Option, + ) { + let mut table_lines = Vec::::new(); + + let tree_vines = String::from("tree_vines"); + + handle_text_tree_line_extra_deps_group( + input_dep_kind, + &mut table_lines, + tree_vines.clone(), + ); + + if expected_kind_group_name.is_some() { + assert_eq!(table_lines.len(), 1); + assert_eq!( + table_lines.first().unwrap().as_str(), + format!( + "{}{}{}", + table_row_empty(), + tree_vines, + expected_kind_group_name.unwrap(), + ) + ); + } else { + assert!(table_lines.is_empty()); + } + } + + #[rstest( + input_crate_forbids_unsafe, + input_total_inc, + input_unsafe_found, + expected_crate_detection_status, + expected_none_detected_forbids_unsafe, + expected_none_detected_allows_unsafe, + expected_unsafe_detected, + case( + true, + 1, + false, + CrateDetectionStatus::NoneDetectedForbidsUnsafe, + 1, + 0, + 0 + ), + case( + true, + 0, + false, + CrateDetectionStatus::NoneDetectedForbidsUnsafe, + 0, + 0, + 0 + ), + case( + false, + 1, + false, + CrateDetectionStatus::NoneDetectedAllowsUnsafe, + 0, + 1, + 0 + ), + case( + false, + 0, + false, + CrateDetectionStatus::NoneDetectedAllowsUnsafe, + 0, + 0, + 0 + ), + case(false, 1, true, CrateDetectionStatus::UnsafeDetected, 0, 0, 1), + case(false, 0, true, CrateDetectionStatus::UnsafeDetected, 0, 0, 0) + )] + fn get_crate_detection_status_and_update_package_counts_test( + input_crate_forbids_unsafe: bool, + input_total_inc: i32, + input_unsafe_found: bool, + expected_crate_detection_status: CrateDetectionStatus, + expected_none_detected_forbids_unsafe: i32, + expected_none_detected_allows_unsafe: i32, + expected_unsafe_detected: i32, + ) { + let mut handle_package_parameters = HandlePackageParameters { + total_package_counts: &mut TotalPackageCounts { + none_detected_forbids_unsafe: 0, + none_detected_allows_unsafe: 0, + unsafe_detected: 0, + total_counter_block: Default::default(), + total_unused_counter_block: Default::default(), + }, + visited_package_ids: &mut Default::default(), + warning_count: &mut 0, + }; + + let crate_detection_status = + get_crate_detection_status_and_update_package_counts( + input_crate_forbids_unsafe, + &mut handle_package_parameters, + input_total_inc, + input_unsafe_found, + ); + + assert_eq!(crate_detection_status, expected_crate_detection_status); + + assert_eq!( + handle_package_parameters + .total_package_counts + .none_detected_forbids_unsafe, + expected_none_detected_forbids_unsafe + ); + + assert_eq!( + handle_package_parameters + .total_package_counts + .none_detected_allows_unsafe, + expected_none_detected_allows_unsafe + ); + + assert_eq!( + handle_package_parameters + .total_package_counts + .unsafe_detected, + expected_unsafe_detected + ); + } +} diff --git a/cargo-geiger/src/format/table/total_package_counts.rs b/cargo-geiger/src/format/table/total_package_counts.rs new file mode 100644 index 00000000..6f24e75c --- /dev/null +++ b/cargo-geiger/src/format/table/total_package_counts.rs @@ -0,0 +1,37 @@ +use crate::format::CrateDetectionStatus; + +use geiger::CounterBlock; + +pub struct TotalPackageCounts { + pub none_detected_forbids_unsafe: i32, + pub none_detected_allows_unsafe: i32, + pub unsafe_detected: i32, + pub total_counter_block: CounterBlock, + pub total_unused_counter_block: CounterBlock, +} + +impl TotalPackageCounts { + pub fn new() -> TotalPackageCounts { + TotalPackageCounts { + none_detected_forbids_unsafe: 0, + none_detected_allows_unsafe: 0, + unsafe_detected: 0, + total_counter_block: CounterBlock::default(), + total_unused_counter_block: CounterBlock::default(), + } + } + + pub fn get_total_detection_status(&self) -> CrateDetectionStatus { + match ( + self.none_detected_forbids_unsafe > 0, + self.none_detected_allows_unsafe > 0, + self.unsafe_detected > 0, + ) { + (_, _, true) => CrateDetectionStatus::UnsafeDetected, + (true, false, false) => { + CrateDetectionStatus::NoneDetectedForbidsUnsafe + } + _ => CrateDetectionStatus::NoneDetectedAllowsUnsafe, + } + } +} diff --git a/cargo-geiger/src/graph.rs b/cargo-geiger/src/graph.rs index b1e56617..42d42e16 100644 --- a/cargo-geiger/src/graph.rs +++ b/cargo-geiger/src/graph.rs @@ -12,6 +12,7 @@ use petgraph::graph::NodeIndex; use std::collections::hash_map::Entry; use std::collections::HashMap; +#[derive(Debug, PartialEq)] pub enum ExtraDeps { All, Build, @@ -202,20 +203,95 @@ fn build_graph_prerequisites<'a>( mod graph_tests { use super::*; - #[test] - fn extra_deps_allows_test() { - assert_eq!(ExtraDeps::All.allows(DepKind::Normal), true); - assert_eq!(ExtraDeps::Build.allows(DepKind::Normal), true); - assert_eq!(ExtraDeps::Dev.allows(DepKind::Normal), true); - assert_eq!(ExtraDeps::NoMore.allows(DepKind::Normal), true); + use fake::{Fake, Faker}; + use rstest::*; - assert_eq!(ExtraDeps::All.allows(DepKind::Build), true); - assert_eq!(ExtraDeps::All.allows(DepKind::Development), true); + #[rstest( + input_extra_deps, + input_dep_kind, + expected_allows, + case(ExtraDeps::All, DepKind::Normal, true), + case(ExtraDeps::Build, DepKind::Normal, true), + case(ExtraDeps::Dev, DepKind::Normal, true), + case(ExtraDeps::NoMore, DepKind::Normal, true), + case(ExtraDeps::All, DepKind::Build, true), + case(ExtraDeps::All, DepKind::Development, true), + case(ExtraDeps::Build, DepKind::Build, true), + case(ExtraDeps::Build, DepKind::Development, false), + case(ExtraDeps::Dev, DepKind::Build, false), + case(ExtraDeps::Dev, DepKind::Development, true) + )] + fn extra_deps_allows_test( + input_extra_deps: ExtraDeps, + input_dep_kind: DepKind, + expected_allows: bool, + ) { + assert_eq!(input_extra_deps.allows(input_dep_kind), expected_allows); + } + + #[rstest( + input_all_deps, + input_build_deps, + input_dev_deps, + expected_extra_deps, + case(true, false, false, ExtraDeps::All), + case(false, true, false, ExtraDeps::Build), + case(false, false, true, ExtraDeps::Dev), + case(false, false, false, ExtraDeps::NoMore) + )] + fn build_graph_prerequisites_extra_deps_test( + input_all_deps: bool, + input_build_deps: bool, + input_dev_deps: bool, + expected_extra_deps: ExtraDeps, + ) { + let mut args: Args = Faker.fake(); + + args.all_deps = input_all_deps; + args.build_deps = input_build_deps; + args.dev_deps = input_dev_deps; + + let config_host = InternedString::new("config_host"); + + let result = build_graph_prerequisites(&args, &config_host); + + assert!(result.is_ok()); + + let (extra_deps, _) = result.unwrap(); + + assert_eq!(extra_deps, expected_extra_deps); + } + + #[rstest( + input_all_targets, + input_target, + expected_target, + case(true, None, None), + case(false, None, Some("default_config_host")), + case( + false, + Some(String::from("provided_config_host")), + Some("provided_config_host") + ) + )] + fn build_graph_prerequisites_all_targets_test( + input_all_targets: bool, + input_target: Option, + expected_target: Option<&str>, + ) { + let mut args: Args = Faker.fake(); + + args.all_targets = input_all_targets; + args.target = input_target; + + let config_host = InternedString::new("default_config_host"); + + let result = build_graph_prerequisites(&args, &config_host); + + assert!(result.is_ok()); - assert_eq!(ExtraDeps::Build.allows(DepKind::Build), true); - assert_eq!(ExtraDeps::Build.allows(DepKind::Development), false); + let (_, target) = result.unwrap(); - assert_eq!(ExtraDeps::Dev.allows(DepKind::Build), false); - assert_eq!(ExtraDeps::Dev.allows(DepKind::Development), true); + assert_eq!(target, expected_target); } } diff --git a/cargo-geiger/src/main.rs b/cargo-geiger/src/main.rs index 7a6ba2a3..0e047154 100644 --- a/cargo-geiger/src/main.rs +++ b/cargo-geiger/src/main.rs @@ -113,7 +113,7 @@ fn main() { cargo::exit_with_error(e.into(), &mut shell) } }; - let args = Args::parse_args().unwrap(); + let args = Args::parse_args(pico_args::Arguments::from_env()).unwrap(); if let Err(e) = real_main(&args, &mut config) { let mut shell = Shell::new(); cargo::exit_with_error(e, &mut shell) diff --git a/cargo-geiger/src/rs_file.rs b/cargo-geiger/src/rs_file.rs index d5b67239..f79db969 100644 --- a/cargo-geiger/src/rs_file.rs +++ b/cargo-geiger/src/rs_file.rs @@ -257,51 +257,67 @@ fn parse_rustc_dep_info( #[cfg(test)] mod rs_file_tests { use super::*; - - #[test] - fn into_rs_code_file_test() { + use rstest::*; + + #[rstest( + input_target_kind, + expected_rs_file, + case( + TargetKind::Lib(vec![]), + RsFile::LibRoot( + Path::new("test_path.ext").to_path_buf() + ) + ), + case( + TargetKind::Bin, + RsFile::BinRoot( + Path::new("test_path.ext").to_path_buf() + ) + ), + case( + TargetKind::Test, + RsFile::Other( + Path::new("test_path.ext").to_path_buf() + ) + ), + case( + TargetKind::Bench, + RsFile::Other( + Path::new("test_path.ext").to_path_buf() + ) + ), + case( + TargetKind::ExampleLib(vec![]), + RsFile::Other( + Path::new("test_path.ext").to_path_buf() + ) + ), + case( + TargetKind::ExampleBin, + RsFile::Other( + Path::new("test_path.ext").to_path_buf() + ) + ), + case( + TargetKind::CustomBuild, + RsFile::CustomBuildRoot( + Path::new("test_path.ext").to_path_buf() + ) + ), + )] + fn into_rs_code_file_test( + input_target_kind: TargetKind, + expected_rs_file: RsFile + ) { let path_buf = Path::new("test_path.ext").to_path_buf(); assert_eq!( - into_rs_code_file(&TargetKind::Lib(vec![]), path_buf.clone()), - RsFile::LibRoot(path_buf.clone()) - ); - - assert_eq!( - into_rs_code_file(&TargetKind::Bin, path_buf.clone()), - RsFile::BinRoot(path_buf.clone()) - ); - - assert_eq!( - into_rs_code_file(&TargetKind::Test, path_buf.clone()), - RsFile::Other(path_buf.clone()) - ); - - assert_eq!( - into_rs_code_file(&TargetKind::Bench, path_buf.clone()), - RsFile::Other(path_buf.clone()) - ); - - assert_eq!( - into_rs_code_file( - &TargetKind::ExampleLib(vec![]), - path_buf.clone() - ), - RsFile::Other(path_buf.clone()) - ); - - assert_eq!( - into_rs_code_file(&TargetKind::ExampleBin, path_buf.clone()), - RsFile::Other(path_buf.clone()) - ); - - assert_eq!( - into_rs_code_file(&TargetKind::CustomBuild, path_buf.clone()), - RsFile::CustomBuildRoot(path_buf.clone()) + into_rs_code_file(&input_target_kind, path_buf), + expected_rs_file ); } - #[test] + #[rstest] fn is_file_with_ext_test() { let config = Config::default().unwrap(); let cwd = config.cwd(); diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index 99432e13..134fcc34 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -4,7 +4,7 @@ mod forbid; mod report; use crate::args::Args; -use crate::format::print::PrintConfig; +use crate::format::print_config::PrintConfig; use crate::graph::Graph; use crate::rs_file::RsFileMetricsWrapper; @@ -22,7 +22,7 @@ use std::path::PathBuf; /// Provides a more terse and searchable name for the wrapped generic /// collection. pub struct GeigerContext { - pub pack_id_to_metrics: HashMap, + pub package_id_to_metrics: HashMap, } #[derive(Debug, Default)] @@ -143,7 +143,7 @@ fn list_files_used_but_not_scanned( rs_files_used: &HashSet, ) -> Vec { let scanned_files = geiger_context - .pack_id_to_metrics + .package_id_to_metrics .iter() .flat_map(|(_, v)| v.rs_path_to_metrics.keys()) .collect::>(); @@ -174,7 +174,7 @@ fn package_metrics<'a>( let dep = graph.graph[dep_index].id; package.push_dependency(dep, *edge.weight()); } - match geiger_context.pack_id_to_metrics.get(&id) { + match geiger_context.package_id_to_metrics.get(&id) { Some(m) => Some((package, Some(m))), None => { eprintln!("WARNING: No metrics found for package: {}", id); @@ -194,9 +194,10 @@ mod scan_tests { }; use geiger::Count; + use rstest::*; use std::{collections::HashSet, path::PathBuf}; - #[test] + #[rstest] fn construct_rs_files_used_lines_test() { let mut rs_files_used = HashSet::::new(); @@ -216,7 +217,7 @@ mod scan_tests { ); } - #[test] + #[rstest] fn unsafe_stats_from_nothing_are_empty() { let stats = unsafe_stats(&Default::default(), &Default::default()); let expected = UnsafeInfo { @@ -226,21 +227,21 @@ mod scan_tests { assert_eq!(stats, expected); } - #[test] + #[rstest] fn unsafe_stats_report_forbid_unsafe_as_true_if_all_entry_points_forbid_unsafe( ) { let metrics = metrics_from_iter(vec![( "foo.rs", MetricsBuilder::default() .forbids_unsafe(true) - .is_entry_point(true) + .set_is_crate_entry_point(true) .build(), )]); let stats = unsafe_stats(&metrics, &set_of_paths(&["foo.rs"])); assert!(stats.forbids_unsafe) } - #[test] + #[rstest] fn unsafe_stats_report_forbid_unsafe_as_false_if_one_entry_point_allows_unsafe( ) { let metrics = metrics_from_iter(vec![ @@ -248,14 +249,14 @@ mod scan_tests { "foo.rs", MetricsBuilder::default() .forbids_unsafe(true) - .is_entry_point(true) + .set_is_crate_entry_point(true) .build(), ), ( "bar.rs", MetricsBuilder::default() .forbids_unsafe(false) - .is_entry_point(true) + .set_is_crate_entry_point(true) .build(), ), ]); @@ -264,7 +265,7 @@ mod scan_tests { assert!(!stats.forbids_unsafe) } - #[test] + #[rstest] fn unsafe_stats_accumulate_counters() { let metrics = metrics_from_iter(vec![ ("foo.rs", MetricsBuilder::default().functions(2, 1).build()), @@ -323,7 +324,7 @@ mod scan_tests { self } - fn is_entry_point(mut self, yes: bool) -> Self { + fn set_is_crate_entry_point(mut self, yes: bool) -> Self { self.inner.is_crate_entry_point = yes; self } diff --git a/cargo-geiger/src/scan/default.rs b/cargo-geiger/src/scan/default.rs index 2c05ae77..0eccc340 100644 --- a/cargo-geiger/src/scan/default.rs +++ b/cargo-geiger/src/scan/default.rs @@ -1,7 +1,7 @@ mod table; use crate::args::Args; -use crate::format::print::OutputFormat; +use crate::format::print_config::OutputFormat; use crate::graph::Graph; use crate::rs_file::resolve_rs_file_deps; @@ -153,7 +153,9 @@ mod default_tests { use super::*; use crate::format::Charset; - #[test] + use rstest::*; + + #[rstest] fn build_compile_options_test() { let args_all_features = rand::random(); let args_features = Some(String::from("unit test features")); @@ -202,7 +204,7 @@ mod default_tests { ); } - #[test] + #[rstest] fn construct_scan_mode_default_output_key_lines_test() { let emoji_symbols = EmojiSymbols::new(Charset::Utf8); let output_key_lines = diff --git a/cargo-geiger/src/scan/default/table.rs b/cargo-geiger/src/scan/default/table.rs index 26d0683d..1c329129 100644 --- a/cargo-geiger/src/scan/default/table.rs +++ b/cargo-geiger/src/scan/default/table.rs @@ -1,6 +1,6 @@ use crate::format::emoji_symbols::EmojiSymbols; use crate::format::table::{ - create_table_from_text_tree_lines, UNSAFE_COUNTERS_HEADER, + create_table_from_text_tree_lines, TableParameters, UNSAFE_COUNTERS_HEADER, }; use crate::format::SymbolKind; use crate::graph::Graph; @@ -48,12 +48,16 @@ pub fn scan_to_table( &graph, &scan_parameters.print_config, ); + let table_parameters = TableParameters { + geiger_context: &geiger_context, + print_config: &scan_parameters.print_config, + rs_files_used: &rs_files_used, + }; + let (mut table_lines, mut warning_count) = create_table_from_text_tree_lines( - &geiger_context, package_set, - scan_parameters.print_config, - &rs_files_used, + &table_parameters, text_tree_lines, ); scan_output_lines.append(&mut table_lines); @@ -84,7 +88,7 @@ pub fn scan_to_table( #[derive(Debug)] struct FoundWarningsError { - pub warning_count: u64, + warning_count: u64, } impl Error for FoundWarningsError {} @@ -149,4 +153,4 @@ fn construct_key_lines(emoji_symbols: &EmojiSymbols) -> Vec { output_key_lines.push(String::new()); output_key_lines -} +} \ No newline at end of file diff --git a/cargo-geiger/src/scan/find.rs b/cargo-geiger/src/scan/find.rs index 519d72c3..b3f9f8a8 100644 --- a/cargo-geiger/src/scan/find.rs +++ b/cargo-geiger/src/scan/find.rs @@ -1,4 +1,4 @@ -use crate::format::print::PrintConfig; +use crate::format::print_config::PrintConfig; use crate::rs_file::{ into_rs_code_file, is_file_with_ext, RsFile, RsFileMetricsWrapper, }; @@ -86,7 +86,7 @@ where } let _ = progress_step(i, pack_code_file_count); } - GeigerContext { pack_id_to_metrics } + GeigerContext { package_id_to_metrics: pack_id_to_metrics } } fn find_rs_files_in_dir(dir: &Path) -> impl Iterator { diff --git a/cargo-geiger/src/scan/forbid.rs b/cargo-geiger/src/scan/forbid.rs index 35eca467..b331e3e5 100644 --- a/cargo-geiger/src/scan/forbid.rs +++ b/cargo-geiger/src/scan/forbid.rs @@ -1,6 +1,6 @@ mod table; -use crate::format::print::{OutputFormat, PrintConfig}; +use crate::format::print_config::{OutputFormat, PrintConfig}; use crate::graph::Graph; use super::find::find_unsafe; diff --git a/cargo-geiger/src/scan/forbid/table.rs b/cargo-geiger/src/scan/forbid/table.rs index 1691de8c..bf4fef12 100644 --- a/cargo-geiger/src/scan/forbid/table.rs +++ b/cargo-geiger/src/scan/forbid/table.rs @@ -1,6 +1,6 @@ use crate::format::emoji_symbols::EmojiSymbols; use crate::format::pattern::Pattern; -use crate::format::print::PrintConfig; +use crate::format::print_config::PrintConfig; use crate::format::{get_kind_group_name, SymbolKind}; use crate::graph::Graph; use crate::tree::traversal::walk_dependency_tree; @@ -116,7 +116,7 @@ fn handle_package_text_tree_line( let package = package_set.get_one(package_id).unwrap(); // FIXME let name = format_package_name(package, &print_config.format); - let package_metrics = geiger_ctx.pack_id_to_metrics.get(&package_id); + let package_metrics = geiger_ctx.package_id_to_metrics.get(&package_id); let package_forbids_unsafe = match package_metrics { None => false, // no metrics available, .rs parsing failed? Some(package_metric) => package_metric.rs_path_to_metrics.iter().all( @@ -143,8 +143,9 @@ mod forbid_tests { use cargo::core::Workspace; use cargo::util::important_paths; + use rstest::*; - #[test] + #[rstest] fn construct_scan_mode_forbid_only_output_key_lines_test() { let emoji_symbols = EmojiSymbols::new(Charset::Utf8); let output_key_lines = construct_key_lines(&emoji_symbols); @@ -152,7 +153,7 @@ mod forbid_tests { assert_eq!(output_key_lines.len(), 5); } - #[test] + #[rstest] fn format_package_name_test() { let pattern = Pattern::try_build("{p}").unwrap(); diff --git a/cargo-geiger/src/tree.rs b/cargo-geiger/src/tree.rs index d2f80780..6309e80f 100644 --- a/cargo-geiger/src/tree.rs +++ b/cargo-geiger/src/tree.rs @@ -1,6 +1,6 @@ pub mod traversal; -use crate::format::print::{Prefix, PrintConfig}; +use crate::format::print_config::{Prefix, PrintConfig}; use crate::format::Charset; use cargo::core::dependency::DepKind; @@ -84,34 +84,54 @@ mod tree_tests { use cargo::core::shell::Verbosity; use geiger::IncludeTests; use petgraph::EdgeDirection; - - #[test] - fn construct_tree_vines_string_test() { + use rstest::*; + + #[rstest( + input_prefix, + expected_tree_vines_string, + case( + Prefix::Depth, + "3 " + ), + case( + Prefix::Indent, + "| |-- " + ), + case( + Prefix::None, + "" + ) + )] + fn construct_tree_vines_string_test( + input_prefix: Prefix, + expected_tree_vines_string: &str + ) { let mut levels_continue = vec![true, false, true]; - let print_config = construct_print_config(Prefix::Depth); - let tree_vines_string = - construct_tree_vines_string(&mut levels_continue, &print_config); - - assert_eq!(tree_vines_string, "3 "); - - let print_config = construct_print_config(Prefix::Indent); - let tree_vines_string = - construct_tree_vines_string(&mut levels_continue, &print_config); - - assert_eq!(tree_vines_string, "| |-- "); - - let print_config = construct_print_config(Prefix::None); + let print_config = construct_print_config(input_prefix); let tree_vines_string = construct_tree_vines_string(&mut levels_continue, &print_config); - assert_eq!(tree_vines_string, ""); + assert_eq!(tree_vines_string, expected_tree_vines_string); } - #[test] - fn get_tree_symbols_test() { - assert_eq!(get_tree_symbols(Charset::Utf8), UTF8_TREE_SYMBOLS); - assert_eq!(get_tree_symbols(Charset::Ascii), ASCII_TREE_SYMBOLS) + #[rstest( + input_charset, + expected_tree_symbols, + case( + Charset::Utf8, + UTF8_TREE_SYMBOLS + ), + case( + Charset::Ascii, + ASCII_TREE_SYMBOLS + ) + )] + fn get_tree_symbols_test( + input_charset: Charset, + expected_tree_symbols: TreeSymbols + ) { + assert_eq!(get_tree_symbols(input_charset), expected_tree_symbols); } fn construct_print_config(prefix: Prefix) -> PrintConfig { diff --git a/cargo-geiger/src/tree/traversal.rs b/cargo-geiger/src/tree/traversal.rs index 2aa0e3b4..c81bbc84 100644 --- a/cargo-geiger/src/tree/traversal.rs +++ b/cargo-geiger/src/tree/traversal.rs @@ -1,4 +1,4 @@ -use crate::format::print::{Prefix, PrintConfig}; +use crate::format::print_config::{Prefix, PrintConfig}; use crate::graph::{Graph, Node}; use crate::tree::{get_tree_symbols, TextTreeLine}; diff --git a/geiger/src/lib.rs b/geiger/src/lib.rs index 5cf6e7a4..025d3d6c 100644 --- a/geiger/src/lib.rs +++ b/geiger/src/lib.rs @@ -116,7 +116,7 @@ pub struct RsFileMetrics { pub forbids_unsafe: bool, } -#[derive(PartialEq, Eq, Clone, Copy)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum IncludeTests { Yes, No, @@ -225,10 +225,7 @@ fn file_forbids_unsafe(f: &syn::File) -> bool { use syn::NestedMeta; f.attrs .iter() - .filter(|a| match a.style { - AttrStyle::Inner(_) => true, - _ => false, - }) + .filter(|a| matches!(a.style, AttrStyle::Inner(_))) .filter_map(|a| a.parse_meta().ok()) .filter(|meta| match meta { Meta::List(MetaList {