diff --git a/crates/prek/src/cli/run/run.rs b/crates/prek/src/cli/run/run.rs index d8fa80c02..48769fc9e 100644 --- a/crates/prek/src/cli/run/run.rs +++ b/crates/prek/src/cli/run/run.rs @@ -718,17 +718,18 @@ impl<'a> HookRunSession<'a> { ) .await?; - let needs_diff = group_results - .iter() - .any(|result| result.file_changes == hooks::FileChanges::Unknown); let known_modified_files = group_results .iter() .any(|result| result.file_changes == hooks::FileChanges::Modified); + let needs_diff = !known_modified_files + && group_results + .iter() + .any(|result| result.file_changes == hooks::FileChanges::Unknown); let diff_detected_modifications = diff_tracker.changed_after_group(needs_diff).await?; - if known_modified_files && !needs_diff { - // Native hooks report modifications directly, so no Git snapshot - // was taken. Force one before a later external hook needs a - // before/after comparison. + if known_modified_files { + // The group is already known to have modified files, so a Git + // comparison cannot change its result. A later external hook + // will capture the current worktree before it runs. diff_tracker.invalidate(); } let group_modified_files = known_modified_files || diff_detected_modifications; diff --git a/crates/prek/tests/skipped_hooks.rs b/crates/prek/tests/skipped_hooks.rs index 9d37ae3f8..9cfd0d771 100644 --- a/crates/prek/tests/skipped_hooks.rs +++ b/crates/prek/tests/skipped_hooks.rs @@ -893,6 +893,126 @@ fn read_only_languages_do_not_run_diff_detection() -> Result<()> { Ok(()) } +#[test] +fn same_group_known_modification_skips_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: builtin + hooks: + - id: end-of-file-fixer + priority: 0 + - repo: local + hooks: + - id: noop + name: noop + language: system + entry: python3 -c "pass" + pass_filenames: false + priority: 0 + "#}); + + cwd.child("file.txt").write_str("missing newline")?; + context.git_add("."); + + let output = context.run().env("RUST_LOG", "prek::git=trace").output()?; + + assert!( + !output.status.success(), + "the builtin should report its modification" + ); + assert_eq!(context.read("file.txt"), "missing newline\n"); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout + .lines() + .any(|line| line.contains("noop") && line.contains("Passed")) + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!( + stderr.matches("has_worktree_diff").count(), + 0, + "A known modification should make the same-group quiet diff unnecessary.\n\ + Trace output:\n{stderr}" + ); + assert_eq!( + stderr.matches("diff_worktree").count(), + 0, + "A known modification should make the same-group full diff unnecessary.\n\ + Trace output:\n{stderr}" + ); + + Ok(()) +} + +#[test] +fn same_group_known_modification_rebaselines_later_external_hook() -> Result<()> { + let context = TestContext::new(); + context.init_project(); + + let cwd = context.work_dir(); + context.write_pre_commit_config(indoc::indoc! {r#" + repos: + - repo: builtin + hooks: + - id: end-of-file-fixer + priority: 0 + - repo: local + hooks: + - id: same-group-noop + name: same-group-noop + language: system + entry: python3 -c "pass" + pass_filenames: false + priority: 0 + - id: later-noop + name: later-noop + language: system + entry: python3 -c "pass" + pass_filenames: false + priority: 1 + "#}); + + cwd.child("file.txt").write_str("missing newline")?; + context.git_add("."); + + let output = context.run().env("RUST_LOG", "prek::git=trace").output()?; + + assert!( + !output.status.success(), + "the builtin should report its modification" + ); + assert_eq!(context.read("file.txt"), "missing newline\n"); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout + .lines() + .any(|line| line.contains("later-noop") && line.contains("Passed")) + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!( + stderr.matches("has_worktree_diff").count(), + 0, + "The known modification should invalidate the clean baseline.\n\ + Trace output:\n{stderr}" + ); + assert_eq!( + stderr.matches("diff_worktree").count(), + 2, + "The later external hook should capture and compare the modified worktree.\n\ + Trace output:\n{stderr}" + ); + + Ok(()) +} + #[test] fn modifying_builtin_invalidates_baseline_for_later_external_hook() -> Result<()> { let context = TestContext::new();