Add option to fail on error when running with upload-to-github (#327)

This PR potentially fixes #326 and adds an option to fail on error when running `upload-to-github`. Disclaimer: I'm not well versed in Rust, so feel free to suggest any modifications. Also, if you don't agree with the solution for the problem itself, I don't mind closing this one and the issue. Thank you!
sbdchd · Dec 9, 2023 · ddeed02 · ddeed02
1 parent fdbb684
commit ddeed02
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## v0.25.0 - 2023-12-09
+
+### Added
+
+- added `squawk upload-to-github --fail-on-violations` flag to exit with error if violations are found (#327). Thanks @wmartins!
+
 ## v0.24.2 - 2023-11-07
 
 ### Fixed

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "squawk"
-version = "0.24.2"
+version = "0.25.0"
 authors = ["Steve Dignam <steve@dignam.xyz>"]
 edition = "2018"
 license = "GPL-3.0"

diff --git a/cli/src/reporter.rs b/cli/src/reporter.rs
@@ -452,12 +452,12 @@ const fn get_violations_emoji(count: usize) -> &'static str {
     }
 }
 
-fn get_sql_file_content(violation: ViolationContent) -> Result<String, std::io::Error> {
-    let sql = violation.sql;
+fn get_sql_file_content(violation: &ViolationContent) -> Result<String, std::io::Error> {
+    let sql = &violation.sql;
     let mut buff = Vec::new();
     let violation_count = violation.violations.len();
-    for v in violation.violations {
-        fmt_tty_violation(&mut buff, &v)?;
+    for v in &violation.violations {
+        fmt_tty_violation(&mut buff, v)?;
     }
     let violations_text_raw = &String::from_utf8_lossy(&buff);
     let violations_text = strip_ansi_codes(violations_text_raw);
@@ -498,7 +498,7 @@ fn get_sql_file_content(violation: ViolationContent) -> Result<String, std::io::
     ))
 }
 
-pub fn get_comment_body(files: Vec<ViolationContent>, version: &str) -> String {
+pub fn get_comment_body(files: &[ViolationContent], version: &str) -> String {
     let violations_count: usize = files.iter().map(|x| x.violations.len()).sum();
 
     let violations_emoji = get_violations_emoji(violations_count);
@@ -563,7 +563,7 @@ SELECT 1;
             }],
         }];
 
-        let body = get_comment_body(violations, "0.2.3");
+        let body = get_comment_body(&violations, "0.2.3");
 
         assert_display_snapshot!(body);
     }
@@ -598,7 +598,7 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
             },
         ];
 
-        let body = get_comment_body(violations, "0.2.3");
+        let body = get_comment_body(&violations, "0.2.3");
 
         assert_display_snapshot!(body);
     }
@@ -609,7 +609,7 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
     fn test_generating_no_violations_no_files() {
         let violations = vec![];
 
-        let body = get_comment_body(violations, "0.2.3");
+        let body = get_comment_body(&violations, "0.2.3");
 
         assert_display_snapshot!(body);
     }

diff --git a/cli/src/subcommand.rs b/cli/src/subcommand.rs
@@ -14,6 +14,7 @@ pub enum SquawkError {
     GithubPrivateKeyBase64DecodeError(base64::DecodeError),
     GithubPrivateKeyDecodeError(std::string::FromUtf8Error),
     GithubPrivateKeyMissing,
+    RulesViolatedError { violations: usize, files: usize },
 }
 
 impl std::fmt::Display for SquawkError {
@@ -31,6 +32,9 @@ impl std::fmt::Display for SquawkError {
                 write!(f, "Could not decode GitHub private key to string: {err}")
             }
             Self::GithubPrivateKeyMissing => write!(f, "Missing GitHub private key"),
+            Self::RulesViolatedError { violations, files } => {
+                write!(f, "Found {violations} violation(s) across {files} file(s)")
+            }
         }
     }
 }
@@ -59,6 +63,9 @@ pub enum Command {
         /// --exclude=require-concurrent-index-creation,ban-drop-database
         #[structopt(short, long, use_delimiter = true)]
         exclude: Option<Vec<RuleViolationKind>>,
+        /// Exits with an error if violations are found
+        #[structopt(long)]
+        fail_on_violations: bool,
         #[structopt(long, env = "SQUAWK_GITHUB_PRIVATE_KEY")]
         github_private_key: Option<String>,
         #[structopt(long, env = "SQUAWK_GITHUB_PRIVATE_KEY_BASE64")]
@@ -115,6 +122,7 @@ pub fn check_and_comment_on_pr(
     let Command::UploadToGithub {
         paths,
         exclude,
+        fail_on_violations,
         github_private_key,
         github_token,
         github_app_id,
@@ -138,7 +146,7 @@ pub fn check_and_comment_on_pr(
         return Ok(());
     }
     info!("generating github comment body");
-    let comment_body = get_comment_body(file_results, VERSION);
+    let comment_body = get_comment_body(&file_results, VERSION);
 
     if let Some(github_install_id) = github_install_id {
         if let Some(github_app_id) = github_app_id {
@@ -147,16 +155,15 @@ pub fn check_and_comment_on_pr(
                 get_github_private_key(github_private_key, github_private_key_base64)?;
             let gh = app::GitHub::new(&gh_private_key, github_app_id, github_install_id)?;
 
-            return Ok(comment_on_pr(
+            comment_on_pr(
                 &gh,
                 &github_repo_owner,
                 &github_repo_name,
                 github_pr_number,
                 &comment_body,
-            )?);
+            )?;
         }
-    }
-    if let Some(github_token) = github_token {
+    } else if let Some(github_token) = github_token {
         info!("using github actions client");
         let gh = actions::GitHub::new(&github_token);
         comment_on_pr(
@@ -167,5 +174,15 @@ pub fn check_and_comment_on_pr(
             &comment_body,
         )?;
     }
+
+    let violations: usize = file_results.iter().map(|f| f.violations.len()).sum();
+
+    if fail_on_violations && violations > 0 {
+        return Err(SquawkError::RulesViolatedError {
+            violations,
+            files: file_results.len(),
+        });
+    }
+
     Ok(())
 }
diff --git a/flake.nix b/flake.nix
@@ -18,7 +18,7 @@
           {
             squawk = final.rustPlatform.buildRustPackage {
               pname = "squawk";
-              version = "0.24.2";
+              version = "0.25.0";
 
               cargoLock = {
                 lockFile = ./Cargo.lock;

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "squawk-cli",
-  "version": "0.24.2",
+  "version": "0.25.0",
   "description": "linter for PostgreSQL, focused on migrations",
   "repository": "git@github.com:sbdchd/squawk.git",
   "author": "Steve Dignam <steve@dignam.xyz>",