From c728d04a7ab93d66cdb69ca58e108c6401d53eca Mon Sep 17 00:00:00 2001 From: joshmc Date: Mon, 2 Nov 2020 17:59:30 +0000 Subject: [PATCH] ISSUE-16 - Remove Cargo Core from Traversal Module: * Switch Args struct to use Default * Refactor rs_file module to under scan * Remove Node struct from Graph struct, instead only use PackageId * Pull out function to parse features from args * Pull out cargo core usage from traversal module * Create FeatureArgs struct within Args Signed-off-by: joshmc --- cargo-geiger/src/args.rs | 64 ++++++++++++-- cargo-geiger/src/cli.rs | 83 ++++--------------- cargo-geiger/src/format.rs | 6 ++ cargo-geiger/src/format/print_config.rs | 41 +-------- cargo-geiger/src/format/table.rs | 3 +- cargo-geiger/src/graph.rs | 60 ++------------ cargo-geiger/src/krates_utils.rs | 19 ++--- cargo-geiger/src/main.rs | 16 +--- cargo-geiger/src/scan.rs | 20 +++-- cargo-geiger/src/scan/default.rs | 68 ++++----------- cargo-geiger/src/scan/default/table.rs | 7 +- cargo-geiger/src/scan/find.rs | 2 +- cargo-geiger/src/scan/forbid/table.rs | 12 ++- cargo-geiger/src/{ => scan}/rs_file.rs | 0 .../src/{ => scan}/rs_file/custom_executor.rs | 0 cargo-geiger/src/tree/traversal.rs | 10 ++- .../src/tree/traversal/dependency_kind.rs | 10 +-- .../src/tree/traversal/dependency_node.rs | 27 +++--- 18 files changed, 166 insertions(+), 282 deletions(-) rename cargo-geiger/src/{ => scan}/rs_file.rs (100%) rename cargo-geiger/src/{ => scan}/rs_file/custom_executor.rs (100%) diff --git a/cargo-geiger/src/args.rs b/cargo-geiger/src/args.rs index 57aeb681..e6f7394a 100644 --- a/cargo-geiger/src/args.rs +++ b/cargo-geiger/src/args.rs @@ -55,16 +55,16 @@ OPTIONS: -V, --version Prints version information. "; +#[derive(Default)] pub struct Args { pub all: bool, pub all_deps: bool, - pub all_features: bool, pub all_targets: bool, pub build_deps: bool, pub charset: Charset, pub color: Option, pub dev_deps: bool, - pub features: Option, + pub features_args: FeaturesArgs, pub forbid_only: bool, pub format: String, pub frozen: bool, @@ -73,7 +73,6 @@ pub struct Args { pub invert: bool, pub locked: bool, pub manifest_path: Option, - pub no_default_features: bool, pub no_indent: bool, pub offline: bool, pub package: Option, @@ -93,7 +92,6 @@ impl Args { let args = Args { all: raw_args.contains(["-a", "--all"]), all_deps: raw_args.contains("--all-dependencies"), - all_features: raw_args.contains("--all-features"), all_targets: raw_args.contains("--all-targets"), build_deps: raw_args.contains("--build-dependencies"), charset: raw_args @@ -101,7 +99,13 @@ impl Args { .unwrap_or(Charset::Utf8), color: raw_args.opt_value_from_str("--color")?, dev_deps: raw_args.contains("--dev-dependencies"), - features: raw_args.opt_value_from_str("--features")?, + features_args: FeaturesArgs { + all_features: raw_args.contains("--all-features"), + features: parse_features( + raw_args.opt_value_from_str("--features")?, + ), + no_default_features: raw_args.contains("--no-default-features"), + }, forbid_only: raw_args.contains(["-f", "--forbid-only"]), format: raw_args .opt_value_from_str("--format")? @@ -112,7 +116,6 @@ impl Args { invert: raw_args.contains(["-i", "--invert"]), locked: raw_args.contains("--locked"), manifest_path: raw_args.opt_value_from_str("--manifest-path")?, - no_default_features: raw_args.contains("--no-default-features"), no_indent: raw_args.contains("--no-indent"), offline: raw_args.contains("--offline"), package: raw_args.opt_value_from_str("--manifest-path")?, @@ -142,6 +145,24 @@ impl Args { } } +#[derive(Default)] +pub struct FeaturesArgs { + pub all_features: bool, + pub features: Vec, + pub no_default_features: bool, +} + +fn parse_features(raw_features: Option) -> Vec { + raw_features + .as_ref() + .cloned() + .unwrap_or_else(String::new) + .split(' ') + .map(str::to_owned) + .filter(|f| f != "") + .collect::>() +} + #[cfg(test)] pub mod args_tests { use super::*; @@ -202,4 +223,35 @@ pub mod args_tests { assert_eq!(args.charset, expected_charset); assert_eq!(args.verbose, expected_verbose) } + + #[rstest( + input_raw_features, + expected_features, + case( + Some(String::from("test some features")), + vec![ + String::from("test"), + String::from("some"), + String::from("features") + ] + ), + case( + Some(String::from("test")), + vec![String::from("test")] + ), + case( + Some(String::from("")), + vec![] + ), + case( + None, + vec![] + ) + )] + fn parse_features_test( + input_raw_features: Option, + expected_features: Vec, + ) { + assert_eq!(parse_features(input_raw_features), expected_features); + } } diff --git a/cargo-geiger/src/cli.rs b/cargo-geiger/src/cli.rs index 5d450be3..089bc7df 100644 --- a/cargo-geiger/src/cli.rs +++ b/cargo-geiger/src/cli.rs @@ -8,6 +8,7 @@ // TODO: Investigate how cargo-clippy is implemented. Is it using syn? Is is // using rustc? Is it implementing a compiler plugin? +use crate::args::FeaturesArgs; use crate::Args; // TODO: Consider making this a lib.rs (again) and expose a full API, excluding @@ -37,25 +38,18 @@ pub fn get_cargo_metadata( let mut metadata_command = MetadataCommand::new(); metadata_command.manifest_path(root_manifest_path); - if args.all_features { + if args.features_args.all_features { metadata_command.features(CargoOpt::AllFeatures); } - if args.no_default_features { + if args.features_args.no_default_features { metadata_command.features(CargoOpt::NoDefaultFeatures); } - if args.features.is_some() { - let features = args - .features - .as_ref() - .cloned() - .unwrap_or_else(String::new) - .split(' ') - .map(str::to_owned) - .collect::>(); - - metadata_command.features(CargoOpt::SomeFeatures(features)); + if !args.features_args.features.is_empty() { + metadata_command.features(CargoOpt::SomeFeatures( + args.features_args.features.clone(), + )); } Ok(metadata_command.exec()?) @@ -113,19 +107,17 @@ pub fn get_workspace( } pub fn resolve<'a, 'cfg>( + args: &FeaturesArgs, package_id: PackageId, registry: &mut PackageRegistry<'cfg>, workspace: &'a Workspace<'cfg>, - features: &[String], - all_features: bool, - no_default_features: bool, ) -> CargoResult<(PackageSet<'a>, Resolve)> { let dev_deps = true; // TODO: Review this. - let uses_default_features = !no_default_features; + let uses_default_features = !args.no_default_features; let opts = ResolveOpts::new( dev_deps, - features, - all_features, + &args.features.clone(), + args.all_features, uses_default_features, ); let prev = ops::load_pkg_lockfile(workspace)?; @@ -150,12 +142,11 @@ pub fn resolve<'a, 'cfg>( #[cfg(test)] mod cli_tests { use super::*; - use crate::format::Charset; use rstest::*; #[rstest] fn get_cargo_metadata_test() { - let args = create_args(); + let args = Args::default(); let config = Config::default().unwrap(); let cargo_metadata_result = get_cargo_metadata(&args, &config); @@ -194,7 +185,7 @@ mod cli_tests { #[rstest] fn get_krates_test() { - let args = create_args(); + let args = Args::default(); let config = Config::default().unwrap(); let cargo_metadata = get_cargo_metadata(&args, &config).unwrap(); @@ -243,58 +234,16 @@ mod cli_tests { #[rstest] fn resolve_test() { + let args = FeaturesArgs::default(); let config = Config::default().unwrap(); let manifest_path: Option = None; let workspace = get_workspace(&config, manifest_path).unwrap(); let package = workspace.current().unwrap(); let mut registry = get_registry(&config, &package).unwrap(); - let features: Vec = vec![]; - let all_features = false; - let no_default_features = false; - - let resolve_cargo_result = resolve( - package.package_id(), - &mut registry, - &workspace, - &features, - all_features, - no_default_features, - ); + let resolve_cargo_result = + resolve(&args, package.package_id(), &mut registry, &workspace); assert!(resolve_cargo_result.is_ok()); } - - fn create_args() -> Args { - Args { - all: false, - all_deps: false, - all_features: false, - all_targets: false, - build_deps: false, - charset: Charset::Ascii, - color: None, - dev_deps: false, - features: None, - forbid_only: false, - format: "".to_string(), - frozen: false, - help: false, - include_tests: false, - invert: false, - locked: false, - manifest_path: None, - no_default_features: false, - no_indent: false, - offline: false, - package: None, - prefix_depth: false, - quiet: false, - target: None, - unstable_flags: vec![], - verbose: 0, - version: false, - output_format: None, - } - } } diff --git a/cargo-geiger/src/format.rs b/cargo-geiger/src/format.rs index 7324eff2..7920dccc 100644 --- a/cargo-geiger/src/format.rs +++ b/cargo-geiger/src/format.rs @@ -17,6 +17,12 @@ pub enum Charset { Utf8, } +impl Default for Charset { + fn default() -> Self { + Charset::Ascii + } +} + #[derive(Debug, PartialEq)] pub enum Chunk { License, diff --git a/cargo-geiger/src/format/print_config.rs b/cargo-geiger/src/format/print_config.rs index 6d719400..2cac9fa7 100644 --- a/cargo-geiger/src/format/print_config.rs +++ b/cargo-geiger/src/format/print_config.rs @@ -122,7 +122,7 @@ mod print_config_tests { input_invert_bool: bool, expected_edge_direction: EdgeDirection, ) { - let mut args = create_args(); + let mut args = Args::default(); args.invert = input_invert_bool; let print_config_result = PrintConfig::new(&args); @@ -144,7 +144,7 @@ mod print_config_tests { input_include_tests_bool: bool, expected_include_tests: IncludeTests, ) { - let mut args = create_args(); + let mut args = Args::default(); args.include_tests = input_include_tests_bool; let print_config_result = PrintConfig::new(&args); @@ -170,7 +170,7 @@ mod print_config_tests { input_no_indent_bool: bool, expected_output_prefix: Prefix, ) { - let mut args = create_args(); + let mut args = Args::default(); args.prefix_depth = input_prefix_depth_bool; args.no_indent = input_no_indent_bool; @@ -191,7 +191,7 @@ mod print_config_tests { input_verbosity_u32: u32, expected_verbosity: Verbosity, ) { - let mut args = create_args(); + let mut args = Args::default(); args.verbose = input_verbosity_u32; let print_config_result = PrintConfig::new(&args); @@ -227,37 +227,4 @@ mod print_config_tests { expected_colorized_string ); } - - fn create_args() -> Args { - Args { - all: false, - all_deps: false, - all_features: false, - all_targets: false, - build_deps: false, - charset: Charset::Ascii, - color: None, - dev_deps: false, - features: None, - forbid_only: false, - format: "".to_string(), - frozen: false, - help: false, - include_tests: false, - invert: false, - locked: false, - manifest_path: None, - no_default_features: false, - no_indent: false, - offline: false, - package: None, - prefix_depth: false, - quiet: false, - target: None, - unstable_flags: vec![], - verbose: 0, - version: false, - output_format: None, - } - } } diff --git a/cargo-geiger/src/format/table.rs b/cargo-geiger/src/format/table.rs index 8bb6cf48..9e62cc31 100644 --- a/cargo-geiger/src/format/table.rs +++ b/cargo-geiger/src/format/table.rs @@ -146,8 +146,7 @@ fn table_row_empty() -> String { mod table_tests { use super::*; - use crate::rs_file::RsFileMetricsWrapper; - use crate::scan::{unsafe_stats, PackageMetrics}; + use crate::scan::{unsafe_stats, PackageMetrics, RsFileMetricsWrapper}; use geiger::RsFileMetrics; use rstest::*; diff --git a/cargo-geiger/src/graph.rs b/cargo-geiger/src/graph.rs index 266257eb..007d3dbe 100644 --- a/cargo-geiger/src/graph.rs +++ b/cargo-geiger/src/graph.rs @@ -34,18 +34,10 @@ impl ExtraDeps { /// Representation of the package dependency graph pub struct Graph { - pub graph: petgraph::Graph, + pub graph: petgraph::Graph, pub nodes: HashMap, } -/// Representation of a node within the package dependency graph -pub struct Node { - pub id: PackageId, - // TODO: Investigate why this was needed before the separation of printing - // and graph traversal and if it should be added back. - //pack: &'a Package, -} - // Almost unmodified compared to the original in cargo-tree, should be fairly // simple to move this and the dependency graph structure out to a library. /// Function to build a graph of packages dependencies @@ -65,13 +57,9 @@ pub fn build_graph<'a>( graph: petgraph::Graph::new(), nodes: HashMap::new(), }; - let node = Node { - id: root_package_id, - //pack: packages.get_one(root)?, - }; graph .nodes - .insert(root_package_id, graph.graph.add_node(node)); + .insert(root_package_id, graph.graph.add_node(root_package_id)); let mut pending_packages = vec![root_package_id]; @@ -112,11 +100,7 @@ fn add_graph_node_if_not_present_and_edge( Entry::Occupied(e) => *e.get(), Entry::Vacant(e) => { pending_packages.push(dependency_package_id); - let node = Node { - id: dependency_package_id, - //pack: packages.get_one(dep_id)?, - }; - *e.insert(graph.graph.add_node(node)) + *e.insert(graph.graph.add_node(dependency_package_id)) } }; graph @@ -201,7 +185,6 @@ fn build_graph_prerequisites<'a>( #[cfg(test)] mod graph_tests { use super::*; - use crate::format::Charset; use rstest::*; #[rstest( @@ -243,7 +226,7 @@ mod graph_tests { input_dev_deps: bool, expected_extra_deps: ExtraDeps, ) { - let mut args = create_args(); + let mut args = Args::default(); args.all_deps = input_all_deps; args.build_deps = input_build_deps; args.dev_deps = input_dev_deps; @@ -274,7 +257,7 @@ mod graph_tests { input_target: Option, expected_target: Option<&str>, ) { - let mut args = create_args(); + let mut args = Args::default(); args.all_targets = input_all_targets; args.target = input_target; @@ -289,37 +272,4 @@ mod graph_tests { assert_eq!(target, expected_target); } - - fn create_args() -> Args { - Args { - all: false, - all_deps: false, - all_features: false, - all_targets: false, - build_deps: false, - charset: Charset::Ascii, - color: None, - dev_deps: false, - features: None, - forbid_only: false, - format: "".to_string(), - frozen: false, - help: false, - include_tests: false, - invert: false, - locked: false, - manifest_path: None, - no_default_features: false, - no_indent: false, - offline: false, - package: None, - prefix_depth: false, - quiet: false, - target: None, - unstable_flags: vec![], - verbose: 0, - version: false, - output_format: None, - } - } } diff --git a/cargo-geiger/src/krates_utils.rs b/cargo-geiger/src/krates_utils.rs index 7f051012..7a74b119 100644 --- a/cargo-geiger/src/krates_utils.rs +++ b/cargo-geiger/src/krates_utils.rs @@ -63,7 +63,6 @@ impl ToPackageId for cargo_metadata::PackageId { package_set: &PackageSet, ) -> PackageId { let node = krates.node_for_kid(&self).unwrap(); - package_set .package_ids() .filter(|p| { @@ -108,6 +107,7 @@ pub trait ToPackageId { mod krates_utils_tests { use super::*; + use crate::args::FeaturesArgs; use crate::cli::{get_registry, get_workspace, resolve}; use cargo::Config; @@ -132,25 +132,16 @@ mod krates_utils_tests { #[rstest] fn to_package_id_test() { + let args = FeaturesArgs::default(); let config = Config::default().unwrap(); let manifest_path: Option = None; let workspace = get_workspace(&config, manifest_path).unwrap(); let package = workspace.current().unwrap(); let mut registry = get_registry(&config, &package).unwrap(); - let features: Vec = vec![]; - let all_features = false; - let no_default_features = false; - - let (package_set, _) = resolve( - package.package_id(), - &mut registry, - &workspace, - &features, - all_features, - no_default_features, - ) - .unwrap(); + let (package_set, _) = + resolve(&args, package.package_id(), &mut registry, &workspace) + .unwrap(); let metadata = construct_metadata(); let krates = Builder::new() diff --git a/cargo-geiger/src/main.rs b/cargo-geiger/src/main.rs index a1413dcb..6fd5a286 100644 --- a/cargo-geiger/src/main.rs +++ b/cargo-geiger/src/main.rs @@ -15,7 +15,6 @@ mod cli; mod format; mod graph; mod krates_utils; -mod rs_file; mod scan; mod tree; @@ -61,7 +60,7 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult { ColorChoice::CargoAuto => {} } - let cargo_metadata = get_cargo_metadata(args, config)?; + let cargo_metadata = get_cargo_metadata(&args, config)?; let krates = get_krates(&cargo_metadata)?; let cargo_metadata_parameters = CargoMetadataParameters { @@ -72,23 +71,14 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult { let workspace = get_workspace(config, args.manifest_path.clone())?; let package = workspace.current()?; let mut registry = get_registry(config, &package)?; - let features = args - .features - .as_ref() - .cloned() - .unwrap_or_else(String::new) - .split(' ') - .map(str::to_owned) - .collect::>(); let (package_set, resolve) = resolve( + &args.features_args, package.package_id(), &mut registry, &workspace, - &features, - args.all_features, - args.no_default_features, )?; + let package_ids = package_set.package_ids().collect::>(); let package_set = registry.get(&package_ids)?; diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index 81643e8e..77354aeb 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -1,11 +1,13 @@ mod default; mod find; mod forbid; +mod rs_file; use crate::args::Args; use crate::format::print_config::PrintConfig; use crate::graph::Graph; -use crate::rs_file::RsFileMetricsWrapper; + +pub use rs_file::RsFileMetricsWrapper; use default::scan_unsafe; use forbid::scan_forbid_unsafe; @@ -170,23 +172,26 @@ fn package_metrics<'a>( let mut visited = HashSet::new(); std::iter::from_fn(move || { let i = indices.pop()?; - let id = graph.graph[i].id; - let mut package = PackageInfo::new(from_cargo_package_id(id)); + let package_id = graph.graph[i]; + let mut package = PackageInfo::new(from_cargo_package_id(package_id)); for edge in graph.graph.edges(i) { let dep_index = edge.target(); if visited.insert(dep_index) { indices.push(dep_index); } - let dep = from_cargo_package_id(graph.graph[dep_index].id); + let dep = from_cargo_package_id(graph.graph[dep_index]); package.add_dependency( dep, from_cargo_dependency_kind(*edge.weight()), ); } - match geiger_context.package_id_to_metrics.get(&id) { + match geiger_context.package_id_to_metrics.get(&package_id) { Some(m) => Some((package, Some(m))), None => { - eprintln!("WARNING: No metrics found for package: {}", id); + eprintln!( + "WARNING: No metrics found for package: {}", + package_id + ); Some((package, None)) } } @@ -248,7 +253,8 @@ fn from_cargo_dependency_kind(kind: DepKind) -> DependencyKind { mod scan_tests { use super::*; - use crate::{rs_file::RsFileMetricsWrapper, scan::PackageMetrics}; + use crate::scan::PackageMetrics; + use rs_file::RsFileMetricsWrapper; use cargo_geiger_serde::{Count, UnsafeInfo}; use rstest::*; diff --git a/cargo-geiger/src/scan/default.rs b/cargo-geiger/src/scan/default.rs index 45d1479d..cc633a93 100644 --- a/cargo-geiger/src/scan/default.rs +++ b/cargo-geiger/src/scan/default.rs @@ -1,10 +1,10 @@ mod table; -use crate::args::Args; +use crate::args::FeaturesArgs; use crate::format::print_config::OutputFormat; use crate::graph::Graph; use crate::krates_utils::CargoMetadataParameters; -use crate::rs_file::resolve_rs_file_deps; +use crate::scan::rs_file::resolve_rs_file_deps; use super::find::find_unsafe; use super::{ @@ -53,21 +53,13 @@ pub fn scan_unsafe( /// constructed without providing all standard cargo options, TODO: Open an issue /// in cargo? fn build_compile_options<'a>( - args: &'a Args, + args: &'a FeaturesArgs, config: &'a Config, ) -> CompileOptions { - let features = args - .features - .as_ref() - .cloned() - .unwrap_or_else(String::new) - .split(' ') - .map(str::to_owned) - .collect::>(); let mut compile_options = CompileOptions::new(&config, CompileMode::Check { test: false }) .unwrap(); - compile_options.features = features; + compile_options.features = args.features.clone(); compile_options.all_features = args.all_features; compile_options.no_default_features = args.no_default_features; @@ -101,8 +93,10 @@ fn scan( scan_parameters: &ScanParameters, workspace: &Workspace, ) -> Result { - let compile_options = - build_compile_options(scan_parameters.args, scan_parameters.config); + let compile_options = build_compile_options( + &scan_parameters.args.features_args, + scan_parameters.config, + ); let rs_files_used = resolve_rs_file_deps(&compile_options, workspace).unwrap(); let geiger_context = find_unsafe( @@ -168,26 +162,29 @@ fn scan_to_report( #[cfg(test)] mod default_tests { use super::*; - use crate::format::Charset; use rstest::*; #[rstest( input_features, expected_compile_features, case( - Some(String::from("unit test features")), + vec![ + String::from("unit"), + String::from("test"), + String::from("features") + ], vec!["unit", "test", "features"], ), case( - Some(String::from("")), + vec![String::from("")], vec![""], ) )] fn build_compile_options_test( - input_features: Option, + input_features: Vec, expected_compile_features: Vec<&str>, ) { - let mut args = create_args(); + let mut args = FeaturesArgs::default(); args.all_features = rand::random(); args.features = input_features; args.no_default_features = rand::random(); @@ -202,37 +199,4 @@ mod default_tests { args.no_default_features ); } - - fn create_args() -> Args { - Args { - all: false, - all_deps: false, - all_features: false, - all_targets: false, - build_deps: false, - charset: Charset::Utf8, - color: None, - dev_deps: false, - features: None, - forbid_only: false, - format: "".to_string(), - frozen: false, - help: false, - include_tests: false, - invert: false, - locked: false, - manifest_path: None, - no_default_features: false, - no_indent: false, - offline: false, - package: None, - prefix_depth: false, - quiet: false, - target: None, - unstable_flags: vec![], - verbose: 0, - version: false, - output_format: None, - } - } } diff --git a/cargo-geiger/src/scan/default/table.rs b/cargo-geiger/src/scan/default/table.rs index 530153cf..fc2c0061 100644 --- a/cargo-geiger/src/scan/default/table.rs +++ b/cargo-geiger/src/scan/default/table.rs @@ -12,7 +12,7 @@ use super::super::{ }; use super::scan; -use crate::krates_utils::CargoMetadataParameters; +use crate::krates_utils::{CargoMetadataParameters, ToCargoMetadataPackageId}; use cargo::core::shell::Verbosity; use cargo::core::{PackageId, PackageSet, Workspace}; use cargo::{CliError, CliResult}; @@ -51,9 +51,12 @@ pub fn scan_to_table( scan_output_lines.append(&mut output_key_lines); let text_tree_lines = walk_dependency_tree( - root_package_id, + cargo_metadata_parameters, &graph, + package_set, &scan_parameters.print_config, + root_package_id + .to_cargo_metadata_package_id(cargo_metadata_parameters.metadata), ); let table_parameters = TableParameters { geiger_context: &geiger_context, diff --git a/cargo-geiger/src/scan/find.rs b/cargo-geiger/src/scan/find.rs index 2dd0e5ad..199efe76 100644 --- a/cargo-geiger/src/scan/find.rs +++ b/cargo-geiger/src/scan/find.rs @@ -2,7 +2,7 @@ use crate::format::print_config::PrintConfig; use crate::krates_utils::{ CargoMetadataParameters, GetRoot, ToCargoMetadataPackage, ToPackageId, }; -use crate::rs_file::{ +use crate::scan::rs_file::{ into_is_entry_point_and_path_buf, into_rs_code_file, into_target_kind, is_file_with_ext, RsFile, RsFileMetricsWrapper, }; diff --git a/cargo-geiger/src/scan/forbid/table.rs b/cargo-geiger/src/scan/forbid/table.rs index 63a7fa47..1da19565 100644 --- a/cargo-geiger/src/scan/forbid/table.rs +++ b/cargo-geiger/src/scan/forbid/table.rs @@ -3,7 +3,7 @@ use crate::format::pattern::Pattern; use crate::format::print_config::PrintConfig; use crate::format::{get_kind_group_name, SymbolKind}; use crate::graph::Graph; -use crate::krates_utils::CargoMetadataParameters; +use crate::krates_utils::{CargoMetadataParameters, ToCargoMetadataPackageId}; use crate::tree::traversal::walk_dependency_tree; use crate::tree::TextTreeLine; @@ -29,8 +29,14 @@ pub fn scan_forbid_to_table( let mut output_key_lines = construct_key_lines(&emoji_symbols); scan_output_lines.append(&mut output_key_lines); - let tree_lines = - walk_dependency_tree(root_package_id, &graph, &print_config); + let tree_lines = walk_dependency_tree( + cargo_metadata_parameters, + &graph, + package_set, + &print_config, + root_package_id + .to_cargo_metadata_package_id(cargo_metadata_parameters.metadata), + ); for tree_line in tree_lines { match tree_line { TextTreeLine::ExtraDepsGroup { kind, tree_vines } => { diff --git a/cargo-geiger/src/rs_file.rs b/cargo-geiger/src/scan/rs_file.rs similarity index 100% rename from cargo-geiger/src/rs_file.rs rename to cargo-geiger/src/scan/rs_file.rs diff --git a/cargo-geiger/src/rs_file/custom_executor.rs b/cargo-geiger/src/scan/rs_file/custom_executor.rs similarity index 100% rename from cargo-geiger/src/rs_file/custom_executor.rs rename to cargo-geiger/src/scan/rs_file/custom_executor.rs diff --git a/cargo-geiger/src/tree/traversal.rs b/cargo-geiger/src/tree/traversal.rs index 6ef196d6..28740a32 100644 --- a/cargo-geiger/src/tree/traversal.rs +++ b/cargo-geiger/src/tree/traversal.rs @@ -9,7 +9,8 @@ use super::construct_tree_vines_string; use dependency_kind::walk_dependency_kind; use dependency_node::walk_dependency_node; -use cargo::core::PackageId; +use crate::krates_utils::{CargoMetadataParameters, ToPackageId}; +use cargo::core::PackageSet; use std::collections::HashSet; /// Printing the returned TextTreeLines in order is expected to produce a nice @@ -19,13 +20,16 @@ use std::collections::HashSet; /// TODO: Consider separating the tree vine building from the tree traversal. /// pub fn walk_dependency_tree( - root_package_id: PackageId, + cargo_metadata_parameters: &CargoMetadataParameters, graph: &Graph, + package_set: &PackageSet, print_config: &PrintConfig, + root_package_id: cargo_metadata::PackageId, ) -> Vec { let mut visited_deps = HashSet::new(); let mut levels_continue = vec![]; - let node = &graph.graph[graph.nodes[&root_package_id]]; + let node = &graph.graph[graph.nodes[&root_package_id + .to_package_id(cargo_metadata_parameters.krates, package_set)]]; walk_dependency_node( node, graph, diff --git a/cargo-geiger/src/tree/traversal/dependency_kind.rs b/cargo-geiger/src/tree/traversal/dependency_kind.rs index 11a8dca0..edf569ba 100644 --- a/cargo-geiger/src/tree/traversal/dependency_kind.rs +++ b/cargo-geiger/src/tree/traversal/dependency_kind.rs @@ -1,5 +1,5 @@ use crate::format::print_config::{Prefix, PrintConfig}; -use crate::graph::{Graph, Node}; +use crate::graph::Graph; use crate::tree::{get_tree_symbols, TextTreeLine, TreeSymbols}; use super::dependency_node::walk_dependency_node; @@ -12,7 +12,7 @@ use std::slice::Iter; pub fn walk_dependency_kind( dep_kind: DepKind, - deps: &mut Vec<&Node>, + deps: &mut Vec<&PackageId>, graph: &Graph, visited_deps: &mut HashSet, levels_continue: &mut Vec, @@ -23,7 +23,7 @@ pub fn walk_dependency_kind( } // Resolve uses Hash data types internally but we want consistent output ordering - deps.sort_by_key(|n| n.id); + deps.sort_by_key(|n| *n); let tree_symbols = get_tree_symbols(print_config.charset); let mut text_tree_lines = Vec::new(); @@ -52,10 +52,10 @@ pub fn walk_dependency_kind( } fn handle_walk_dependency_node( - dependency: &Node, + dependency: &PackageId, graph: &Graph, levels_continue: &mut Vec, - node_iterator: &mut Peekable>, + node_iterator: &mut Peekable>, print_config: &PrintConfig, text_tree_lines: &mut Vec, visited_deps: &mut HashSet, diff --git a/cargo-geiger/src/tree/traversal/dependency_node.rs b/cargo-geiger/src/tree/traversal/dependency_node.rs index d9c12659..b74922e2 100644 --- a/cargo-geiger/src/tree/traversal/dependency_node.rs +++ b/cargo-geiger/src/tree/traversal/dependency_node.rs @@ -1,5 +1,5 @@ use crate::format::print_config::PrintConfig; -use crate::graph::{Graph, Node}; +use crate::graph::Graph; use crate::tree::TextTreeLine; use super::construct_tree_vines_string; @@ -12,17 +12,17 @@ use petgraph::EdgeDirection; use std::collections::{HashMap, HashSet}; pub fn walk_dependency_node( - package: &Node, + package: &PackageId, graph: &Graph, visited_deps: &mut HashSet, levels_continue: &mut Vec, print_config: &PrintConfig, ) -> Vec { - let new = print_config.all || visited_deps.insert(package.id); + let new = print_config.all || visited_deps.insert(*package); let tree_vines = construct_tree_vines_string(levels_continue, print_config); let mut all_out_text_tree_lines = vec![TextTreeLine::Package { - id: package.id, + id: *package, tree_vines, }]; @@ -51,10 +51,10 @@ pub fn walk_dependency_node( fn construct_dependency_type_nodes_hashmap<'a>( graph: &'a Graph, - package: &Node, + package: &PackageId, print_config: &PrintConfig, -) -> HashMap> { - let mut dependency_type_nodes: HashMap> = [ +) -> HashMap> { + let mut dependency_type_nodes: HashMap> = [ (DepKind::Build, vec![]), (DepKind::Development, vec![]), (DepKind::Normal, vec![]), @@ -65,7 +65,7 @@ fn construct_dependency_type_nodes_hashmap<'a>( for edge in graph .graph - .edges_directed(graph.nodes[&package.id], print_config.direction) + .edges_directed(graph.nodes[&package], print_config.direction) { let dependency = match print_config.direction { EdgeDirection::Incoming => &graph.graph[edge.source()], @@ -139,17 +139,14 @@ mod dependency_node_tests { expected_development_nodes_length: usize, expected_normal_nodes_length: usize, ) { - let mut inner_graph = petgraph::Graph::::new(); + let mut inner_graph = petgraph::Graph::::new(); let mut nodes = HashMap::::new(); let package_ids = create_package_id_vec(7); let print_config = create_print_config(input_edge_direction); for package_id in &package_ids { - nodes.insert( - *package_id, - inner_graph.add_node(Node { id: *package_id }), - ); + nodes.insert(*package_id, inner_graph.add_node(*package_id)); } add_edges_to_graph( @@ -167,7 +164,7 @@ mod dependency_node_tests { let dependency_type_nodes_hashmap = construct_dependency_type_nodes_hashmap( &graph, - &Node { id: package_ids[0] }, + &package_ids[0], &print_config, ); @@ -187,7 +184,7 @@ mod dependency_node_tests { fn add_edges_to_graph( directed_edges: &[(usize, usize, DepKind)], - graph: &mut petgraph::Graph, + graph: &mut petgraph::Graph, nodes: &HashMap, package_ids: &[PackageId], ) {