From 0f5ba7ba8f000821c61e18829beebb260bcfd503 Mon Sep 17 00:00:00 2001 From: joshmc Date: Thu, 4 Feb 2021 19:27:16 +0000 Subject: [PATCH] NOISSUE - Clean up error handling, remove unwrap() calls, logging meaningful messages --- cargo-geiger/src/format/display.rs | 26 +-- .../src/format/table/handle_text_tree_line.rs | 15 +- cargo-geiger/src/graph.rs | 105 +++++++----- cargo-geiger/src/mapping.rs | 31 ++-- cargo-geiger/src/mapping/krates.rs | 74 ++++---- cargo-geiger/src/mapping/metadata.rs | 161 +++++++++++------- cargo-geiger/src/readme.rs | 52 ++++-- cargo-geiger/src/scan.rs | 80 ++++++--- cargo-geiger/src/scan/find.rs | 12 +- ...est2_package_with_shallow_deps.stdout.snap | 2 +- 10 files changed, 333 insertions(+), 225 deletions(-) diff --git a/cargo-geiger/src/format/display.rs b/cargo-geiger/src/format/display.rs index 0ced53d6..052f61a6 100644 --- a/cargo-geiger/src/format/display.rs +++ b/cargo-geiger/src/format/display.rs @@ -2,8 +2,7 @@ use crate::format::pattern::Pattern; use crate::format::Chunk; use crate::mapping::{ CargoMetadataParameters, GetLicenceFromCargoMetadataPackageId, - GetPackageNameFromCargoMetadataPackageId, - GetPackageVersionFromCargoMetadataPackageId, + GetPackageNameAndVersionFromCargoMetadataPackageId, GetRepositoryFromCargoMetadataPackageId, }; @@ -32,22 +31,13 @@ impl<'a> fmt::Display for Display<'a> { } } Chunk::Package => { - (write!( - fmt, - "{} {}", - self.cargo_metadata_parameters - .krates - .get_package_name_from_cargo_metadata_package_id( - self.package - ) - .unwrap(), - self.cargo_metadata_parameters - .krates - .get_package_version_from_cargo_metadata_package_id( - self.package - ) - .unwrap() - ))? + if let Some((package_name, package_version)) = self.cargo_metadata_parameters + .krates + .get_package_name_and_version_from_cargo_metadata_package_id(self.package) { + (write!(fmt, "{} {}", package_name, package_version))? + } else { + eprintln!("Failed to format Package: {}", self.package) + } } Chunk::Raw(ref s) => (fmt.write_str(s))?, Chunk::Repository => { 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 f7bab4d4..754faede 100644 --- a/cargo-geiger/src/format/table/handle_text_tree_line.rs +++ b/cargo-geiger/src/format/table/handle_text_tree_line.rs @@ -22,14 +22,15 @@ pub fn handle_text_tree_line_extra_deps_group( table_lines: &mut Vec, tree_vines: String, ) { - let name = get_kind_group_name(dep_kind); - if name.is_none() { - return; + if let Some(name) = get_kind_group_name(dep_kind) { + // TODO: Fix the alignment on macOS (others too?) + table_lines.push(format!( + "{}{}{}", + table_row_empty(), + tree_vines, + name + )); } - let name = name.unwrap(); - - // TODO: Fix the alignment on macOS (others too?) - table_lines.push(format!("{}{}{}", table_row_empty(), tree_vines, name)); } pub fn handle_text_tree_line_package( diff --git a/cargo-geiger/src/graph.rs b/cargo-geiger/src/graph.rs index 4b341302..b09d541b 100644 --- a/cargo-geiger/src/graph.rs +++ b/cargo-geiger/src/graph.rs @@ -8,7 +8,7 @@ use cargo::core::Workspace; use cargo::util::interning::InternedString; use cargo::util::CargoResult; use cargo::Config; -use cargo_metadata::{DependencyKind, PackageId}; +use cargo_metadata::{Dependency, DependencyKind, Package, PackageId}; use cargo_platform::Cfg; use petgraph::graph::NodeIndex; use std::collections::hash_map::Entry; @@ -124,49 +124,39 @@ fn add_package_dependencies_to_graph( pending_packages: &mut Vec, ) { let index = graph.nodes[&package_id]; - let package = cargo_metadata_parameters - .krates - .node_for_kid(&package_id) - .unwrap() - .krate - .clone(); - for (dependency_package_id, _) in cargo_metadata_parameters + let krates_node_option = + cargo_metadata_parameters.krates.node_for_kid(&package_id); + + let dep_not_replaced_option = cargo_metadata_parameters .metadata - .deps_not_replaced(package_id) - { - let dependency_iterator = package - .dependencies - .iter() - .filter(|d| { - d.matches_ignoring_source( - cargo_metadata_parameters.krates, - 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), - } - }) - }) - .unwrap_or(true) - }); + .deps_not_replaced(package_id.clone()); - for dependency in dependency_iterator { - add_graph_node_if_not_present_and_edge( - dependency, - dependency_package_id.clone(), - graph, - index, - pending_packages, - ); + match (krates_node_option, dep_not_replaced_option) { + (Some(krates_node), Some(dependencies)) => { + let package = krates_node.krate.clone(); + + for (dependency_package_id, _) in dependencies { + let dependency_iterator = filter_dependencies( + cargo_metadata_parameters, + &dependency_package_id, + graph_configuration, + &package, + ); + + for dependency in dependency_iterator { + add_graph_node_if_not_present_and_edge( + dependency, + dependency_package_id.clone(), + graph, + index, + pending_packages, + ); + } + } + } + _ => { + eprintln!("Failed to add package dependencies to graph for Package Id: {}", package_id) } } } @@ -195,6 +185,39 @@ fn build_graph_prerequisites<'a>( (extra_deps, target) } +fn filter_dependencies<'a>( + cargo_metadata_parameters: &'a CargoMetadataParameters, + dependency_package_id: &'a PackageId, + graph_configuration: &'a GraphConfiguration, + package: &'a Package, +) -> Vec<&'a Dependency> { + package + .dependencies + .iter() + .filter(|d| { + d.matches_ignoring_source( + cargo_metadata_parameters.krates, + dependency_package_id.clone(), + ) + .unwrap_or(false) + }) + .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), + }, + ) + }) + .unwrap_or(true) + }) + .collect::>() +} + #[cfg(test)] mod graph_tests { use super::*; diff --git a/cargo-geiger/src/mapping.rs b/cargo-geiger/src/mapping.rs index da183cb6..c47e0d1f 100644 --- a/cargo-geiger/src/mapping.rs +++ b/cargo-geiger/src/mapping.rs @@ -19,10 +19,12 @@ pub trait DepsNotReplaced { fn deps_not_replaced( &self, package_id: cargo_metadata::PackageId, - ) -> Vec<( - cargo_metadata::PackageId, - HashSet, - )>; + ) -> Option< + Vec<( + cargo_metadata::PackageId, + HashSet, + )>, + >; } pub trait GetLicenceFromCargoMetadataPackageId { @@ -32,18 +34,11 @@ pub trait GetLicenceFromCargoMetadataPackageId { ) -> Option; } -pub trait GetPackageNameFromCargoMetadataPackageId { - fn get_package_name_from_cargo_metadata_package_id( +pub trait GetPackageNameAndVersionFromCargoMetadataPackageId { + fn get_package_name_and_version_from_cargo_metadata_package_id( &self, package_id: &cargo_metadata::PackageId, - ) -> Option; -} - -pub trait GetPackageVersionFromCargoMetadataPackageId { - fn get_package_version_from_cargo_metadata_package_id( - &self, - package_id: &cargo_metadata::PackageId, - ) -> Option; + ) -> Option<(String, cargo_metadata::Version)>; } pub trait GetRepositoryFromCargoMetadataPackageId { @@ -54,7 +49,7 @@ pub trait GetRepositoryFromCargoMetadataPackageId { } pub trait GetRoot { - fn get_root(&self) -> PathBuf; + fn get_root(&self) -> Option; } pub trait MatchesIgnoringSource { @@ -62,7 +57,7 @@ pub trait MatchesIgnoringSource { &self, krates: &Krates, package_id: cargo_metadata::PackageId, - ) -> bool; + ) -> Option; } pub trait QueryResolve { @@ -76,14 +71,14 @@ pub trait ToCargoCoreDepKind { pub trait ToCargoGeigerDependencyKind { fn to_cargo_geiger_dependency_kind( &self, - ) -> cargo_geiger_serde::DependencyKind; + ) -> Option; } pub trait ToCargoGeigerPackageId { fn to_cargo_geiger_package_id( &self, metadata: &Metadata, - ) -> cargo_geiger_serde::PackageId; + ) -> Option; } pub trait ToCargoGeigerSource { diff --git a/cargo-geiger/src/mapping/krates.rs b/cargo-geiger/src/mapping/krates.rs index 2a42cf1c..4f3b1880 100644 --- a/cargo-geiger/src/mapping/krates.rs +++ b/cargo-geiger/src/mapping/krates.rs @@ -1,7 +1,6 @@ use crate::mapping::{ GetLicenceFromCargoMetadataPackageId, - GetPackageNameFromCargoMetadataPackageId, - GetPackageVersionFromCargoMetadataPackageId, + GetPackageNameAndVersionFromCargoMetadataPackageId, GetRepositoryFromCargoMetadataPackageId, QueryResolve, }; @@ -18,23 +17,14 @@ impl GetLicenceFromCargoMetadataPackageId for Krates { } } -impl GetPackageNameFromCargoMetadataPackageId for Krates { - fn get_package_name_from_cargo_metadata_package_id( +impl GetPackageNameAndVersionFromCargoMetadataPackageId for Krates { + fn get_package_name_and_version_from_cargo_metadata_package_id( &self, package_id: &cargo_metadata::PackageId, - ) -> Option { - self.node_for_kid(package_id) - .map(|package| package.krate.clone().name) - } -} - -impl GetPackageVersionFromCargoMetadataPackageId for Krates { - fn get_package_version_from_cargo_metadata_package_id( - &self, - package_id: &cargo_metadata::PackageId, - ) -> Option { - self.node_for_kid(package_id) - .map(|package| package.krate.clone().version) + ) -> Option<(String, cargo_metadata::Version)> { + self.node_for_kid(package_id).map(|package| { + (package.krate.clone().name, package.krate.clone().version) + }) } } @@ -50,12 +40,18 @@ impl GetRepositoryFromCargoMetadataPackageId for Krates { impl QueryResolve for Krates { fn query_resolve(&self, query: &str) -> Option { - let package_spec = PkgSpec::from_str(query).unwrap(); - self.krates_by_name(package_spec.name.as_str()) - .filter(|(_, node)| package_spec.matches(&node.krate)) - .map(|(_, node)| node.krate.clone().id) - .collect::>() - .pop() + match PkgSpec::from_str(query) { + Ok(package_spec) => self + .krates_by_name(package_spec.name.as_str()) + .filter(|(_, node)| package_spec.matches(&node.krate)) + .map(|(_, node)| node.krate.clone().id) + .collect::>() + .pop(), + _ => { + eprintln!("Failed to construct PkgSpec from string: {}", query); + None + } + } } } @@ -82,8 +78,10 @@ mod krates_tests { 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) + let (package_name, _) = krates + .get_package_name_and_version_from_cargo_metadata_package_id( + &package.id, + ) .unwrap(); assert_eq!(package_name, package.name); } @@ -92,8 +90,10 @@ mod krates_tests { 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) + let (_, package_version) = krates + .get_package_name_and_version_from_cargo_metadata_package_id( + &package.id, + ) .unwrap(); assert_eq!(package_version, package.version); } @@ -147,19 +147,15 @@ mod krates_tests { let (krates, _) = construct_krates_and_metadata(); let package_id = krates.query_resolve(input_query_string).unwrap(); - assert_eq!( - krates - .get_package_name_from_cargo_metadata_package_id(&package_id) - .unwrap(), - expected_package_name - ); + let (package_name, package_version) = krates + .get_package_name_and_version_from_cargo_metadata_package_id( + &package_id, + ) + .unwrap(); - assert_eq!( - krates - .get_package_version_from_cargo_metadata_package_id(&package_id) - .unwrap(), - expected_package_version - ); + assert_eq!(package_name, expected_package_name); + + assert_eq!(package_version, expected_package_version); } fn construct_krates_and_metadata() -> (Krates, Metadata) { diff --git a/cargo-geiger/src/mapping/metadata.rs b/cargo-geiger/src/mapping/metadata.rs index 6ef242de..9c202d7d 100644 --- a/cargo-geiger/src/mapping/metadata.rs +++ b/cargo-geiger/src/mapping/metadata.rs @@ -1,7 +1,7 @@ use super::{ - DepsNotReplaced, GetPackageNameFromCargoMetadataPackageId, - GetPackageVersionFromCargoMetadataPackageId, GetRoot, - MatchesIgnoringSource, ToCargoGeigerPackageId, ToCargoMetadataPackageId, + DepsNotReplaced, GetPackageNameAndVersionFromCargoMetadataPackageId, + GetRoot, MatchesIgnoringSource, ToCargoGeigerPackageId, + ToCargoMetadataPackageId, }; use crate::mapping::{ @@ -17,36 +17,52 @@ impl DepsNotReplaced for cargo_metadata::Metadata { fn deps_not_replaced( &self, package_id: cargo_metadata::PackageId, - ) -> Vec<( - cargo_metadata::PackageId, - HashSet, - )> { + ) -> Option< + Vec<( + cargo_metadata::PackageId, + HashSet, + )>, + > { let mut cargo_metadata_deps_not_replaced = vec![]; let mut package_id_hashset = HashSet::::new(); - for dep in package_id - .to_cargo_metadata_package(self) - .unwrap() - .dependencies - { - if let Some(package_id) = dep.to_cargo_metadata_package_id(self) { - if !package_id_hashset.contains(&package_id) { - cargo_metadata_deps_not_replaced.push(( - package_id.clone(), - HashSet::::new(), - )); - package_id_hashset.insert(package_id); + match package_id.to_cargo_metadata_package(self) { + Some(package) => { + for dependency in package.dependencies { + if let Some(package_id) = + dependency.to_cargo_metadata_package_id(self) + { + if !package_id_hashset.contains(&package_id) { + cargo_metadata_deps_not_replaced.push(( + package_id.clone(), + HashSet::::new(), + )); + package_id_hashset.insert(package_id); + } + } } + Some(cargo_metadata_deps_not_replaced) + } + None => { + eprintln!("Failed to convert Package Id: {} to Cargo Metadata Package", package_id); + None } } - - cargo_metadata_deps_not_replaced } } impl GetRoot for cargo_metadata::Package { - fn get_root(&self) -> PathBuf { - self.manifest_path.parent().unwrap().to_path_buf() + fn get_root(&self) -> Option { + match self.manifest_path.parent() { + Some(path) => Some(path.to_path_buf()), + None => { + eprintln!( + "Failed to get root for: {} {:?}", + self.name, self.version + ); + None + } + } } } @@ -55,34 +71,44 @@ impl MatchesIgnoringSource for cargo_metadata::Dependency { &self, krates: &Krates, package_id: cargo_metadata::PackageId, - ) -> bool { - self.name - == krates - .get_package_name_from_cargo_metadata_package_id(&package_id) - .unwrap() - && self.req.matches( - &krates - .get_package_version_from_cargo_metadata_package_id( - &package_id, - ) - .unwrap(), - ) + ) -> Option { + match krates + .get_package_name_and_version_from_cargo_metadata_package_id( + &package_id, + ) { + Some((name, version)) => { + Some(name == self.name && self.req.matches(&version)) + } + _ => { + eprintln!( + "Failed to match (ignoring source) package: {} with version: {}", + self.name, + self.req + ); + None + } + } } } impl ToCargoGeigerDependencyKind for cargo_metadata::DependencyKind { fn to_cargo_geiger_dependency_kind( &self, - ) -> cargo_geiger_serde::DependencyKind { + ) -> Option { match self { - DependencyKind::Build => cargo_geiger_serde::DependencyKind::Build, + DependencyKind::Build => { + Some(cargo_geiger_serde::DependencyKind::Build) + } DependencyKind::Development => { - cargo_geiger_serde::DependencyKind::Development + Some(cargo_geiger_serde::DependencyKind::Development) } DependencyKind::Normal => { - cargo_geiger_serde::DependencyKind::Normal + Some(cargo_geiger_serde::DependencyKind::Normal) + } + _ => { + eprintln!("Unrecognised Dependency Kind"); + None } - _ => panic!("Unrecognised Dependency Kind"), } } } @@ -91,14 +117,21 @@ impl ToCargoGeigerPackageId for cargo_metadata::PackageId { fn to_cargo_geiger_package_id( &self, metadata: &Metadata, - ) -> cargo_geiger_serde::PackageId { - let package = self.to_cargo_metadata_package(metadata).unwrap(); - let metadata_source = self.to_cargo_geiger_source(metadata); - - cargo_geiger_serde::PackageId { - name: package.name, - version: package.version, - source: metadata_source, + ) -> Option { + match self.to_cargo_metadata_package(metadata) { + Some(package) => { + let metadata_source = self.to_cargo_geiger_source(metadata); + + Some(cargo_geiger_serde::PackageId { + name: package.name, + version: package.version, + source: metadata_source, + }) + } + None => { + eprintln!("Failed to convert PackageId: {} to Package", self); + None + } } } } @@ -138,7 +171,7 @@ mod metadata_tests { use super::*; use super::super::{ - GetPackageNameFromCargoMetadataPackageId, ToCargoCoreDepKind, + GetPackageNameAndVersionFromCargoMetadataPackageId, ToCargoCoreDepKind, }; use crate::args::FeaturesArgs; @@ -174,8 +207,9 @@ mod metadata_tests { .unwrap(); let deps_not_replaced = resolve.deps_not_replaced(package.package_id()); - let cargo_metadata_deps_not_replaced = - metadata.deps_not_replaced(cargo_metadata_package_id); + let cargo_metadata_deps_not_replaced = metadata + .deps_not_replaced(cargo_metadata_package_id) + .unwrap(); let mut cargo_core_package_names = deps_not_replaced .map(|(p, _)| p.name().to_string()) @@ -184,9 +218,10 @@ mod metadata_tests { let mut cargo_metadata_package_names = cargo_metadata_deps_not_replaced .iter() .map(|(p, _)| { - krates - .get_package_name_from_cargo_metadata_package_id(p) - .unwrap() + let (name, _) = krates + .get_package_name_and_version_from_cargo_metadata_package_id(p) + .unwrap(); + name }) .collect::>(); @@ -200,7 +235,7 @@ mod metadata_tests { fn get_root_test() { let (_, metadata) = construct_krates_and_metadata(); let package = metadata.root_package().unwrap(); - let package_root = package.get_root(); + let package_root = package.get_root().unwrap(); assert_eq!( package_root, package.manifest_path.parent().unwrap().to_path_buf() @@ -208,14 +243,16 @@ mod metadata_tests { } #[rstest] - fn matches_ignoring_source_test() { + fn matches_ignoring_source() { let (krates, metadata) = construct_krates_and_metadata(); let package = metadata.root_package().unwrap(); let dependency = package.dependencies.clone().pop().unwrap(); assert_eq!( - dependency.matches_ignoring_source(&krates, package.clone().id), + dependency + .matches_ignoring_source(&krates, package.clone().id) + .unwrap(), false ); @@ -231,7 +268,9 @@ mod metadata_tests { .unwrap(); assert!( - dependency.matches_ignoring_source(&krates, dependency_package_id), + dependency + .matches_ignoring_source(&krates, dependency_package_id) + .unwrap(), true ); } @@ -259,8 +298,10 @@ mod metadata_tests { let root_package = metadata.root_package().unwrap(); - let cargo_geiger_package_id = - root_package.id.to_cargo_geiger_package_id(&metadata); + let cargo_geiger_package_id = root_package + .id + .to_cargo_geiger_package_id(&metadata) + .unwrap(); assert_eq!(cargo_geiger_package_id.name, root_package.name); diff --git a/cargo-geiger/src/readme.rs b/cargo-geiger/src/readme.rs index b38ad6ce..4b260fc6 100644 --- a/cargo-geiger/src/readme.rs +++ b/cargo-geiger/src/readme.rs @@ -3,7 +3,7 @@ use crate::args::ReadmeArgs; use cargo::{CliError, CliResult}; use regex::Regex; use std::fs::File; -use std::io::{BufRead, BufReader, Write}; +use std::io::{BufRead, BufReader, Error, Write}; use std::path::PathBuf; /// Name of README FILE @@ -24,25 +24,30 @@ pub fn create_or_replace_section_in_readme( if !readme_path_buf.exists() { eprintln!( - "File {} does not exist. To construct a Cargo Geiger Safety Report section, please first create a README.", + "File: {} does not exist. To construct a Cargo Geiger Safety Report section, please first create a README.", readme_path_buf.to_str().unwrap() ); return CliResult::Err(CliError::code(1)); } let mut readme_content = - BufReader::new(File::open(readme_path_buf.clone()).unwrap()) - .lines() - .map(|l| l.unwrap()) - .collect::>(); + read_file_contents(&readme_path_buf).map_err(|e| { + eprintln!( + "Failed to read contents from file: {}", + readme_path_buf.to_str().unwrap() + ); + anyhow::Error::from(e) + })?; update_readme_content(readme_args, &mut readme_content, scan_output_lines); - let mut readme_file = File::create(readme_path_buf).unwrap(); - - for line in readme_content { - writeln!(readme_file, "{}", line).unwrap(); - } + write_lines_to_file(&readme_content, &readme_path_buf).map_err(|e| { + eprintln!( + "Failed to write lines to file: {}", + readme_path_buf.to_str().unwrap() + ); + anyhow::Error::from(e) + })?; Ok(()) } @@ -111,6 +116,17 @@ fn get_readme_path_buf_from_arguments_or_default( } } +/// Read the contents of a file line by line. +fn read_file_contents(path_buf: &PathBuf) -> Result, Error> { + let file = File::open(path_buf)?; + let buf_reader = BufReader::new(file); + + Ok(buf_reader + .lines() + .filter_map(|l| l.ok()) + .collect::>()) +} + /// Update the content of a README.md with a Scan Result. When the section doesn't exist, it will /// be created with an `h2` level header, otherwise it will preserve the level of the existing /// header @@ -176,6 +192,20 @@ fn update_readme_content( } } +/// Write a Vec line by line to a file, overwriting the current file, if it exists. +fn write_lines_to_file( + lines: &[String], + path_buf: &PathBuf, +) -> Result<(), Error> { + let mut readme_file = File::create(path_buf)?; + + for line in lines { + writeln!(readme_file, "{}", line)? + } + + Ok(()) +} + #[cfg(test)] mod readme_tests { use super::*; diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index d1916782..c839d404 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -193,34 +193,62 @@ fn package_metrics( let mut indices = vec![root_index]; let mut visited = HashSet::new(); - while !indices.is_empty() { - let i = indices.pop().unwrap(); - let package_id = graph.graph[i].clone(); - let mut package = PackageInfo::new( - package_id - .to_cargo_geiger_package_id(cargo_metadata_parameters.metadata), - ); - for edge in graph.graph.edges(i) { - let dep_index = edge.target(); - if visited.insert(dep_index) { - indices.push(dep_index); + while let Some(index) = indices.pop() { + let package_id = graph.graph[index].clone(); + + if let Some(package) = package_id + .to_cargo_geiger_package_id(cargo_metadata_parameters.metadata) + { + let mut package_info = PackageInfo::new(package); + + 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] + ) + } + } } - let dep = graph.graph[dep_index] - .to_cargo_geiger_package_id(cargo_metadata_parameters.metadata); - package.add_dependency( - dep, - edge.weight().to_cargo_geiger_dependency_kind(), - ); - } - match geiger_context.package_id_to_metrics.get(&package_id) { - Some(m) => package_metrics.push((package, Some(m.clone()))), - None => { - eprintln!( - "WARNING: No metrics found for package: {}", - package_id - ); - package_metrics.push((package, None)) + match geiger_context.package_id_to_metrics.get(&package_id) { + Some(m) => { + package_metrics.push((package_info, Some(m.clone()))) + } + None => { + eprintln!( + "WARNING: No metrics found for package: {}", + package_id + ); + package_metrics.push((package_info, None)) + } } } } diff --git a/cargo-geiger/src/scan/find.rs b/cargo-geiger/src/scan/find.rs index 8222f615..83ffa776 100644 --- a/cargo-geiger/src/scan/find.rs +++ b/cargo-geiger/src/scan/find.rs @@ -130,18 +130,22 @@ fn find_rs_files_in_package(package: &cargo_metadata::Package) -> Vec { targets.push(target); } let mut rs_files = Vec::new(); - for path_bufs in find_rs_files_in_dir(package.clone().get_root().as_path()) - { - if !canon_targets.contains_key(&path_bufs) { - rs_files.push(RsFile::Other(path_bufs)); + + if let Some(root_path) = package.clone().get_root() { + for path_bufs in find_rs_files_in_dir(root_path.as_path()) { + if !canon_targets.contains_key(&path_bufs) { + rs_files.push(RsFile::Other(path_bufs)); + } } } + for (path_buf, targets) in canon_targets.into_iter() { for target in targets { let target_kind = into_target_kind(target.clone().kind); rs_files.push(into_rs_code_file(&target_kind, path_buf.clone())); } } + rs_files } diff --git a/cargo-geiger/tests/snapshots/integration_tests__test2_package_with_shallow_deps.stdout.snap b/cargo-geiger/tests/snapshots/integration_tests__test2_package_with_shallow_deps.stdout.snap index caf1ee2a..ff0e3b24 100644 --- a/cargo-geiger/tests/snapshots/integration_tests__test2_package_with_shallow_deps.stdout.snap +++ b/cargo-geiger/tests/snapshots/integration_tests__test2_package_with_shallow_deps.stdout.snap @@ -1,5 +1,5 @@ --- -source: cargo-geiger/tests/mod.rs +source: cargo-geiger/tests/integration_tests.rs expression: stdout ---