Skip to content

Commit

Permalink
feat: add a flag & config for skipping path glob patterns (#350)
Browse files Browse the repository at this point in the history
Adding a CLI flag as well as a field in the config file for skipping paths using glob patterns.  This is useful for introducing the linter on an already live service which had linter warnings on previous migrations.
  • Loading branch information
rdaniels6813 authored May 31, 2024
1 parent fa0ec26 commit d2a8e26
Show file tree
Hide file tree
Showing 16 changed files with 127 additions and 17 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## v0.29.0 - 2024-05-30

### Added

- added `--excluded-paths` flag and `excluded_paths = ["example.sql"]` configuration option to ignore paths when searching for files (#350). Thanks @rdaniels6813!

## v0.28.0 - 2024-01-12

### Changed
Expand Down
7 changes: 4 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ OPTIONS:
--dump-ast <ast-format>
Output AST in JSON [possible values: Raw, Parsed, Debug]
--exclude-path <excluded-path>...
Paths to exclude
For example: --exclude-path=005_user_ids.sql --exclude-path=009_account_emails.sql
--exclude-path='*user_ids.sql'
-e, --exclude <rule>...
Exclude specific warnings
Expand Down
3 changes: 2 additions & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "squawk"
version = "0.28.0"
version = "0.29.0"
authors = ["Steve Dignam <[email protected]>"]
edition = "2018"
license = "GPL-3.0"
Expand All @@ -27,6 +27,7 @@ squawk-parser = { version = "0.0.0", path = "../parser" }
squawk-linter = { version = "0.0.0", path = "../linter" }
squawk-github = { version = "0.0.0", path = "../github" }
toml = "0.5.9"
glob = "0.3.1"

[dev-dependencies]
insta = "0.16.0"
13 changes: 13 additions & 0 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ impl std::fmt::Display for ConfigError {

#[derive(Debug, Default, Deserialize)]
pub struct Config {
#[serde(default)]
pub excluded_paths: Vec<String>,
#[serde(default)]
pub excluded_rules: Vec<RuleViolationKind>,
#[serde(default)]
Expand Down Expand Up @@ -98,6 +100,7 @@ mod test_config {
let squawk_toml = NamedTempFile::new().expect("generate tempFile");
let file = r#"
pg_version = "19.1"
excluded_paths = ["example.sql"]
excluded_rules = ["require-concurrent-index-creation"]
assume_in_transaction = true
Expand Down Expand Up @@ -126,6 +129,16 @@ excluded_rules = ["require-concurrent-index-creation"]
assert_debug_snapshot!(Config::parse(Some(squawk_toml.path().to_path_buf())));
}
#[test]
fn test_load_excluded_paths() {
let squawk_toml = NamedTempFile::new().expect("generate tempFile");
let file = r#"
excluded_paths = ["example.sql"]
"#;
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_assume_in_transaction() {
let squawk_toml = NamedTempFile::new().expect("generate tempFile");
let file = r#"
Expand Down
28 changes: 28 additions & 0 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::reporter::{
use crate::subcommand::{check_and_comment_on_pr, Command};
use atty::Stream;
use config::Config;
use glob::Pattern;
use log::info;
use simplelog::CombinedLogger;
use squawk_linter::versions::Version;
Expand All @@ -40,6 +41,14 @@ struct Opt {
/// Paths to search
#[structopt(value_name = "path")]
paths: Vec<String>,
/// Paths to exclude
///
/// For example:
/// --exclude-path=005_user_ids.sql --exclude-path=009_account_emails.sql
///
/// --exclude-path='*user_ids.sql'
#[structopt(long = "exclude-path")]
excluded_path: Option<Vec<String>>,
/// Exclude specific warnings
///
/// For example:
Expand Down Expand Up @@ -117,6 +126,22 @@ fn main() {
conf.excluded_rules
};

// 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
};
let excluded_path_patterns = excluded_paths
.iter()
.map(|excluded_path| {
Pattern::new(excluded_path).unwrap_or_else(|e| {
eprintln!("Pattern error: {e}");
process::exit(1);
})
})
.collect::<Vec<Pattern>>();

let pg_version = if let Some(pg_version) = opts.pg_version {
Some(pg_version)
} else {
Expand All @@ -135,6 +160,7 @@ fn main() {

info!("pg version: {:?}", pg_version);
info!("excluded rules: {:?}", &excluded_rules);
info!("excluded paths: {:?}", &excluded_paths);
info!("assume in a transaction: {:?}", assume_in_transaction);

let mut clap_app = Opt::clap();
Expand All @@ -149,6 +175,7 @@ fn main() {
is_stdin,
opts.stdin_filepath,
&excluded_rules,
&excluded_path_patterns,
pg_version,
assume_in_transaction,
),
Expand All @@ -167,6 +194,7 @@ fn main() {
read_stdin,
opts.stdin_filepath,
&excluded_rules,
&excluded_path_patterns,
pg_version,
assume_in_transaction,
) {
Expand Down
32 changes: 22 additions & 10 deletions cli/src/reporter.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use console::strip_ansi_codes;
use console::style;
use glob::Pattern;
use log::info;
use serde::Serialize;
use squawk_linter::errors::CheckSqlError;
Expand Down Expand Up @@ -164,6 +165,7 @@ pub fn check_files(
read_stdin: bool,
stdin_path: Option<String>,
excluded_rules: &[RuleViolationKind],
excluded_paths: &[Pattern],
pg_version: Option<Version>,
assume_in_transaction: bool,
) -> Result<Vec<ViolationContent>, CheckFilesError> {
Expand All @@ -188,15 +190,25 @@ pub fn check_files(
}

for path in paths {
info!("checking file path: {}", path);
let sql = get_sql_from_path(path)?;
output_violations.push(process_violations(
&sql,
path,
excluded_rules,
pg_version,
assume_in_transaction,
));
if let Some(pattern) = excluded_paths
.iter()
.find(|&excluded| excluded.matches(path))
{
info!(
"skipping excluded file path: {}. pattern: {}",
path, pattern
);
} else {
info!("checking file path: {}", path);
let sql = get_sql_from_path(path)?;
output_violations.push(process_violations(
&sql,
path,
excluded_rules,
pg_version,
assume_in_transaction,
));
}
}
Ok(output_violations)
}
Expand Down Expand Up @@ -520,7 +532,7 @@ pub fn get_comment_body(files: &[ViolationContent], version: &str) -> String {
violation_count = violations_count,
file_count = files.len(),
sql_file_content = files
.into_iter()
.iter()
.filter_map(|x| get_sql_file_content(x).ok())
.collect::<Vec<String>>()
.join("\n"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
---
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: Some(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
---
source: cli/src/config.rs
expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"

---
Ok(
Some(
Config {
excluded_paths: [
"example.sql",
],
excluded_rules: [
RequireConcurrentIndexCreation,
],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: cli/src/config.rs
expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"

---
Ok(
Some(
Config {
excluded_paths: [
"example.sql",
],
excluded_rules: [],
pg_version: None,
assume_in_transaction: None,
},
),
)
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
---
source: cli/src/config.rs
expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"

---
Ok(
Some(
Config {
excluded_paths: [],
excluded_rules: [
RequireConcurrentIndexCreation,
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
---
source: cli/src/config.rs
expression: "Config::parse(Some(squawk_toml.path().to_path_buf()))"

---
Ok(
Some(
Config {
excluded_paths: [],
excluded_rules: [],
pg_version: Some(
Version {
Expand Down
3 changes: 3 additions & 0 deletions cli/src/subcommand.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::reporter::{check_files, get_comment_body, CheckFilesError};

use glob::Pattern;
use log::info;
use squawk_github::{actions, app, comment_on_pr, GitHubApi, GithubError};
use squawk_linter::{versions::Version, violations::RuleViolationKind};
Expand Down Expand Up @@ -157,6 +158,7 @@ pub fn check_and_comment_on_pr(
is_stdin: bool,
stdin_path: Option<String>,
root_cmd_exclude: &[RuleViolationKind],
root_cmd_exclude_paths: &[Pattern],
pg_version: Option<Version>,
assume_in_transaction: bool,
) -> Result<(), SquawkError> {
Expand Down Expand Up @@ -188,6 +190,7 @@ pub fn check_and_comment_on_pr(
is_stdin,
stdin_path,
&concat(&exclude.unwrap_or_default(), root_cmd_exclude),
root_cmd_exclude_paths,
pg_version,
assume_in_transaction,
)?;
Expand Down
14 changes: 13 additions & 1 deletion docs/docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ Individual rules can be disabled via the `--exclude` flag
squawk --exclude=adding-field-with-default,disallowed-unique-constraint example.sql
```

## files

Files can be excluded from linting via the `--exclude-path` flag. Glob matching is supported and the flag can be provided multiple times.

```shell
squawk --exclude-path=005_user_ids.sql --exclude-path='*user_ids.sql' migrations/*
```

## `.squawk.toml` configuration file

Rules can be disabled with a configuration file.
Expand All @@ -31,7 +39,7 @@ By default, Squawk will traverse up from the current directory to find a `.squaw
squawk --config=~/.squawk.toml example.sql
```

The `--exclude` and `--pg-version` flags will always be prioritized over the configuration file.
The `--exclude`, `--exclude-path`, and `--pg-version` flags will always be prioritized over the configuration file.


## example `.squawk.toml` configurations
Expand Down Expand Up @@ -70,6 +78,10 @@ excluded_rules = [
"require-concurrent-index-deletion",
]
assume_in_transaction = true
excluded_paths = [
"005_user_ids.sql",
"*user_ids.sql",
]
```


Expand Down
2 changes: 1 addition & 1 deletion flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
{
squawk = final.rustPlatform.buildRustPackage {
pname = "squawk";
version = "0.28.0";
version = "0.29.0";

cargoLock = {
lockFile = ./Cargo.lock;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "squawk-cli",
"version": "0.28.0",
"version": "0.29.0",
"description": "linter for PostgreSQL, focused on migrations",
"repository": "[email protected]:sbdchd/squawk.git",
"author": "Steve Dignam <[email protected]>",
Expand Down

0 comments on commit d2a8e26

Please sign in to comment.