diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs b/crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs index 067e02b83..ced1d3071 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs @@ -1,10 +1,11 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use clap::Parser; use rustc_hash::FxHashSet; use crate::git::{get_added_files, get_lfs_files}; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; @@ -17,13 +18,17 @@ struct Args { enforce_all: bool, #[arg(long = "maxkb", default_value = "500")] max_kb: u64, + #[arg(value_name = "FILENAMES")] + filenames: Vec, } pub(crate) async fn check_added_large_files( hook: &Hook, filenames: &[&Path], ) -> anyhow::Result<(i32, Vec)> { - let args = Args::try_parse_from(hook.entry.expect_direct().split_with_args(&hook.args)?)?; + let args: Args = parse_hook_args(hook)?; + let all_filenames = hook_filenames(&args.filenames, filenames).collect::>(); + let filenames = all_filenames.as_slice(); let candidate_filenames; let filenames = if args.enforce_all { diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_case_conflict.rs b/crates/prek/src/hooks/pre_commit_hooks/check_case_conflict.rs index 6e68c4e25..8725731bd 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_case_conflict.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_case_conflict.rs @@ -8,11 +8,14 @@ use rustc_hash::FxHashSet; use crate::git; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; pub(crate) async fn check_case_conflict( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec)> { + let args: FilenamesArgs = parse_hook_args(hook)?; + let filenames = hook_filenames(&args.filenames, filenames).collect::>(); let work_dir = hook.work_dir(); // Get all files in the repo. @@ -25,7 +28,7 @@ pub(crate) async fn check_case_conflict( // Get relevant files (filenames + added files) and include their parent directories. let added = git::get_added_files(work_dir).await?; let mut relevant_files_with_dirs: FxHashSet<&Path> = FxHashSet::default(); - for filename in filenames { + for filename in &filenames { insert_path_and_parents(&mut relevant_files_with_dirs, filename); } for path in &added { diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_executables_have_shebangs.rs b/crates/prek/src/hooks/pre_commit_hooks/check_executables_have_shebangs.rs index 3099c3f64..8bd8bf54f 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_executables_have_shebangs.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_executables_have_shebangs.rs @@ -8,6 +8,7 @@ use crate::hook::Hook; use crate::hooks::pre_commit_hooks::shebangs::{ file_has_shebang, git_index_stage_output, matching_git_index_paths_by_executable_bit, }; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; use rustc_hash::FxHashSet; @@ -16,6 +17,8 @@ pub(crate) async fn check_executables_have_shebangs( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec), anyhow::Error> { + let args: FilenamesArgs = parse_hook_args(hook)?; + let filenames = hook_filenames(&args.filenames, filenames).collect::>(); if filenames.is_empty() { return Ok((0, Vec::new())); } @@ -34,10 +37,10 @@ pub(crate) async fn check_executables_have_shebangs( let (code, output) = if tracks_executable_bit { // core.fileMode=true means the platform honors the executable bit, so trust the FS metadata. // The `executables-have-shebangs` hook already restricts inputs to executable text files (`types: [text, executable]`). - os_check_shebangs(file_base, filenames).await? + os_check_shebangs(file_base, &filenames).await? } else { // If on win32 use git to check executable bit - git_check_shebangs(file_base, filenames).await? + git_check_shebangs(file_base, &filenames).await? }; Ok((code, output)) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_json.rs b/crates/prek/src/hooks/pre_commit_hooks/check_json.rs index 70016b9f6..24a939dc0 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_json.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_json.rs @@ -6,12 +6,14 @@ use rustc_hash::FxHashSet; use serde::{Deserialize, Deserializer}; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; pub(crate) async fn check_json(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { + let args: FilenamesArgs = parse_hook_args(hook)?; run_concurrent_file_checks( - filenames.iter().copied(), + hook_filenames(&args.filenames, filenames), *INTERNAL_CONCURRENCY, |filename| check_file(hook.project().relative_path(), filename), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_merge_conflict.rs b/crates/prek/src/hooks/pre_commit_hooks/check_merge_conflict.rs index 8d355a706..e4c119106 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_merge_conflict.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_merge_conflict.rs @@ -1,5 +1,5 @@ use std::io::Write; -use std::path::Path; +use std::path::{Path, PathBuf}; use anyhow::Result; use clap::Parser; @@ -7,6 +7,7 @@ use tokio::io::AsyncBufReadExt; use crate::git::get_git_dir; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; @@ -22,13 +23,15 @@ const SEPARATOR_PATTERNS: &[&[u8]] = &[b"======= ", b"=======\r\n", b"=======\n" struct Args { #[arg(long)] assume_in_merge: bool, + #[arg(value_name = "FILENAMES")] + filenames: Vec, } pub(crate) async fn check_merge_conflict( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec)> { - let args = Args::try_parse_from(hook.entry.expect_direct().split_with_args(&hook.args)?)?; + let args: Args = parse_hook_args(hook)?; // Check if we're in a merge state or assuming merge if !args.assume_in_merge && !is_in_merge().await? { @@ -36,7 +39,7 @@ pub(crate) async fn check_merge_conflict( } run_concurrent_file_checks( - filenames.iter().copied(), + hook_filenames(&args.filenames, filenames), *INTERNAL_CONCURRENCY, |filename| check_file(hook.project().relative_path(), filename), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_shebang_scripts_are_executable.rs b/crates/prek/src/hooks/pre_commit_hooks/check_shebang_scripts_are_executable.rs index 8d289c729..b26d8c991 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_shebang_scripts_are_executable.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_shebang_scripts_are_executable.rs @@ -7,6 +7,7 @@ use crate::hook::Hook; use crate::hooks::pre_commit_hooks::shebangs::{ file_has_shebang, git_index_stage_output, matching_git_index_paths_by_executable_bit, }; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; use rustc_hash::FxHashSet; @@ -15,13 +16,15 @@ pub(crate) async fn check_shebang_scripts_are_executable( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec), anyhow::Error> { + let args: FilenamesArgs = parse_hook_args(hook)?; + let filenames = hook_filenames(&args.filenames, filenames).collect::>(); if filenames.is_empty() { return Ok((0, Vec::new())); } let file_base = hook.project().relative_path(); let stdout = git_index_stage_output(file_base).await?; - let filenames: FxHashSet<_> = filenames.iter().copied().collect(); + let filenames: FxHashSet<_> = filenames.into_iter().collect(); let entries = matching_git_index_paths_by_executable_bit(&stdout, file_base, &filenames, false); run_concurrent_file_checks(entries, *INTERNAL_CONCURRENCY, |file| async move { diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_symlinks.rs b/crates/prek/src/hooks/pre_commit_hooks/check_symlinks.rs index 26ae066cc..37ee0de86 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_symlinks.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_symlinks.rs @@ -3,12 +3,14 @@ use std::path::Path; use anyhow::Result; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; pub(crate) async fn check_symlinks(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { + let args: FilenamesArgs = parse_hook_args(hook)?; run_concurrent_file_checks( - filenames.iter().copied(), + hook_filenames(&args.filenames, filenames), *INTERNAL_CONCURRENCY, |filename| check_file(hook.project().relative_path(), filename), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_toml.rs b/crates/prek/src/hooks/pre_commit_hooks/check_toml.rs index 767cb6942..b01b2487a 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_toml.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_toml.rs @@ -4,12 +4,14 @@ use std::path::Path; use anyhow::Result; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; pub(crate) async fn check_toml(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { + let args: FilenamesArgs = parse_hook_args(hook)?; run_concurrent_file_checks( - filenames.iter().copied(), + hook_filenames(&args.filenames, filenames), *INTERNAL_CONCURRENCY, |filename| check_file(hook.project().relative_path(), filename), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_vcs_permalinks.rs b/crates/prek/src/hooks/pre_commit_hooks/check_vcs_permalinks.rs index b74786387..94184ad63 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_vcs_permalinks.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_vcs_permalinks.rs @@ -1,6 +1,6 @@ use std::collections::BTreeSet; use std::io::{BufRead, BufReader, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::Arc; use anyhow::Result; @@ -9,6 +9,7 @@ use memchr::memmem; use regex::bytes::{Match, Regex}; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; @@ -19,6 +20,8 @@ use crate::run::INTERNAL_CONCURRENCY; struct Args { #[arg(long = "additional-github-domain")] additional_github_domains: Vec, + #[arg(value_name = "FILENAMES")] + filenames: Vec, } #[derive(Debug)] @@ -82,14 +85,14 @@ pub(crate) async fn check_vcs_permalinks( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec)> { - let args = Args::try_parse_from(hook.entry.expect_direct().split_with_args(&hook.args)?)?; + let args: Args = parse_hook_args(hook)?; let matcher = Arc::new(GithubNonPermalinkMatcher::new( args.additional_github_domains, )); let file_base = hook.project().relative_path(); run_concurrent_file_checks( - filenames.iter().copied(), + hook_filenames(&args.filenames, filenames), *INTERNAL_CONCURRENCY, |filename| check_file(file_base, filename, &matcher), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_xml.rs b/crates/prek/src/hooks/pre_commit_hooks/check_xml.rs index fe129237d..d52f40997 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_xml.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_xml.rs @@ -4,12 +4,14 @@ use anyhow::Result; use xml::reader::ParserConfig; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; pub(crate) async fn check_xml(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { + let args: FilenamesArgs = parse_hook_args(hook)?; run_concurrent_file_checks( - filenames.iter().copied(), + hook_filenames(&args.filenames, filenames), *INTERNAL_CONCURRENCY, |filename| check_file(hook.project().relative_path(), filename), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_yaml.rs b/crates/prek/src/hooks/pre_commit_hooks/check_yaml.rs index 151a941b1..cb6760a5d 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_yaml.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_yaml.rs @@ -1,10 +1,11 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use anyhow::Result; use clap::Parser; use serde::de::IgnoredAny; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; @@ -15,16 +16,18 @@ use crate::run::INTERNAL_CONCURRENCY; struct Args { #[arg(long, short = 'm', alias = "multi")] allow_multiple_documents: bool, + #[arg(value_name = "FILENAMES")] + filenames: Vec, // `--unsafe` flag is not supported yet. // #[arg(long)] // r#unsafe: bool, } pub(crate) async fn check_yaml(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { - let args = Args::try_parse_from(hook.entry.expect_direct().split_with_args(&hook.args)?)?; + let args: Args = parse_hook_args(hook)?; run_concurrent_file_checks( - filenames.iter().copied(), + hook_filenames(&args.filenames, filenames), *INTERNAL_CONCURRENCY, |filename| { check_file( diff --git a/crates/prek/src/hooks/pre_commit_hooks/destroyed_symlinks.rs b/crates/prek/src/hooks/pre_commit_hooks/destroyed_symlinks.rs index 8841fb486..2b6210bd4 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/destroyed_symlinks.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/destroyed_symlinks.rs @@ -7,12 +7,15 @@ use rustc_hash::FxHashSet; use crate::git; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; const ORDINARY_CHANGED_ENTRY_MARKER: &str = "1"; const PERMS_LINK: u32 = 0o120_000; const PERMS_NONEXIST: u32 = 0; pub(crate) async fn destroyed_symlinks(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { + let args: FilenamesArgs = parse_hook_args(hook)?; + let filenames = hook_filenames(&args.filenames, filenames).collect::>(); let status_output = git_status_output(hook.work_dir()).await?; let entries = status_output .split(|&byte| byte == b'\0') @@ -22,7 +25,7 @@ pub(crate) async fn destroyed_symlinks(hook: &Hook, filenames: &[&Path]) -> Resu Err(err) => Some(Err(err)), }); - let destroyed_links = find_destroyed_symlinks(hook, filenames, entries).await?; + let destroyed_links = find_destroyed_symlinks(hook, &filenames, entries).await?; if destroyed_links.is_empty() { return Ok((0, Vec::new())); } diff --git a/crates/prek/src/hooks/pre_commit_hooks/detect_private_key.rs b/crates/prek/src/hooks/pre_commit_hooks/detect_private_key.rs index 9c3ff27fe..981231335 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/detect_private_key.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/detect_private_key.rs @@ -6,6 +6,7 @@ use aho_corasick::AhoCorasick; use anyhow::Result; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; use crate::hooks::run_concurrent_file_checks; use crate::run::INTERNAL_CONCURRENCY; @@ -42,8 +43,9 @@ static PRIVATE_KEY_MATCHER: LazyLock = LazyLock::new(|| { }); pub(crate) async fn detect_private_key(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { + let args: FilenamesArgs = parse_hook_args(hook)?; run_concurrent_file_checks( - filenames.iter().copied(), + hook_filenames(&args.filenames, filenames), *INTERNAL_CONCURRENCY, |filename| check_file(hook.project().relative_path(), filename), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/file_contents_sorter.rs b/crates/prek/src/hooks/pre_commit_hooks/file_contents_sorter.rs index b47463a4d..c974852c5 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/file_contents_sorter.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/file_contents_sorter.rs @@ -1,11 +1,11 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use anyhow::Result; use bstr::ByteSlice; use clap::Parser; use crate::hook::Hook; -use crate::hooks::run_concurrent_file_checks; +use crate::hooks::pre_commit_hooks::{parse_hook_args, run_file_checks}; use crate::run::INTERNAL_CONCURRENCY; #[derive(Parser)] @@ -17,17 +17,20 @@ struct Args { ignore_case: bool, #[arg(long, conflicts_with = "ignore_case")] unique: bool, + #[arg(value_name = "FILENAMES")] + filenames: Vec, } pub(crate) async fn file_contents_sorter( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec)> { - let args = Args::try_parse_from(hook.entry.expect_direct().split_with_args(&hook.args)?)?; + let args: Args = parse_hook_args(hook)?; let file_base = hook.project().relative_path(); - run_concurrent_file_checks( - filenames.iter().copied(), + run_file_checks( + &args.filenames, + filenames, *INTERNAL_CONCURRENCY, |filename| sort_file(file_base, filename, args.ignore_case, args.unique), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/fix_byte_order_marker.rs b/crates/prek/src/hooks/pre_commit_hooks/fix_byte_order_marker.rs index aa6560481..d70af9703 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/fix_byte_order_marker.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/fix_byte_order_marker.rs @@ -4,7 +4,7 @@ use anyhow::Result; use tokio::io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt, SeekFrom}; use crate::hook::Hook; -use crate::hooks::run_concurrent_file_checks; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, parse_hook_args, run_file_checks}; use crate::run::INTERNAL_CONCURRENCY; const UTF8_BOM: &[u8] = b"\xef\xbb\xbf"; @@ -14,8 +14,10 @@ pub(crate) async fn fix_byte_order_marker( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec)> { - run_concurrent_file_checks( - filenames.iter().copied(), + let args: FilenamesArgs = parse_hook_args(hook)?; + run_file_checks( + &args.filenames, + filenames, *INTERNAL_CONCURRENCY, |filename| fix_file(hook.project().relative_path(), filename), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/fix_end_of_file.rs b/crates/prek/src/hooks/pre_commit_hooks/fix_end_of_file.rs index 8257a1edb..73c707855 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/fix_end_of_file.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/fix_end_of_file.rs @@ -4,12 +4,14 @@ use std::path::Path; use anyhow::Result; use crate::hook::Hook; -use crate::hooks::run_concurrent_file_checks; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, parse_hook_args, run_file_checks}; use crate::run::INTERNAL_CONCURRENCY; pub(crate) async fn fix_end_of_file(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { - run_concurrent_file_checks( - filenames.iter().copied(), + let args: FilenamesArgs = parse_hook_args(hook)?; + run_file_checks( + &args.filenames, + filenames, *INTERNAL_CONCURRENCY, |filename| fix_file(hook.project().relative_path(), filename), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/fix_trailing_whitespace.rs b/crates/prek/src/hooks/pre_commit_hooks/fix_trailing_whitespace.rs index 993a409ca..7357cb4ca 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/fix_trailing_whitespace.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/fix_trailing_whitespace.rs @@ -1,4 +1,4 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use std::str::FromStr; use anyhow::Result; @@ -6,7 +6,7 @@ use bstr::ByteSlice; use clap::Parser; use crate::hook::Hook; -use crate::hooks::run_concurrent_file_checks; +use crate::hooks::pre_commit_hooks::{parse_hook_args, run_file_checks}; use crate::run::INTERNAL_CONCURRENCY; const MARKDOWN_LINE_BREAK: &[u8] = b" "; @@ -33,6 +33,8 @@ struct Args { // so, we use Chars to achieve it. #[arg(long)] chars: Option, + #[arg(value_name = "FILENAMES")] + filenames: Vec, } impl Args { @@ -62,7 +64,7 @@ pub(crate) async fn fix_trailing_whitespace( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec)> { - let args = Args::try_parse_from(hook.entry.expect_direct().split_with_args(&hook.args)?)?; + let args: Args = parse_hook_args(hook)?; let force_markdown = args.force_markdown(); let markdown_exts = args.markdown_exts()?; @@ -71,8 +73,9 @@ pub(crate) async fn fix_trailing_whitespace( .as_ref() .map_or(&[][..], |chars| chars.0.as_slice()); - run_concurrent_file_checks( - filenames.iter().copied(), + run_file_checks( + &args.filenames, + filenames, *INTERNAL_CONCURRENCY, |filename| { fix_file( diff --git a/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs b/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs index 7e03e9361..b4240878d 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/forbid_new_submodules.rs @@ -6,11 +6,13 @@ use prek_consts::env_vars::{EnvVars, EnvVarsRead}; use crate::git; use crate::hook::Hook; +use crate::hooks::pre_commit_hooks::{FilenamesArgs, hook_filenames, parse_hook_args}; pub(crate) async fn forbid_new_submodules( hook: &Hook, filenames: &[&Path], ) -> Result<(i32, Vec), anyhow::Error> { + let args: FilenamesArgs = parse_hook_args(hook)?; let diff_arg = if let (Ok(from_ref), Ok(to_ref)) = ( EnvVars.var("PRE_COMMIT_FROM_REF"), EnvVars.var("PRE_COMMIT_TO_REF"), @@ -30,7 +32,7 @@ pub(crate) async fn forbid_new_submodules( .arg("-z") .arg(diff_arg.as_ref()) .arg("--") - .file_args(filenames) + .file_args(hook_filenames(&args.filenames, filenames)) .check(true) .output() .await? diff --git a/crates/prek/src/hooks/pre_commit_hooks/mixed_line_ending.rs b/crates/prek/src/hooks/pre_commit_hooks/mixed_line_ending.rs index 183a07b80..7f3301dea 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/mixed_line_ending.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/mixed_line_ending.rs @@ -1,11 +1,11 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use anyhow::Result; use bstr::ByteSlice; use clap::{Parser, ValueEnum}; use crate::hook::Hook; -use crate::hooks::run_concurrent_file_checks; +use crate::hooks::pre_commit_hooks::{parse_hook_args, run_file_checks}; use crate::run::INTERNAL_CONCURRENCY; const CRLF: &[u8] = b"\r\n"; @@ -21,6 +21,8 @@ struct Args { /// or a specified line ending. #[clap(long, short, value_enum, default_value_t = FixMode::Auto)] fix: FixMode, + #[arg(value_name = "FILENAMES")] + filenames: Vec, } #[derive(Copy, Clone, Debug, Default, ValueEnum)] @@ -59,10 +61,11 @@ impl LineEndingCounts { } pub(crate) async fn mixed_line_ending(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { - let args = Args::try_parse_from(hook.entry.expect_direct().split_with_args(&hook.args)?)?; + let args: Args = parse_hook_args(hook)?; - run_concurrent_file_checks( - filenames.iter().copied(), + run_file_checks( + &args.filenames, + filenames, *INTERNAL_CONCURRENCY, |filename| fix_file(hook.project().relative_path(), filename, args.fix), ) diff --git a/crates/prek/src/hooks/pre_commit_hooks/mod.rs b/crates/prek/src/hooks/pre_commit_hooks/mod.rs index 0d746fb69..db663316c 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/mod.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/mod.rs @@ -1,9 +1,12 @@ -use std::path::Path; +use std::future::Future; +use std::path::{Path, PathBuf}; use anyhow::Result; +use clap::Parser; use tracing::debug; use crate::hook::Hook; +use crate::hooks::run_concurrent_file_checks; use super::HookFuture; @@ -52,6 +55,67 @@ pub(crate) use mixed_line_ending::mixed_line_ending; pub(crate) use no_commit_to_branch::no_commit_to_branch; pub(crate) use pretty_format_json::pretty_format_json; +#[derive(Parser)] +#[command(disable_help_subcommand = true)] +#[command(disable_version_flag = true)] +#[command(disable_help_flag = true)] +pub(crate) struct FilenamesArgs { + #[arg(value_name = "FILENAMES")] + pub(crate) filenames: Vec, +} + +pub(crate) fn parse_hook_args(hook: &Hook) -> Result { + Ok(T::try_parse_from( + hook.entry.expect_direct().split_with_args(&hook.args)?, + )?) +} + +pub(crate) fn hook_filenames<'a>( + configured: &'a [PathBuf], + selected: &'a [&Path], +) -> impl Iterator + 'a { + configured + .iter() + .map(PathBuf::as_path) + .chain(selected.iter().copied()) +} + +pub(crate) async fn run_file_checks<'a, F, Fut>( + explicit: &'a [PathBuf], + selected: &'a [&Path], + concurrency: usize, + check: F, +) -> Result<(i32, Vec)> +where + F: Fn(&'a Path) -> Fut, + Fut: Future)>>, +{ + // Keep the common case on the existing concurrent path without an extra accumulator. + if explicit.is_empty() { + return run_concurrent_file_checks(selected.iter().copied(), concurrency, check).await; + } + + // Filenames from `entry` or `args` may repeat or overlap with `selected`, so finish + // them serially in CLI order before starting the selected batch. + let mut code = 0; + let mut output = Vec::new(); + for filename in explicit { + let (file_code, file_output) = check(filename).await?; + code |= file_code; + output.extend(file_output); + } + if selected.is_empty() { + return Ok((code, output)); + } + + // The explicit batch is complete, so selected filenames retain their normal concurrency. + let (selected_code, selected_output) = + run_concurrent_file_checks(selected.iter().copied(), concurrency, check).await?; + code |= selected_code; + output.extend(selected_output); + Ok((code, output)) +} + /// Hooks from `https://github.com/pre-commit/pre-commit-hooks`. #[derive(strum::EnumString)] #[strum(serialize_all = "kebab-case")] @@ -153,3 +217,42 @@ impl PreCommitHooks { pub(crate) fn is_pre_commit_hooks(url: &str) -> bool { url == "https://github.com/pre-commit/pre-commit-hooks" } + +#[cfg(test)] +mod tests { + use std::cell::RefCell; + use std::rc::Rc; + + use super::*; + + #[tokio::test(flavor = "current_thread")] + async fn explicit_filenames_run_serially_before_selected_filenames() { + let explicit = vec![PathBuf::from("shared"), PathBuf::from("shared")]; + let selected = [Path::new("shared"), Path::new("selected")]; + let events = Rc::new(RefCell::new(Vec::new())); + + run_file_checks(&explicit, &selected, 2, |path| { + let events = Rc::clone(&events); + async move { + events.borrow_mut().push(format!("start {path:?}")); + tokio::task::yield_now().await; + events.borrow_mut().push(format!("end {path:?}")); + Ok((0, Vec::new())) + } + }) + .await + .unwrap(); + + let events = events.borrow(); + assert_eq!(events.len(), 8); + assert_eq!( + &events[..4], + [ + "start \"shared\"", + "end \"shared\"", + "start \"shared\"", + "end \"shared\"", + ] + ); + } +} diff --git a/crates/prek/src/hooks/pre_commit_hooks/pretty_format_json.rs b/crates/prek/src/hooks/pre_commit_hooks/pretty_format_json.rs index 0f12e417e..642723c39 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/pretty_format_json.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/pretty_format_json.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; use std::io; -use std::path::Path; +use std::path::{Path, PathBuf}; use anyhow::Result; use clap::Parser; @@ -10,7 +10,7 @@ use serde_json::ser::{Formatter, PrettyFormatter}; use similar::TextDiff; use crate::hook::Hook; -use crate::hooks::run_concurrent_file_checks; +use crate::hooks::pre_commit_hooks::{parse_hook_args, run_file_checks}; use crate::run::INTERNAL_CONCURRENCY; #[derive(Parser, Debug)] @@ -32,6 +32,8 @@ struct Args { #[arg(long, value_delimiter = ',')] top_keys: Vec, + #[arg(value_name = "FILENAMES")] + filenames: Vec, } struct PreparedArgs { @@ -69,11 +71,12 @@ impl From<&Args> for PreparedArgs { } pub(crate) async fn pretty_format_json(hook: &Hook, filenames: &[&Path]) -> Result<(i32, Vec)> { - let args = Args::try_parse_from(hook.entry.expect_direct().split_with_args(&hook.args)?)?; + let args: Args = parse_hook_args(hook)?; let prepared = PreparedArgs::from(&args); - run_concurrent_file_checks( - filenames.iter().copied(), + run_file_checks( + &args.filenames, + filenames, *INTERNAL_CONCURRENCY, |filename| check_file(hook.project().relative_path(), filename, &prepared), ) @@ -708,6 +711,7 @@ mod tests { "version".to_string(), "name".to_string(), ], + filenames: Vec::new(), }; let prepared = PreparedArgs::from(&args); diff --git a/crates/prek/tests/builtin_hooks.rs b/crates/prek/tests/builtin_hooks.rs index 643ddb83d..7c1edbbb1 100644 --- a/crates/prek/tests/builtin_hooks.rs +++ b/crates/prek/tests/builtin_hooks.rs @@ -373,6 +373,46 @@ fn file_contents_sorter_hook() -> Result<()> { Ok(()) } +#[test] +fn builtin_hook_checks_filename_from_args_after_options() -> Result<()> { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: builtin + hooks: + - id: file-contents-sorter + args: [--ignore-case, configured.txt, configured.txt, selected.txt] + files: ^selected\.txt$ + "}); + + let cwd = context.work_dir(); + cwd.child("configured.txt").write_str("beta\nAlpha\n")?; + cwd.child("selected.txt").write_str("Beta\nalpha\n")?; + context.git_add("."); + + cmd_snapshot!(context.filters(), context.run(), @r" + success: false + exit_code: 1 + ----- stdout ----- + file contents sorter.....................................................Failed + - hook id: file-contents-sorter + - exit code: 1 + - files were modified by this hook + + Sorting configured.txt + Sorting selected.txt + + ----- stderr ----- + "); + + assert_eq!(context.read("configured.txt"), "Alpha\nbeta\n"); + assert_eq!(context.read("selected.txt"), "alpha\nBeta\n"); + + Ok(()) +} + #[test] fn forbid_new_submodules_hook_in_workspace_project() -> Result<()> { let context = TestContext::new(); diff --git a/crates/prek/tests/run.rs b/crates/prek/tests/run.rs index e3cfe1458..c7036f7e7 100644 --- a/crates/prek/tests/run.rs +++ b/crates/prek/tests/run.rs @@ -72,6 +72,45 @@ fn run_basic() -> Result<()> { Ok(()) } +#[test] +fn fast_path_checks_filenames_from_entry_and_args() -> Result<()> { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: check-json + entry: check-json from-entry.json + args: [from-args.json] + files: ^selected\.json$ + "}); + + let cwd = context.work_dir(); + cwd.child("from-entry.json").write_str("invalid")?; + cwd.child("from-args.json").write_str("invalid")?; + cwd.child("selected.json").write_str("{}")?; + context.git_add("."); + + cmd_snapshot!(context.filters(), context.run(), @r" + success: false + exit_code: 1 + ----- stdout ----- + check json...............................................................Failed + - hook id: check-json + - exit code: 1 + + from-entry.json: Failed to json decode (expected value at line 1 column 1) + from-args.json: Failed to json decode (expected value at line 1 column 1) + + ----- stderr ----- + "); + + Ok(()) +} + #[test] fn run_preserves_stdout_stderr_order() -> Result<()> { let context = TestContext::new();