Skip to content

Commit

Permalink
Merge pull request #162 from jmcconnell26/ISSUE-151-FixNotWritingRepo…
Browse files Browse the repository at this point in the history
…rtIfWarningFound

ISSUE-151 - Fix a bug where a report wasn't written if any warning
  • Loading branch information
anderejd authored Dec 7, 2020
2 parents 8a84902 + 2e51b9a commit db35ec7
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 56 deletions.
9 changes: 6 additions & 3 deletions cargo-geiger/src/format/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ 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::scan::{GeigerContext, ScanResult};
use crate::tree::TextTreeLine;

use handle_text_tree_line::{
Expand Down Expand Up @@ -34,7 +34,7 @@ pub fn create_table_from_text_tree_lines(
cargo_metadata_parameters: &CargoMetadataParameters,
table_parameters: &TableParameters,
text_tree_lines: Vec<TextTreeLine>,
) -> (Vec<String>, u64) {
) -> ScanResult {
let mut table_lines = Vec::<String>::new();
let mut total_package_counts = TotalPackageCounts::new();
let mut warning_count = 0;
Expand Down Expand Up @@ -87,7 +87,10 @@ pub fn create_table_from_text_tree_lines(

table_lines.push(String::new());

(table_lines, warning_count)
ScanResult {
scan_output_lines: table_lines,
warning_count,
}
}

pub struct TableParameters<'a> {
Expand Down
14 changes: 12 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, ScanResult};

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

let scan_output_lines = scan(
let ScanResult {
scan_output_lines,
warning_count,
} = scan(
args,
&cargo_metadata_parameters,
config,
Expand All @@ -105,6 +108,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
30 changes: 26 additions & 4 deletions cargo-geiger/src/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,29 @@ 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)
}
}

pub struct ScanResult {
pub scan_output_lines: Vec<String>,
pub warning_count: u64,
}

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

let scan_parameters = ScanParameters {
Expand Down Expand Up @@ -87,7 +108,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 +117,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 +126,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
12 changes: 8 additions & 4 deletions cargo-geiger/src/scan/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::scan::rs_file::resolve_rs_file_deps;
use super::find::find_unsafe;
use super::{
list_files_used_but_not_scanned, package_metrics, unsafe_stats,
ScanDetails, ScanMode, ScanParameters,
ScanDetails, ScanMode, ScanParameters, ScanResult,
};

use table::scan_to_table;
Expand All @@ -27,7 +27,7 @@ pub fn scan_unsafe(
root_package_id: PackageId,
scan_parameters: &ScanParameters,
workspace: &Workspace,
) -> Result<Vec<String>, CliError> {
) -> Result<ScanResult, 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<ScanResult, CliError> {
let ScanDetails {
rs_files_used,
geiger_context,
Expand Down Expand Up @@ -148,7 +148,11 @@ fn scan_to_report(
let s = match output_format {
OutputFormat::Json => serde_json::to_string(&report).unwrap(),
};
Ok(vec![s])

Ok(ScanResult {
scan_output_lines: vec![s],
warning_count: 0,
})
}

#[cfg(test)]
Expand Down
54 changes: 18 additions & 36 deletions cargo-geiger/src/scan/default/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::tree::traversal::walk_dependency_tree;

use super::super::{
construct_rs_files_used_lines, list_files_used_but_not_scanned,
ScanDetails, ScanParameters,
ScanDetails, ScanParameters, ScanResult,
};
use super::scan;

Expand All @@ -18,17 +18,15 @@ 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> {
let mut scan_output_lines = Vec::<String>::new();
) -> Result<ScanResult, CliError> {
let mut combined_scan_output_lines = Vec::<String>::new();

let ScanDetails {
rs_files_used,
Expand All @@ -38,12 +36,12 @@ pub fn scan_to_table(
if scan_parameters.print_config.verbosity == Verbosity::Verbose {
let mut rs_files_used_lines =
construct_rs_files_used_lines(&rs_files_used);
scan_output_lines.append(&mut rs_files_used_lines);
combined_scan_output_lines.append(&mut rs_files_used_lines);
}

let emoji_symbols = EmojiSymbols::new(scan_parameters.print_config.charset);
let mut output_key_lines = construct_key_lines(&emoji_symbols);
scan_output_lines.append(&mut output_key_lines);
combined_scan_output_lines.append(&mut output_key_lines);

let text_tree_lines = walk_dependency_tree(
cargo_metadata_parameters,
Expand All @@ -57,13 +55,15 @@ pub fn scan_to_table(
rs_files_used: &rs_files_used,
};

let (mut table_lines, mut warning_count) =
create_table_from_text_tree_lines(
cargo_metadata_parameters,
&table_parameters,
text_tree_lines,
);
scan_output_lines.append(&mut table_lines);
let ScanResult {
mut scan_output_lines,
mut warning_count,
} = create_table_from_text_tree_lines(
cargo_metadata_parameters,
&table_parameters,
text_tree_lines,
);
combined_scan_output_lines.append(&mut scan_output_lines);

let used_but_not_scanned =
list_files_used_but_not_scanned(&geiger_context, &rs_files_used);
Expand All @@ -75,28 +75,10 @@ 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(ScanResult {
scan_output_lines: combined_scan_output_lines,
warning_count,
})
}

fn construct_key_lines(emoji_symbols: &EmojiSymbols) -> Vec<String> {
Expand Down
11 changes: 7 additions & 4 deletions cargo-geiger/src/scan/forbid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::graph::Graph;
use crate::mapping::CargoMetadataParameters;

use super::find::find_unsafe;
use super::{package_metrics, ScanMode, ScanParameters};
use super::{package_metrics, ScanMode, ScanParameters, ScanResult};

use table::scan_forbid_to_table;

Expand All @@ -18,7 +18,7 @@ pub fn scan_forbid_unsafe(
graph: &Graph,
root_package_id: PackageId,
scan_parameters: &ScanParameters,
) -> Result<Vec<String>, CliError> {
) -> Result<ScanResult, 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<ScanResult, CliError> {
let geiger_context = find_unsafe(
cargo_metadata_parameters,
config,
Expand Down Expand Up @@ -81,5 +81,8 @@ fn scan_forbid_to_report(
OutputFormat::Json => serde_json::to_string(&report).unwrap(),
};

Ok(vec![s])
Ok(ScanResult {
scan_output_lines: vec![s],
warning_count: 0,
})
}
9 changes: 6 additions & 3 deletions cargo-geiger/src/scan/forbid/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::tree::traversal::walk_dependency_tree;
use crate::tree::TextTreeLine;

use super::super::find::find_unsafe;
use super::super::ScanMode;
use super::super::{ScanMode, ScanResult};

use cargo::{CliError, Config};
use cargo_metadata::PackageId;
Expand All @@ -21,7 +21,7 @@ pub fn scan_forbid_to_table(
graph: &Graph,
print_config: &PrintConfig,
root_package_id: PackageId,
) -> Result<Vec<String>, CliError> {
) -> Result<ScanResult, CliError> {
let mut scan_output_lines = Vec::<String>::new();
let emoji_symbols = EmojiSymbols::new(print_config.charset);

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

Ok(scan_output_lines)
Ok(ScanResult {
scan_output_lines,
warning_count: 0,
})
}

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

0 comments on commit db35ec7

Please sign in to comment.