support more options in upload-to-github subcommand (#353)

- support configuration for fail_on_violations - support parent flags in upload-to-github command
sbdchd · Jun 13, 2024 · a7d59a9 · a7d59a9
1 parent f0dba18
commit a7d59a9
Show file tree

Hide file tree

Showing 15 changed files with 95 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## v1.1.0 - 2024-06-13
+
+### Changed
+
+- support configuration for "fail_on_violations" via `upload_to_github.fail_on_violations` (#353)
+- support root flags within upload-to-github subcommand (#353)
+
 ## v1.0.0 - 2024-06-11
 
 ### Changed

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 = "1.0.0"
+version = "1.1.0"
 authors = ["Steve Dignam <steve@dignam.xyz>"]
 edition = "2018"
 license = "GPL-3.0"

diff --git a/cli/src/config.rs b/cli/src/config.rs
@@ -36,6 +36,12 @@ impl std::fmt::Display for ConfigError {
     }
 }
 
+#[derive(Debug, Default, Deserialize)]
+pub struct UploadToGitHubConfig {
+    #[serde(default)]
+    pub fail_on_violations: Option<bool>,
+}
+
 #[derive(Debug, Default, Deserialize)]
 pub struct Config {
     #[serde(default)]
@@ -46,6 +52,8 @@ pub struct Config {
     pub pg_version: Option<Version>,
     #[serde(default)]
     pub assume_in_transaction: Option<bool>,
+    #[serde(default)]
+    pub upload_to_github: UploadToGitHubConfig,
 }
 
 impl Config {
@@ -148,4 +156,14 @@ assume_in_transaction = false
         fs::write(&squawk_toml, file).expect("Unable to write file");
         assert_debug_snapshot!(Config::parse(Some(squawk_toml.path().to_path_buf())));
     }
+    #[test]
+    fn test_load_fail_on_violations() {
+        let squawk_toml = NamedTempFile::new().expect("generate tempFile");
+        let file = r#"
+[upload_to_github]
+fail_on_violations = true        
+        "#;
+        fs::write(&squawk_toml, file).expect("Unable to write file");
+        assert_debug_snapshot!(Config::parse(Some(squawk_toml.path().to_path_buf())));
+    }
 }
diff --git a/cli/src/main.rs b/cli/src/main.rs
@@ -48,7 +48,7 @@ struct Opt {
     /// --exclude-path=005_user_ids.sql --exclude-path=009_account_emails.sql
     ///
     /// --exclude-path='*user_ids.sql'
-    #[structopt(long = "exclude-path")]
+    #[structopt(long = "exclude-path", global = true)]
     excluded_path: Option<Vec<String>>,
     /// Exclude specific warnings
     ///
@@ -58,14 +58,15 @@ struct Opt {
         short = "e",
         long = "exclude",
         value_name = "rule",
-        use_delimiter = true
+        use_delimiter = true,
+        global = true
     )]
     excluded_rules: Option<Vec<RuleViolationKind>>,
     /// Specify postgres version
     ///
     /// For example:
     /// --pg-version=13.0
-    #[structopt(long)]
+    #[structopt(long, global = true)]
     pg_version: Option<Version>,
     /// List all available rules
     #[structopt(long)]
@@ -85,17 +86,22 @@ struct Opt {
     #[structopt(subcommand)]
     cmd: Option<Command>,
     /// Enable debug logging output
-    #[structopt(long)]
+    #[structopt(long, global = true)]
     verbose: bool,
     /// Path to the squawk config file (.squawk.toml)
-    #[structopt(short = "c", long = "config")]
+    #[structopt(short = "c", long = "config", global = true)]
     config_path: Option<PathBuf>,
     /// Assume that a transaction will wrap each SQL file when run by a migration tool
     ///
     /// Use --no-assume-in-transaction to override any config file that sets this
-    #[structopt(long)]
+    #[structopt(long, global = true)]
     assume_in_transaction: bool,
-    #[structopt(long, hidden = true, conflicts_with = "assume-in-transaction")]
+    #[structopt(
+        long,
+        hidden = true,
+        conflicts_with = "assume-in-transaction",
+        global = true
+    )]
     no_assume_in_transaction: bool,
 }
 
@@ -124,14 +130,14 @@ fn main() {
     let excluded_rules = if let Some(excluded_rules) = opts.excluded_rules {
         excluded_rules
     } else {
-        conf.excluded_rules
+        conf.excluded_rules.clone()
     };
 
     // the --exclude-path flag completely overrides the configuration file.
     let excluded_paths = if let Some(excluded_paths) = opts.excluded_path {
         excluded_paths
     } else {
-        conf.excluded_paths
+        conf.excluded_paths.clone()
     };
     let pg_version = if let Some(pg_version) = opts.pg_version {
         Some(pg_version)
@@ -175,6 +181,7 @@ fn main() {
         exit(
             check_and_comment_on_pr(
                 subcommand,
+                &conf,
                 is_stdin,
                 opts.stdin_filepath,
                 &excluded_rules,

diff --git a/cli/src/snapshots/squawk__config__test_config__load_assume_in_transaction.snap b/cli/src/snapshots/squawk__config__test_config__load_assume_in_transaction.snap
@@ -1,7 +1,6 @@
 ---
 source: cli/src/config.rs
 expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"
-
 ---
 Ok(
     Some(
@@ -12,6 +11,9 @@ Ok(
             assume_in_transaction: Some(
                 false,
             ),
+            upload_to_github: UploadToGitHubConfig {
+                fail_on_violations: None,
+            },
         },
     ),
 )
diff --git a/cli/src/snapshots/squawk__config__test_config__load_cfg_full.snap b/cli/src/snapshots/squawk__config__test_config__load_cfg_full.snap
@@ -1,7 +1,6 @@
 ---
 source: cli/src/config.rs
 expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"
-
 ---
 Ok(
     Some(
@@ -24,6 +23,9 @@ Ok(
             assume_in_transaction: Some(
                 true,
             ),
+            upload_to_github: UploadToGitHubConfig {
+                fail_on_violations: None,
+            },
         },
     ),
 )
diff --git a/cli/src/snapshots/squawk__config__test_config__load_excluded_paths.snap b/cli/src/snapshots/squawk__config__test_config__load_excluded_paths.snap
@@ -1,7 +1,6 @@
 ---
 source: cli/src/config.rs
 expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"
-
 ---
 Ok(
     Some(
@@ -12,6 +11,9 @@ Ok(
             excluded_rules: [],
             pg_version: None,
             assume_in_transaction: None,
+            upload_to_github: UploadToGitHubConfig {
+                fail_on_violations: None,
+            },
         },
     ),
 )
diff --git a/cli/src/snapshots/squawk__config__test_config__load_excluded_rules.snap b/cli/src/snapshots/squawk__config__test_config__load_excluded_rules.snap
@@ -1,7 +1,6 @@
 ---
 source: cli/src/config.rs
 expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"
-
 ---
 Ok(
     Some(
@@ -12,6 +11,9 @@ Ok(
             ],
             pg_version: None,
             assume_in_transaction: None,
+            upload_to_github: UploadToGitHubConfig {
+                fail_on_violations: None,
+            },
         },
     ),
 )
diff --git a/cli/src/snapshots/squawk__config__test_config__load_fail_on_violations.snap b/cli/src/snapshots/squawk__config__test_config__load_fail_on_violations.snap
@@ -0,0 +1,19 @@
+---
+source: cli/src/config.rs
+expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"
+---
+Ok(
+    Some(
+        Config {
+            excluded_paths: [],
+            excluded_rules: [],
+            pg_version: None,
+            assume_in_transaction: None,
+            upload_to_github: UploadToGitHubConfig {
+                fail_on_violations: Some(
+                    true,
+                ),
+            },
+        },
+    ),
+)
diff --git a/cli/src/snapshots/squawk__config__test_config__load_pg_version.snap b/cli/src/snapshots/squawk__config__test_config__load_pg_version.snap
@@ -1,7 +1,6 @@
 ---
 source: cli/src/config.rs
 expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"
-
 ---
 Ok(
     Some(
@@ -18,6 +17,9 @@ Ok(
                 },
             ),
             assume_in_transaction: None,
+            upload_to_github: UploadToGitHubConfig {
+                fail_on_violations: None,
+            },
         },
     ),
 )
diff --git a/cli/src/subcommand.rs b/cli/src/subcommand.rs
@@ -1,8 +1,9 @@
+#![allow(clippy::too_many_arguments)]
+use crate::config::Config;
 use crate::{
     file_finding::{find_paths, FindFilesError},
     reporter::{check_files, get_comment_body, CheckFilesError},
 };
-
 use log::info;
 use squawk_github::{actions, app, comment_on_pr, GitHubApi, GithubError};
 use squawk_linter::{versions::Version, violations::RuleViolationKind};
@@ -83,12 +84,6 @@ pub enum Command {
     UploadToGithub {
         /// Paths to search
         paths: Vec<String>,
-        /// Exclude specific warnings
-        ///
-        /// For example:
-        /// --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,
@@ -132,11 +127,6 @@ fn get_github_private_key(
     }
 }
 
-fn concat(a: &[RuleViolationKind], b: &[RuleViolationKind]) -> Vec<RuleViolationKind> {
-    // from: https://stackoverflow.com/a/53476705/3720597
-    [a, b].concat()
-}
-
 fn create_gh_app(
     github_install_id: Option<i64>,
     github_app_id: Option<i64>,
@@ -166,16 +156,16 @@ fn create_gh_app(
 
 pub fn check_and_comment_on_pr(
     cmd: Command,
+    cfg: &Config,
     is_stdin: bool,
     stdin_path: Option<String>,
-    root_cmd_exclude: &[RuleViolationKind],
-    root_cmd_exclude_paths: &[String],
+    exclude: &[RuleViolationKind],
+    exclude_paths: &[String],
     pg_version: Option<Version>,
     assume_in_transaction: bool,
 ) -> Result<(), SquawkError> {
     let Command::UploadToGithub {
         paths,
-        exclude,
         fail_on_violations,
         github_private_key,
         github_token,
@@ -187,6 +177,13 @@ pub fn check_and_comment_on_pr(
         github_private_key_base64,
     } = cmd;
 
+    let fail_on_violations =
+        if let Some(fail_on_violations_cfg) = cfg.upload_to_github.fail_on_violations {
+            fail_on_violations_cfg
+        } else {
+            fail_on_violations
+        };
+
     let github_app = create_gh_app(
         github_install_id,
         github_app_id,
@@ -195,14 +192,14 @@ pub fn check_and_comment_on_pr(
         github_private_key_base64,
     )?;
 
-    let found_paths = find_paths(&paths, root_cmd_exclude_paths)?;
+    let found_paths = find_paths(&paths, exclude_paths)?;
 
     info!("checking files");
     let file_results = check_files(
         &found_paths,
         is_stdin,
         stdin_path,
-        &concat(&exclude.unwrap_or_default(), root_cmd_exclude),
+        exclude,
         pg_version,
         assume_in_transaction,
     )?;

diff --git a/docs/docs/cli.md b/docs/docs/cli.md
@@ -82,6 +82,8 @@ excluded_paths = [
     "005_user_ids.sql",
     "*user_ids.sql",
 ]
+[upload_to_github]
+fail_on_violations = true
 ```
 
 

diff --git a/flake.nix b/flake.nix
@@ -18,7 +18,7 @@
           {
             squawk = final.rustPlatform.buildRustPackage {
               pname = "squawk";
-              version = "1.0.0";
+              version = "1.1.0";
 
               cargoLock = {
                 lockFile = ./Cargo.lock;

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