Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion crates/prek/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,8 @@ pub(crate) struct RunArgs {
///
/// Can be specified multiple times; a hook may match any specified group.
/// When combined with `--require-group`, both filters must match.
/// `@ungrouped` matches hooks without groups, while `@builtin` matches
/// hooks from the builtin repository.
#[arg(long = "group", value_name = "GROUP")]
pub(crate) groups: Vec<String>,

Expand All @@ -690,12 +692,14 @@ pub(crate) struct RunArgs {
///
/// For example, `--require-group fast --group format --group lint-only`
/// selects hooks in `fast` and either `format` or `lint-only`.
/// The special selectors `@ungrouped` and `@builtin` are also supported.
#[arg(long = "require-group", value_name = "GROUP")]
pub(crate) required_groups: Vec<String>,

/// Do not run hooks belonging to the specified group.
///
/// Can be specified multiple times. Exclusion wins over inclusion.
/// The special selectors `@ungrouped` and `@builtin` are also supported.
#[arg(long = "no-group", value_name = "GROUP")]
pub(crate) no_groups: Vec<String>,
}
Expand Down Expand Up @@ -782,19 +786,22 @@ pub(crate) struct ListArgs {

/// Show hooks belonging to the specified group.
///
/// Can be specified multiple times.
/// Can be specified multiple times. `@ungrouped` matches hooks without
/// groups, while `@builtin` matches hooks from the builtin repository.
#[arg(long = "group", value_name = "GROUP")]
pub(crate) groups: Vec<String>,

/// Show hooks belonging to every specified group.
///
/// Can be specified multiple times. Composes with `--group` and `--no-group`.
/// The special selectors `@ungrouped` and `@builtin` are also supported.
#[arg(long = "require-group", value_name = "GROUP")]
pub(crate) required_groups: Vec<String>,

/// Do not show hooks belonging to the specified group.
///
/// Can be specified multiple times. Exclusion wins over inclusion.
/// The special selectors `@ungrouped` and `@builtin` are also supported.
#[arg(long = "no-group", value_name = "GROUP")]
pub(crate) no_groups: Vec<String>,

Expand Down
73 changes: 56 additions & 17 deletions crates/prek/src/cli/run/selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use std::fmt::Display;
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};

use crate::config::validate_name;
use crate::hook::Hook;
use crate::config::validate_group_name;
use crate::hook::{Hook, Repo};
use crate::warn_user;

use anyhow::anyhow;
Expand Down Expand Up @@ -424,12 +424,45 @@ impl Selectors {

#[derive(Debug, Clone, Default)]
pub(crate) struct GroupFilters {
include_any: Vec<String>,
require_all: Vec<String>,
exclude_any: Vec<String>,
include_any: Vec<GroupSelector>,
require_all: Vec<GroupSelector>,
exclude_any: Vec<GroupSelector>,
usage: Arc<Mutex<FilterUsage>>,
}

#[derive(Debug, Clone)]
enum GroupSelector {
Named(String),
Ungrouped,
Builtin,
}

const UNGROUPED_GROUP: &str = "@ungrouped";
const BUILTIN_GROUP: &str = "@builtin";

impl GroupSelector {
fn parse(group: &str) -> Result<Self, &'static str> {
match group {
UNGROUPED_GROUP => Ok(Self::Ungrouped),
BUILTIN_GROUP => Ok(Self::Builtin),
_ => {
validate_group_name(group)?;
Ok(Self::Named(group.to_owned()))
}
}
}
}

impl Display for GroupSelector {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Named(group) => f.write_str(group),
Self::Ungrouped => f.write_str(UNGROUPED_GROUP),
Self::Builtin => f.write_str(BUILTIN_GROUP),
}
}
}

impl GroupFilters {
pub(crate) fn parse(
include_any: &[String],
Expand All @@ -438,21 +471,20 @@ impl GroupFilters {
) -> Result<Self, Error> {
let parse_groups = |flag: &'static str, groups: &[String]| {
let mut seen = FxHashSet::default();
let mut names = Vec::new();
let mut selectors = Vec::new();

for group in groups {
if let Err(reason) = validate_name(group) {
return Err(Error::GroupSelector {
let selector =
GroupSelector::parse(group).map_err(|reason| Error::GroupSelector {
selector: format!("{flag}={group}"),
source: anyhow!("group name {reason}"),
});
}
})?;
if seen.insert(group.as_str()) {
names.push(group.clone());
selectors.push(selector);
}
}

Ok(names)
Ok(selectors)
};

Ok(Self {
Expand All @@ -467,7 +499,7 @@ impl GroupFilters {
!self.include_any.is_empty() || !self.require_all.is_empty() || !self.exclude_any.is_empty()
}

fn matches_groups(&self, contains_group: impl Fn(&str) -> bool) -> bool {
fn matches_groups(&self, contains_group: impl Fn(&GroupSelector) -> bool) -> bool {
let mut usage = self.usage.lock().unwrap();

let mut matches_any_excluded = false;
Expand Down Expand Up @@ -499,13 +531,20 @@ impl GroupFilters {
}

pub(crate) fn matches_hook(&self, hook: &Hook) -> bool {
self.matches_groups(|group| hook.groups.contains(group))
self.matches_groups(|group| match group {
GroupSelector::Named(group) => hook.groups.contains(group),
GroupSelector::Ungrouped => hook.groups.is_empty(),
GroupSelector::Builtin => matches!(hook.repo(), Repo::Builtin),
})
}

pub(crate) fn matches_configured_hook(&self, hook: &ConfiguredHook<'_>) -> bool {
self.matches_groups(|group| {
hook.groups
.is_some_and(|groups| groups.iter().any(|hook_group| hook_group == group))
self.matches_groups(|group| match group {
GroupSelector::Named(group) => hook
.groups
.is_some_and(|groups| groups.iter().any(|hook_group| hook_group == group)),
GroupSelector::Ungrouped => hook.groups.is_none_or(<[String]>::is_empty),
GroupSelector::Builtin => false,
})
}

Expand Down
12 changes: 6 additions & 6 deletions crates/prek/src/config/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,8 +710,8 @@ pub(crate) struct RemoteHook {
/// It is not allowed in manifests (e.g. `.pre-commit-hooks.yaml`).
pub priority: Option<Priority>,
/// User-defined hook groups used by `prek run --group` and `--no-group`.
/// Group names cannot be empty or contain whitespace.
#[cfg_attr(feature = "schemars", schemars(inner(regex(pattern = r"^\S+$"))))]
/// Group names cannot be empty, contain whitespace, or start with `@`.
#[cfg_attr(feature = "schemars", schemars(inner(regex(pattern = r"^[^@\s]\S*$"))))]
pub groups: Option<Vec<String>>,
#[cfg_attr(feature = "schemars", serde(flatten))]
pub options: HookOptions,
Expand Down Expand Up @@ -752,8 +752,8 @@ pub(crate) struct LocalHook {
/// Hooks with the same priority can run in parallel.
pub priority: Option<Priority>,
/// User-defined hook groups used by `prek run --group` and `--no-group`.
/// Group names cannot be empty or contain whitespace.
#[cfg_attr(feature = "schemars", schemars(inner(regex(pattern = r"^\S+$"))))]
/// Group names cannot be empty, contain whitespace, or start with `@`.
#[cfg_attr(feature = "schemars", schemars(inner(regex(pattern = r"^[^@\s]\S*$"))))]
pub groups: Option<Vec<String>>,
#[cfg_attr(feature = "schemars", serde(flatten))]
pub options: HookOptions,
Expand Down Expand Up @@ -795,7 +795,7 @@ pub(crate) struct MetaHook {
/// Hooks with the same priority can run in parallel.
pub priority: Option<Priority>,
/// User-defined hook groups used by `prek run --group` and `--no-group`.
/// Group names cannot be empty or contain whitespace.
/// Group names cannot be empty, contain whitespace, or start with `@`.
pub groups: Option<Vec<String>>,
pub options: HookOptions,
}
Expand Down Expand Up @@ -884,7 +884,7 @@ pub(crate) struct BuiltinHook {
/// Hooks with the same priority can run in parallel.
pub priority: Option<Priority>,
/// User-defined hook groups used by `prek run --group` and `--no-group`.
/// Group names cannot be empty or contain whitespace.
/// Group names cannot be empty, contain whitespace, or start with `@`.
pub groups: Option<Vec<String>>,
/// Common hook options.
///
Expand Down
6 changes: 5 additions & 1 deletion crates/prek/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub(crate) use hook::{
MetaHook, PassFilenames, RemoteHook, Shell, Stage, Stages, ToolchainPreference,
};
pub(crate) use pattern::{FilePattern, GlobPatterns};
pub(crate) use priority::{Priority, PriorityAlias, validate_name};
pub(crate) use priority::{Priority, PriorityAlias, validate_group_name};
#[cfg(feature = "schemars")]
pub(crate) use repo::{BuiltinRepo, LocalRepo, MetaRepo};
pub(crate) use repo::{RemoteRepo, RemoteRepoKey, Repo};
Expand Down Expand Up @@ -409,6 +409,10 @@ mod tests {
r#"["ci\tslow"]"#,
"group name `ci\tslow` cannot contain whitespace",
),
(
r#"["@builtin"]"#,
"group name `@builtin` uses the reserved `@` prefix",
),
] {
let yaml = indoc::formatdoc! {r"
repos:
Expand Down
13 changes: 11 additions & 2 deletions crates/prek/src/config/priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use serde::{Deserialize, Deserializer};

use super::Error;

pub(crate) fn validate_name(name: &str) -> Result<(), &'static str> {
fn validate_name(name: &str) -> Result<(), &'static str> {
if name.is_empty() {
Err("cannot be empty")
} else if name.chars().any(char::is_whitespace) {
Expand All @@ -16,6 +16,15 @@ pub(crate) fn validate_name(name: &str) -> Result<(), &'static str> {
}
}

pub(crate) fn validate_group_name(name: &str) -> Result<(), &'static str> {
validate_name(name)?;
if name.starts_with('@') {
Err("uses the reserved `@` prefix")
} else {
Ok(())
}
}

#[derive(Clone, Eq, Ord, PartialEq, PartialOrd)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub(crate) struct PriorityAlias(
Expand Down Expand Up @@ -122,7 +131,7 @@ where
let groups = Option::<Vec<String>>::deserialize(deserializer)?;
if let Some(groups) = &groups {
for group in groups {
if let Err(reason) = validate_name(group) {
if let Err(reason) = validate_group_name(group) {
Comment thread
j178 marked this conversation as resolved.
return Err(D::Error::custom(format!("group name `{group}` {reason}")));
}
}
Expand Down
20 changes: 20 additions & 0 deletions crates/prek/tests/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,26 @@ fn list_with_group_filter() {
----- stderr -----
");

cmd_snapshot!(context, context.list().arg("--group").arg("@ungrouped"), @r"
success: true
exit_code: 0
----- stdout -----
.:ungrouped

----- stderr -----
");

cmd_snapshot!(context, context.list().arg("--no-group").arg("@ungrouped"), @r"
success: true
exit_code: 0
----- stdout -----
.:fast-lint
.:slow-lint
.:format

----- stderr -----
");

cmd_snapshot!(context, context.list().arg("--group").arg("ci").arg("--no-group").arg("slow"), @r"
success: true
exit_code: 0
Expand Down
73 changes: 72 additions & 1 deletion crates/prek/tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,67 @@ fn run_group_without_stage_selects_hooks_across_stages() {
"#);
}

#[test]
fn run_reserved_groups_select_ungrouped_and_builtin_hooks() {
let context = TestEnv::new().with_config(indoc::indoc! {r#"
repos:
- repo: builtin
hooks:
- id: end-of-file-fixer
groups: [other]

- repo: local
hooks:
- id: ungrouped
name: Ungrouped
language: system
entry: python3 -c "print('ungrouped')"
always_run: true
- id: ci
name: CI
language: system
entry: python3 -c "print('ci')"
always_run: true
groups: [ci]
- id: other
name: Other
language: system
entry: python3 -c "print('other')"
always_run: true
groups: [other]

- repo: https://notexistentatallnevergonnahappen.com/nonexistent/repo
rev: v1.0.0
hooks:
- id: remote-other
groups: [other]
"#});

context.git_add_all();

cmd_snapshot!(context,
context
.run()
.arg("--all-files")
.arg("--group")
.arg("ci")
.arg("--group")
.arg("@ungrouped")
.arg("--group")
.arg("@builtin"),
@r#"
success: true
exit_code: 0
----- stdout -----
fix end of files.........................................................Passed
Ungrouped................................................................Passed
CI.......................................................................Passed

----- stderr -----
"#
);
}

#[test]
fn run_required_group_without_stage_warns_when_only_message_file_hooks_match() {
let context = TestEnv::new().with_config(indoc::indoc! {r#"
Expand Down Expand Up @@ -1183,7 +1244,7 @@ fn run_unknown_group_selectors_warn_and_empty_selection_fails() {
}

#[test]
fn run_group_selectors_reject_whitespace() {
fn run_group_selectors_reject_invalid_names() {
let context = TestEnv::new().with_config(indoc::indoc! {r#"
repos:
- repo: local
Expand Down Expand Up @@ -1217,6 +1278,16 @@ fn run_group_selectors_reject_whitespace() {
error: Invalid group selector: `--no-group=ci slow`
caused by: group name cannot contain whitespace
"#);

cmd_snapshot!(context, context.run().arg("--all-files").arg("--group").arg("@custom"), @r#"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: Invalid group selector: `--group=@custom`
caused by: group name uses the reserved `@` prefix
"#);
}

#[test]
Expand Down
Loading
Loading