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..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; +use petgraph::{Direction, EdgeDirection}; 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..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,7 +9,9 @@ use super::TableParameters; use super::{table_row, table_row_empty}; use cargo_metadata::{DependencyKind, PackageId}; +use colored::ColoredString; use std::collections::HashSet; +use std::fmt::Display; pub struct HandlePackageParameters<'a> { pub total_package_counts: &'a mut TotalPackageCounts, @@ -112,6 +114,28 @@ 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(); @@ -139,7 +163,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( @@ -174,6 +198,8 @@ fn get_crate_detection_status_and_update_package_counts( mod handle_text_tree_line_tests { use super::*; + use crate::format::print_config::PrintConfig; + use colored::Colorize; use rstest::*; #[rstest( @@ -219,6 +245,57 @@ 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/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/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..d46e0676 100644 --- a/cargo-geiger/src/mapping.rs +++ b/cargo-geiger/src/mapping.rs @@ -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> { @@ -71,7 +73,7 @@ pub trait ToCargoCoreDepKind { pub trait ToCargoGeigerDependencyKind { fn to_cargo_geiger_dependency_kind( &self, - ) -> Option; + ) -> Option; } pub trait ToCargoGeigerPackageId { diff --git a/cargo-geiger/src/mapping/geiger.rs b/cargo-geiger/src/mapping/geiger.rs index 85b760d3..9c56949d 100644 --- a/cargo-geiger/src/mapping/geiger.rs +++ b/cargo-geiger/src/mapping/geiger.rs @@ -3,11 +3,14 @@ use super::{ToCargoGeigerSource, ToCargoMetadataPackage}; use cargo_metadata::Metadata; use url::Url; -impl ToCargoGeigerSource for cargo_metadata::PackageId { +use cargo_geiger_serde::Source as CargoGeigerSerdeSource; +use cargo_metadata::PackageId as CargoMetadataPackageId; + +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 +20,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 +48,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 +60,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 +77,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 +92,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 +107,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 +116,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/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 9c202d7d..781549b2 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -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, @@ -176,6 +176,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 +185,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 +319,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 c839d404..60326a68 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -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; @@ -47,6 +51,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, } @@ -172,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() @@ -203,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) { @@ -256,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::*; @@ -263,9 +286,80 @@ 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() { @@ -287,6 +381,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..47236e67 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)));