From 3a066b89a8c4ea0c7a45a74007b824f7c418673e Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Fri, 10 Oct 2025 00:47:10 -0400 Subject: [PATCH 1/3] internal: refactor cli commands --- crates/squawk/src/cmd.rs | 89 ++++++++++++++++++++++++++++ crates/squawk/src/debug.rs | 45 ++++++++------- crates/squawk/src/github.rs | 21 +++---- crates/squawk/src/main.rs | 76 ++++++++---------------- crates/squawk/src/reporter.rs | 105 +++++++++++++++------------------- 5 files changed, 195 insertions(+), 141 deletions(-) create mode 100644 crates/squawk/src/cmd.rs diff --git a/crates/squawk/src/cmd.rs b/crates/squawk/src/cmd.rs new file mode 100644 index 00000000..29ddd67c --- /dev/null +++ b/crates/squawk/src/cmd.rs @@ -0,0 +1,89 @@ +use std::{path::PathBuf, process}; + +use crate::{ + Command, config::Config, debug::DebugArgs, file_finding::find_paths, reporter::LintArgs, +}; + +pub(crate) struct Stdin { + pub(crate) path: Option, +} + +pub(crate) enum Input { + Stdin(Stdin), + Paths(Vec), +} + +pub(crate) enum Cmd { + Debug(DebugArgs), + Lint(LintArgs), + Help, + None, + Server, + UploadToGithub(Config), +} + +impl Cmd { + fn resolve_cli(conf: Config) -> Cmd { + // TODO: do we need to do the same thing for the github command? + let found_paths = + find_paths(&conf.path_patterns, &conf.excluded_paths).unwrap_or_else(|e| { + eprintln!("Failed to find files: {e}"); + process::exit(1); + }); + if found_paths.is_empty() && !conf.path_patterns.is_empty() { + eprintln!( + "Failed to find files for provided patterns: {:?}", + conf.path_patterns + ); + if !conf.no_error_on_unmatched_pattern { + process::exit(1); + } + } + if !found_paths.is_empty() || conf.is_stdin { + let read_stdin = found_paths.is_empty() && conf.is_stdin; + let input = if read_stdin { + Input::Stdin(Stdin { + path: conf.stdin_filepath, + }) + } else { + Input::Paths(found_paths) + }; + if let Some(debug_option) = conf.debug { + return Cmd::Debug(DebugArgs { + input, + debug_option, + verbose: conf.verbose, + }); + } else { + return Cmd::Lint(LintArgs { + input, + excluded_rules: conf.excluded_rules, + pg_version: conf.pg_version, + assume_in_transaction: conf.assume_in_transaction, + reporter: conf.reporter, + github_annotations: conf.github_annotations, + }); + } + } else if !conf.no_error_on_unmatched_pattern { + return Cmd::Help; + } else { + return Cmd::None; + } + } +} + +impl Cmd { + pub(crate) fn from(opts: crate::Opts) -> Cmd { + match opts.cmd { + Some(Command::Server) => Cmd::Server, + Some(Command::UploadToGithub(_)) => { + let conf = Config::from(opts); + Cmd::UploadToGithub(conf) + } + None => { + let conf = Config::from(opts); + Cmd::resolve_cli(conf) + } + } + } +} diff --git a/crates/squawk/src/debug.rs b/crates/squawk/src/debug.rs index 0bbbda17..4e0d1745 100644 --- a/crates/squawk/src/debug.rs +++ b/crates/squawk/src/debug.rs @@ -1,4 +1,4 @@ -use std::{io, path::PathBuf}; +use std::io; use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle}; use anyhow::Result; @@ -7,23 +7,24 @@ use squawk_syntax::{ast::AstNode, syntax_error::SyntaxError}; use crate::{ DebugOption, + cmd::Input, file::{sql_from_path, sql_from_stdin}, }; -pub(crate) fn debug( - f: &mut W, - paths: &[PathBuf], - read_stdin: bool, - debug_option: &DebugOption, - verbose: bool, -) -> Result<()> { +pub(crate) struct DebugArgs { + pub(crate) input: Input, + pub(crate) debug_option: DebugOption, + pub(crate) verbose: bool, +} + +pub(crate) fn debug(f: &mut W, args: DebugArgs) -> Result<()> { let process_dump_ast = |sql: &str, filename: &str, f: &mut W| -> Result<()> { - match debug_option { + match args.debug_option { DebugOption::Lex => { let tokens = squawk_lexer::tokenize(sql); let mut start = 0; for token in tokens { - if verbose { + if args.verbose { let content = &sql[start as usize..(start + token.len) as usize]; start += token.len; writeln!(f, "{content:?} @ {:?}", token.kind)?; @@ -34,7 +35,7 @@ pub(crate) fn debug( } DebugOption::Parse => { let parse = squawk_syntax::SourceFile::parse(sql); - if verbose { + if args.verbose { writeln!(f, "{}\n---", parse.syntax_node())?; } writeln!(f, "{:#?}", parse.syntax_node())?; @@ -65,16 +66,20 @@ pub(crate) fn debug( } Ok(()) }; - if read_stdin { - let sql = sql_from_stdin()?; - process_dump_ast(&sql, "stdin", f)?; - return Ok(()); - } - for path in paths { - let sql = sql_from_path(path)?; - process_dump_ast(&sql, &path.to_string_lossy(), f)?; - } + match args.input { + Input::Stdin(_) => { + let sql = sql_from_stdin()?; + process_dump_ast(&sql, "stdin", f)?; + } + Input::Paths(path_bufs) => { + for path in path_bufs { + let sql = sql_from_path(&path)?; + process_dump_ast(&sql, &path.to_string_lossy(), f)?; + } + } + }; + Ok(()) } diff --git a/crates/squawk/src/github.rs b/crates/squawk/src/github.rs index 56730ebc..627c13f6 100644 --- a/crates/squawk/src/github.rs +++ b/crates/squawk/src/github.rs @@ -1,7 +1,8 @@ -use crate::UploadToGithubArgs; +use crate::cmd::Input; use crate::config::Config; use crate::reporter::{CheckReport, fmt_github_annotations, fmt_tty_violation}; -use crate::{file_finding::find_paths, reporter::check_files}; +use crate::{LintArgs, UploadToGithubArgs}; +use crate::{file_finding::find_paths, reporter::lint_files}; use anyhow::{Context, Result, anyhow, bail}; use console::strip_ansi_codes; use log::info; @@ -117,14 +118,14 @@ pub fn check_and_comment_on_pr(cfg: Config) -> Result<()> { let found_paths = find_paths(&paths, &cfg.excluded_paths)?; info!("checking files"); - let file_results = check_files( - &found_paths, - cfg.is_stdin, - &cfg.stdin_filepath, - &cfg.excluded_rules, - cfg.pg_version, - cfg.assume_in_transaction, - )?; + let file_results = lint_files(&LintArgs { + input: Input::Paths(found_paths), + excluded_rules: cfg.excluded_rules, + pg_version: cfg.pg_version, + assume_in_transaction: cfg.assume_in_transaction, + reporter: cfg.reporter, + github_annotations: cfg.github_annotations, + })?; // We should only leave a comment when there are files checked. if paths.is_empty() { diff --git a/crates/squawk/src/main.rs b/crates/squawk/src/main.rs index 8025ea59..48d56bf3 100644 --- a/crates/squawk/src/main.rs +++ b/crates/squawk/src/main.rs @@ -1,21 +1,22 @@ +mod cmd; mod config; mod debug; mod file; mod file_finding; mod github; mod reporter; -use crate::config::Config; -use crate::file_finding::find_paths; +use crate::cmd::Cmd; +use crate::reporter::LintArgs; use anyhow::{Context, Result}; use clap::{CommandFactory, Parser, Subcommand, ValueEnum}; use debug::debug; -use reporter::check_and_dump_files; +use reporter::lint_and_report; use simplelog::CombinedLogger; use squawk_linter::{Rule, Version}; use std::io; use std::panic; use std::path::PathBuf; -use std::process::{self, ExitCode}; +use std::process::ExitCode; #[derive(Parser, Debug)] pub struct UploadToGithubArgs { @@ -182,58 +183,29 @@ Please open an issue at https://github.com/sbdchd/squawk/issues/new with the log .expect("problem creating logger"); } - match opts.cmd { - Some(Command::Server) => { + match Cmd::from(opts) { + Cmd::Server => { squawk_server::run().context("language server failed")?; } - Some(Command::UploadToGithub(_)) => { - let conf = Config::from(opts); - github::check_and_comment_on_pr(conf).context("Upload to GitHub failed")?; + Cmd::UploadToGithub(config) => { + github::check_and_comment_on_pr(config).context("Upload to GitHub failed")?; } - None => { - let conf = Config::from(opts); - // TODO: do we need to do the same thing for the github command? - let found_paths = - find_paths(&conf.path_patterns, &conf.excluded_paths).unwrap_or_else(|e| { - eprintln!("Failed to find files: {e}"); - process::exit(1); - }); - if found_paths.is_empty() && !conf.path_patterns.is_empty() { - eprintln!( - "Failed to find files for provided patterns: {:?}", - conf.path_patterns - ); - if !conf.no_error_on_unmatched_pattern { - process::exit(1); - } - } - if !found_paths.is_empty() || conf.is_stdin { - let stdout = io::stdout(); - let mut handle = stdout.lock(); - - let read_stdin = found_paths.is_empty() && conf.is_stdin; - if let Some(kind) = conf.debug { - debug(&mut handle, &found_paths, read_stdin, &kind, conf.verbose)?; - } else { - let reporter = conf.reporter; - let exit_code = check_and_dump_files( - &mut handle, - &found_paths, - read_stdin, - conf.stdin_filepath, - &conf.excluded_rules, - conf.pg_version, - conf.assume_in_transaction, - &reporter, - conf.github_annotations, - )?; - return Ok(exit_code); - } - } else if !conf.no_error_on_unmatched_pattern { - Opts::command().print_long_help()?; - println!(); - } + Cmd::Debug(debug_args) => { + let stdout = io::stdout(); + let mut handle = stdout.lock(); + debug(&mut handle, debug_args)?; + } + Cmd::Lint(lint_args) => { + let stdout = io::stdout(); + let mut handle = stdout.lock(); + return lint_and_report(&mut handle, lint_args); } + Cmd::Help => { + Opts::command().print_long_help()?; + println!(); + } + Cmd::None => (), } + Ok(ExitCode::SUCCESS) } diff --git a/crates/squawk/src/reporter.rs b/crates/squawk/src/reporter.rs index 90a6329d..88197423 100644 --- a/crates/squawk/src/reporter.rs +++ b/crates/squawk/src/reporter.rs @@ -11,9 +11,9 @@ use std::hash::DefaultHasher; use std::hash::Hash; use std::hash::Hasher; use std::io; -use std::path::PathBuf; use std::process::ExitCode; +use crate::cmd::Input; use crate::{ Reporter, file::{sql_from_path, sql_from_stdin}, @@ -128,73 +128,60 @@ fn render_lint_error( Ok(()) } -pub fn check_files( - path_patterns: &[PathBuf], - read_stdin: bool, - stdin_path: &Option, - excluded_rules: &[Rule], - pg_version: Option, - assume_in_transaction: bool, -) -> Result> { +pub(crate) struct LintArgs { + pub(crate) input: Input, + pub(crate) excluded_rules: Vec, + pub(crate) pg_version: Option, + pub(crate) assume_in_transaction: bool, + pub(crate) reporter: Reporter, + pub(crate) github_annotations: bool, +} + +pub fn lint_files(args: &LintArgs) -> Result> { let mut violations = vec![]; - if read_stdin { - info!("reading content from stdin"); - let sql = sql_from_stdin()?; - // ignore stdin if it's empty. - if sql.trim().is_empty() { - info!("ignoring empty stdin"); - } else { - let path = stdin_path.clone().unwrap_or_else(|| "stdin".into()); - let content = check_sql( - &sql, - &path, - excluded_rules, - pg_version, - assume_in_transaction, - ); - violations.push(content); + match &args.input { + Input::Stdin(stdin) => { + info!("reading content from stdin"); + let sql = sql_from_stdin()?; + // ignore stdin if it's empty. + if sql.trim().is_empty() { + info!("ignoring empty stdin"); + } else { + let path = stdin.path.clone().unwrap_or_else(|| "stdin".into()); + let content = check_sql( + &sql, + &path, + &args.excluded_rules, + args.pg_version, + args.assume_in_transaction, + ); + violations.push(content); + } + } + Input::Paths(path_bufs) => { + for path in path_bufs { + info!("checking file path: {}", path.display()); + let sql = sql_from_path(path)?; + let content = check_sql( + &sql, + path.to_str().unwrap(), + &args.excluded_rules, + args.pg_version, + args.assume_in_transaction, + ); + violations.push(content); + } } } - - for path in path_patterns { - info!("checking file path: {}", path.display()); - let sql = sql_from_path(path)?; - let content = check_sql( - &sql, - path.to_str().unwrap(), - excluded_rules, - pg_version, - assume_in_transaction, - ); - violations.push(content); - } - Ok(violations) } -pub fn check_and_dump_files( - f: &mut W, - path_patterns: &[PathBuf], - read_stdin: bool, - stdin_path: Option, - excluded_rules: &[Rule], - pg_version: Option, - assume_in_transaction: bool, - reporter: &Reporter, - github_annotations: bool, -) -> Result { - let violations = check_files( - path_patterns, - read_stdin, - &stdin_path, - excluded_rules, - pg_version, - assume_in_transaction, - )?; +pub fn lint_and_report(f: &mut W, args: LintArgs) -> Result { + let violations = lint_files(&args)?; let ok = violations.iter().map(|x| x.violations.len()).sum::() == 0; - print_violations(f, violations, reporter, github_annotations)?; + print_violations(f, violations, &args.reporter, args.github_annotations)?; Ok(if ok { ExitCode::SUCCESS From 764f4806cc3b59bf9f4b295b1869085ad85fce98 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Fri, 10 Oct 2025 01:15:37 -0400 Subject: [PATCH 2/3] nit --- crates/squawk/src/cmd.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/squawk/src/cmd.rs b/crates/squawk/src/cmd.rs index 29ddd67c..0c8a87de 100644 --- a/crates/squawk/src/cmd.rs +++ b/crates/squawk/src/cmd.rs @@ -70,9 +70,7 @@ impl Cmd { return Cmd::None; } } -} -impl Cmd { pub(crate) fn from(opts: crate::Opts) -> Cmd { match opts.cmd { Some(Command::Server) => Cmd::Server, From 4aa210d7a6fce2c4c18a9dd60cf20dd43b2d8fb1 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Fri, 10 Oct 2025 01:16:30 -0400 Subject: [PATCH 3/3] lint --- crates/squawk/src/cmd.rs | 4 ++-- crates/squawk/src/main.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/squawk/src/cmd.rs b/crates/squawk/src/cmd.rs index 0c8a87de..f2eaece7 100644 --- a/crates/squawk/src/cmd.rs +++ b/crates/squawk/src/cmd.rs @@ -19,7 +19,7 @@ pub(crate) enum Cmd { Help, None, Server, - UploadToGithub(Config), + UploadToGithub(Box), } impl Cmd { @@ -76,7 +76,7 @@ impl Cmd { Some(Command::Server) => Cmd::Server, Some(Command::UploadToGithub(_)) => { let conf = Config::from(opts); - Cmd::UploadToGithub(conf) + Cmd::UploadToGithub(Box::new(conf)) } None => { let conf = Config::from(opts); diff --git a/crates/squawk/src/main.rs b/crates/squawk/src/main.rs index 48d56bf3..af00cf6b 100644 --- a/crates/squawk/src/main.rs +++ b/crates/squawk/src/main.rs @@ -188,7 +188,7 @@ Please open an issue at https://github.com/sbdchd/squawk/issues/new with the log squawk_server::run().context("language server failed")?; } Cmd::UploadToGithub(config) => { - github::check_and_comment_on_pr(config).context("Upload to GitHub failed")?; + github::check_and_comment_on_pr(*config).context("Upload to GitHub failed")?; } Cmd::Debug(debug_args) => { let stdout = io::stdout();