Skip to content

Validate path arg, fix non-cwd resolution, refactor _fix_file#7

Merged
lesnik512 merged 1 commit into
mainfrom
code-review-fixes-round-2
May 31, 2026
Merged

Validate path arg, fix non-cwd resolution, refactor _fix_file#7
lesnik512 merged 1 commit into
mainfrom
code-review-fixes-round-2

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Second round of code-review followups.

Summary

  • Bug — non-cwd invocation: match_tree_files yields paths relative to the supplied path, but the code opened them relative to cwd. Running eof-fixer /some/other/dir from anywhere except inside that dir raised FileNotFoundError. The existing test suite masked this because it always os.chdir(tempdir) before invoking. Resolve with path / filename. Adds a regression test.
  • Bug — argparse validation (Fix missing newline in output, clarify docs #6): path was declared "path to directory" but never validated. Passing a file raised an opaque pathspec error; passing a nonexistent path raised FileNotFoundError. Now exits via parser.error("path is not a directory: <p>") with code 2.
  • Bug — read-only files in --check mode (require 100% cov #8): every file was opened rb+ even in check mode, which blew up on read-only files for no reason (no writes were going to happen). Now opens rb when check=True. Adds a test that chmods a file to 0o444 and verifies --check reports it without raising.
  • Refactor (#10): _fix_file carried # noqa: C901, PLR0911 because it did binary detection, end-of-file inspection, and writing all in one body. Split into _detect_trailing(file_obj) -> tuple[str, int] (pure inspection — returns ("none", 0) / ("append_lf", 0) / ("truncate", offset)) and a small dispatcher in _fix_file. Both suppressions removed. No behavior change.
  • Docs (migrate to ty and update packages #5): README note that an appended terminator is always LF, regardless of the file's existing CRLF/CR style — matches upstream pre-commit-hooks behavior.
  • Tests (ci: align with modern-di #9): new coverage for CRLF (no/one/many trailing), CR-only, UTF-8 BOM, null byte beyond the first 1024 bytes, symlinks (skipped where unavailable), arg validation rejecting files and nonexistent paths, read-only files in --check, and the non-cwd regression. Factored a small _run_main_in() context manager that handles cwd/argv/stdout/stderr swapping; existing tests left untouched. Coverage stays at 100%.

No public API or output-format changes.

Test plan

  • just lint — ruff format, ruff check, ty — all pass with no new suppressions.
  • just test — 15/15 pass, 100% coverage on eof_fixer/main.py and tests.
  • grep -nE "noqa: (C901 |PLR0911)" eof_fixer/main.py — empty.
  • Smoke: eof-fixer pyproject.toml → exit 2, stderr "path is not a directory: …".
  • Smoke: eof-fixer /nonexistent → exit 2, stderr "path is not a directory: …".
  • Smoke: tmp=$(mktemp -d) && printf a > "$tmp/ro.txt" && chmod 444 "$tmp/ro.txt" && eof-fixer "$tmp" --check → prints Fixing ro.txt, exit 1, no traceback.

🤖 Generated with Claude Code

- Reject non-directory paths up front via `parser.error` instead of
  letting `pathspec` raise an obscure error later (#6).
- Open files `rb` in `--check` mode so read-only files don't raise
  `OSError`; the write paths in `_fix_file` were already guarded (#8).
- Resolve walked filenames against the supplied `path`, not cwd.
  Previously `eof-fixer /some/dir` from elsewhere raised
  `FileNotFoundError` on every file.
- Split `_fix_file` into `_detect_trailing` (pure inspection) +
  dispatch, dropping `# noqa: C901, PLR0911` (#10).
- README: note that an appended terminator is always LF regardless
  of the file's existing CRLF/CR style (#5).
- Add tests for CRLF / CR / BOM / late null byte / symlinks /
  read-only-in-check / arg validation / non-cwd invocation, all
  encoding current behavior (#9). Coverage stays at 100%.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lesnik512 lesnik512 self-assigned this May 31, 2026
@lesnik512 lesnik512 merged commit 205d5df into main May 31, 2026
6 checks passed
@lesnik512 lesnik512 deleted the code-review-fixes-round-2 branch May 31, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant