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
4 changes: 2 additions & 2 deletions crates/prek/src/hooks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ fn fast_path_hook(hook: &Hook) -> Option<PreCommitHooks> {
pub(crate) fn requires_diff_tracking(hook: &Hook) -> bool {
match hook.repo() {
Repo::Meta { .. } | Repo::Builtin { .. } => false,
Repo::Remote { .. } => fast_path_hook(hook).is_none(),
Repo::Local { .. } => true,
Repo::Remote { .. } => fast_path_hook(hook).is_none() && hook.language.can_modify_files(),
Repo::Local { .. } => hook.language.can_modify_files(),
}
}

Expand Down
10 changes: 9 additions & 1 deletion crates/prek/src/languages/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ impl Language {
}
}

pub(crate) fn can_modify_files(self) -> bool {
!matches!(self, Self::Fail | Self::Pygrep)
}

pub(crate) fn supports_install_env(self) -> bool {
match self {
Self::Bun
Expand Down Expand Up @@ -351,7 +355,11 @@ impl Language {
Repo::Remote { .. } | Repo::Local { .. } => Box::pin(async move {
let (exit_status, output) =
self.backend().run(store, hook, filenames, reporter).await?;
Ok(HookOutput::unknown(exit_status, output))
if self.can_modify_files() {
Ok(HookOutput::unknown(exit_status, output))
} else {
Ok(HookOutput::unchanged(exit_status, output))
}
}),
};

Expand Down
55 changes: 55 additions & 0 deletions crates/prek/tests/skipped_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,61 @@ fn read_only_builtin_hook_does_not_run_diff_detection() -> Result<()> {
Ok(())
}

#[test]
fn read_only_languages_do_not_run_diff_detection() -> Result<()> {
let context = TestContext::new();
context.init_project();

let cwd = context.work_dir();
context.write_pre_commit_config(indoc::indoc! {r"
repos:
- repo: local
hooks:
- id: fail
name: fail
language: fail
entry: expected failure
files: \.txt$
- id: pygrep
name: pygrep
language: pygrep
entry: not-present
files: \.txt$
"});

cwd.child("file.txt").write_str("original\n")?;
context.git_add(".");

let output = context
.run()
.arg("--all-files")
.env("RUST_LOG", "prek::git=trace")
.output()?;

assert!(!output.status.success(), "the fail hook should fail");

let stdout = String::from_utf8_lossy(&output.stdout);
assert!(stdout.contains("fail") && stdout.contains("Failed"));
assert!(stdout.contains("pygrep") && stdout.contains("Passed"));
assert!(!stdout.contains("files were modified by this hook"));

let stderr = String::from_utf8_lossy(&output.stderr);
assert_eq!(
stderr.matches("has_worktree_diff").count(),
0,
"Read-only languages should not require a worktree diff check.\n\
Trace output:\n{stderr}"
);
assert_eq!(
stderr.matches("diff_worktree").count(),
0,
"Read-only languages should not require a full worktree diff.\n\
Trace output:\n{stderr}"
);

Ok(())
}

#[test]
fn modifying_builtin_invalidates_baseline_for_later_external_hook() -> Result<()> {
let context = TestContext::new();
Expand Down