From aa8d1437d1ef4a927134f08a52c70685d216e392 Mon Sep 17 00:00:00 2001 From: joshmc Date: Sun, 21 Feb 2021 17:40:35 +0000 Subject: [PATCH 1/4] NOISSUE - Add further testing to handle_text_tree_line --- Cargo.lock | 1 - cargo-geiger/Cargo.toml | 1 - cargo-geiger/src/format/print_config.rs | 48 ++++++----- .../src/format/table/handle_text_tree_line.rs | 83 ++++++++++++++++++- cargo-geiger/src/main.rs | 1 - cargo-geiger/src/mapping.rs | 8 +- cargo-geiger/src/mapping/geiger.rs | 37 ++++++--- cargo-geiger/src/mapping/metadata.rs | 4 +- cargo-geiger/src/scan.rs | 3 +- 9 files changed, 139 insertions(+), 47 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/format/print_config.rs b/cargo-geiger/src/format/print_config.rs index 27bf89bc..17e7cd8e 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::{EdgeDirection, Direction}; 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..ffb53b14 100644 --- a/cargo-geiger/src/format/table/handle_text_tree_line.rs +++ b/cargo-geiger/src/format/table/handle_text_tree_line.rs @@ -10,6 +10,8 @@ use super::{table_row, table_row_empty}; use cargo_metadata::{DependencyKind, PackageId}; use std::collections::HashSet; +use colored::ColoredString; +use std::fmt::Display; pub struct HandlePackageParameters<'a> { pub total_package_counts: &'a mut TotalPackageCounts, @@ -112,6 +114,27 @@ 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(); @@ -126,7 +149,7 @@ pub fn handle_text_tree_line_package( // symbol or something in the Terminal app on macOS. if emoji_symbols.will_output_emoji() && table_parameters.print_config.output_format - != OutputFormat::GitHubMarkdown + != OutputFormat::GitHubMarkdown { 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. @@ -139,7 +162,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( @@ -175,6 +198,8 @@ mod handle_text_tree_line_tests { use super::*; use rstest::*; + use crate::format::print_config::PrintConfig; + use colored::Colorize; #[rstest( input_dep_kind, @@ -219,6 +244,60 @@ 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/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 c47e0d1f..f79b29a9 100644 --- a/cargo-geiger/src/mapping.rs +++ b/cargo-geiger/src/mapping.rs @@ -1,4 +1,4 @@ -mod geiger; +pub mod geiger; mod krates; mod metadata; @@ -68,12 +68,6 @@ pub trait ToCargoCoreDepKind { fn to_cargo_core_dep_kind(&self) -> DepKind; } -pub trait ToCargoGeigerDependencyKind { - fn to_cargo_geiger_dependency_kind( - &self, - ) -> Option; -} - pub trait ToCargoGeigerPackageId { fn to_cargo_geiger_package_id( &self, diff --git a/cargo-geiger/src/mapping/geiger.rs b/cargo-geiger/src/mapping/geiger.rs index 85b760d3..09868582 100644 --- a/cargo-geiger/src/mapping/geiger.rs +++ b/cargo-geiger/src/mapping/geiger.rs @@ -3,11 +3,22 @@ use super::{ToCargoGeigerSource, ToCargoMetadataPackage}; use cargo_metadata::Metadata; use url::Url; -impl ToCargoGeigerSource for cargo_metadata::PackageId { +use cargo_metadata::PackageId as CargoMetadataPackageId; + +use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; +use cargo_geiger_serde::Source as CargoGeigerSerdeSource; + +pub trait ToCargoGeigerDependencyKind { + fn to_cargo_geiger_dependency_kind( + &self, + ) -> Option; +} + +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 { @@ -17,14 +28,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(), @@ -45,7 +56,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), } @@ -57,8 +68,8 @@ fn handle_source_repr(source_repr: &str) -> cargo_geiger_serde::Source { } fn handle_path_source( - package_id: &cargo_metadata::PackageId, -) -> cargo_geiger_serde::Source { + package_id: &CargoMetadataPackageId, +) -> CargoGeigerSerdeSource { let raw_repr = package_id.repr.clone(); let raw_path_repr = raw_repr[1..raw_repr.len() - 1] .split("+file://") @@ -74,7 +85,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)] @@ -89,14 +100,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") } @@ -104,7 +115,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); @@ -113,11 +124,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/metadata.rs b/cargo-geiger/src/mapping/metadata.rs index 9c202d7d..2db746b6 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -5,7 +5,7 @@ use super::{ }; use crate::mapping::{ - ToCargoGeigerDependencyKind, ToCargoGeigerSource, ToCargoMetadataPackage, + geiger::ToCargoGeigerDependencyKind, ToCargoGeigerSource, ToCargoMetadataPackage, }; use cargo_metadata::{DependencyKind, Metadata, PackageId}; @@ -13,7 +13,7 @@ use krates::Krates; use std::collections::HashSet; use std::path::PathBuf; -impl DepsNotReplaced for cargo_metadata::Metadata { +impl DepsNotReplaced for Metadata { fn deps_not_replaced( &self, package_id: cargo_metadata::PackageId, diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index c839d404..d85e0f79 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -7,7 +7,7 @@ use crate::args::Args; use crate::format::print_config::PrintConfig; use crate::graph::Graph; use crate::mapping::{ - CargoMetadataParameters, ToCargoGeigerDependencyKind, + CargoMetadataParameters, geiger::ToCargoGeigerDependencyKind, ToCargoGeigerPackageId, }; @@ -47,6 +47,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, } From 70a4a36d9de0621c8d9cf3588f7efdc1b9b6d95f Mon Sep 17 00:00:00 2001 From: joshmc Date: Tue, 23 Feb 2021 19:37:20 +0000 Subject: [PATCH 2/4] NOISSUE - Add further unit testing --- cargo-geiger/src/format/print_config.rs | 14 +- .../src/format/table/handle_text_tree_line.rs | 22 +- cargo-geiger/src/lib.rs | 21 ++ cargo-geiger/src/mapping/krates.rs | 18 +- cargo-geiger/src/mapping/metadata.rs | 21 +- cargo-geiger/src/scan.rs | 246 +++++++++++++++--- cargo-geiger/src/scan/rs_file.rs | 7 +- 7 files changed, 263 insertions(+), 86 deletions(-) diff --git a/cargo-geiger/src/format/print_config.rs b/cargo-geiger/src/format/print_config.rs index 17e7cd8e..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, Direction}; +use petgraph::{Direction, EdgeDirection}; use strum_macros::EnumString; #[derive(Clone, Copy, Debug, PartialEq)] @@ -56,7 +56,7 @@ impl PrintConfig { let direction = match args.invert { true => EdgeDirection::Incoming, - false => EdgeDirection::Outgoing + false => EdgeDirection::Outgoing, }; let format = Pattern::try_build(&args.format).map_err(|e| { @@ -71,18 +71,18 @@ impl PrintConfig { let include_tests = match args.include_tests { true => IncludeTests::Yes, - false => IncludeTests::No + false => IncludeTests::No, }; let prefix = match (args.prefix_depth, args.no_indent) { (true, _) => Prefix::Depth, (false, true) => Prefix::None, - (false, false) => Prefix::Indent + (false, false) => Prefix::Indent, }; let verbosity = match args.verbose { 0 => Verbosity::Normal, - _ => Verbosity::Verbose + _ => Verbosity::Verbose, }; Ok(PrintConfig { @@ -100,7 +100,7 @@ impl PrintConfig { impl Default for PrintConfig { fn default() -> Self { - PrintConfig{ + PrintConfig { all: false, allow_partial_results: false, direction: Direction::Outgoing, @@ -108,7 +108,7 @@ impl Default for PrintConfig { include_tests: IncludeTests::Yes, prefix: Prefix::Depth, output_format: Default::default(), - verbosity: Verbosity::Verbose + verbosity: Verbosity::Verbose, } } } 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 ffb53b14..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,8 +9,8 @@ use super::TableParameters; use super::{table_row, table_row_empty}; use cargo_metadata::{DependencyKind, PackageId}; -use std::collections::HashSet; use colored::ColoredString; +use std::collections::HashSet; use std::fmt::Display; pub struct HandlePackageParameters<'a> { @@ -121,7 +121,7 @@ pub fn handle_text_tree_line_package( package_name, table_parameters, tree_vines, - unsafe_info + unsafe_info, ); table_lines.push(line); @@ -134,7 +134,8 @@ fn construct_package_text_tree_line( package_name: ColoredString, table_parameters: &TableParameters, tree_vines: String, - unsafe_info: ColoredString) -> String { + unsafe_info: ColoredString, +) -> String { let shift_chars = unsafe_info.chars().count() + 4; let mut line = String::new(); @@ -149,7 +150,7 @@ fn construct_package_text_tree_line( // symbol or something in the Terminal app on macOS. if emoji_symbols.will_output_emoji() && table_parameters.print_config.output_format - != OutputFormat::GitHubMarkdown + != OutputFormat::GitHubMarkdown { 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. @@ -197,9 +198,9 @@ fn get_crate_detection_status_and_update_package_counts( mod handle_text_tree_line_tests { use super::*; - use rstest::*; use crate::format::print_config::PrintConfig; use colored::Colorize; + use rstest::*; #[rstest( input_dep_kind, @@ -266,7 +267,7 @@ mod handle_text_tree_line_tests { input_crate_detection_status: CrateDetectionStatus, input_output_format: OutputFormat, input_symbol_kind: SymbolKind, - expected_package_text_tree_line: String + expected_package_text_tree_line: String, ) { let emoji_symbols = EmojiSymbols::new(input_output_format); let icon = emoji_symbols.emoji(input_symbol_kind); @@ -277,7 +278,7 @@ mod handle_text_tree_line_tests { output_format: input_output_format, ..Default::default() }, - rs_files_used: &Default::default() + rs_files_used: &Default::default(), }; let tree_vines = String::from("tree_vines"); let unsafe_info = ColoredString::from("unsafe_info").normal(); @@ -289,13 +290,10 @@ mod handle_text_tree_line_tests { package_name, &table_parameters, tree_vines, - unsafe_info + unsafe_info, ); - assert_eq!( - package_text_tree_line, - expected_package_text_tree_line - ); + assert_eq!(package_text_tree_line, expected_package_text_tree_line); } #[rstest( 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/mapping/krates.rs b/cargo-geiger/src/mapping/krates.rs index 4f3b1880..23f7fd45 100644 --- a/cargo-geiger/src/mapping/krates.rs +++ b/cargo-geiger/src/mapping/krates.rs @@ -58,9 +58,9 @@ impl QueryResolve for Krates { #[cfg(test)] mod krates_tests { use super::*; + use crate::lib_tests::construct_krates_and_metadata; - use cargo_metadata::{CargoOpt, Metadata, MetadataCommand, Version}; - use krates::Builder as KratesBuilder; + use cargo_metadata::Version; use rstest::*; #[rstest] @@ -157,18 +157,4 @@ mod krates_tests { 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 2db746b6..52e70685 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -5,7 +5,8 @@ use super::{ }; use crate::mapping::{ - geiger::ToCargoGeigerDependencyKind, ToCargoGeigerSource, ToCargoMetadataPackage, + geiger::ToCargoGeigerDependencyKind, ToCargoGeigerSource, + ToCargoMetadataPackage, }; use cargo_metadata::{DependencyKind, Metadata, PackageId}; @@ -176,6 +177,7 @@ mod metadata_tests { use crate::args::FeaturesArgs; use crate::cli::get_workspace; + use crate::lib_tests::construct_krates_and_metadata; use cargo::core::dependency::DepKind; use cargo::core::registry::PackageRegistry; @@ -184,8 +186,7 @@ mod metadata_tests { Package, PackageId, PackageIdSpec, PackageSet, Resolve, Workspace, }; use cargo::{ops, CargoResult, Config}; - use cargo_metadata::{CargoOpt, Metadata, MetadataCommand}; - use krates::Builder as KratesBuilder; + use cargo_metadata::Metadata; use rstest::*; use std::path::PathBuf; @@ -319,20 +320,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 d85e0f79..cb0bb49c 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -7,7 +7,7 @@ use crate::args::Args; use crate::format::print_config::PrintConfig; use crate::graph::Graph; use crate::mapping::{ - CargoMetadataParameters, geiger::ToCargoGeigerDependencyKind, + geiger::ToCargoGeigerDependencyKind, CargoMetadataParameters, ToCargoGeigerPackageId, }; @@ -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; @@ -173,8 +177,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() @@ -204,39 +211,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) { @@ -257,6 +244,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::*; @@ -264,9 +286,83 @@ 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() { @@ -288,6 +384,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..87619d5b 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))); @@ -447,4 +447,9 @@ mod rs_file_tests { assert_eq!(is_file_with_ext(&entry, "rs"), false); } } + + #[rstest] + fn add_dir_entries_to_path_buf_hash_set_test() { + + } } From f63840034df7a845d3eefe89048c378998e4d6f8 Mon Sep 17 00:00:00 2001 From: joshmc Date: Wed, 24 Feb 2021 17:55:12 +0000 Subject: [PATCH 3/4] NOISSUE - Clean up --- cargo-geiger/src/mapping.rs | 10 +++++++++- cargo-geiger/src/mapping/geiger.rs | 8 -------- cargo-geiger/src/mapping/metadata.rs | 2 +- cargo-geiger/src/scan.rs | 2 +- cargo-geiger/src/scan/rs_file.rs | 5 ----- 5 files changed, 11 insertions(+), 16 deletions(-) diff --git a/cargo-geiger/src/mapping.rs b/cargo-geiger/src/mapping.rs index f79b29a9..d46e0676 100644 --- a/cargo-geiger/src/mapping.rs +++ b/cargo-geiger/src/mapping.rs @@ -1,4 +1,4 @@ -pub mod geiger; +mod geiger; mod krates; mod metadata; @@ -8,6 +8,8 @@ use cargo_metadata::Metadata; use std::collections::HashSet; use std::path::PathBuf; +use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; + /// Holds a pointer to both a `Krates` graph, and the `Metadata` struct /// which are often required together pub struct CargoMetadataParameters<'a> { @@ -68,6 +70,12 @@ pub trait ToCargoCoreDepKind { fn to_cargo_core_dep_kind(&self) -> DepKind; } +pub trait ToCargoGeigerDependencyKind { + fn to_cargo_geiger_dependency_kind( + &self, + ) -> Option; +} + pub trait ToCargoGeigerPackageId { fn to_cargo_geiger_package_id( &self, diff --git a/cargo-geiger/src/mapping/geiger.rs b/cargo-geiger/src/mapping/geiger.rs index 09868582..83f6568b 100644 --- a/cargo-geiger/src/mapping/geiger.rs +++ b/cargo-geiger/src/mapping/geiger.rs @@ -4,16 +4,8 @@ use cargo_metadata::Metadata; use url::Url; use cargo_metadata::PackageId as CargoMetadataPackageId; - -use cargo_geiger_serde::DependencyKind as CargoGeigerSerdeDependencyKind; use cargo_geiger_serde::Source as CargoGeigerSerdeSource; -pub trait ToCargoGeigerDependencyKind { - fn to_cargo_geiger_dependency_kind( - &self, - ) -> Option; -} - impl ToCargoGeigerSource for CargoMetadataPackageId { fn to_cargo_geiger_source( &self, diff --git a/cargo-geiger/src/mapping/metadata.rs b/cargo-geiger/src/mapping/metadata.rs index 52e70685..34f3e4f9 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -5,7 +5,7 @@ use super::{ }; use crate::mapping::{ - geiger::ToCargoGeigerDependencyKind, ToCargoGeigerSource, + ToCargoGeigerDependencyKind, ToCargoGeigerSource, ToCargoMetadataPackage, }; diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index cb0bb49c..acc8b4aa 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -7,7 +7,7 @@ use crate::args::Args; use crate::format::print_config::PrintConfig; use crate::graph::Graph; use crate::mapping::{ - geiger::ToCargoGeigerDependencyKind, CargoMetadataParameters, + ToCargoGeigerDependencyKind, CargoMetadataParameters, ToCargoGeigerPackageId, }; diff --git a/cargo-geiger/src/scan/rs_file.rs b/cargo-geiger/src/scan/rs_file.rs index 87619d5b..47236e67 100644 --- a/cargo-geiger/src/scan/rs_file.rs +++ b/cargo-geiger/src/scan/rs_file.rs @@ -447,9 +447,4 @@ mod rs_file_tests { assert_eq!(is_file_with_ext(&entry, "rs"), false); } } - - #[rstest] - fn add_dir_entries_to_path_buf_hash_set_test() { - - } } From 0cf1f5fb19680a9c50fb28dbb8201d4e9b3704f8 Mon Sep 17 00:00:00 2001 From: joshmc Date: Wed, 24 Feb 2021 17:58:11 +0000 Subject: [PATCH 4/4] NOISSUE - Run cargo fmt --- cargo-geiger/src/mapping/geiger.rs | 2 +- cargo-geiger/src/mapping/metadata.rs | 3 +-- cargo-geiger/src/scan.rs | 19 ++++++++----------- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/cargo-geiger/src/mapping/geiger.rs b/cargo-geiger/src/mapping/geiger.rs index 83f6568b..9c56949d 100644 --- a/cargo-geiger/src/mapping/geiger.rs +++ b/cargo-geiger/src/mapping/geiger.rs @@ -3,8 +3,8 @@ use super::{ToCargoGeigerSource, ToCargoMetadataPackage}; use cargo_metadata::Metadata; use url::Url; -use cargo_metadata::PackageId as CargoMetadataPackageId; use cargo_geiger_serde::Source as CargoGeigerSerdeSource; +use cargo_metadata::PackageId as CargoMetadataPackageId; impl ToCargoGeigerSource for CargoMetadataPackageId { fn to_cargo_geiger_source( diff --git a/cargo-geiger/src/mapping/metadata.rs b/cargo-geiger/src/mapping/metadata.rs index 34f3e4f9..781549b2 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -5,8 +5,7 @@ use super::{ }; use crate::mapping::{ - ToCargoGeigerDependencyKind, ToCargoGeigerSource, - ToCargoMetadataPackage, + ToCargoGeigerDependencyKind, ToCargoGeigerSource, ToCargoMetadataPackage, }; use cargo_metadata::{DependencyKind, Metadata, PackageId}; diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index acc8b4aa..60326a68 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -7,7 +7,7 @@ use crate::args::Args; use crate::format::print_config::PrintConfig; use crate::graph::Graph; use crate::mapping::{ - ToCargoGeigerDependencyKind, CargoMetadataParameters, + CargoMetadataParameters, ToCargoGeigerDependencyKind, ToCargoGeigerPackageId, }; @@ -296,18 +296,12 @@ mod scan_tests { #[rstest( input_dependency_kind_option, expected_package_info_dependency_length, - case( - Some(DependencyKind::Normal), - 1, - ), - case( - None, - 0 - ) + 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 + expected_package_info_dependency_length: usize, ) { let (krates, metadata) = construct_krates_and_metadata(); let package_id = metadata.root_package().unwrap().id.clone(); @@ -361,7 +355,10 @@ mod scan_tests { ); assert_eq!(visited, vec![dependency_index].iter().cloned().collect()); - assert_eq!(package_info.dependencies.len(), expected_package_info_dependency_length) + assert_eq!( + package_info.dependencies.len(), + expected_package_info_dependency_length + ) } #[rstest]