fix: tidy fix defaults match tidy check issues (#1424)

SebTardif · web-flow · commit 1f2d2a95adea · 2026-07-04T17:22:38.000Z
## Summary

fixrealloop multi-file round found that bare `tidy fix --apply` reported
success with zero files changed while `tidy check` still flagged missing
final newline and trailing whitespace on the same tree.

- Default `tidy fix` (no write-policy flags, no
`--respect-editorconfig`) now enables final-newline and
trailing-whitespace fixes.
- Explicit flags or EditorConfig keep opt-in behavior unchanged.
- Docs/help examples updated.

## Test plan

- [x] Unit tests for check-parity defaults and explicit trim-only
- [x] Existing EditorConfig per-extension integration test
- [x] `make check-fast`
- [x] Release binary: messy file cleaned, subsequent check clean

Signed-off-by: Sebastien Tardif &lt;sebtardif@ncf.ca&gt;
diff --git a/docs/reference/README.md b/docs/reference/README.md
@@ -840,9 +840,9 @@ Use these when newline and whitespace correctness is the main concern.
 <!-- ref:tidy-action:fix -->
 ### `tidy fix`
 
-- **What it does:** Applies newline and whitespace normalization to text files. Binary and invalid UTF-8 files are skipped.
+- **What it does:** Applies newline and whitespace normalization to text files. Binary and invalid UTF-8 files are skipped. With no write-policy flags (and without `--respect-editorconfig`), it enables final-newline and trailing-whitespace fixes so it matches the issues bare `tidy check` always reports. Pass explicit flags (or EditorConfig) to narrow the fix set.
 - **Use when:** Existing files already need cleanup and the cleanup itself is the task.
-- **Prefer instead:** Use write policy flags when normalization should only apply to files already being touched by another write command.
+- **Prefer instead:** Use write policy flags on another write command when normalization should only apply to files already being touched by that command.
 
 ## `tx` reference
 
diff --git a/src/cmd/tidy/fix.rs b/src/cmd/tidy/fix.rs
@@ -114,6 +114,49 @@ fn emit_tidy_fix_output(
     Ok(())
 }
 
+/// Write-policy fields for `tidy fix` after applying check-parity defaults.
+struct TidyFixPolicy {
+    ensure_final_newline: bool,
+    trim_trailing_whitespace: bool,
+    normalize_eol: Option<crate::cli::global::EolMode>,
+    collapse_blanks: bool,
+}
+
+/// When the user did not pass any write-policy flag and is not only
+/// indenting/dedenting, enable the same normalizations that `tidy check`
+/// always reports (final newline + trailing whitespace). Otherwise
+/// `tidy fix --apply` is a silent no-op while `tidy check` still fails
+/// (fixrealloop feature gap).
+fn effective_tidy_fix_policy(
+    global: &GlobalFlags,
+    dedent: Option<&str>,
+    indent: Option<&str>,
+) -> TidyFixPolicy {
+    // EditorConfig opt-in is also "explicit policy": do not force check-parity
+    // defaults on top of per-file EditorConfig rules.
+    let any_explicit_policy = global.ensure_final_newline
+        || global.trim_trailing_whitespace
+        || global.normalize_eol.is_some()
+        || global.collapse_blanks
+        || global.respect_editorconfig
+        || dedent.is_some()
+        || indent.is_some();
+    if any_explicit_policy {
+        return TidyFixPolicy {
+            ensure_final_newline: global.ensure_final_newline,
+            trim_trailing_whitespace: global.trim_trailing_whitespace,
+            normalize_eol: global.normalize_eol,
+            collapse_blanks: global.collapse_blanks,
+        };
+    }
+    TidyFixPolicy {
+        ensure_final_newline: true,
+        trim_trailing_whitespace: true,
+        normalize_eol: None,
+        collapse_blanks: false,
+    }
+}
+
 /// Run `tidy fix` for the given paths and optional dedent/indent/lines.
 pub(super) fn run_fix(
     paths: Vec<String>,
@@ -127,6 +170,8 @@ pub(super) fn run_fix(
         anyhow::bail!("--dedent and --indent cannot both be set");
     }
 
+    let policy_flags = effective_tidy_fix_policy(global, dedent.as_deref(), indent.as_deref());
+
     let cwd = global.resolve_cwd()?;
     global.check_paths_contained(&cwd, &paths)?;
     let glob_matcher = crate::build_glob_matcher_from_global(global)?;
@@ -141,6 +186,18 @@ pub(super) fn run_fix(
     let quiet = global.quiet;
     let dedent_ref = dedent.as_deref();
     let indent_ref = indent.as_deref();
+    // policy_from_flags reads ensure/trim/eol from GlobalFlags. Overlay the
+    // effective tidy defaults (and keep editorconfig opt-in from the caller).
+    let policy_global = GlobalFlags {
+        ensure_final_newline: policy_flags.ensure_final_newline,
+        trim_trailing_whitespace: policy_flags.trim_trailing_whitespace,
+        normalize_eol: policy_flags.normalize_eol,
+        collapse_blanks: policy_flags.collapse_blanks,
+        respect_editorconfig: global.respect_editorconfig,
+        quiet: global.quiet,
+        ..GlobalFlags::default()
+    };
+
     let dirty_rel_paths: Vec<String> = crate::par_process_files(
         &fix_file_paths,
         glob_matcher.as_ref(),
@@ -150,7 +207,7 @@ pub(super) fn run_fix(
                 Some(text) => text,
                 None => return None,
             };
-            let policy = policy_from_flags(global, Some(file_path));
+            let policy = policy_from_flags(&policy_global, Some(file_path));
             let mut fixed = apply_policy(&original, &policy).into_owned();
             if let Some(spec) = dedent_ref {
                 fixed = crate::write::dedent_content(&fixed, spec, line_range);
@@ -176,8 +233,8 @@ pub(super) fn run_fix(
         return Ok(exit::SUCCESS);
     }
 
-    let eol_str = global.normalize_eol.map(eol_mode_to_str);
-    let collapse = if global.collapse_blanks {
+    let eol_str = policy_flags.normalize_eol.map(eol_mode_to_str);
+    let collapse = if policy_flags.collapse_blanks {
         Some(true)
     } else {
         None
@@ -186,8 +243,8 @@ pub(super) fn run_fix(
         .iter()
         .map(|rel_path| Operation::TidyFix {
             path: rel_path.clone(),
-            ensure_final_newline: Some(global.ensure_final_newline),
-            trim_trailing_whitespace: Some(global.trim_trailing_whitespace),
+            ensure_final_newline: Some(policy_flags.ensure_final_newline),
+            trim_trailing_whitespace: Some(policy_flags.trim_trailing_whitespace),
             normalize_eol: eol_str.map(String::from),
             collapse_blanks: collapse,
             dedent: dedent.clone(),
@@ -196,6 +253,8 @@ pub(super) fn run_fix(
         })
         .collect();
 
+    // Mode/apply still come from the caller's global flags; only the
+    // write-policy fields use the effective check-parity defaults above.
     let (cwd, result) = crate::cmd::output::stage_for_write(WriteSource::Operations(ops), global)?;
 
     tidy_fix_output(global, result, &dirty_rel_paths, &cwd)
diff --git a/src/cmd/tidy/mod.rs b/src/cmd/tidy/mod.rs
@@ -14,9 +14,14 @@ use clap::Args;
 #[derive(Debug, Args)]
 #[command(after_help = "\
 EXAMPLES:
-  patchloom tidy src/ --ensure-final-newline
-  patchloom tidy . --trim-trailing-whitespace --apply
-  patchloom tidy . --normalize-eol lf --check")]
+  patchloom tidy fix . --apply
+  patchloom tidy fix src/ --ensure-final-newline --apply
+  patchloom tidy fix . --trim-trailing-whitespace --apply
+  patchloom tidy check . --normalize-eol lf
+
+With no write-policy flags, `tidy fix` enables final-newline and trailing-
+whitespace fixes (the same issues `tidy check` always reports). Pass explicit
+flags to narrow the fix set.")]
 pub struct TidyArgs {
     #[command(subcommand)]
     pub action: TidyAction,
diff --git a/src/cmd/tidy/tests.rs b/src/cmd/tidy/tests.rs
@@ -214,6 +214,71 @@ fn fix_adds_missing_newline() {
     );
 }
 
+/// Bare `tidy fix --apply` must fix issues that bare `tidy check` reports
+/// (final newline + trailing whitespace). fixrealloop feature gap.
+#[test]
+fn fix_defaults_match_check_parity() {
+    let tmp = TempDir::new().unwrap();
+    let file = tmp.path().join("messy.txt");
+    // Trailing spaces and no final newline &mdash; both reported by tidy check.
+    std::fs::write(&file, b"line  ").unwrap();
+
+    let mut global = GlobalFlags::test_with_cwd(tmp.path());
+    global.apply = true;
+    // Deliberately leave ensure_final_newline / trim_trailing_whitespace false
+    // so the check-parity defaults must kick in.
+
+    let args = TidyArgs {
+        action: TidyAction::Fix {
+            paths: vec![".".to_string()],
+            dedent: None,
+            indent: None,
+            lines: None,
+        },
+        write: Default::default(),
+    };
+    let code = run(args, &global).unwrap();
+    assert_eq!(code, exit::SUCCESS);
+
+    let content = std::fs::read_to_string(&file).unwrap();
+    assert_eq!(
+        content, "line\n",
+        "bare tidy fix should trim trailing ws and add final newline"
+    );
+}
+
+/// Explicit single-policy flag must not auto-enable the other defaults.
+#[test]
+fn fix_explicit_trim_only_does_not_force_final_newline() {
+    let tmp = TempDir::new().unwrap();
+    let file = tmp.path().join("trim_only.txt");
+    std::fs::write(&file, b"line  ").unwrap();
+
+    let mut global = GlobalFlags::test_with_cwd(tmp.path());
+    global.apply = true;
+    global.trim_trailing_whitespace = true;
+    // ensure_final_newline left false; because an explicit policy flag is set,
+    // check-parity defaults must not force final newline.
+
+    let args = TidyArgs {
+        action: TidyAction::Fix {
+            paths: vec![".".to_string()],
+            dedent: None,
+            indent: None,
+            lines: None,
+        },
+        write: Default::default(),
+    };
+    let code = run(args, &global).unwrap();
+    assert_eq!(code, exit::SUCCESS);
+
+    let content = std::fs::read_to_string(&file).unwrap();
+    assert_eq!(
+        content, "line",
+        "explicit --trim-trailing-whitespace only should not force final newline"
+    );
+}
+
 #[test]
 fn fix_normalizes_eol() {
     let tmp = TempDir::new().unwrap();
