Skip to content

Commit

Permalink
ISSUE-16 - Remove usage of ManifestMetadata:
Browse files Browse the repository at this point in the history
* Add functions to extract licence and repository from
  `cargo_metadata::PackageId`
* Remove `ManifestMetadata` from display
* Add testing

Signed-off-by: joshmc <[email protected]>
  • Loading branch information
jmcconnell26 committed Nov 16, 2020
1 parent 24a783d commit 486010e
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 78 deletions.
45 changes: 20 additions & 25 deletions cargo-geiger/src/format/display.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,32 @@
use crate::format::pattern::Pattern;
use crate::format::Chunk;

use crate::mapping::{
CargoMetadataParameters, GetPackageNameFromCargoMetadataPackageId,
CargoMetadataParameters, GetLicenceFromCargoMetadataPackageId,
GetPackageNameFromCargoMetadataPackageId,
GetPackageVersionFromCargoMetadataPackageId,
GetRepositoryFromCargoMetadataPackageId,
};
use cargo::core::manifest::ManifestMetadata;

use std::fmt;

pub struct Display<'a> {
pub cargo_metadata_parameters: &'a CargoMetadataParameters<'a>,
pub pattern: &'a Pattern,
pub package: &'a cargo_metadata::PackageId,
pub metadata: &'a ManifestMetadata,
}

impl<'a> fmt::Display for Display<'a> {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
for chunk in &self.pattern.0 {
match *chunk {
Chunk::License => {
if let Some(ref license) = self.metadata.license {
if let Some(ref license) = self
.cargo_metadata_parameters
.krates
.get_licence_from_cargo_metadata_package_id(
self.package,
)
{
(write!(fmt, "{}", license))?
}
}
Expand All @@ -42,7 +48,13 @@ impl<'a> fmt::Display for Display<'a> {
}
Chunk::Raw(ref s) => (fmt.write_str(s))?,
Chunk::Repository => {
if let Some(ref repository) = self.metadata.repository {
if let Some(ref repository) = self
.cargo_metadata_parameters
.krates
.get_repository_from_cargo_metadata_package_id(
self.package,
)
{
(write!(fmt, "{}", repository))?
}
}
Expand All @@ -59,7 +71,6 @@ pub mod display_tests {
use crate::format::pattern::Pattern;
use crate::format::Chunk;

use cargo::core::manifest::ManifestMetadata;
use cargo_metadata::{CargoOpt, MetadataCommand};
use krates::Builder as KratesBuilder;
use rstest::*;
Expand All @@ -69,7 +80,7 @@ pub mod display_tests {
expected_formatted_string,
case(
Pattern(vec![Chunk::License]),
"licence_string"
"Apache-2.0/MIT"
),
case(
Pattern(vec![Chunk::Package]),
Expand All @@ -81,7 +92,7 @@ pub mod display_tests {
),
case(
Pattern(vec![Chunk::Repository]),
"repository_string"
"https://github.com/rust-secure-code/cargo-geiger"
)
)]
fn display_format_fmt_test(
Expand All @@ -100,29 +111,13 @@ pub mod display_tests {

let package_id = metadata.root_package().unwrap().id.clone();

let manifest_metadata = ManifestMetadata {
authors: vec![],
keywords: vec![],
categories: vec![],
license: Some(String::from("licence_string")),
license_file: None,
description: None,
readme: None,
homepage: None,
repository: Some(String::from("repository_string")),
documentation: None,
badges: Default::default(),
links: None,
};

let display = Display {
cargo_metadata_parameters: &CargoMetadataParameters {
krates: &krates,
metadata: &metadata,
},
pattern: &input_pattern,
package: &package_id,
metadata: &manifest_metadata,
};

assert_eq!(format!("{}", display), expected_formatted_string);
Expand Down
3 changes: 0 additions & 3 deletions cargo-geiger/src/format/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::format::{Chunk, RawChunk};
use super::display::Display;

use crate::mapping::CargoMetadataParameters;
use cargo::core::manifest::ManifestMetadata;
use std::error::Error;

#[derive(Debug, PartialEq)]
Expand All @@ -14,14 +13,12 @@ impl Pattern {
pub fn display<'a>(
&'a self,
cargo_metadata_parameters: &'a CargoMetadataParameters,
metadata: &'a ManifestMetadata,
package: &'a cargo_metadata::PackageId,
) -> Display<'a> {
Display {
cargo_metadata_parameters,
pattern: self,
package,
metadata,
}
}

Expand Down
5 changes: 1 addition & 4 deletions cargo-geiger/src/format/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod total_package_counts;
use crate::format::emoji_symbols::EmojiSymbols;
use crate::format::print_config::{colorize, PrintConfig};
use crate::format::CrateDetectionStatus;
use crate::mapping::CargoMetadataParameters;
use crate::scan::GeigerContext;
use crate::tree::TextTreeLine;

Expand All @@ -13,8 +14,6 @@ use handle_text_tree_line::{
};
use total_package_counts::TotalPackageCounts;

use crate::mapping::CargoMetadataParameters;
use cargo::core::package::PackageSet;
use cargo_geiger_serde::{Count, CounterBlock};
use std::collections::HashSet;
use std::path::PathBuf;
Expand All @@ -33,7 +32,6 @@ pub const UNSAFE_COUNTERS_HEADER: [&str; 6] = [

pub fn create_table_from_text_tree_lines(
cargo_metadata_parameters: &CargoMetadataParameters,
package_set: &PackageSet,
table_parameters: &TableParameters,
text_tree_lines: Vec<TextTreeLine>,
) -> (Vec<String>, u64) {
Expand Down Expand Up @@ -67,7 +65,6 @@ pub fn create_table_from_text_tree_lines(
&emoji_symbols,
&mut handle_package_parameters,
package_id,
package_set,
&mut table_lines,
table_parameters,
tree_vines,
Expand Down
17 changes: 5 additions & 12 deletions cargo-geiger/src/format/table/handle_text_tree_line.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use crate::format::emoji_symbols::EmojiSymbols;
use crate::format::print_config::colorize;
use crate::format::{get_kind_group_name, CrateDetectionStatus, SymbolKind};
use crate::mapping::{CargoMetadataParameters, ToPackage};
use crate::mapping::CargoMetadataParameters;
use crate::scan::unsafe_stats;

use super::total_package_counts::TotalPackageCounts;
use super::TableParameters;
use super::{table_row, table_row_empty};

use cargo::core::dependency::DepKind;
use cargo::core::package::PackageSet;
use std::collections::HashSet;

pub struct HandlePackageParameters<'a> {
Expand All @@ -33,13 +32,11 @@ pub fn handle_text_tree_line_extra_deps_group(
table_lines.push(format!("{}{}{}", table_row_empty(), tree_vines, name));
}

#[allow(clippy::too_many_arguments)]
pub fn handle_text_tree_line_package(
cargo_metadata_parameters: &CargoMetadataParameters,
emoji_symbols: &EmojiSymbols,
handle_package_parameters: &mut HandlePackageParameters,
package_id: cargo_metadata::PackageId,
package_set: &PackageSet,
table_lines: &mut Vec<String>,
table_parameters: &TableParameters,
tree_vines: String,
Expand All @@ -48,9 +45,6 @@ pub fn handle_text_tree_line_package(
.visited_package_ids
.insert(package_id.clone());

let package =
package_id.to_package(cargo_metadata_parameters.krates, package_set);

let package_metrics = match table_parameters
.geiger_context
.package_id_to_metrics
Expand Down Expand Up @@ -99,11 +93,10 @@ pub fn handle_text_tree_line_package(
let package_name = colorize(
format!(
"{}",
table_parameters.print_config.format.display(
cargo_metadata_parameters,
package.manifest().metadata(),
&package_id
)
table_parameters
.print_config
.format
.display(cargo_metadata_parameters, &package_id)
),
&crate_detection_status,
);
Expand Down
15 changes: 10 additions & 5 deletions cargo-geiger/src/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ mod metadata;

use ::krates::Krates;
use cargo::core::dependency::DepKind;
use cargo::core::manifest::ManifestMetadata;
use cargo::core::{Package, PackageId, PackageSet, Resolve};
use cargo_metadata::{DependencyKind, Metadata};
use std::collections::HashSet;
Expand All @@ -28,12 +27,11 @@ pub trait DepsNotReplaced {
)>;
}

pub trait GetManifestMetadataFromCargoMetadataPackageId {
fn get_manifest_metadata_from_cargo_metadata_package_id(
pub trait GetLicenceFromCargoMetadataPackageId {
fn get_licence_from_cargo_metadata_package_id(
&self,
package_id: &cargo_metadata::PackageId,
package_set: &PackageSet,
) -> ManifestMetadata;
) -> Option<String>;
}

pub trait GetPackageNameFromCargoMetadataPackageId {
Expand All @@ -50,6 +48,13 @@ pub trait GetPackageVersionFromCargoMetadataPackageId {
) -> cargo_metadata::Version;
}

pub trait GetRepositoryFromCargoMetadataPackageId {
fn get_repository_from_cargo_metadata_package_id(
&self,
package_id: &cargo_metadata::PackageId,
) -> Option<String>;
}

pub trait GetRoot {
fn get_root(&self) -> PathBuf;
}
Expand Down
53 changes: 43 additions & 10 deletions cargo-geiger/src/mapping/krates.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
use crate::mapping::{
GetManifestMetadataFromCargoMetadataPackageId,
GetLicenceFromCargoMetadataPackageId,
GetPackageNameFromCargoMetadataPackageId,
GetPackageVersionFromCargoMetadataPackageId, ToPackage,
GetPackageVersionFromCargoMetadataPackageId,
GetRepositoryFromCargoMetadataPackageId,
};

use cargo::core::manifest::ManifestMetadata;
use cargo::core::PackageSet;
use krates::Krates;

impl GetManifestMetadataFromCargoMetadataPackageId for Krates {
fn get_manifest_metadata_from_cargo_metadata_package_id(
impl GetLicenceFromCargoMetadataPackageId for Krates {
fn get_licence_from_cargo_metadata_package_id(
&self,
package_id: &cargo_metadata::PackageId,
package_set: &PackageSet,
) -> ManifestMetadata {
let package = package_id.to_package(self, package_set);
package.manifest().metadata().clone()
) -> Option<String> {
let package = self.node_for_kid(package_id);
package.unwrap().krate.clone().license
}
}

Expand All @@ -39,6 +37,16 @@ impl GetPackageVersionFromCargoMetadataPackageId for Krates {
}
}

impl GetRepositoryFromCargoMetadataPackageId for Krates {
fn get_repository_from_cargo_metadata_package_id(
&self,
package_id: &cargo_metadata::PackageId,
) -> Option<String> {
let package = self.node_for_kid(package_id);
package.unwrap().krate.clone().repository
}
}

#[cfg(test)]
mod krates_tests {
use super::*;
Expand All @@ -47,6 +55,17 @@ mod krates_tests {
use krates::Builder as KratesBuilder;
use rstest::*;

#[rstest]
fn get_licence_from_cargo_metadata_package_id_test() {
let (krates, metadata) = construct_krates_and_metadata();
let package = metadata.root_package().unwrap();
let licence_option =
krates.get_licence_from_cargo_metadata_package_id(&package.id);
assert!(licence_option.is_some());
let licence = licence_option.unwrap();
assert_eq!(licence, String::from("Apache-2.0/MIT"))
}

#[rstest]
fn get_package_name_from_cargo_metadata_package_id_test() {
let (krates, metadata) = construct_krates_and_metadata();
Expand All @@ -65,6 +84,20 @@ mod krates_tests {
assert_eq!(package_version, package.version);
}

#[rstest]
fn get_repository_from_cargo_metadata_package_id_test() {
let (krates, metadata) = construct_krates_and_metadata();
let package = metadata.root_package().unwrap();
let repository_option =
krates.get_repository_from_cargo_metadata_package_id(&package.id);
assert!(repository_option.is_some());
let repository = repository_option.unwrap();
assert_eq!(
repository,
String::from("https://github.com/rust-secure-code/cargo-geiger")
);
}

fn construct_krates_and_metadata() -> (Krates, Metadata) {
let metadata = MetadataCommand::new()
.manifest_path("./Cargo.toml")
Expand Down
1 change: 0 additions & 1 deletion cargo-geiger/src/scan/default/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ pub fn scan_to_table(
let (mut table_lines, mut warning_count) =
create_table_from_text_tree_lines(
cargo_metadata_parameters,
package_set,
&table_parameters,
text_tree_lines,
);
Expand Down
Loading

0 comments on commit 486010e

Please sign in to comment.