From 4479d0fdff1ab65cf677510992bf0b4f17ef277b Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 5 Oct 2020 13:24:22 -0700 Subject: [PATCH 1/7] Move serialized types to their own crate For #117 --- Cargo.lock | 9 +- cargo-geiger-serde/Cargo.toml | 5 +- cargo-geiger-serde/src/lib.rs | 26 +++-- cargo-geiger-serde/src/package_id.rs | 14 +++ cargo-geiger-serde/src/report.rs | 166 +++++++++++++++++++++++++++ cargo-geiger-serde/src/source.rs | 18 +++ cargo-geiger/Cargo.toml | 1 + cargo-geiger/src/format/table.rs | 2 +- cargo-geiger/src/scan.rs | 48 ++++++-- cargo-geiger/src/scan/default.rs | 2 +- cargo-geiger/src/scan/forbid.rs | 2 +- cargo-geiger/src/scan/report.rs | 63 ---------- geiger/Cargo.toml | 2 +- geiger/src/lib.rs | 73 +----------- 14 files changed, 274 insertions(+), 157 deletions(-) create mode 100644 cargo-geiger-serde/src/package_id.rs create mode 100644 cargo-geiger-serde/src/report.rs create mode 100644 cargo-geiger-serde/src/source.rs delete mode 100644 cargo-geiger/src/scan/report.rs diff --git a/Cargo.lock b/Cargo.lock index efbed76b..176ddce1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -192,6 +192,7 @@ dependencies = [ "assert_cmd", "better-panic", "cargo", + "cargo-geiger-serde", "cargo-platform", "colored", "console 0.11.3", @@ -213,6 +214,11 @@ dependencies = [ [[package]] name = "cargo-geiger-serde" version = "0.1.0" +dependencies = [ + "semver 0.10.0", + "serde", + "url", +] [[package]] name = "cargo-platform" @@ -523,8 +529,8 @@ dependencies = [ name = "geiger" version = "0.4.5" dependencies = [ + "cargo-geiger-serde", "proc-macro2", - "serde", "syn", ] @@ -1448,6 +1454,7 @@ dependencies = [ "idna", "matches", "percent-encoding", + "serde", ] [[package]] diff --git a/cargo-geiger-serde/Cargo.toml b/cargo-geiger-serde/Cargo.toml index 95c5fe4e..d5329c38 100644 --- a/cargo-geiger-serde/Cargo.toml +++ b/cargo-geiger-serde/Cargo.toml @@ -6,6 +6,7 @@ license = "Apache-2.0/MIT" name = "cargo-geiger-serde" version = "0.1.0" -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - [dependencies] +semver = "0.10.0" +serde = { version = "1.0.116", features = ["derive"] } +url = { version = "2.1.1", features = ["serde"] } diff --git a/cargo-geiger-serde/src/lib.rs b/cargo-geiger-serde/src/lib.rs index 31e1bb20..c9d29867 100644 --- a/cargo-geiger-serde/src/lib.rs +++ b/cargo-geiger-serde/src/lib.rs @@ -1,7 +1,19 @@ -#[cfg(test)] -mod tests { - #[test] - fn it_works() { - assert_eq!(2 + 2, 4); - } -} +//! cargo-geiger-serde ☢ +//! ======== +//! +//! This crate provides definitions to serialize the unsafety report. + +#![forbid(unsafe_code)] +#![forbid(warnings)] + +mod package_id; +mod report; +mod source; + +pub use package_id::PackageId; +pub use report::{ + Count, CounterBlock, DependencyKind, PackageInfo, QuickReportEntry, QuickSafetyReport, + ReportEntry, SafetyReport, UnsafeInfo, +}; +pub use source::Source; + diff --git a/cargo-geiger-serde/src/package_id.rs b/cargo-geiger-serde/src/package_id.rs new file mode 100644 index 00000000..9a4ef0c3 --- /dev/null +++ b/cargo-geiger-serde/src/package_id.rs @@ -0,0 +1,14 @@ +use crate::Source; +use semver::Version; +use serde::{Deserialize, Serialize}; + +/// Identifies a package in the dependency tree +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct PackageId { + /// Package name + pub name: String, + /// Package version + pub version: Version, + /// Package source (e.g. repository, crate registry) + pub source: Source, +} diff --git a/cargo-geiger-serde/src/report.rs b/cargo-geiger-serde/src/report.rs new file mode 100644 index 00000000..5c4c84ad --- /dev/null +++ b/cargo-geiger-serde/src/report.rs @@ -0,0 +1,166 @@ +use crate::PackageId; +use serde::{Deserialize, Serialize}; +use std::{ + ops::{Add, AddAssign}, + path::PathBuf, +}; + +/// Package dependency information +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct PackageInfo { + pub id: PackageId, + pub dependencies: Vec, + pub dev_dependencies: Vec, + pub build_dependencies: Vec, +} + +impl PackageInfo { + pub fn new(id: PackageId) -> Self { + PackageInfo { + id, + dependencies: Vec::new(), + dev_dependencies: Vec::new(), + build_dependencies: Vec::new(), + } + } + + pub fn push_dependency(&mut self, dep: PackageId, kind: DependencyKind) { + match kind { + DependencyKind::Normal => self.dependencies.push(dep), + DependencyKind::Development => self.dev_dependencies.push(dep), + DependencyKind::Build => self.build_dependencies.push(dep), + } + } +} + +/// Entry of the report generated from scanning for packages that forbid the use of `unsafe` +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct QuickReportEntry { + pub package: PackageInfo, + /// Whether this package forbids the use of `unsafe` + pub forbids_unsafe: bool, +} + +/// Report generated from scanning for packages that forbid the use of `unsafe` +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] +pub struct QuickSafetyReport { + /// Packages that were scanned successfully + pub packages: Vec, + /// Packages that were not scanned successfully + pub packages_without_metrics: Vec, +} + +/// Entry of the report generated from scanning for the use of `unsafe` +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct ReportEntry { + pub package: PackageInfo, + /// Unsafety scan results + pub unsafety: UnsafeInfo, +} + +/// Report generated from scanning for the use of `unsafe` +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] +pub struct SafetyReport { + pub packages: Vec, + pub packages_without_metrics: Vec, + pub used_but_not_scanned_files: Vec, +} + +/// Unsafety usage in a package +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] +pub struct UnsafeInfo { + /// Unsafe usage statistics for code used by the project + pub used: CounterBlock, + /// Unsafe usage statistics for code not used by the project + pub unused: CounterBlock, + /// Whether this package forbids the use of `unsafe` + pub forbids_unsafe: bool, +} + +/// Kind of dependency for a package +#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +pub enum DependencyKind { + /// Dependency in the `[dependencies]` section of `Cargo.toml` + Normal, + /// Dependency in the `[dev-dependencies]` section of `Cargo.toml` + Development, + /// Dependency in the `[build-dependencies]` section of `Cargo.toml` + Build, +} + +/// Statistics about the use of `unsafe` +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] +pub struct Count { + /// Number of safe items + pub safe: u64, + /// Number of unsafe items + pub unsafe_: u64, +} + +impl Count { + /// Increments the safe or unsafe counter by 1 + pub fn count(&mut self, is_unsafe: bool) { + if is_unsafe { + self.unsafe_ += 1; + } else { + self.safe += 1; + } + } +} + +impl Add for Count { + type Output = Count; + + fn add(self, other: Count) -> Count { + Count { + safe: self.safe + other.safe, + unsafe_: self.unsafe_ + other.unsafe_, + } + } +} + +impl AddAssign for Count { + fn add_assign(&mut self, rhs: Count) { + *self = self.clone() + rhs; + } +} + +/// Unsafe usage metrics collection. +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] +pub struct CounterBlock { + pub functions: Count, + pub exprs: Count, + pub item_impls: Count, + pub item_traits: Count, + pub methods: Count, +} + +impl CounterBlock { + pub fn has_unsafe(&self) -> bool { + self.functions.unsafe_ > 0 + || self.exprs.unsafe_ > 0 + || self.item_impls.unsafe_ > 0 + || self.item_traits.unsafe_ > 0 + || self.methods.unsafe_ > 0 + } +} + +impl Add for CounterBlock { + type Output = CounterBlock; + + fn add(self, other: CounterBlock) -> CounterBlock { + CounterBlock { + functions: self.functions + other.functions, + exprs: self.exprs + other.exprs, + item_impls: self.item_impls + other.item_impls, + item_traits: self.item_traits + other.item_traits, + methods: self.methods + other.methods, + } + } +} + +impl AddAssign for CounterBlock { + fn add_assign(&mut self, rhs: Self) { + *self = self.clone() + rhs; + } +} diff --git a/cargo-geiger-serde/src/source.rs b/cargo-geiger-serde/src/source.rs new file mode 100644 index 00000000..ccbf8381 --- /dev/null +++ b/cargo-geiger-serde/src/source.rs @@ -0,0 +1,18 @@ +use serde::{Deserialize, Serialize}; +use url::Url; + +/// Source of a package (where it is fetched from) +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub enum Source { + Git { + url: Url, + rev: String, + }, + Registry { + name: String, + url: Url, + }, + Path(Url), + LocalRegistry(Url), + Directory(Url), +} diff --git a/cargo-geiger/Cargo.toml b/cargo-geiger/Cargo.toml index 68f9a171..b39e3bf1 100644 --- a/cargo-geiger/Cargo.toml +++ b/cargo-geiger/Cargo.toml @@ -15,6 +15,7 @@ maintenance = { status = "experimental" } [dependencies] cargo = "0.47.0" +cargo-geiger-serde = { path = "../cargo-geiger-serde", version = "0.1.0" } cargo-platform = "0.1.1" colored = "2.0.0" console = "0.11.3" diff --git a/cargo-geiger/src/format/table.rs b/cargo-geiger/src/format/table.rs index b9109fb5..0c43518a 100644 --- a/cargo-geiger/src/format/table.rs +++ b/cargo-geiger/src/format/table.rs @@ -5,7 +5,7 @@ use crate::scan::{unsafe_stats, GeigerContext}; use crate::tree::TextTreeLine; use cargo::core::package::PackageSet; -use geiger::{Count, CounterBlock}; +use cargo_geiger_serde::{Count, CounterBlock}; use std::collections::HashSet; use std::path::PathBuf; diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index 99432e13..8a65a5d7 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -1,7 +1,6 @@ mod default; mod find; mod forbid; -mod report; use crate::args::Args; use crate::format::print::PrintConfig; @@ -10,11 +9,11 @@ use crate::rs_file::RsFileMetricsWrapper; use default::scan_unsafe; use forbid::scan_forbid_unsafe; -use report::{PackageInfo, UnsafeInfo}; use cargo::core::{PackageId, PackageSet, Workspace}; +use cargo::core::dependency::DepKind; use cargo::{CliResult, Config}; -use geiger::CounterBlock; +use cargo_geiger_serde::{CounterBlock, DependencyKind, PackageInfo, UnsafeInfo}; use petgraph::visit::EdgeRef; use std::collections::{HashMap, HashSet}; use std::path::PathBuf; @@ -165,14 +164,14 @@ fn package_metrics<'a>( std::iter::from_fn(move || { let i = indices.pop()?; let id = graph.graph[i].id; - let mut package = PackageInfo::new(id); + let mut package = PackageInfo::new(from_cargo_package_id(id)); for edge in graph.graph.edges(i) { let dep_index = edge.target(); if visited.insert(dep_index) { indices.push(dep_index); } - let dep = graph.graph[dep_index].id; - package.push_dependency(dep, *edge.weight()); + let dep = from_cargo_package_id(graph.graph[dep_index].id); + package.push_dependency(dep, from_cargo_dependency_kind(*edge.weight())); } match geiger_context.pack_id_to_metrics.get(&id) { Some(m) => Some((package, Some(m))), @@ -184,16 +183,49 @@ fn package_metrics<'a>( }) } +fn from_cargo_package_id(id: PackageId) -> cargo_geiger_serde::PackageId { + let source = id.source_id(); + let source_url = source.url().clone(); + let source = if source.is_git() { + cargo_geiger_serde::Source::Git { + url: source_url, + rev: source.precise().expect("Git revision should be known").to_string(), + } + } else if source.is_path() { + cargo_geiger_serde::Source::Path(source_url) + } else if source.is_registry() { + cargo_geiger_serde::Source::Registry { + name: source.display_registry_name(), + url: source_url, + } + } else { + panic!("Unsupported source type: {:?}", source) + }; + cargo_geiger_serde::PackageId { + name: id.name().to_string(), + version: id.version().clone(), + source, + } +} + +fn from_cargo_dependency_kind(kind: DepKind) -> DependencyKind { + match kind { + DepKind::Normal => DependencyKind::Normal, + DepKind::Development => DependencyKind::Development, + DepKind::Build => DependencyKind::Build, + } +} + #[cfg(test)] mod scan_tests { use super::*; use crate::{ rs_file::RsFileMetricsWrapper, - scan::{report::UnsafeInfo, PackageMetrics}, + scan::PackageMetrics, }; - use geiger::Count; + use cargo_geiger_serde::{Count, UnsafeInfo}; use std::{collections::HashSet, path::PathBuf}; #[test] diff --git a/cargo-geiger/src/scan/default.rs b/cargo-geiger/src/scan/default.rs index 2c05ae77..9455e4ba 100644 --- a/cargo-geiger/src/scan/default.rs +++ b/cargo-geiger/src/scan/default.rs @@ -6,7 +6,6 @@ use crate::graph::Graph; use crate::rs_file::resolve_rs_file_deps; use super::find::find_unsafe; -use super::report::{ReportEntry, SafetyReport}; use super::{ list_files_used_but_not_scanned, package_metrics, unsafe_stats, ScanDetails, ScanMode, ScanParameters, @@ -18,6 +17,7 @@ use cargo::core::compiler::CompileMode; use cargo::core::{PackageId, PackageSet, Workspace}; use cargo::ops::CompileOptions; use cargo::{CliError, CliResult, Config}; +use cargo_geiger_serde::{ReportEntry, SafetyReport}; pub fn scan_unsafe( workspace: &Workspace, diff --git a/cargo-geiger/src/scan/forbid.rs b/cargo-geiger/src/scan/forbid.rs index 35eca467..cedde296 100644 --- a/cargo-geiger/src/scan/forbid.rs +++ b/cargo-geiger/src/scan/forbid.rs @@ -4,13 +4,13 @@ use crate::format::print::{OutputFormat, PrintConfig}; use crate::graph::Graph; use super::find::find_unsafe; -use super::report::{QuickReportEntry, QuickSafetyReport}; use super::{package_metrics, ScanMode, ScanParameters}; use table::scan_forbid_to_table; use cargo::core::{PackageId, PackageSet}; use cargo::{CliResult, Config}; +use cargo_geiger_serde::{QuickReportEntry, QuickSafetyReport}; pub fn scan_forbid_unsafe( package_set: &PackageSet, diff --git a/cargo-geiger/src/scan/report.rs b/cargo-geiger/src/scan/report.rs deleted file mode 100644 index be0ed5bb..00000000 --- a/cargo-geiger/src/scan/report.rs +++ /dev/null @@ -1,63 +0,0 @@ -use cargo::core::{dependency::DepKind, PackageId}; -use geiger::CounterBlock; -use serde::{Deserialize, Serialize}; -use std::path::PathBuf; - -#[derive(Clone, Debug, Deserialize, Serialize)] -pub struct PackageInfo { - pub id: PackageId, - pub dependencies: Vec, - pub dev_dependencies: Vec, - pub build_dependencies: Vec, -} - -impl PackageInfo { - pub fn new(id: PackageId) -> Self { - PackageInfo { - id, - dependencies: Vec::new(), - dev_dependencies: Vec::new(), - build_dependencies: Vec::new(), - } - } - - pub fn push_dependency(&mut self, dep: PackageId, kind: DepKind) { - match kind { - DepKind::Normal => self.dependencies.push(dep), - DepKind::Development => self.dev_dependencies.push(dep), - DepKind::Build => self.build_dependencies.push(dep), - } - } -} - -#[derive(Clone, Debug, Deserialize, Serialize)] -pub struct QuickReportEntry { - pub package: PackageInfo, - pub forbids_unsafe: bool, -} - -#[derive(Clone, Debug, Default, Deserialize, Serialize)] -pub struct QuickSafetyReport { - pub packages: Vec, - pub packages_without_metrics: Vec, -} - -#[derive(Clone, Debug, Deserialize, Serialize)] -pub struct ReportEntry { - pub package: PackageInfo, - pub unsafety: UnsafeInfo, -} - -#[derive(Clone, Debug, Default, Deserialize, Serialize)] -pub struct SafetyReport { - pub packages: Vec, - pub packages_without_metrics: Vec, - pub used_but_not_scanned_files: Vec, -} - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct UnsafeInfo { - pub used: CounterBlock, - pub unused: CounterBlock, - pub forbids_unsafe: bool, -} diff --git a/geiger/Cargo.toml b/geiger/Cargo.toml index 0bccb401..5bf5f8fd 100644 --- a/geiger/Cargo.toml +++ b/geiger/Cargo.toml @@ -14,6 +14,6 @@ license = "Apache-2.0/MIT" maintenance = { status = "experimental" } [dependencies] -serde = { version = "1.0.116", features = ["derive"] } +cargo-geiger-serde = { path = "../cargo-geiger-serde", version = "0.1.0" } syn = { version = "1.0.34", features = ["parsing", "printing", "clone-impls", "full", "extra-traits", "visit"] } proc-macro2 = "1.0.18" diff --git a/geiger/src/lib.rs b/geiger/src/lib.rs index 5cf6e7a4..46ec9c86 100644 --- a/geiger/src/lib.rs +++ b/geiger/src/lib.rs @@ -7,13 +7,12 @@ #![forbid(unsafe_code)] #![forbid(warnings)] -use serde::{Deserialize, Serialize}; +use cargo_geiger_serde::CounterBlock; use std::error::Error; use std::fmt; use std::fs::File; use std::io; use std::io::Read; -use std::ops::{Add, AddAssign}; use std::path::Path; use std::path::PathBuf; use std::string::FromUtf8Error; @@ -36,76 +35,6 @@ impl fmt::Display for ScanFileError { } } -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct Count { - /// Number of safe items - pub safe: u64, - - /// Number of unsafe items - pub unsafe_: u64, -} - -impl Count { - fn count(&mut self, is_unsafe: bool) { - if is_unsafe { - self.unsafe_ += 1; - } else { - self.safe += 1; - } - } -} - -impl Add for Count { - type Output = Count; - - fn add(self, other: Count) -> Count { - Count { - safe: self.safe + other.safe, - unsafe_: self.unsafe_ + other.unsafe_, - } - } -} - -/// Unsafe usage metrics collection. -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct CounterBlock { - pub functions: Count, - pub exprs: Count, - pub item_impls: Count, - pub item_traits: Count, - pub methods: Count, -} - -impl CounterBlock { - pub fn has_unsafe(&self) -> bool { - self.functions.unsafe_ > 0 - || self.exprs.unsafe_ > 0 - || self.item_impls.unsafe_ > 0 - || self.item_traits.unsafe_ > 0 - || self.methods.unsafe_ > 0 - } -} - -impl Add for CounterBlock { - type Output = CounterBlock; - - fn add(self, other: CounterBlock) -> CounterBlock { - CounterBlock { - functions: self.functions + other.functions, - exprs: self.exprs + other.exprs, - item_impls: self.item_impls + other.item_impls, - item_traits: self.item_traits + other.item_traits, - methods: self.methods + other.methods, - } - } -} - -impl AddAssign for CounterBlock { - fn add_assign(&mut self, rhs: Self) { - *self = self.clone() + rhs; - } -} - /// Scan result for a single `.rs` file. #[derive(Debug, Default)] pub struct RsFileMetrics { From 68cbc0f3c7f12c29a4c0c8f4819811d146caed08 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Wed, 14 Oct 2020 23:50:16 -0700 Subject: [PATCH 2/7] Fix test4 package name --- .../r#mod__test4_workspace_with_top_level_package.stdout.snap | 2 +- test_crates/test4_workspace_with_top_level_package/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cargo-geiger/tests/snapshots/r#mod__test4_workspace_with_top_level_package.stdout.snap b/cargo-geiger/tests/snapshots/r#mod__test4_workspace_with_top_level_package.stdout.snap index 956b8d32..4f9b2236 100644 --- a/cargo-geiger/tests/snapshots/r#mod__test4_workspace_with_top_level_package.stdout.snap +++ b/cargo-geiger/tests/snapshots/r#mod__test4_workspace_with_top_level_package.stdout.snap @@ -14,7 +14,7 @@ Symbols: Functions Expressions Impls Traits Methods Dependency -0/0 0/1 0/0 0/0 0/0 ? test4_workspace_with_virtual_manifest 0.1.0 +0/0 0/1 0/0 0/0 0/0 ? test4_workspace_with_top_level_package 0.1.0 1/1 2/2 0/0 0/0 0/0 ! `-- test1_package_with_no_deps 0.1.0 1/1 2/3 0/0 0/0 0/0 diff --git a/test_crates/test4_workspace_with_top_level_package/Cargo.toml b/test_crates/test4_workspace_with_top_level_package/Cargo.toml index f3d6dbb2..0ac6e97f 100644 --- a/test_crates/test4_workspace_with_top_level_package/Cargo.toml +++ b/test_crates/test4_workspace_with_top_level_package/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "test4_workspace_with_virtual_manifest" +name = "test4_workspace_with_top_level_package" version = "0.1.0" edition = "2018" From 2434f48d7beebf7d14b1440bd8ddd45abb9c3687 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Thu, 15 Oct 2020 08:33:36 -0700 Subject: [PATCH 3/7] Refactor types and add tests - Maps and sets are used to enfore unicity of package IDs and paths. - Serialization is made deterministic by ordering sequences. This allows diff'ing outputs. - Test full and quick reports with existing crate data set. - Copy test data to allow tests to run concurrently (otherwise there would be IO errors, e.g. about file deletion). --- Cargo.lock | 10 + cargo-geiger-serde/src/package_id.rs | 2 +- cargo-geiger-serde/src/report.rs | 144 ++++- cargo-geiger-serde/src/source.rs | 2 +- cargo-geiger/Cargo.toml | 4 + cargo-geiger/src/scan.rs | 2 +- cargo-geiger/src/scan/default.rs | 6 +- cargo-geiger/src/scan/forbid.rs | 4 +- cargo-geiger/tests/mod.rs | 788 ++++++++++++++++++++++++++- 9 files changed, 921 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 176ddce1..f6f120b7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -197,6 +197,7 @@ dependencies = [ "colored", "console 0.11.3", "env_logger", + "fs_extra", "geiger", "insta", "petgraph", @@ -204,10 +205,13 @@ dependencies = [ "rand", "regex", "rstest", + "semver 0.10.0", "serde", "serde_json", "strum", "strum_macros", + "tempfile", + "url", "walkdir", ] @@ -515,6 +519,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" +[[package]] +name = "fs_extra" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2022715d62ab30faffd124d40b76f4134a550a87792276512b18d63272333394" + [[package]] name = "fwdansi" version = "1.1.0" diff --git a/cargo-geiger-serde/src/package_id.rs b/cargo-geiger-serde/src/package_id.rs index 9a4ef0c3..d79e5558 100644 --- a/cargo-geiger-serde/src/package_id.rs +++ b/cargo-geiger-serde/src/package_id.rs @@ -3,7 +3,7 @@ use semver::Version; use serde::{Deserialize, Serialize}; /// Identifies a package in the dependency tree -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct PackageId { /// Package name pub name: String, diff --git a/cargo-geiger-serde/src/report.rs b/cargo-geiger-serde/src/report.rs index 5c4c84ad..70ddb1a0 100644 --- a/cargo-geiger-serde/src/report.rs +++ b/cargo-geiger-serde/src/report.rs @@ -1,6 +1,7 @@ use crate::PackageId; use serde::{Deserialize, Serialize}; use std::{ + collections::{HashMap, HashSet}, ops::{Add, AddAssign}, path::PathBuf, }; @@ -9,27 +10,30 @@ use std::{ #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct PackageInfo { pub id: PackageId, - pub dependencies: Vec, - pub dev_dependencies: Vec, - pub build_dependencies: Vec, + #[serde(serialize_with = "set_serde::serialize")] + pub dependencies: HashSet, + #[serde(serialize_with = "set_serde::serialize")] + pub dev_dependencies: HashSet, + #[serde(serialize_with = "set_serde::serialize")] + pub build_dependencies: HashSet, } impl PackageInfo { pub fn new(id: PackageId) -> Self { PackageInfo { id, - dependencies: Vec::new(), - dev_dependencies: Vec::new(), - build_dependencies: Vec::new(), + dependencies: Default::default(), + dev_dependencies: Default::default(), + build_dependencies: Default::default(), } } - pub fn push_dependency(&mut self, dep: PackageId, kind: DependencyKind) { + pub fn add_dependency(&mut self, dep: PackageId, kind: DependencyKind) { match kind { - DependencyKind::Normal => self.dependencies.push(dep), - DependencyKind::Development => self.dev_dependencies.push(dep), - DependencyKind::Build => self.build_dependencies.push(dep), - } + DependencyKind::Normal => self.dependencies.insert(dep), + DependencyKind::Development => self.dev_dependencies.insert(dep), + DependencyKind::Build => self.build_dependencies.insert(dep), + }; } } @@ -45,9 +49,11 @@ pub struct QuickReportEntry { #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] pub struct QuickSafetyReport { /// Packages that were scanned successfully - pub packages: Vec, + #[serde(with = "entry_serde")] + pub packages: HashMap, /// Packages that were not scanned successfully - pub packages_without_metrics: Vec, + #[serde(serialize_with = "set_serde::serialize")] + pub packages_without_metrics: HashSet, } /// Entry of the report generated from scanning for the use of `unsafe` @@ -61,9 +67,12 @@ pub struct ReportEntry { /// Report generated from scanning for the use of `unsafe` #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] pub struct SafetyReport { - pub packages: Vec, - pub packages_without_metrics: Vec, - pub used_but_not_scanned_files: Vec, + #[serde(with = "entry_serde")] + pub packages: HashMap, + #[serde(serialize_with = "set_serde::serialize")] + pub packages_without_metrics: HashSet, + #[serde(serialize_with = "set_serde::serialize")] + pub used_but_not_scanned_files: HashSet, } /// Unsafety usage in a package @@ -164,3 +173,106 @@ impl AddAssign for CounterBlock { *self = self.clone() + rhs; } } + +trait Entry { + fn package_id(&self) -> &PackageId; +} + +impl Entry for ReportEntry { + fn package_id(&self) -> &PackageId { + &self.package.id + } +} + +impl Entry for QuickReportEntry { + fn package_id(&self) -> &PackageId { + &self.package.id + } +} + +mod entry_serde { + use crate::PackageId; + use serde::{ + ser::SerializeSeq, + Deserialize, Deserializer, Serialize, Serializer, + }; + use std::{ + collections::HashMap, + fmt, + marker::PhantomData, + }; + + pub(super) fn serialize( + map: &HashMap, + serializer: S, + ) -> Result + where + T: Serialize + super::Entry, + S: Serializer, + { + let mut values = map.values().collect::>(); + values.sort_by(|a, b| a.package_id().cmp(b.package_id())); + let mut seq = serializer.serialize_seq(Some(values.len()))?; + for value in values { + seq.serialize_element(value)?; + } + seq.end() + } + + pub(super) fn deserialize<'de, T, D>(deserializer: D) -> Result, D::Error> + where + T: Deserialize<'de> + super::Entry, + D: Deserializer<'de>, + { + struct Visitor(PhantomData U>); + + impl<'d, U> serde::de::Visitor<'d> for Visitor + where + U: Deserialize<'d> + super::Entry, + { + type Value = HashMap; + + fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("a sequence") + } + + fn visit_seq(self, mut seq: A) -> Result + where + A: serde::de::SeqAccess<'d>, + { + let mut map = HashMap::new(); + while let Some(item) = seq.next_element::()? { + map.insert(item.package_id().clone(), item); + } + Ok(map) + } + } + + deserializer.deserialize_seq(Visitor(PhantomData)) + } +} + +mod set_serde { + use serde::{ + ser::SerializeSeq, + Serialize, Serializer, + }; + use std::collections::HashSet; + + pub(super) fn serialize( + set: &HashSet, + serializer: S, + ) -> Result + where + T: Serialize + Ord, + S: Serializer, + { + let mut values = set.iter().collect::>(); + values.sort_by(|a, b| a.cmp(b)); + let mut seq = serializer.serialize_seq(Some(values.len()))?; + for value in values { + seq.serialize_element(value)?; + } + seq.end() + } +} diff --git a/cargo-geiger-serde/src/source.rs b/cargo-geiger-serde/src/source.rs index ccbf8381..ddb2007f 100644 --- a/cargo-geiger-serde/src/source.rs +++ b/cargo-geiger-serde/src/source.rs @@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize}; use url::Url; /// Source of a package (where it is fetched from) -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub enum Source { Git { url: Url, diff --git a/cargo-geiger/Cargo.toml b/cargo-geiger/Cargo.toml index b39e3bf1..faaac74e 100644 --- a/cargo-geiger/Cargo.toml +++ b/cargo-geiger/Cargo.toml @@ -36,7 +36,11 @@ vendored-openssl = ["cargo/vendored-openssl"] [dev-dependencies] assert_cmd = "1.0.1" better-panic = "0.2.0" +fs_extra = "1.2.0" insta = "0.16.1" rand = "0.7.3" regex = "1.3.9" rstest = "0.6.4" +semver = "0.10.0" +tempfile = "3.1.0" +url = "2.1.1" diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index 8a65a5d7..eb584e20 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -171,7 +171,7 @@ fn package_metrics<'a>( indices.push(dep_index); } let dep = from_cargo_package_id(graph.graph[dep_index].id); - package.push_dependency(dep, from_cargo_dependency_kind(*edge.weight())); + package.add_dependency(dep, from_cargo_dependency_kind(*edge.weight())); } match geiger_context.pack_id_to_metrics.get(&id) { Some(m) => Some((package, Some(m))), diff --git a/cargo-geiger/src/scan/default.rs b/cargo-geiger/src/scan/default.rs index 9455e4ba..9d3fcef5 100644 --- a/cargo-geiger/src/scan/default.rs +++ b/cargo-geiger/src/scan/default.rs @@ -131,16 +131,16 @@ fn scan_to_report( let pack_metrics = match pack_metrics { Some(m) => m, None => { - report.packages_without_metrics.push(package.id); + report.packages_without_metrics.insert(package.id); continue; } }; let unsafety = unsafe_stats(pack_metrics, &rs_files_used); let entry = ReportEntry { package, unsafety }; - report.packages.push(entry); + report.packages.insert(entry.package.id.clone(), entry); } report.used_but_not_scanned_files = - list_files_used_but_not_scanned(&geiger_context, &rs_files_used); + list_files_used_but_not_scanned(&geiger_context, &rs_files_used).into_iter().collect(); let s = match output_format { OutputFormat::Json => serde_json::to_string(&report).unwrap(), }; diff --git a/cargo-geiger/src/scan/forbid.rs b/cargo-geiger/src/scan/forbid.rs index cedde296..4d7d0f09 100644 --- a/cargo-geiger/src/scan/forbid.rs +++ b/cargo-geiger/src/scan/forbid.rs @@ -54,7 +54,7 @@ fn scan_forbid_to_report( let pack_metrics = match package_metrics { Some(m) => m, None => { - report.packages_without_metrics.push(package.id); + report.packages_without_metrics.insert(package.id); continue; } }; @@ -67,7 +67,7 @@ fn scan_forbid_to_report( package, forbids_unsafe, }; - report.packages.push(entry); + report.packages.insert(entry.package.id.clone(), entry); } let s = match output_format { OutputFormat::Json => serde_json::to_string(&report).unwrap(), diff --git a/cargo-geiger/tests/mod.rs b/cargo-geiger/tests/mod.rs index 3146d485..7668c1c1 100644 --- a/cargo-geiger/tests/mod.rs +++ b/cargo-geiger/tests/mod.rs @@ -2,12 +2,21 @@ #![forbid(warnings)] use assert_cmd::prelude::*; +use cargo_geiger_serde::{ + Count, CounterBlock, PackageId, PackageInfo, QuickReportEntry, QuickSafetyReport, ReportEntry, + SafetyReport, Source, UnsafeInfo, +}; use insta::assert_snapshot; use rstest::rstest; +use semver::Version; +use tempfile::TempDir; +use url::Url; +use std::collections::{HashMap, HashSet}; use std::env; -use std::path::Path; -use std::process::Command; +use std::hash::Hash; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; #[rstest( name, @@ -22,21 +31,7 @@ use std::process::Command; fn test_package(name: &str) { better_panic::install(); - let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); - let test_case_root_dir = - Path::new(&manifest_dir).join("../test_crates").join(name); - - let result = Command::cargo_bin("cargo-geiger") - .unwrap() - .arg("geiger") - .arg("--color=never") - .arg("--quiet") - .arg("--charset=ascii") - .arg("--all-targets") - .arg("--all-features") - .current_dir(test_case_root_dir) - .output() - .expect("failed to run `cargo-geiger`"); + let result = run_geiger(name); let stderr_filename = format!("{}.stderr", name); let stderr = String::from_utf8(result.stderr) @@ -57,3 +52,762 @@ fn test_package(name: &str) { assert!(result.status.success(), "`cargo-geiger` failed"); } } + +#[test] +fn serialize_test1_report() { + Test1.run(); +} + +#[test] +fn serialize_test2_report() { + Test2.run(); +} + +#[test] +fn serialize_test3_report() { + Test3.run(); +} + +#[test] +fn serialize_test4_report() { + Test4.run(); +} + +#[test] +fn serialize_test6_report() { + Test6.run(); +} + +#[test] +fn serialize_test7_report() { + Test7.run(); +} + +#[test] +fn serialize_test1_quick_report() { + Test1.run_quick(); +} + +#[test] +fn serialize_test2_quick_report() { + Test2.run_quick(); +} + +#[test] +fn serialize_test3_quick_report() { + Test3.run_quick(); +} + +#[test] +fn serialize_test4_quick_report() { + Test4.run_quick(); +} + +#[test] +fn serialize_test6_quick_report() { + Test6.run_quick(); +} + +#[test] +fn serialize_test7_quick_report() { + Test7.run_quick(); +} + +trait Test { + const NAME: &'static str; + + fn expected_report(&self, cx: &Context) -> SafetyReport; + fn expected_report_entry(&self, cx: &Context) -> ReportEntry; + + fn expected_quick_report(&self, cx: &Context) -> QuickSafetyReport { + to_quick_report(self.expected_report(cx)) + } + + fn run(&self) { + let (output, cx) = run_geiger_json(Self::NAME); + assert!(output.status.success()); + let actual = serde_json::from_slice::(&output.stdout).unwrap(); + assert_eq!(actual, self.expected_report(&cx)); + } + + fn run_quick(&self) { + let (output, cx) = run_geiger_json_quick(Self::NAME); + assert!(output.status.success()); + let actual = serde_json::from_slice::(&output.stdout).unwrap(); + assert_eq!(actual, self.expected_quick_report(&cx)); + } +} + +struct Test1; + +impl Test for Test1 { + const NAME: &'static str = "test1_package_with_no_deps"; + + fn expected_report(&self, cx: &Context) -> SafetyReport { + single_entry_safety_report(self.expected_report_entry(cx)) + } + + fn expected_report_entry(&self, cx: &Context) -> ReportEntry { + ReportEntry { + package: PackageInfo::new(make_package_id(cx, Self::NAME)), + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 1, unsafe_: 1 }, + exprs: Count { safe: 4, unsafe_: 2 }, + ..Default::default() + }, + ..Default::default() + }, + } + } +} + +struct Test2; + +impl Test for Test2 { + const NAME: &'static str = "test2_package_with_shallow_deps"; + + fn expected_report(&self, cx: &Context) -> SafetyReport { + let mut report = single_entry_safety_report(self.expected_report_entry(cx)); + merge_test_reports(&mut report, Test1.expected_report(cx)); + merge_test_reports(&mut report, external::ref_slice_safety_report()); + report + } + + fn expected_report_entry(&self, cx: &Context) -> ReportEntry { + ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![ + make_package_id(cx, Test1::NAME), + external::ref_slice_package_id(), + ]), + ..PackageInfo::new(make_package_id(cx, Self::NAME)) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 0, unsafe_: 1 }, + exprs: Count { safe: 0, unsafe_: 4 }, + ..Default::default() + }, + ..Default::default() + }, + } + } +} + +struct Test3; + +impl Test for Test3 { + const NAME: &'static str = "test3_package_with_nested_deps"; + + fn expected_report(&self, cx: &Context) -> SafetyReport { + let mut report = single_entry_safety_report(self.expected_report_entry(cx)); + merge_test_reports(&mut report, external::itertools_safety_report()); + merge_test_reports(&mut report, external::doc_comment_safety_report()); + merge_test_reports(&mut report, Test2.expected_report(cx)); + report + } + + fn expected_report_entry(&self, cx: &Context) -> ReportEntry { + ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![ + make_package_id(cx, Test2::NAME), + external::itertools_package_id(), + external::doc_comment_package_id(), + ]), + ..PackageInfo::new(make_package_id(cx, Self::NAME)) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 1, unsafe_: 0 }, + exprs: Count { safe: 6, unsafe_: 1 }, + ..Default::default() + }, + ..Default::default() + }, + } + } +} + +struct Test4; + +impl Test for Test4 { + const NAME: &'static str = "test4_workspace_with_top_level_package"; + + fn expected_report(&self, cx: &Context) -> SafetyReport { + let mut report = single_entry_safety_report(self.expected_report_entry(cx)); + merge_test_reports(&mut report, Test1.expected_report(cx)); + report + } + + fn expected_report_entry(&self, cx: &Context) -> ReportEntry { + ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![make_package_id(cx, Test1::NAME)]), + ..PackageInfo::new(make_package_id(cx, Self::NAME)) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 1, unsafe_: 0 }, + exprs: Count { safe: 1, unsafe_: 0 }, + ..Default::default() + }, + unused: CounterBlock { + functions: Count { safe: 1, unsafe_: 0 }, + exprs: Count { safe: 1, unsafe_: 1 }, + ..Default::default() + }, + ..Default::default() + }, + } + } +} + +struct Test6; + +impl Test for Test6 { + const NAME: &'static str = "test6_cargo_lock_out_of_date"; + + fn expected_report(&self, cx: &Context) -> SafetyReport { + let mut report = single_entry_safety_report(self.expected_report_entry(cx)); + merge_test_reports(&mut report, external::generational_arena_safety_report()); + merge_test_reports(&mut report, external::idna_safety_report()); + report + } + + fn expected_report_entry(&self, cx: &Context) -> ReportEntry { + ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![ + external::generational_arena_package_id(), + external::idna_package_id(), + ]), + ..PackageInfo::new(make_package_id(cx, Self::NAME)) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 1, unsafe_: 0 }, + exprs: Count { safe: 1, unsafe_: 0 }, + ..Default::default() + }, + forbids_unsafe: true, + ..Default::default() + }, + } + } +} + +struct Test7; + +impl Test for Test7 { + const NAME: &'static str = "test7_package_with_patched_dep"; + + fn expected_report(&self, cx: &Context) -> SafetyReport { + let mut report = single_entry_safety_report(self.expected_report_entry(cx)); + merge_test_reports(&mut report, external::num_cpus_safety_report(cx)); + report + } + + fn expected_report_entry(&self, cx: &Context) -> ReportEntry { + ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![external::num_cpus_package_id(cx)]), + ..PackageInfo::new(make_package_id(cx, Self::NAME)) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 1, unsafe_: 0 }, + exprs: Count { safe: 1, unsafe_: 0 }, + ..Default::default() + }, + forbids_unsafe: true, + ..Default::default() + }, + } + } +} + +fn run_geiger(test_name: &str) -> Output { + run_geiger_with(test_name, None::<&str>).0 +} + +fn run_geiger_json(test_name: &str) -> (Output, Context) { + run_geiger_with(test_name, &["--json"]) +} + +fn run_geiger_json_quick(test_name: &str) -> (Output, Context) { + run_geiger_with(test_name, &["--forbid-only", "--json"]) +} + +fn run_geiger_with(test_name: &str, extra_args: I) -> (Output, Context) +where + I: IntoIterator, + I::Item: AsRef, +{ + let cx = Context::new(); + let output = Command::cargo_bin("cargo-geiger") + .unwrap() + .arg("geiger") + .arg("--color=never") + .arg("--quiet") + .arg("--charset=ascii") + .arg("--all-targets") + .arg("--all-features") + .args(extra_args) + .current_dir(cx.crate_dir(test_name)) + .output() + .expect("failed to run `cargo-geiger`"); + (output, cx) +} + +fn make_source(cx: &Context, name: &str) -> Source { + Source::Path(Url::from_file_path(cx.crate_dir(name)).unwrap()) +} + +fn make_workspace_source(cx: &Context, workspace: &str, name: &str) -> Source { + Source::Path(Url::from_file_path(cx.workspace_crate_dir(workspace, name)).unwrap()) +} + +struct Context { + dir: TempDir, +} + +impl Context { + fn new() -> Self { + let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); + let path = Path::new(&manifest_dir).join("../test_crates"); + let dir = TempDir::new().unwrap(); + let copy_options = fs_extra::dir::CopyOptions { + content_only: true, + ..Default::default() + }; + fs_extra::dir::copy(&path, dir.path(), ©_options).expect("Failed to copy tests"); + Context { dir } + } + + fn crate_dir(&self, name: &str) -> PathBuf { + self.dir.path().join(name) + } + + fn workspace_crate_dir(&self, workspace: &str, name: &str) -> PathBuf { + let mut p = self.dir.path().to_owned(); + p.extend(&[workspace, name]); + p + } +} + +fn make_package_id(cx: &Context, name: &str) -> PackageId { + PackageId { + name: name.into(), + version: Version::new(0, 1, 0), + source: make_source(cx, name), + } +} + +fn report_entry_list_to_map(entries: I) -> HashMap +where + I: IntoIterator, +{ + entries.into_iter().map(|e| (e.package.id.clone(), e)).collect() +} + +fn to_set(items: I) -> HashSet +where + I: IntoIterator, + I::Item: Hash + Eq, +{ + items.into_iter().collect() +} + +// This function does not handle all merges but works well enough to avoid repetition in these +// tests. +fn merge_test_reports(report: &mut SafetyReport, other: SafetyReport) { + report.packages.extend(other.packages); + report.packages_without_metrics.extend(other.packages_without_metrics); + report.used_but_not_scanned_files.extend(other.used_but_not_scanned_files); +} + +fn to_quick_report(report: SafetyReport) -> QuickSafetyReport { + let entries = report + .packages + .into_iter() + .map(|(id, entry)| { + let quick_entry = QuickReportEntry { + package: entry.package, + forbids_unsafe: entry.unsafety.forbids_unsafe, + }; + (id, quick_entry) + }) + .collect(); + QuickSafetyReport { + packages: entries, + packages_without_metrics: report.packages_without_metrics, + } +} + +fn single_entry_safety_report(entry: ReportEntry) -> SafetyReport { + SafetyReport { + packages: report_entry_list_to_map(vec![entry]), + ..Default::default() + } +} + +mod external { + use cargo_geiger_serde::{ + Count, CounterBlock, PackageId, PackageInfo, ReportEntry, SafetyReport, Source, UnsafeInfo, + }; + use semver::Version; + use super::{merge_test_reports, single_entry_safety_report, to_set, Context, Test}; + use url::Url; + + fn crates_io_source() -> Source { + Source::Registry { + name: "crates.io".into(), + url: Url::parse("https://github.com/rust-lang/crates.io-index").unwrap(), + } + } + + pub(super) fn ref_slice_package_id() -> PackageId { + PackageId { + name: "ref_slice".into(), + version: Version::new(1, 1, 1), + source: crates_io_source(), + } + } + + pub(super) fn ref_slice_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo::new(ref_slice_package_id()), + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 4, unsafe_: 0 }, + exprs: Count { safe: 10, unsafe_: 2 }, + ..Default::default() + }, + ..Default::default() + }, + }; + single_entry_safety_report(entry) + } + + pub(super) fn either_package_id() -> PackageId { + PackageId { + name: "either".into(), + version: Version::new(1, 5, 2), + source: crates_io_source(), + } + } + + pub(super) fn either_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo::new(either_package_id()), + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 6, unsafe_: 0 }, + exprs: Count { safe: 102, unsafe_: 0 }, + item_impls: Count { safe: 21, unsafe_: 0 }, + methods: Count { safe: 50, unsafe_: 0 }, + ..Default::default() + }, + ..Default::default() + }, + }; + single_entry_safety_report(entry) + } + + pub(super) fn doc_comment_package_id() -> PackageId { + PackageId { + name: "doc-comment".into(), + version: Version::new(0, 3, 1), + source: crates_io_source(), + } + } + + pub(super) fn doc_comment_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo::new(doc_comment_package_id()), + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 1, unsafe_: 0 }, + exprs: Count { safe: 37, unsafe_: 0 }, + ..Default::default() + }, + ..Default::default() + }, + }; + single_entry_safety_report(entry) + } + + pub(super) fn itertools_package_id() -> PackageId { + PackageId { + name: "itertools".into(), + version: Version::new(0, 8, 0), + source: Source::Git { + url: Url::parse("https://github.com/rust-itertools/itertools.git").unwrap(), + rev: "8761fbefb3b209cf41829f8dba38044b69c1d8dd".into(), + }, + } + } + + pub(super) fn itertools_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![either_package_id()]), + ..PackageInfo::new(itertools_package_id()) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 79, unsafe_: 0 }, + exprs: Count { safe: 2413, unsafe_: 0 }, + item_impls: Count { safe: 129, unsafe_: 0 }, + item_traits: Count { safe: 7, unsafe_: 0 }, + methods: Count { safe: 180, unsafe_: 0 }, + }, + unused: CounterBlock { + functions: Count { safe: 67, unsafe_: 0 }, + exprs: Count { safe: 1210, unsafe_: 72 }, + item_impls: Count { safe: 24, unsafe_: 3 }, + item_traits: Count { safe: 2, unsafe_: 1 }, + methods: Count { safe: 29, unsafe_: 3 }, + }, + ..Default::default() + } + }; + let mut report = single_entry_safety_report(entry); + merge_test_reports(&mut report, either_safety_report()); + report + } + + pub(super) fn cfg_if_package_id() -> PackageId { + PackageId { + name: "cfg-if".into(), + version: Version::new(0, 1, 9), + source: crates_io_source(), + } + } + + pub(super) fn cfg_if_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo::new(cfg_if_package_id()), + unsafety: Default::default(), + }; + single_entry_safety_report(entry) + } + + pub(super) fn generational_arena_package_id() -> PackageId { + PackageId { + name: "generational-arena".into(), + version: Version::new(0, 2, 2), + source: crates_io_source(), + } + } + + pub(super) fn generational_arena_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![cfg_if_package_id()]), + ..PackageInfo::new(generational_arena_package_id()) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + exprs: Count { safe: 372, unsafe_: 0 }, + item_impls: Count { safe: 21, unsafe_: 0 }, + methods: Count { safe: 39, unsafe_: 0 }, + ..Default::default() + }, + unused: CounterBlock { + functions: Count { safe: 6, unsafe_: 0 }, + exprs: Count { safe: 243, unsafe_: 0 }, + item_impls: Count { safe: 7, unsafe_: 0 }, + methods: Count { safe: 8, unsafe_: 0 }, + ..Default::default() + }, + forbids_unsafe: true, + }, + }; + let mut report = single_entry_safety_report(entry); + merge_test_reports(&mut report, cfg_if_safety_report()); + report + } + + pub(super) fn idna_package_id() -> PackageId { + PackageId { + name: "idna".into(), + version: Version::new(0, 1, 5), + source: crates_io_source(), + } + } + + pub(super) fn idna_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![ + matches_package_id(), + unicode_bidi_package_id(), + unicode_normalization_package_id(), + ]), + ..PackageInfo::new(idna_package_id()) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 17, unsafe_: 0 }, + exprs: Count { safe: 13596, unsafe_: 1 }, + ..Default::default() + }, + unused: CounterBlock { + functions: Count { safe: 7, unsafe_: 0 }, + exprs: Count { safe: 185, unsafe_: 0 }, + ..Default::default() + }, + ..Default::default() + }, + }; + let mut report = single_entry_safety_report(entry); + merge_test_reports(&mut report, matches_safety_report()); + merge_test_reports(&mut report, unicode_bidi_safety_report()); + merge_test_reports(&mut report, unicode_normalization_safety_report()); + report + } + + pub(super) fn matches_package_id() -> PackageId { + PackageId { + name: "matches".into(), + version: Version::new(0, 1, 8), + source: crates_io_source(), + } + } + + pub(super) fn matches_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo::new(matches_package_id()), + unsafety: Default::default(), + }; + single_entry_safety_report(entry) + } + + pub(super) fn smallvec_package_id() -> PackageId { + PackageId { + name: "smallvec".into(), + version: Version::new(0, 6, 9), + source: crates_io_source(), + } + } + + pub(super) fn smallvec_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo::new(smallvec_package_id()), + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 0, unsafe_: 2 }, + exprs: Count { safe: 291, unsafe_: 354 }, + item_impls: Count { safe: 48, unsafe_: 4 }, + item_traits: Count { safe: 3, unsafe_: 1 }, + methods: Count { safe: 92, unsafe_: 13 }, + }, + unused: CounterBlock { + functions: Count { safe: 18, unsafe_: 0 }, + exprs: Count { safe: 126, unsafe_: 0 }, + item_impls: Count { safe: 2, unsafe_: 0 }, + item_traits: Count { safe: 1, unsafe_: 0 }, + methods: Count { safe: 14, unsafe_: 0 }, + }, + ..Default::default() + } + }; + single_entry_safety_report(entry) + } + + pub(super) fn unicode_bidi_package_id() -> PackageId { + PackageId { + name: "unicode-bidi".into(), + version: Version::new(0, 3, 4), + source: crates_io_source(), + } + } + + pub(super) fn unicode_bidi_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![matches_package_id()]), + ..PackageInfo::new(unicode_bidi_package_id()) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 16, unsafe_: 0 }, + exprs: Count { safe: 2119, unsafe_: 0 }, + item_impls: Count { safe: 8, unsafe_: 0 }, + methods: Count { safe: 31, unsafe_: 0 }, + ..Default::default() + }, + forbids_unsafe: true, + ..Default::default() + }, + }; + let mut report = single_entry_safety_report(entry); + merge_test_reports(&mut report, matches_safety_report()); + report + } + + pub(super) fn unicode_normalization_package_id() -> PackageId { + PackageId { + name: "unicode-normalization".into(), + version: Version::new(0, 1, 8), + source: crates_io_source(), + } + } + + pub(super) fn unicode_normalization_safety_report() -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![smallvec_package_id()]), + ..PackageInfo::new(unicode_normalization_package_id()) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 37, unsafe_: 0 }, + exprs: Count { safe: 12901, unsafe_: 20 }, + item_impls: Count { safe: 9, unsafe_: 0 }, + item_traits: Count { safe: 1, unsafe_: 0 }, + methods: Count { safe: 21, unsafe_: 0 }, + }, + unused: CounterBlock { + functions: Count { safe: 22, unsafe_: 0 }, + exprs: Count { safe: 84, unsafe_: 0 }, + ..Default::default() + }, + ..Default::default() + }, + }; + let mut report = single_entry_safety_report(entry); + merge_test_reports(&mut report, smallvec_safety_report()); + report + } + + pub(super) fn num_cpus_package_id(cx: &Context) -> PackageId { + PackageId { + name: "num_cpus".into(), + version: Version::new(1, 10, 1), + source: super::make_workspace_source(cx, "support", "num_cpus"), + } + } + + pub(super) fn num_cpus_safety_report(cx: &Context) -> SafetyReport { + let entry = ReportEntry { + package: PackageInfo { + dependencies: to_set(vec![super::make_package_id(cx, super::Test1::NAME)]), + ..PackageInfo::new(num_cpus_package_id(cx)) + }, + unsafety: UnsafeInfo { + used: CounterBlock { + functions: Count { safe: 1, unsafe_: 0 }, + ..Default::default() + }, + ..Default::default() + }, + }; + let mut report = single_entry_safety_report(entry); + merge_test_reports(&mut report, super::Test1.expected_report(cx)); + report + } +} From fb6c4798d57735af751d5e045bda5bd293b0af5b Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Thu, 15 Oct 2020 09:45:19 -0700 Subject: [PATCH 4/7] Canonicalize paths in test reports So that they can be compared to the actual reports. --- cargo-geiger/tests/mod.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/cargo-geiger/tests/mod.rs b/cargo-geiger/tests/mod.rs index 7668c1c1..2a9461fc 100644 --- a/cargo-geiger/tests/mod.rs +++ b/cargo-geiger/tests/mod.rs @@ -370,28 +370,31 @@ fn make_workspace_source(cx: &Context, workspace: &str, name: &str) -> Source { } struct Context { - dir: TempDir, + _dir: TempDir, + path: PathBuf, } impl Context { fn new() -> Self { let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); - let path = Path::new(&manifest_dir).join("../test_crates"); + let src_path = Path::new(&manifest_dir).join("../test_crates"); let dir = TempDir::new().unwrap(); let copy_options = fs_extra::dir::CopyOptions { content_only: true, ..Default::default() }; - fs_extra::dir::copy(&path, dir.path(), ©_options).expect("Failed to copy tests"); - Context { dir } + fs_extra::dir::copy(&src_path, dir.path(), ©_options).expect("Failed to copy tests"); + let path = dir.path().canonicalize().expect("Failed to canonicalize temporary path"); + let _dir = dir; + Context { _dir, path } } fn crate_dir(&self, name: &str) -> PathBuf { - self.dir.path().join(name) + self.path.join(name) } fn workspace_crate_dir(&self, workspace: &str, name: &str) -> PathBuf { - let mut p = self.dir.path().to_owned(); + let mut p = self.path.clone(); p.extend(&[workspace, name]); p } From 1918304e9e8cf2c61cde67d40181a8754cffca54 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Thu, 15 Oct 2020 11:26:56 -0700 Subject: [PATCH 5/7] LocalRegistry and Directory are handled as Registry As per the `cargo` docs, they are respectively a local filesystem-based registry and a directory-based registry. --- cargo-geiger-serde/src/source.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/cargo-geiger-serde/src/source.rs b/cargo-geiger-serde/src/source.rs index ddb2007f..10df0596 100644 --- a/cargo-geiger-serde/src/source.rs +++ b/cargo-geiger-serde/src/source.rs @@ -13,6 +13,4 @@ pub enum Source { url: Url, }, Path(Url), - LocalRegistry(Url), - Directory(Url), } From 2aea3afdfe32f60d718bd467d80a8b161e64e179 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Thu, 15 Oct 2020 12:05:15 -0700 Subject: [PATCH 6/7] Canonicalize paths returned by cargo --- cargo-geiger/Cargo.toml | 2 +- cargo-geiger/src/scan.rs | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/cargo-geiger/Cargo.toml b/cargo-geiger/Cargo.toml index faaac74e..0f7766ac 100644 --- a/cargo-geiger/Cargo.toml +++ b/cargo-geiger/Cargo.toml @@ -29,6 +29,7 @@ strum = "0.19.2" strum_macros = "0.19.2" walkdir = "2.3.1" anyhow = "1.0.31" +url = "2.1.1" [features] vendored-openssl = ["cargo/vendored-openssl"] @@ -43,4 +44,3 @@ regex = "1.3.9" rstest = "0.6.4" semver = "0.10.0" tempfile = "3.1.0" -url = "2.1.1" diff --git a/cargo-geiger/src/scan.rs b/cargo-geiger/src/scan.rs index eb584e20..79bc0474 100644 --- a/cargo-geiger/src/scan.rs +++ b/cargo-geiger/src/scan.rs @@ -17,6 +17,7 @@ use cargo_geiger_serde::{CounterBlock, DependencyKind, PackageInfo, UnsafeInfo}; use petgraph::visit::EdgeRef; use std::collections::{HashMap, HashSet}; use std::path::PathBuf; +use url::Url; /// Provides a more terse and searchable name for the wrapped generic /// collection. @@ -185,7 +186,19 @@ fn package_metrics<'a>( fn from_cargo_package_id(id: PackageId) -> cargo_geiger_serde::PackageId { let source = id.source_id(); - let source_url = source.url().clone(); + let source_url = source.url(); + // Canonicalize paths as cargo does not seem to do so on all platforms. + let source_url = if source_url.scheme() == "file" { + match source_url.to_file_path() { + Ok(p) => { + let p = p.canonicalize().expect("A package source path could not be canonicalized"); + Url::from_file_path(p).expect("A URL could not be created from a file path") + } + Err(_) => source_url.clone(), + } + } else { + source_url.clone() + }; let source = if source.is_git() { cargo_geiger_serde::Source::Git { url: source_url, From aa4ef4f8eb9aafa26f0b754d44c78340dbc6a60e Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Fri, 16 Oct 2020 00:31:08 -0700 Subject: [PATCH 7/7] Work around cargo bug with UNC paths `cargo` fails to use an overriding path dependency when the manifest path given to `cargo build` is a UNC path (starting with `\\?\`). This was causing test7 test cases to fail. --- cargo-geiger/tests/mod.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cargo-geiger/tests/mod.rs b/cargo-geiger/tests/mod.rs index 2a9461fc..e07f16a0 100644 --- a/cargo-geiger/tests/mod.rs +++ b/cargo-geiger/tests/mod.rs @@ -385,6 +385,17 @@ impl Context { }; fs_extra::dir::copy(&src_path, dir.path(), ©_options).expect("Failed to copy tests"); let path = dir.path().canonicalize().expect("Failed to canonicalize temporary path"); + // Canonicalizing on Windows returns a UNC path (starting with `\\?\`). + // `cargo build` (as of 1.47.0) fails to use an overriding path dependency if the manifest + // given to `cargo build` is a UNC path. Roudtripping to URL gets rid of the UNC prefix. + let path = if cfg!(windows) { + Url::from_file_path(path) + .expect("URL from path must succeed") + .to_file_path() + .expect("Roundtripping path to URL must succeed") + } else { + path + }; let _dir = dir; Context { _dir, path } }