Skip to content

Commit

Permalink
CSV output for pks check (#11)
Browse files Browse the repository at this point in the history
* adding csv support
  • Loading branch information
perryqh authored Aug 29, 2024
1 parent 8f8e87d commit d55cce0
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 10 deletions.
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
18 changes: 15 additions & 3 deletions src/packs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -68,14 +70,24 @@ pub fn create(

pub fn check(
configuration: &Configuration,
output_format: OutputFormat,
files: Vec<String>,
) -> 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(())
}

Expand Down
6 changes: 3 additions & 3 deletions src/packs/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ pub(crate) trait ValidatorInterface {

#[derive(Debug, PartialEq)]
pub struct CheckAllResult {
reportable_violations: HashSet<Violation>,
stale_violations: Vec<ViolationIdentifier>,
strict_mode_violations: HashSet<Violation>,
pub reportable_violations: HashSet<Violation>,
pub stale_violations: Vec<ViolationIdentifier>,
pub strict_mode_violations: HashSet<Violation>,
}

impl CheckAllResult {
Expand Down
16 changes: 13 additions & 3 deletions src/packs/cli.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -70,6 +70,9 @@ enum Command {
#[arg(long)]
ignore_recorded_violations: bool,

#[arg(short, long, default_value = "packwerk")]
output_format: OutputFormat,

files: Vec<String>,
},

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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 => {
Expand Down
50 changes: 50 additions & 0 deletions src/packs/csv.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use itertools::chain;

use super::checker::{build_strict_violation_message, CheckAllResult};

pub fn write_csv<W: std::io::Write>(
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(())
}
39 changes: 39 additions & 0 deletions tests/check_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,25 @@ fn test_check_with_package_todo_file() -> Result<(), Box<dyn Error>> {
Ok(())
}

#[test]
fn test_check_with_package_todo_file_csv() -> Result<(), Box<dyn Error>> {
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<dyn Error>> {
Expand Down Expand Up @@ -308,6 +327,26 @@ fn test_check_with_strict_mode() -> Result<(), Box<dyn Error>> {
Ok(())
}

#[test]
fn test_check_with_strict_mode_output_csv() -> Result<(), Box<dyn Error>> {
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<dyn Error>> {
let project_root = "tests/fixtures/simple_app";
Expand Down

0 comments on commit d55cce0

Please sign in to comment.