Skip to content

Commit

Permalink
glob match in file finding (#352)
Browse files Browse the repository at this point in the history
We can now find files via glob matching.

```
squawk 'migrations/*.sql'
```
  • Loading branch information
chdsbd authored Jun 12, 2024
1 parent 5c18629 commit f0dba18
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 55 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]

## v1.0.0 - 2024-06-11

### Changed

- provided paths now support glob matching. `squawk 'migrations/*.sql'` (#352)

## v0.29.0 - 2024-05-30

### Added
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

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

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "squawk"
version = "0.29.0"
version = "1.0.0"
authors = ["Steve Dignam <[email protected]>"]
edition = "2018"
license = "GPL-3.0"
Expand Down
70 changes: 70 additions & 0 deletions cli/src/file_finding.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use std::path::PathBuf;

use log::info;

#[derive(Debug)]
pub enum FindFilesError {
PatternError(glob::PatternError),
GlobError(glob::GlobError),
}

impl std::fmt::Display for FindFilesError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match *self {
Self::PatternError(ref err) => {
write!(f, "Failed to build pattern: {err}")
}
Self::GlobError(ref err) => {
write!(f, "Failed to read file: {err}")
}
}
}
}

impl std::convert::From<glob::PatternError> for FindFilesError {
fn from(e: glob::PatternError) -> Self {
Self::PatternError(e)
}
}
impl std::convert::From<glob::GlobError> for FindFilesError {
fn from(e: glob::GlobError) -> Self {
Self::GlobError(e)
}
}

/// Given a list of patterns or paths, along with exclusion patterns, find matching files.
pub fn find_paths(
path_patterns: &[String],
exclude_patterns: &[String],
) -> Result<Vec<PathBuf>, FindFilesError> {
let mut matched_paths = vec![];
let exclude_paths: Vec<_> = exclude_patterns
.iter()
.map(|x| glob::Pattern::new(x))
.collect::<Result<_, _>>()?;
for path in path_patterns
.iter()
.flat_map(|x| glob::glob(x))
.flatten()
.collect::<Result<Vec<_>, _>>()?
{
if path.is_dir() {
info!("skipping directory path: {}", path.display());
continue;
}
if let Some(pattern) = exclude_paths
.iter()
.find(|&excluded| excluded.matches(path.to_str().unwrap()))
{
info!(
"skipping excluded file path: {}. pattern: {}",
path.display(),
pattern
);

continue;
}
matched_paths.push(path);
}
Ok(matched_paths)
}
40 changes: 21 additions & 19 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,18 @@
#[allow(clippy::enum_variant_names)]
#[allow(clippy::module_name_repetitions)]
mod config;
mod file_finding;
mod reporter;
mod subcommand;

use crate::file_finding::find_paths;
use crate::reporter::{
check_files, dump_ast_for_paths, explain_rule, list_rules, print_violations, DumpAstOption,
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 @@ -38,9 +39,9 @@ fn exit<E: std::fmt::Display, T>(res: Result<T, E>, msg: &str) -> ! {
#[allow(clippy::struct_excessive_bools)]
#[derive(StructOpt, Debug)]
struct Opt {
/// Paths to search
/// Paths or patterns to search
#[structopt(value_name = "path")]
paths: Vec<String>,
path_patterns: Vec<String>,
/// Paths to exclude
///
/// For example:
Expand Down Expand Up @@ -132,16 +133,6 @@ fn main() {
} 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 @@ -168,33 +159,44 @@ fn main() {
let mut handle = stdout.lock();

let is_stdin = !atty::is(Stream::Stdin);

let found_paths = find_paths(&opts.path_patterns, &excluded_paths).unwrap_or_else(|e| {
eprintln!("Failed to find files: {e}");
process::exit(1);
});
if found_paths.is_empty() && !opts.path_patterns.is_empty() {
eprintln!(
"Failed to find files for provided patterns: {:?}",
opts.path_patterns
);
process::exit(1);
}
if let Some(subcommand) = opts.cmd {
exit(
check_and_comment_on_pr(
subcommand,
is_stdin,
opts.stdin_filepath,
&excluded_rules,
&excluded_path_patterns,
&excluded_paths,
pg_version,
assume_in_transaction,
),
"Upload to GitHub failed",
);
} else if !opts.paths.is_empty() || is_stdin {
let read_stdin = opts.paths.is_empty() && is_stdin;
} else if !found_paths.is_empty() || is_stdin {
let read_stdin = found_paths.is_empty() && is_stdin;
if let Some(dump_ast_kind) = opts.dump_ast {
exit(
dump_ast_for_paths(&mut handle, &opts.paths, read_stdin, &dump_ast_kind),
dump_ast_for_paths(&mut handle, &found_paths, read_stdin, &dump_ast_kind),
"Failed to dump AST",
);
} else {
match check_files(
&opts.paths,
&found_paths,
read_stdin,
opts.stdin_filepath,
&excluded_rules,
&excluded_path_patterns,
pg_version,
assume_in_transaction,
) {
Expand Down
39 changes: 14 additions & 25 deletions cli/src/reporter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use console::strip_ansi_codes;
use console::style;
use glob::Pattern;
use log::info;
use serde::Serialize;
use squawk_linter::errors::CheckSqlError;
Expand All @@ -13,10 +12,11 @@ use std::convert::TryFrom;
use std::fs::File;
use std::io;
use std::io::prelude::*;
use std::path::PathBuf;
use structopt::clap::arg_enum;
use structopt::StructOpt;

fn get_sql_from_path(path: &str) -> Result<String, std::io::Error> {
fn get_sql_from_path(path: &PathBuf) -> Result<String, std::io::Error> {
let mut file = File::open(path)?;
let mut contents = String::new();
file.read_to_string(&mut contents).map(|_| contents)
Expand Down Expand Up @@ -68,7 +68,7 @@ impl std::convert::From<serde_json::error::Error> for DumpAstError {

pub fn dump_ast_for_paths<W: io::Write>(
f: &mut W,
paths: &[String],
paths: &[PathBuf],
read_stdin: bool,
dump_ast: &DumpAstOption,
) -> Result<(), DumpAstError> {
Expand Down Expand Up @@ -161,11 +161,10 @@ fn process_violations(
}

pub fn check_files(
paths: &[String],
path_patterns: &[PathBuf],
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 @@ -189,26 +188,16 @@ pub fn check_files(
}
}

for path in paths {
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,
));
}
for path in path_patterns {
info!("checking file path: {}", path.display());
let sql = get_sql_from_path(path)?;
output_violations.push(process_violations(
&sql,
path.to_str().unwrap(),
excluded_rules,
pg_version,
assume_in_transaction,
));
}
Ok(output_violations)
}
Expand Down
22 changes: 17 additions & 5 deletions cli/src/subcommand.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::reporter::{check_files, get_comment_body, CheckFilesError};
use crate::{
file_finding::{find_paths, FindFilesError},
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 All @@ -11,6 +13,7 @@ const VERSION: &str = env!("CARGO_PKG_VERSION");
#[derive(Debug)]
pub enum SquawkError {
CheckFilesError(CheckFilesError),
FindFilesError(FindFilesError),
GithubError(GithubError),
GithubPrivateKeyBase64DecodeError(base64::DecodeError),
GithubPrivateKeyDecodeError(std::string::FromUtf8Error),
Expand All @@ -25,6 +28,9 @@ impl std::fmt::Display for SquawkError {
Self::CheckFilesError(ref err) => {
write!(f, "Failed to dump AST: {err}")
}
Self::FindFilesError(ref err) => {
write!(f, "Failed to find files: {err}")
}
Self::GithubError(ref err) => err.fmt(f),
Self::GithubPrivateKeyBase64DecodeError(ref err) => write!(
f,
Expand Down Expand Up @@ -65,6 +71,11 @@ impl std::convert::From<CheckFilesError> for SquawkError {
Self::CheckFilesError(e)
}
}
impl std::convert::From<FindFilesError> for SquawkError {
fn from(e: FindFilesError) -> Self {
Self::FindFilesError(e)
}
}

#[derive(StructOpt, Debug)]
pub enum Command {
Expand Down Expand Up @@ -158,7 +169,7 @@ pub fn check_and_comment_on_pr(
is_stdin: bool,
stdin_path: Option<String>,
root_cmd_exclude: &[RuleViolationKind],
root_cmd_exclude_paths: &[Pattern],
root_cmd_exclude_paths: &[String],
pg_version: Option<Version>,
assume_in_transaction: bool,
) -> Result<(), SquawkError> {
Expand All @@ -184,13 +195,14 @@ pub fn check_and_comment_on_pr(
github_private_key_base64,
)?;

let found_paths = find_paths(&paths, root_cmd_exclude_paths)?;

info!("checking files");
let file_results = check_files(
&paths,
&found_paths,
is_stdin,
stdin_path,
&concat(&exclude.unwrap_or_default(), root_cmd_exclude),
root_cmd_exclude_paths,
pg_version,
assume_in_transaction,
)?;
Expand Down
4 changes: 2 additions & 2 deletions docs/docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ title: CLI

```bash
# lint a file or multiple
squawk migration_001.sql migration_002.sql migration_003.sql
squawk migration_001.sql migration_002.sql migration_003.sql 'migrations/*.sql'

# lint from standard in
cat migration.sql | squawk
Expand All @@ -26,7 +26,7 @@ squawk --exclude=adding-field-with-default,disallowed-unique-constraint example.
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 --exclude-path=005_user_ids.sql --exclude-path='*user_ids.sql' 'migrations/*.sql'
```

## `.squawk.toml` configuration file
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.29.0";
version = "1.0.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.29.0",
"version": "1.0.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 f0dba18

Please sign in to comment.