diff --git a/cargo-geiger/src/args.rs b/cargo-geiger/src/args.rs index e6f7394a..c9776fb1 100644 --- a/cargo-geiger/src/args.rs +++ b/cargo-geiger/src/args.rs @@ -58,12 +58,9 @@ OPTIONS: #[derive(Default)] pub struct Args { pub all: bool, - pub all_deps: bool, - pub all_targets: bool, - pub build_deps: bool, pub charset: Charset, pub color: Option, - pub dev_deps: bool, + pub deps_args: DepsArgs, pub features_args: FeaturesArgs, pub forbid_only: bool, pub format: String, @@ -78,7 +75,7 @@ pub struct Args { pub package: Option, pub prefix_depth: bool, pub quiet: bool, - pub target: Option, + pub target_args: TargetArgs, pub unstable_flags: Vec, pub verbose: u32, pub version: bool, @@ -91,14 +88,15 @@ impl Args { ) -> Result> { let args = Args { all: raw_args.contains(["-a", "--all"]), - all_deps: raw_args.contains("--all-dependencies"), - all_targets: raw_args.contains("--all-targets"), - build_deps: raw_args.contains("--build-dependencies"), charset: raw_args .opt_value_from_str("--charset")? .unwrap_or(Charset::Utf8), color: raw_args.opt_value_from_str("--color")?, - dev_deps: raw_args.contains("--dev-dependencies"), + deps_args: DepsArgs { + all_deps: raw_args.contains("--all-dependencies"), + build_deps: raw_args.contains("--build-dependencies"), + dev_deps: raw_args.contains("--dev-dependencies"), + }, features_args: FeaturesArgs { all_features: raw_args.contains("--all-features"), features: parse_features( @@ -121,7 +119,10 @@ impl Args { 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")?, + target_args: TargetArgs { + all_targets: raw_args.contains("--all-targets"), + 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()) @@ -145,6 +146,13 @@ impl Args { } } +#[derive(Default)] +pub struct DepsArgs { + pub all_deps: bool, + pub build_deps: bool, + pub dev_deps: bool, +} + #[derive(Default)] pub struct FeaturesArgs { pub all_features: bool, @@ -152,6 +160,12 @@ pub struct FeaturesArgs { pub no_default_features: bool, } +#[derive(Default)] +pub struct TargetArgs { + pub all_targets: bool, + pub target: Option, +} + fn parse_features(raw_features: Option) -> Vec { raw_features .as_ref() diff --git a/cargo-geiger/src/graph.rs b/cargo-geiger/src/graph.rs index 007d3dbe..d2a1b21a 100644 --- a/cargo-geiger/src/graph.rs +++ b/cargo-geiger/src/graph.rs @@ -1,16 +1,21 @@ -use crate::args::Args; +use crate::args::{Args, DepsArgs, TargetArgs}; use crate::cli::get_cfgs; +use crate::krates_utils::{ + CargoMetadataParameters, DepsNotReplaced, MatchesIgnoringSource, + Replacement, +}; -use cargo::core::dependency::DepKind; use cargo::core::package::PackageSet; -use cargo::core::{Dependency, PackageId, Resolve, Workspace}; +use cargo::core::{Resolve, Workspace}; use cargo::util::interning::InternedString; use cargo::util::CargoResult; use cargo::Config; -use cargo_platform::Cfg; +use cargo_metadata::DependencyKind; +use cargo_platform::{Cfg, Platform}; use petgraph::graph::NodeIndex; use std::collections::hash_map::Entry; use std::collections::HashMap; +use std::str::FromStr; #[derive(Debug, PartialEq)] pub enum ExtraDeps { @@ -21,12 +26,12 @@ pub enum ExtraDeps { } impl ExtraDeps { - pub fn allows(&self, dep: DepKind) -> bool { + pub fn allows(&self, dep: DependencyKind) -> bool { match (self, dep) { - (_, DepKind::Normal) => true, + (_, DependencyKind::Normal) => true, (ExtraDeps::All, _) => true, - (ExtraDeps::Build, DepKind::Build) => true, - (ExtraDeps::Dev, DepKind::Development) => true, + (ExtraDeps::Build, DependencyKind::Build) => true, + (ExtraDeps::Dev, DependencyKind::Development) => true, _ => false, } } @@ -34,8 +39,11 @@ impl ExtraDeps { /// Representation of the package dependency graph pub struct Graph { - pub graph: petgraph::Graph, - pub nodes: HashMap, + pub graph: petgraph::Graph< + cargo_metadata::PackageId, + cargo_metadata::DependencyKind, + >, + pub nodes: HashMap, } // Almost unmodified compared to the original in cargo-tree, should be fairly @@ -43,23 +51,29 @@ pub struct Graph { /// Function to build a graph of packages dependencies pub fn build_graph<'a>( args: &Args, + cargo_metadata_parameters: &'a CargoMetadataParameters, config: &Config, resolve: &'a Resolve, package_set: &'a PackageSet, - root_package_id: PackageId, + root_package_id: cargo_metadata::PackageId, workspace: &Workspace, ) -> CargoResult { let config_host = config.load_global_rustc(Some(&workspace))?.host; - let (extra_deps, target) = build_graph_prerequisites(args, &config_host)?; - let cfgs = get_cfgs(config, &args.target, &workspace)?; + let (extra_deps, target) = build_graph_prerequisites( + &config_host, + &args.deps_args, + &args.target_args, + )?; + let cfgs = get_cfgs(config, &args.target_args.target, &workspace)?; let mut graph = Graph { graph: petgraph::Graph::new(), nodes: HashMap::new(), }; - graph - .nodes - .insert(root_package_id, graph.graph.add_node(root_package_id)); + graph.nodes.insert( + root_package_id.clone(), + graph.graph.add_node(root_package_id.clone()), + ); let mut pending_packages = vec![root_package_id]; @@ -71,9 +85,10 @@ pub fn build_graph<'a>( while let Some(package_id) = pending_packages.pop() { add_package_dependencies_to_graph( - resolve, + cargo_metadata_parameters, package_id, package_set, + resolve, &graph_configuration, &mut graph, &mut pending_packages, @@ -90,65 +105,88 @@ struct GraphConfiguration<'a> { } fn add_graph_node_if_not_present_and_edge( - dependency: &Dependency, - dependency_package_id: PackageId, + dependency: &cargo_metadata::Dependency, + dependency_package_id: cargo_metadata::PackageId, graph: &mut Graph, index: NodeIndex, - pending_packages: &mut Vec, + pending_packages: &mut Vec, ) { - let dependency_index = match graph.nodes.entry(dependency_package_id) { - Entry::Occupied(e) => *e.get(), - Entry::Vacant(e) => { - pending_packages.push(dependency_package_id); - *e.insert(graph.graph.add_node(dependency_package_id)) - } - }; + let dependency_index = + match graph.nodes.entry(dependency_package_id.clone()) { + Entry::Occupied(e) => *e.get(), + Entry::Vacant(e) => { + pending_packages.push(dependency_package_id.clone()); + *e.insert(graph.graph.add_node(dependency_package_id)) + } + }; graph .graph - .add_edge(index, dependency_index, dependency.kind()); + .add_edge(index, dependency_index, dependency.kind); } fn add_package_dependencies_to_graph<'a>( - resolve: &'a Resolve, - package_id: PackageId, + cargo_metadata_parameters: &CargoMetadataParameters, + package_id: cargo_metadata::PackageId, package_set: &'a PackageSet, + resolve: &'a Resolve, graph_configuration: &GraphConfiguration, graph: &mut Graph, - pending_packages: &mut Vec, + pending_packages: &mut Vec, ) -> CargoResult<()> { let index = graph.nodes[&package_id]; - let package = package_set.get_one(package_id)?; - - for (raw_dependency_package_id, _) in resolve.deps_not_replaced(package_id) + let package = cargo_metadata_parameters + .krates + .node_for_kid(&package_id) + .unwrap() + .krate + .clone(); + + for (raw_dependency_package_id, _) in + cargo_metadata_parameters.metadata.deps_not_replaced( + cargo_metadata_parameters.krates, + package_id, + package_set, + resolve, + ) { let dependency_iterator = package - .dependencies() + .dependencies .iter() - .filter(|d| d.matches_ignoring_source(raw_dependency_package_id)) - .filter(|d| graph_configuration.extra_deps.allows(d.kind())) .filter(|d| { - d.platform() + d.matches_ignoring_source( + cargo_metadata_parameters.krates, + raw_dependency_package_id.clone(), + ) + }) + .filter(|d| graph_configuration.extra_deps.allows(d.kind)) + .filter(|d| { + d.target + .as_ref() .and_then(|p| { graph_configuration.target.map(|t| { match graph_configuration.cfgs { None => false, - Some(cfgs) => p.matches(t, cfgs), + Some(cfgs) => { + (Platform::from_str(p.repr.as_str())) + .unwrap() + .matches(t, cfgs) + } } }) }) .unwrap_or(true) }); - let dependency_package_id = - match resolve.replacement(raw_dependency_package_id) { - Some(id) => id, - None => raw_dependency_package_id, - }; + let dependency_package_id = raw_dependency_package_id.replace( + cargo_metadata_parameters, + package_set, + resolve, + ); for dependency in dependency_iterator { add_graph_node_if_not_present_and_edge( dependency, - dependency_package_id, + dependency_package_id.clone(), graph, index, pending_packages, @@ -160,23 +198,24 @@ fn add_package_dependencies_to_graph<'a>( } fn build_graph_prerequisites<'a>( - args: &'a Args, config_host: &'a InternedString, + deps_args: &'a DepsArgs, + target_args: &'a TargetArgs, ) -> CargoResult<(ExtraDeps, Option<&'a str>)> { - let extra_deps = if args.all_deps { + let extra_deps = if deps_args.all_deps { ExtraDeps::All - } else if args.build_deps { + } else if deps_args.build_deps { ExtraDeps::Build - } else if args.dev_deps { + } else if deps_args.dev_deps { ExtraDeps::Dev } else { ExtraDeps::NoMore }; - let target = if args.all_targets { + let target = if target_args.all_targets { None } else { - Some(args.target.as_deref().unwrap_or(&config_host)) + Some(target_args.target.as_deref().unwrap_or(&config_host)) }; Ok((extra_deps, target)) @@ -189,51 +228,78 @@ mod graph_tests { #[rstest( input_extra_deps, - input_dep_kind, + input_dependency_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) + case(ExtraDeps::All, DependencyKind::Normal, true), + case(ExtraDeps::Build, DependencyKind::Normal, true), + case(ExtraDeps::Dev, DependencyKind::Normal, true), + case(ExtraDeps::NoMore, DependencyKind::Normal, true), + case(ExtraDeps::All, DependencyKind::Build, true), + case(ExtraDeps::All, DependencyKind::Development, true), + case(ExtraDeps::Build, DependencyKind::Build, true), + case(ExtraDeps::Build, DependencyKind::Development, false), + case(ExtraDeps::Dev, DependencyKind::Build, false), + case(ExtraDeps::Dev, DependencyKind::Development, true) )] fn extra_deps_allows_test( input_extra_deps: ExtraDeps, - input_dep_kind: DepKind, + input_dependency_kind: DependencyKind, expected_allows: bool, ) { - assert_eq!(input_extra_deps.allows(input_dep_kind), expected_allows); + assert_eq!( + input_extra_deps.allows(input_dependency_kind), + expected_allows + ); } #[rstest( - input_all_deps, - input_build_deps, - input_dev_deps, + input_deps_args, 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) + case( + DepsArgs { + all_deps: true, + build_deps: false, + dev_deps: false + }, + ExtraDeps::All + ), + case( + DepsArgs { + all_deps: false, + build_deps: true, + dev_deps: false + }, + ExtraDeps::Build + ), + case( + DepsArgs { + all_deps: false, + build_deps: false, + dev_deps: true + }, + ExtraDeps::Dev + ), + case( + DepsArgs { + all_deps: false, + build_deps: false, + dev_deps: false + }, + ExtraDeps::NoMore + ) )] fn build_graph_prerequisites_extra_deps_test( - input_all_deps: bool, - input_build_deps: bool, - input_dev_deps: bool, + input_deps_args: DepsArgs, expected_extra_deps: ExtraDeps, ) { - let mut args = Args::default(); - 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 target_args = TargetArgs::default(); - let result = build_graph_prerequisites(&args, &config_host); + let result = build_graph_prerequisites( + &config_host, + &input_deps_args, + &target_args, + ); assert!(result.is_ok()); let (extra_deps, _) = result.unwrap(); @@ -241,35 +307,44 @@ mod graph_tests { } #[rstest( - input_all_targets, - input_target, + input_target_args, expected_target, - case(true, None, None), - case(false, None, Some("default_config_host")), case( - false, - Some(String::from("provided_config_host")), + TargetArgs { + all_targets: true, + target: None + }, + None + ), + case( + TargetArgs { + all_targets: false, + target: None + }, + Some("default_config_host")), + case( + TargetArgs { + all_targets: false, + target: Some(String::from("provided_config_host")), + }, Some("provided_config_host") ) )] fn build_graph_prerequisites_all_targets_test( - input_all_targets: bool, - input_target: Option, + input_target_args: TargetArgs, expected_target: Option<&str>, ) { - let mut args = Args::default(); - - args.all_targets = input_all_targets; - args.target = input_target; - let config_host = InternedString::new("default_config_host"); + let deps_args = DepsArgs::default(); - let result = build_graph_prerequisites(&args, &config_host); + let result = build_graph_prerequisites( + &config_host, + &deps_args, + &input_target_args, + ); assert!(result.is_ok()); - let (_, target) = result.unwrap(); - assert_eq!(target, expected_target); } } diff --git a/cargo-geiger/src/krates_utils.rs b/cargo-geiger/src/krates_utils.rs index 7a74b119..b3b6f959 100644 --- a/cargo-geiger/src/krates_utils.rs +++ b/cargo-geiger/src/krates_utils.rs @@ -1,6 +1,8 @@ -use cargo::core::{Package, PackageId, PackageSet}; -use cargo_metadata::Metadata; +use cargo::core::dependency::DepKind; +use cargo::core::{Package, PackageId, PackageSet, Resolve}; +use cargo_metadata::{DependencyKind, Metadata}; use krates::Krates; +use std::collections::HashSet; use std::path::PathBuf; pub struct CargoMetadataParameters<'a> { @@ -8,12 +10,117 @@ pub struct CargoMetadataParameters<'a> { pub metadata: &'a Metadata, } +impl DepsNotReplaced for cargo_metadata::Metadata { + fn deps_not_replaced( + &self, + krates: &Krates, + package_id: cargo_metadata::PackageId, + package_set: &PackageSet, + resolve: &Resolve, + ) -> Vec<( + cargo_metadata::PackageId, + HashSet, + )> { + let cargo_core_package_id = + package_id.to_package_id(krates, package_set); + let deps_not_replaced = + resolve.deps_not_replaced(cargo_core_package_id); + + let mut cargo_metadata_deps_not_replaced = vec![]; + + for (dep_package_id, _) in deps_not_replaced { + cargo_metadata_deps_not_replaced.push(( + dep_package_id.to_cargo_metadata_package_id(self), + HashSet::::new(), + )) + } + + cargo_metadata_deps_not_replaced + } +} + +impl GetPackageNameFromCargoMetadataPackageId for Krates { + fn get_package_name_from_cargo_metadata_package_id( + &self, + package_id: &cargo_metadata::PackageId, + ) -> String { + let package = self.node_for_kid(package_id); + package.unwrap().krate.clone().name + } +} + +impl GetPackageVersionFromCargoMetadataPackageId for Krates { + fn get_package_version_from_cargo_metadata_package_id( + &self, + package_id: &cargo_metadata::PackageId, + ) -> cargo_metadata::Version { + let package = self.node_for_kid(package_id); + package.unwrap().krate.clone().version + } +} + impl GetRoot for cargo_metadata::Package { fn get_root(&self) -> PathBuf { self.manifest_path.parent().unwrap().to_path_buf() } } +impl MatchesIgnoringSource for cargo_metadata::Dependency { + fn matches_ignoring_source( + &self, + krates: &Krates, + package_id: cargo_metadata::PackageId, + ) -> bool { + self.name + == krates + .get_package_name_from_cargo_metadata_package_id(&package_id) + && self.req.matches( + &krates.get_package_version_from_cargo_metadata_package_id( + &package_id, + ), + ) + } +} + +impl Replacement for cargo_metadata::PackageId { + fn replace( + &self, + cargo_metadata_parameters: &CargoMetadataParameters, + package_set: &PackageSet, + resolve: &Resolve, + ) -> cargo_metadata::PackageId { + let package_id = + self.to_package_id(cargo_metadata_parameters.krates, package_set); + match resolve.replacement(package_id) { + Some(id) => id.to_cargo_metadata_package_id( + cargo_metadata_parameters.metadata, + ), + None => self.clone(), + } + } +} + +impl ToCargoCoreDepKind for DependencyKind { + fn to_cargo_core_dep_kind(&self) -> DepKind { + match self { + DependencyKind::Build => DepKind::Build, + DependencyKind::Development => DepKind::Development, + DependencyKind::Normal => DepKind::Normal, + _ => panic!("Unknown dependency kind"), + } + } +} + +impl ToCargoMetadataDependencyKind for DepKind { + fn to_cargo_metadata_dependency_kind(&self) -> DependencyKind { + match self { + DepKind::Build => DependencyKind::Build, + DepKind::Development => DependencyKind::Development, + DepKind::Normal => DependencyKind::Normal, + } + } +} + impl ToCargoMetadataPackage for Package { fn to_cargo_metadata_package( &self, @@ -77,10 +184,62 @@ impl ToPackageId for cargo_metadata::PackageId { } } +pub trait DepsNotReplaced { + fn deps_not_replaced( + &self, + krates: &Krates, + package_id: cargo_metadata::PackageId, + package_set: &PackageSet, + resolve: &Resolve, + ) -> Vec<( + cargo_metadata::PackageId, + HashSet, + )>; +} + +pub trait GetPackageNameFromCargoMetadataPackageId { + fn get_package_name_from_cargo_metadata_package_id( + &self, + package_id: &cargo_metadata::PackageId, + ) -> String; +} + +pub trait GetPackageVersionFromCargoMetadataPackageId { + fn get_package_version_from_cargo_metadata_package_id( + &self, + package_id: &cargo_metadata::PackageId, + ) -> cargo_metadata::Version; +} + pub trait GetRoot { fn get_root(&self) -> PathBuf; } +pub trait MatchesIgnoringSource { + fn matches_ignoring_source( + &self, + krates: &Krates, + package_id: cargo_metadata::PackageId, + ) -> bool; +} + +pub trait Replacement { + fn replace( + &self, + cargo_metadata_parameters: &CargoMetadataParameters, + package_set: &PackageSet, + resolve: &Resolve, + ) -> cargo_metadata::PackageId; +} + +pub trait ToCargoCoreDepKind { + fn to_cargo_core_dep_kind(&self) -> DepKind; +} + +pub trait ToCargoMetadataDependencyKind { + fn to_cargo_metadata_dependency_kind(&self) -> DependencyKind; +} + pub trait ToCargoMetadataPackage { fn to_cargo_metadata_package( &self, @@ -116,6 +275,131 @@ mod krates_utils_tests { use rstest::*; use std::path::PathBuf; + #[rstest] + fn deps_not_replaced_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 (package_set, resolve) = + resolve(&args, package.package_id(), &mut registry, &workspace) + .unwrap(); + + let (krates, metadata) = construct_krates_and_metadata(); + let cargo_metadata_package_id = + package.package_id().to_cargo_metadata_package_id(&metadata); + + let deps_not_replaced = resolve.deps_not_replaced(package.package_id()); + let cargo_metadata_deps_not_replaced = metadata.deps_not_replaced( + &krates, + cargo_metadata_package_id, + &package_set, + &resolve, + ); + + let mut cargo_core_package_names = deps_not_replaced + .map(|(p, _)| p.name().to_string()) + .collect::>(); + + let mut cargo_metadata_package_names = cargo_metadata_deps_not_replaced + .iter() + .map(|(p, _)| { + krates.get_package_name_from_cargo_metadata_package_id(p) + }) + .collect::>(); + + cargo_core_package_names.sort(); + cargo_metadata_package_names.sort(); + + assert_eq!(cargo_core_package_names, cargo_metadata_package_names); + } + + #[rstest] + fn get_package_name_from_cargo_metadata_package_id_test() { + let (krates, metadata) = construct_krates_and_metadata(); + let package = metadata.root_package().unwrap(); + let package_name = + krates.get_package_name_from_cargo_metadata_package_id(&package.id); + assert_eq!(package_name, package.name); + } + + #[rstest] + fn get_package_version_from_cargo_metadata_package_id_test() { + let (krates, metadata) = construct_krates_and_metadata(); + let package = metadata.root_package().unwrap(); + let package_version = krates + .get_package_version_from_cargo_metadata_package_id(&package.id); + assert_eq!(package_version, package.version); + } + + #[rstest] + fn replace_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 (package_set, resolve) = + resolve(&args, package.package_id(), &mut registry, &workspace) + .unwrap(); + + let (krates, metadata) = construct_krates_and_metadata(); + let cargo_metadata_package_id = + package.package_id().to_cargo_metadata_package_id(&metadata); + let cargo_metadata_parameters = CargoMetadataParameters { + krates: &krates, + metadata: &metadata, + }; + + assert_eq!( + cargo_metadata_package_id, + cargo_metadata_package_id.replace( + &cargo_metadata_parameters, + &package_set, + &resolve + ) + ) + } + + #[rstest( + input_dependency_kind, + expected_dep_kind, + case(DependencyKind::Build, DepKind::Build), + case(DependencyKind::Development, DepKind::Development), + case(DependencyKind::Normal, DepKind::Normal) + )] + fn to_cargo_core_dep_kind( + input_dependency_kind: DependencyKind, + expected_dep_kind: DepKind, + ) { + assert_eq!( + input_dependency_kind.to_cargo_core_dep_kind(), + expected_dep_kind + ) + } + + #[rstest( + input_dep_kind, + expected_dependency_kind, + case(DepKind::Build, DependencyKind::Build), + case(DepKind::Development, DependencyKind::Development), + case(DepKind::Normal, DependencyKind::Normal) + )] + fn to_cargo_metadata_dependency_kind_test( + input_dep_kind: DepKind, + expected_dependency_kind: DependencyKind, + ) { + assert_eq!( + input_dep_kind.to_cargo_metadata_dependency_kind(), + expected_dependency_kind + ); + } + #[rstest] fn to_cargo_metadata_package_id_test() { let config = Config::default().unwrap(); @@ -123,7 +407,7 @@ mod krates_utils_tests { let workspace = get_workspace(&config, manifest_path).unwrap(); let package = workspace.current().unwrap(); - let metadata = construct_metadata(); + let (_, metadata) = construct_krates_and_metadata(); let cargo_metadata_package_id = package.package_id().to_cargo_metadata_package_id(&metadata); @@ -143,10 +427,7 @@ mod krates_utils_tests { resolve(&args, package.package_id(), &mut registry, &workspace) .unwrap(); - let metadata = construct_metadata(); - let krates = Builder::new() - .build_with_metadata(metadata.clone(), |_| ()) - .unwrap(); + let (krates, metadata) = construct_krates_and_metadata(); let cargo_metadata_package = metadata.root_package().unwrap(); let package_id = cargo_metadata_package @@ -157,11 +438,17 @@ mod krates_utils_tests { assert_eq!(cargo_metadata_package.name, package_id.name().to_string()); } - fn construct_metadata() -> Metadata { - MetadataCommand::new() + fn construct_krates_and_metadata() -> (Krates, Metadata) { + let metadata = MetadataCommand::new() .manifest_path("./Cargo.toml") .features(CargoOpt::AllFeatures) .exec() - .unwrap() + .unwrap(); + + let krates = Builder::new() + .build_with_metadata(metadata.clone(), |_| ()) + .unwrap(); + + (krates, metadata) } } diff --git a/cargo-geiger/src/main.rs b/cargo-geiger/src/main.rs index 6fd5a286..275e77f0 100644 --- a/cargo-geiger/src/main.rs +++ b/cargo-geiger/src/main.rs @@ -25,7 +25,7 @@ use crate::cli::{ use crate::graph::build_graph; use crate::scan::scan; -use crate::krates_utils::CargoMetadataParameters; +use crate::krates_utils::{CargoMetadataParameters, ToCargoMetadataPackage}; use cargo::core::shell::{ColorChoice, Shell}; use cargo::{CliResult, Config}; @@ -69,12 +69,18 @@ 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 root_package = workspace.current()?; + let mut registry = get_registry(config, &root_package)?; + + // `cargo_metadata.root_package()` will return `None` when called on a virtual + // manifest + let cargo_metadata_root_package_id = root_package + .to_cargo_metadata_package(cargo_metadata_parameters.metadata) + .id; let (package_set, resolve) = resolve( &args.features_args, - package.package_id(), + root_package.package_id(), &mut registry, &workspace, )?; @@ -84,15 +90,16 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult { let root_package_id = match args.package { Some(ref pkg) => resolve.query(pkg)?, - None => package.package_id(), + None => root_package.package_id(), }; let graph = build_graph( args, + &cargo_metadata_parameters, config, &resolve, &package_set, - package.package_id(), + cargo_metadata_root_package_id, &workspace, )?; diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index 77354aeb..05e4723e 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -12,7 +12,10 @@ pub use rs_file::RsFileMetricsWrapper; use default::scan_unsafe; use forbid::scan_forbid_unsafe; -use crate::krates_utils::CargoMetadataParameters; +use crate::krates_utils::{ + CargoMetadataParameters, ToCargoCoreDepKind, ToCargoMetadataPackageId, + ToPackageId, +}; use cargo::core::dependency::DepKind; use cargo::core::{PackageId, PackageSet, Workspace}; use cargo::{CliResult, Config}; @@ -162,40 +165,55 @@ fn list_files_used_but_not_scanned( .collect() } -fn package_metrics<'a>( - geiger_context: &'a GeigerContext, - graph: &'a Graph, +fn package_metrics( + cargo_metadata_parameters: &CargoMetadataParameters, + geiger_context: &GeigerContext, + graph: &Graph, + package_set: &PackageSet, root_package_id: PackageId, -) -> impl Iterator)> { - let root_index = graph.nodes[&root_package_id]; +) -> Vec<(PackageInfo, Option)> { + let mut package_metrics = + Vec::<(PackageInfo, Option)>::new(); + let root_index = graph.nodes[&root_package_id + .to_cargo_metadata_package_id(cargo_metadata_parameters.metadata)]; let mut indices = vec![root_index]; let mut visited = HashSet::new(); - std::iter::from_fn(move || { - let i = indices.pop()?; - let package_id = graph.graph[i]; + + while !indices.is_empty() { + let i = indices.pop().unwrap(); + let package_id = graph.graph[i] + .to_package_id(cargo_metadata_parameters.krates, package_set); 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]); + let dep = + from_cargo_package_id(graph.graph[dep_index].to_package_id( + cargo_metadata_parameters.krates, + package_set, + )); package.add_dependency( dep, - from_cargo_dependency_kind(*edge.weight()), + from_cargo_dependency_kind( + edge.weight().to_cargo_core_dep_kind(), + ), ); } match geiger_context.package_id_to_metrics.get(&package_id) { - Some(m) => Some((package, Some(m))), + Some(m) => package_metrics.push((package, Some(m.clone()))), None => { eprintln!( "WARNING: No metrics found for package: {}", package_id ); - Some((package, None)) + package_metrics.push((package, None)) } } - }) + } + + package_metrics } fn from_cargo_package_id(id: PackageId) -> cargo_geiger_serde::PackageId { diff --git a/cargo-geiger/src/scan/default.rs b/cargo-geiger/src/scan/default.rs index cc633a93..ea6d7085 100644 --- a/cargo-geiger/src/scan/default.rs +++ b/cargo-geiger/src/scan/default.rs @@ -131,9 +131,13 @@ fn scan_to_report( workspace, )?; let mut report = SafetyReport::default(); - for (package, package_metrics_option) in - package_metrics(&geiger_context, graph, root_package_id) - { + for (package, package_metrics_option) in package_metrics( + cargo_metadata_parameters, + &geiger_context, + graph, + package_set, + root_package_id, + ) { let package_metrics = match package_metrics_option { Some(m) => m, None => { @@ -141,7 +145,7 @@ fn scan_to_report( continue; } }; - let unsafe_info = unsafe_stats(package_metrics, &rs_files_used); + let unsafe_info = unsafe_stats(&package_metrics, &rs_files_used); let entry = ReportEntry { package, unsafety: unsafe_info, diff --git a/cargo-geiger/src/scan/forbid.rs b/cargo-geiger/src/scan/forbid.rs index 072150d3..961f22bf 100644 --- a/cargo-geiger/src/scan/forbid.rs +++ b/cargo-geiger/src/scan/forbid.rs @@ -58,9 +58,13 @@ fn scan_forbid_to_report( print_config, )?; let mut report = QuickSafetyReport::default(); - for (package, package_metrics) in - package_metrics(&geiger_context, graph, root_package_id) - { + for (package, package_metrics) in package_metrics( + cargo_metadata_parameters, + &geiger_context, + graph, + package_set, + root_package_id, + ) { let pack_metrics = match package_metrics { Some(m) => m, None => { diff --git a/cargo-geiger/src/tree/traversal.rs b/cargo-geiger/src/tree/traversal.rs index 28740a32..e0f2c623 100644 --- a/cargo-geiger/src/tree/traversal.rs +++ b/cargo-geiger/src/tree/traversal.rs @@ -3,16 +3,23 @@ mod dependency_node; use crate::format::print_config::PrintConfig; use crate::graph::Graph; +use crate::krates_utils::{CargoMetadataParameters, ToPackageId}; use crate::tree::TextTreeLine; use super::construct_tree_vines_string; use dependency_kind::walk_dependency_kind; use dependency_node::walk_dependency_node; -use crate::krates_utils::{CargoMetadataParameters, ToPackageId}; -use cargo::core::PackageSet; +use cargo::core::{PackageId, PackageSet}; use std::collections::HashSet; +pub struct WalkDependencyParameters<'a> { + pub graph: &'a Graph, + pub levels_continue: &'a mut Vec, + pub print_config: &'a PrintConfig, + pub visited_deps: &'a mut HashSet, +} + /// Printing the returned TextTreeLines in order is expected to produce a nice /// looking tree structure. /// @@ -28,13 +35,19 @@ pub fn walk_dependency_tree( ) -> Vec { let mut visited_deps = HashSet::new(); let mut levels_continue = vec![]; - let node = &graph.graph[graph.nodes[&root_package_id - .to_package_id(cargo_metadata_parameters.krates, package_set)]]; - walk_dependency_node( - node, + + let mut walk_dependency_paramters = WalkDependencyParameters { graph, - &mut visited_deps, - &mut levels_continue, + levels_continue: &mut levels_continue, print_config, + visited_deps: &mut visited_deps, + }; + + let node = &graph.graph[graph.nodes[&root_package_id]]; + walk_dependency_node( + cargo_metadata_parameters, + &node.to_package_id(cargo_metadata_parameters.krates, package_set), + package_set, + &mut walk_dependency_paramters, ) } diff --git a/cargo-geiger/src/tree/traversal/dependency_kind.rs b/cargo-geiger/src/tree/traversal/dependency_kind.rs index edf569ba..c4afa765 100644 --- a/cargo-geiger/src/tree/traversal/dependency_kind.rs +++ b/cargo-geiger/src/tree/traversal/dependency_kind.rs @@ -1,22 +1,21 @@ -use crate::format::print_config::{Prefix, PrintConfig}; -use crate::graph::Graph; +use crate::format::print_config::Prefix; +use crate::tree::traversal::WalkDependencyParameters; use crate::tree::{get_tree_symbols, TextTreeLine, TreeSymbols}; use super::dependency_node::walk_dependency_node; +use crate::krates_utils::CargoMetadataParameters; use cargo::core::dependency::DepKind; -use cargo::core::PackageId; -use std::collections::HashSet; +use cargo::core::{PackageId, PackageSet}; use std::iter::Peekable; use std::slice::Iter; pub fn walk_dependency_kind( + cargo_metadata_parameters: &CargoMetadataParameters, dep_kind: DepKind, - deps: &mut Vec<&PackageId>, - graph: &Graph, - visited_deps: &mut HashSet, - levels_continue: &mut Vec, - print_config: &PrintConfig, + deps: &mut Vec, + package_set: &PackageSet, + walk_dependency_parameters: &mut WalkDependencyParameters, ) -> Vec { if deps.is_empty() { return Vec::new(); @@ -25,12 +24,13 @@ pub fn walk_dependency_kind( // Resolve uses Hash data types internally but we want consistent output ordering deps.sort_by_key(|n| *n); - let tree_symbols = get_tree_symbols(print_config.charset); + let tree_symbols = + get_tree_symbols(walk_dependency_parameters.print_config.charset); let mut text_tree_lines = Vec::new(); - if let Prefix::Indent = print_config.prefix { + if let Prefix::Indent = walk_dependency_parameters.print_config.prefix { push_extra_deps_group_text_tree_line_for_non_normal_dependencies( dep_kind, - levels_continue, + walk_dependency_parameters.levels_continue, &tree_symbols, &mut text_tree_lines, ) @@ -39,36 +39,35 @@ pub fn walk_dependency_kind( let mut node_iterator = deps.iter().peekable(); while let Some(dependency) = node_iterator.next() { handle_walk_dependency_node( + cargo_metadata_parameters, dependency, - graph, - levels_continue, &mut node_iterator, - print_config, + package_set, &mut text_tree_lines, - visited_deps, + walk_dependency_parameters, ); } text_tree_lines } fn handle_walk_dependency_node( + cargo_metadata_parameters: &CargoMetadataParameters, dependency: &PackageId, - graph: &Graph, - levels_continue: &mut Vec, - node_iterator: &mut Peekable>, - print_config: &PrintConfig, + node_iterator: &mut Peekable>, + package_set: &PackageSet, text_tree_lines: &mut Vec, - visited_deps: &mut HashSet, + walk_dependency_parameters: &mut WalkDependencyParameters, ) { - levels_continue.push(node_iterator.peek().is_some()); + walk_dependency_parameters + .levels_continue + .push(node_iterator.peek().is_some()); text_tree_lines.append(&mut walk_dependency_node( + cargo_metadata_parameters, dependency, - graph, - visited_deps, - levels_continue, - print_config, + package_set, + walk_dependency_parameters, )); - levels_continue.pop(); + walk_dependency_parameters.levels_continue.pop(); } fn push_extra_deps_group_text_tree_line_for_non_normal_dependencies( diff --git a/cargo-geiger/src/tree/traversal/dependency_node.rs b/cargo-geiger/src/tree/traversal/dependency_node.rs index b74922e2..1e9e8481 100644 --- a/cargo-geiger/src/tree/traversal/dependency_node.rs +++ b/cargo-geiger/src/tree/traversal/dependency_node.rs @@ -1,25 +1,33 @@ use crate::format::print_config::PrintConfig; use crate::graph::Graph; +use crate::krates_utils::{ + CargoMetadataParameters, ToCargoCoreDepKind, ToCargoMetadataPackageId, + ToPackageId, +}; +use crate::tree::traversal::WalkDependencyParameters; use crate::tree::TextTreeLine; use super::construct_tree_vines_string; use super::walk_dependency_kind; use cargo::core::dependency::DepKind; -use cargo::core::PackageId; +use cargo::core::{PackageId, PackageSet}; use petgraph::visit::EdgeRef; use petgraph::EdgeDirection; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; pub fn walk_dependency_node( + cargo_metadata_parameters: &CargoMetadataParameters, package: &PackageId, - graph: &Graph, - visited_deps: &mut HashSet, - levels_continue: &mut Vec, - print_config: &PrintConfig, + package_set: &PackageSet, + walk_dependency_parameters: &mut WalkDependencyParameters, ) -> Vec { - let new = print_config.all || visited_deps.insert(*package); - let tree_vines = construct_tree_vines_string(levels_continue, print_config); + let new = walk_dependency_parameters.print_config.all + || walk_dependency_parameters.visited_deps.insert(*package); + let tree_vines = construct_tree_vines_string( + walk_dependency_parameters.levels_continue, + walk_dependency_parameters.print_config, + ); let mut all_out_text_tree_lines = vec![TextTreeLine::Package { id: *package, @@ -30,17 +38,26 @@ pub fn walk_dependency_node( return all_out_text_tree_lines; } - let mut dependency_type_nodes = - construct_dependency_type_nodes_hashmap(graph, package, print_config); + let mut dependency_type_nodes = construct_dependency_type_nodes_hashmap( + walk_dependency_parameters.graph, + &package + .to_cargo_metadata_package_id(cargo_metadata_parameters.metadata), + walk_dependency_parameters.print_config, + ); for (dep_kind, nodes) in dependency_type_nodes.iter_mut() { + let mut cargo_core_nodes = nodes + .iter() + .map(|n| { + n.to_package_id(cargo_metadata_parameters.krates, package_set) + }) + .collect::>(); let mut dep_kind_out = walk_dependency_kind( + cargo_metadata_parameters, *dep_kind, - nodes, - graph, - visited_deps, - levels_continue, - print_config, + &mut cargo_core_nodes, + package_set, + walk_dependency_parameters, ); all_out_text_tree_lines.append(&mut dep_kind_out); @@ -51,10 +68,13 @@ pub fn walk_dependency_node( fn construct_dependency_type_nodes_hashmap<'a>( graph: &'a Graph, - package: &PackageId, + package: &cargo_metadata::PackageId, print_config: &PrintConfig, -) -> HashMap> { - let mut dependency_type_nodes: HashMap> = [ +) -> HashMap> { + let mut dependency_type_nodes: HashMap< + DepKind, + Vec, + > = [ (DepKind::Build, vec![]), (DepKind::Development, vec![]), (DepKind::Normal, vec![]), @@ -73,9 +93,9 @@ fn construct_dependency_type_nodes_hashmap<'a>( }; dependency_type_nodes - .get_mut(edge.weight()) + .get_mut(&edge.weight().to_cargo_core_dep_kind()) .unwrap() - .push(dependency); + .push(dependency.clone()); } dependency_type_nodes @@ -85,17 +105,15 @@ fn construct_dependency_type_nodes_hashmap<'a>( mod dependency_node_tests { use super::*; - use crate::cli::get_workspace; use crate::format::pattern::Pattern; use crate::format::print_config::{Prefix, PrintConfig}; use crate::format::Charset; use cargo::core::Verbosity; - use cargo::Config; + use cargo_metadata::DependencyKind; use geiger::IncludeTests; use petgraph::graph::NodeIndex; use rstest::*; - use std::env; #[rstest( input_directed_edges, @@ -105,12 +123,12 @@ mod dependency_node_tests { expected_normal_nodes_length, case( vec![ - (1, 0, DepKind::Build), - (2, 0, DepKind::Build), - (3, 0, DepKind::Build), - (4, 0, DepKind::Development), - (5, 0, DepKind::Development), - (6, 0, DepKind::Normal) + (1, 0, DependencyKind::Build), + (2, 0, DependencyKind::Build), + (3, 0, DependencyKind::Build), + (4, 0, DependencyKind::Development), + (5, 0, DependencyKind::Development), + (6, 0, DependencyKind::Normal) ], EdgeDirection::Incoming, 3, @@ -119,12 +137,12 @@ mod dependency_node_tests { ), case( vec![ - (0, 1, DepKind::Build), - (0, 2, DepKind::Development), - (0, 3, DepKind::Development), - (0, 4, DepKind::Normal), - (0, 5, DepKind::Normal), - (0, 6, DepKind::Normal) + (0, 1, DependencyKind::Build), + (0, 2, DependencyKind::Development), + (0, 3, DependencyKind::Development), + (0, 4, DependencyKind::Normal), + (0, 5, DependencyKind::Normal), + (0, 6, DependencyKind::Normal) ], EdgeDirection::Outgoing, 1, @@ -133,20 +151,26 @@ mod dependency_node_tests { ) )] fn construct_dependency_type_nodes_hashmap_test( - input_directed_edges: Vec<(usize, usize, DepKind)>, + input_directed_edges: Vec<(usize, usize, DependencyKind)>, input_edge_direction: EdgeDirection, expected_build_nodes_length: usize, expected_development_nodes_length: usize, expected_normal_nodes_length: usize, ) { - let mut inner_graph = petgraph::Graph::::new(); - let mut nodes = HashMap::::new(); + let mut inner_graph = petgraph::Graph::< + cargo_metadata::PackageId, + cargo_metadata::DependencyKind, + >::new(); + let mut nodes = HashMap::::new(); - let package_ids = create_package_id_vec(7); + let package_ids = create_cargo_metadata_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(*package_id)); + nodes.insert( + package_id.clone(), + inner_graph.add_node(package_id.clone()), + ); } add_edges_to_graph( @@ -183,10 +207,10 @@ mod dependency_node_tests { } fn add_edges_to_graph( - directed_edges: &[(usize, usize, DepKind)], - graph: &mut petgraph::Graph, - nodes: &HashMap, - package_ids: &[PackageId], + directed_edges: &[(usize, usize, DependencyKind)], + graph: &mut petgraph::Graph, + nodes: &HashMap, + package_ids: &[cargo_metadata::PackageId], ) { for (source_index, target_index, dep_kind) in directed_edges { graph.add_edge( @@ -197,25 +221,15 @@ mod dependency_node_tests { } } - fn create_package_id_vec(count: i32) -> Vec { - let config = Config::default().unwrap(); - let current_working_dir = - env::current_dir().unwrap().join("Cargo.toml"); - let manifest_path_option = Some(current_working_dir); - let workspace = get_workspace(&config, manifest_path_option).unwrap(); - let package = workspace.current().unwrap(); - let source_id = package.dependencies().first().unwrap().source_id(); + fn create_cargo_metadata_package_id_vec( + count: i32, + ) -> Vec { + let mut package_id_vec = vec![]; - let mut package_id_vec: Vec = vec![]; for i in 0..count { - package_id_vec.push( - PackageId::new( - format!("test_name_{}", i), - format!("1.2.{}", i).as_str(), - source_id, - ) - .unwrap(), - ) + package_id_vec.push(cargo_metadata::PackageId { + repr: format!("string_repr_{}", i), + }); } package_id_vec