diff --git a/Cargo.toml b/Cargo.toml index 4cf717c..4e878e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ [package] name = "pks" -version = "0.2.21" +version = "0.2.22" edition = "2021" description = "Welcome! Please see https://github.com/rubyatscale/pks for more information!" license = "MIT" @@ -27,6 +27,7 @@ path = "src/lib.rs" anyhow = { version = "1.0.75", features = [] } # for error handling clap = { version = "4.2.1", features = ["derive"] } # cli clap_derive = "4.2.0" # cli +csv = "1.3.0" # csv de/serialize itertools = "0.13.0" # tools for iterating over iterable things jwalk = "0.8.1" # for walking the file tree path-clean = "1.0.1" # Pathname#cleaname in Ruby diff --git a/src/packs.rs b/src/packs.rs index 6b7420f..b10f66d 100644 --- a/src/packs.rs +++ b/src/packs.rs @@ -10,6 +10,7 @@ pub(crate) mod checker_configuration; pub(crate) mod configuration; pub(crate) mod constant_resolver; pub(crate) mod creator; +pub(crate) mod csv; pub(crate) mod dependencies; pub(crate) mod ignored; pub(crate) mod monkey_patch_detection; @@ -38,6 +39,7 @@ pub(crate) use self::parsing::ruby::zeitwerk::get_zeitwerk_constant_resolver; pub(crate) use self::parsing::ParsedDefinition; pub(crate) use self::parsing::UnresolvedReference; use anyhow::bail; +use cli::OutputFormat; pub(crate) use configuration::Configuration; pub(crate) use package_todo::PackageTodo; @@ -68,14 +70,24 @@ pub fn create( pub fn check( configuration: &Configuration, + output_format: OutputFormat, files: Vec, ) -> anyhow::Result<()> { let result = checker::check_all(configuration, files) .context("Failed to check files")?; - println!("{}", result); - if result.has_violations() { - bail!("Violations found!") + + match output_format { + OutputFormat::Packwerk => { + println!("{}", result); + if result.has_violations() { + bail!("Violations found!") + } + } + OutputFormat::CSV => { + csv::write_csv(&result, std::io::stdout())?; + } } + Ok(()) } diff --git a/src/packs/checker.rs b/src/packs/checker.rs index 318de5b..336f63b 100644 --- a/src/packs/checker.rs +++ b/src/packs/checker.rs @@ -64,9 +64,9 @@ pub(crate) trait ValidatorInterface { #[derive(Debug, PartialEq)] pub struct CheckAllResult { - reportable_violations: HashSet, - stale_violations: Vec, - strict_mode_violations: HashSet, + pub reportable_violations: HashSet, + pub stale_violations: Vec, + pub strict_mode_violations: HashSet, } impl CheckAllResult { diff --git a/src/packs/cli.rs b/src/packs/cli.rs index 0a03f61..a9fec5e 100644 --- a/src/packs/cli.rs +++ b/src/packs/cli.rs @@ -1,7 +1,7 @@ use crate::packs; use crate::packs::file_utils::get_absolute_path; -use clap::{Parser, Subcommand}; +use clap::{Parser, Subcommand, ValueEnum}; use clap_derive::Args; use std::path::PathBuf; use tracing::debug; @@ -70,6 +70,9 @@ enum Command { #[arg(long)] ignore_recorded_violations: bool, + #[arg(short, long, default_value = "packwerk")] + output_format: OutputFormat, + files: Vec, }, @@ -152,6 +155,12 @@ enum Command { ListDefinitions(ListDefinitionsArgs), } +#[derive(ValueEnum, Copy, Clone, Debug, PartialEq, Eq)] +pub enum OutputFormat { + Packwerk, + CSV, +} + #[derive(Debug, Args)] struct ListDefinitionsArgs { /// Show constants with multiple definitions only @@ -242,11 +251,12 @@ pub fn run() -> anyhow::Result<()> { Command::ListIncludedFiles => packs::list_included_files(configuration), Command::Check { ignore_recorded_violations, + output_format, files, } => { configuration.ignore_recorded_violations = ignore_recorded_violations; - packs::check(&configuration, files) + packs::check(&configuration, output_format, files) } Command::CheckContents { ignore_recorded_violations, @@ -257,7 +267,7 @@ pub fn run() -> anyhow::Result<()> { let absolute_path = get_absolute_path(file.clone(), &configuration); configuration.stdin_file_path = Some(absolute_path); - packs::check(&configuration, vec![file]) + packs::check(&configuration, OutputFormat::Packwerk, vec![file]) } Command::Update => packs::update(&configuration), Command::Validate => { diff --git a/src/packs/csv.rs b/src/packs/csv.rs new file mode 100644 index 0000000..d1739b8 --- /dev/null +++ b/src/packs/csv.rs @@ -0,0 +1,50 @@ +use itertools::chain; + +use super::checker::{build_strict_violation_message, CheckAllResult}; + +pub fn write_csv( + result: &CheckAllResult, + writer: W, +) -> anyhow::Result<()> { + let mut wtr = csv::Writer::from_writer(writer); + wtr.write_record([ + "Violation", + "Strict?", + "File", + "Constant", + "Referencing Pack", + "Defining Pack", + "Message", + ])?; + + if !&result.reportable_violations.is_empty() + || !&result.strict_mode_violations.is_empty() + { + let all = chain!( + &result.reportable_violations, + &result.strict_mode_violations + ); + + for violation in all { + let identifier = &violation.identifier; + let message = if violation.identifier.strict { + build_strict_violation_message(&violation.identifier) + } else { + violation.message.to_string() + }; + wtr.serialize(( + &identifier.violation_type, + &identifier.strict, + &identifier.file, + &identifier.constant_name, + &identifier.referencing_pack_name, + &identifier.defining_pack_name, + &message, + ))?; + } + } else { + wtr.serialize(("No violations detected!", "", "", "", "", "", ""))?; + } + wtr.flush()?; + Ok(()) +} diff --git a/tests/check_test.rs b/tests/check_test.rs index 4d30714..ae6b135 100644 --- a/tests/check_test.rs +++ b/tests/check_test.rs @@ -166,6 +166,25 @@ fn test_check_with_package_todo_file() -> Result<(), Box> { Ok(()) } +#[test] +fn test_check_with_package_todo_file_csv() -> Result<(), Box> { + Command::cargo_bin("pks")? + .arg("--project-root") + .arg("tests/fixtures/contains_package_todo") + .arg("--debug") + .arg("check") + .arg("-o") + .arg("csv") + .assert() + .success() + .stdout(predicate::str::contains("Violation,Strict?,File,Constant,Referencing Pack,Defining Pack,Message")) + .stdout(predicate::str::contains("No violations detected!")); + + common::teardown(); + + Ok(()) +} + #[test] fn test_check_with_package_todo_file_ignoring_recorded_violations( ) -> Result<(), Box> { @@ -308,6 +327,26 @@ fn test_check_with_strict_mode() -> Result<(), Box> { Ok(()) } +#[test] +fn test_check_with_strict_mode_output_csv() -> Result<(), Box> { + Command::cargo_bin("pks") + .unwrap() + .arg("--project-root") + .arg("tests/fixtures/uses_strict_mode") + .arg("check") + .arg("-o") + .arg("csv") + .assert() + .stdout(predicate::str::contains("Violation,Strict?,File,Constant,Referencing Pack,Defining Pack,Message")) + .stdout(predicate::str::contains("privacy,true,packs/foo/app/services/foo.rb,::Bar,packs/foo,packs/bar,packs/foo cannot have privacy violations on packs/bar because strict mode is enabled for privacy violations in the enforcing pack\'s package.yml file")) + .stdout(predicate::str::contains( + "privacy,true,packs/foo/app/services/foo.rb,::Bar,packs/foo,packs/bar,packs/foo cannot have privacy violations on packs/bar because strict mode is enabled for privacy violations in the enforcing pack\'s package.yml file", + )); + + common::teardown(); + Ok(()) +} + #[test] fn test_check_contents() -> Result<(), Box> { let project_root = "tests/fixtures/simple_app";