Skip to content

Commit

Permalink
ISSUE-151 - Fix a bug where a report wasn't written if any warning
Browse files Browse the repository at this point in the history
was found:
* When updating the scanning functions to return results as a vec of
  strings, a bug was introduced where the report would only be written
  if no warnings were returned

Signed-off-by: joshmc <josh-mcc@tiscali.co.uk>
  • Loading branch information
jmcconnell26 committed Dec 6, 2020
1 parent dfc6ccb commit 9a14548
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 39 deletions.
11 changes: 9 additions & 2 deletions cargo-geiger/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use cargo_geiger::mapping::{CargoMetadataParameters, QueryResolve};
use cargo_geiger::readme::{
create_or_replace_section_in_readme, README_FILENAME,
};
use cargo_geiger::scan::scan;
use cargo_geiger::scan::{scan, FoundWarningsError};

use cargo::core::shell::Shell;
use cargo::util::important_paths;
Expand Down Expand Up @@ -82,7 +82,7 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult {
},
);

let scan_output_lines = scan(
let (scan_output_lines, warning_count) = scan(
args,
&cargo_metadata_parameters,
config,
Expand All @@ -105,6 +105,13 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult {
}
}

if warning_count > 0 {
return Err(CliError::new(
anyhow::Error::new(FoundWarningsError { warning_count }),
1,
));
}

Ok(())
}

Expand Down
25 changes: 21 additions & 4 deletions cargo-geiger/src/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,24 @@ use cargo_geiger_serde::{CounterBlock, PackageInfo, UnsafeInfo};
use cargo_metadata::PackageId;
use petgraph::visit::EdgeRef;
use std::collections::{HashMap, HashSet};
use std::error::Error;
use std::fmt;
use std::path::PathBuf;

#[derive(Debug)]
pub struct FoundWarningsError {
pub warning_count: u64,
}

impl Error for FoundWarningsError {}

/// Forward Display to Debug.
impl fmt::Display for FoundWarningsError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}

/// Provides a more terse and searchable name for the wrapped generic
/// collection.
pub struct GeigerContext {
Expand Down Expand Up @@ -59,7 +75,7 @@ pub fn scan(
graph: &Graph,
root_package_id: PackageId,
workspace: &Workspace,
) -> Result<Vec<String>, CliError> {
) -> Result<(Vec<String>, u64), CliError> {
let print_config = PrintConfig::new(args)?;

let scan_parameters = ScanParameters {
Expand Down Expand Up @@ -87,7 +103,7 @@ pub fn scan(
}

pub fn unsafe_stats(
pack_metrics: &PackageMetrics,
package_metrics: &PackageMetrics,
rs_files_used: &HashSet<PathBuf>,
) -> UnsafeInfo {
// The crate level "forbids unsafe code" metric __used to__ only
Expand All @@ -96,7 +112,7 @@ pub fn unsafe_stats(
// classified as forbidding unsafe code, all entry point source
// files must declare `forbid(unsafe_code)`. Either a crate
// forbids all unsafe code or it allows it _to some degree_.
let forbids_unsafe = pack_metrics
let forbids_unsafe = package_metrics
.rs_path_to_metrics
.iter()
.filter(|(_, v)| v.is_crate_entry_point)
Expand All @@ -105,7 +121,8 @@ pub fn unsafe_stats(
let mut used = CounterBlock::default();
let mut unused = CounterBlock::default();

for (path_buf, rs_file_metrics_wrapper) in &pack_metrics.rs_path_to_metrics
for (path_buf, rs_file_metrics_wrapper) in
&package_metrics.rs_path_to_metrics
{
let target = if rs_files_used.contains(path_buf) {
&mut used
Expand Down
6 changes: 3 additions & 3 deletions cargo-geiger/src/scan/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub fn scan_unsafe(
root_package_id: PackageId,
scan_parameters: &ScanParameters,
workspace: &Workspace,
) -> Result<Vec<String>, CliError> {
) -> Result<(Vec<String>, u64), CliError> {
match scan_parameters.args.output_format {
Some(output_format) => scan_to_report(
cargo_metadata_parameters,
Expand Down Expand Up @@ -115,7 +115,7 @@ fn scan_to_report(
root_package_id: PackageId,
scan_parameters: &ScanParameters,
workspace: &Workspace,
) -> Result<Vec<String>, CliError> {
) -> Result<(Vec<String>, u64), CliError> {
let ScanDetails {
rs_files_used,
geiger_context,
Expand Down Expand Up @@ -148,7 +148,7 @@ fn scan_to_report(
let s = match output_format {
OutputFormat::Json => serde_json::to_string(&report).unwrap(),
};
Ok(vec![s])
Ok((vec![s], 0))
}

#[cfg(test)]
Expand Down
27 changes: 2 additions & 25 deletions cargo-geiger/src/scan/default/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,14 @@ use cargo::core::Workspace;
use cargo::CliError;
use cargo_metadata::PackageId;
use colored::Colorize;
use std::error::Error;
use std::fmt;

pub fn scan_to_table(
cargo_metadata_parameters: &CargoMetadataParameters,
graph: &Graph,
root_package_id: PackageId,
scan_parameters: &ScanParameters,
workspace: &Workspace,
) -> Result<Vec<String>, CliError> {
) -> Result<(Vec<String>, u64), CliError> {
let mut scan_output_lines = Vec::<String>::new();

let ScanDetails {
Expand Down Expand Up @@ -75,28 +73,7 @@ pub fn scan_to_table(
);
}

if warning_count > 0 {
Err(CliError::new(
anyhow::Error::new(FoundWarningsError { warning_count }),
1,
))
} else {
Ok(scan_output_lines)
}
}

#[derive(Debug)]
struct FoundWarningsError {
warning_count: u64,
}

impl Error for FoundWarningsError {}

/// Forward Display to Debug.
impl fmt::Display for FoundWarningsError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
Ok((scan_output_lines, warning_count))
}

fn construct_key_lines(emoji_symbols: &EmojiSymbols) -> Vec<String> {
Expand Down
6 changes: 3 additions & 3 deletions cargo-geiger/src/scan/forbid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn scan_forbid_unsafe(
graph: &Graph,
root_package_id: PackageId,
scan_parameters: &ScanParameters,
) -> Result<Vec<String>, CliError> {
) -> Result<(Vec<String>, u64), CliError> {
match scan_parameters.args.output_format {
Some(output_format) => scan_forbid_to_report(
cargo_metadata_parameters,
Expand All @@ -45,7 +45,7 @@ fn scan_forbid_to_report(
output_format: OutputFormat,
print_config: &PrintConfig,
root_package_id: PackageId,
) -> Result<Vec<String>, CliError> {
) -> Result<(Vec<String>, u64), CliError> {
let geiger_context = find_unsafe(
cargo_metadata_parameters,
config,
Expand Down Expand Up @@ -81,5 +81,5 @@ fn scan_forbid_to_report(
OutputFormat::Json => serde_json::to_string(&report).unwrap(),
};

Ok(vec![s])
Ok((vec![s], 0))
}
4 changes: 2 additions & 2 deletions cargo-geiger/src/scan/forbid/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn scan_forbid_to_table(
graph: &Graph,
print_config: &PrintConfig,
root_package_id: PackageId,
) -> Result<Vec<String>, CliError> {
) -> Result<(Vec<String>, u64), CliError> {
let mut scan_output_lines = Vec::<String>::new();
let emoji_symbols = EmojiSymbols::new(print_config.charset);

Expand Down Expand Up @@ -70,7 +70,7 @@ pub fn scan_forbid_to_table(
}
}

Ok(scan_output_lines)
Ok((scan_output_lines, 0))
}

fn construct_key_lines(emoji_symbols: &EmojiSymbols) -> Vec<String> {
Expand Down

0 comments on commit 9a14548

Please sign in to comment.