diff --git a/.cargo/config.toml b/.cargo/config.toml index 8255159f..a9f400b3 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -35,5 +35,5 @@ split-debuginfo = "unpacked" t = "nextest run --no-capture" qt = "nextest run --no-fail-fast --no-capture" ci = "nextest run --profile ci --no-capture" -c = "clippy --all-targets" +c = "clippy --all-targets --features tokio,json" b = "build --all-targets" diff --git a/.cursorrules b/.cursorrules index 1a44cde4..3ba7096d 100644 --- a/.cursorrules +++ b/.cursorrules @@ -5,7 +5,7 @@ ## Quick Commands ```bash -cargo fmt && cargo clippy --all-targets && cargo nextest run +cargo fmt && cargo clippy --all-targets --features tokio,json && cargo nextest run # Or use the convenient aliases defined in .cargo/config.toml cargo c && cargo t diff --git a/.devcontainer/welcome.sh b/.devcontainer/welcome.sh index b1a468f9..1e5ce0fe 100755 --- a/.devcontainer/welcome.sh +++ b/.devcontainer/welcome.sh @@ -46,7 +46,7 @@ echo "" echo " Auditing & Quality:" echo " cargo audit # Security vulnerability check" echo " cargo deny check # License/dependency check" -echo " cargo clippy --all-targets # Linting" +echo " cargo clippy --all-targets --features tokio,json # Linting" echo " cargo shear # Unused dependency detection" echo " cargo spellcheck # Documentation spelling" echo " cargo geiger # Unsafe code audit" diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 8cad497c..5d9ce2cb 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -28,7 +28,7 @@ - No `panic!()` or `todo!()` - All fallible operations return `Result` - [ ] I have added tests that prove my fix is effective or my feature works -- [ ] I have run `cargo fmt && cargo clippy --all-targets` with no warnings +- [ ] I have run `cargo fmt && cargo clippy --all-targets --features tokio,json` with no warnings - [ ] I have run `cargo nextest run` and all tests pass ### If Applicable diff --git a/.github/workflows/ci-docs.yml b/.github/workflows/ci-docs.yml index fba0a0e4..be367c26 100644 --- a/.github/workflows/ci-docs.yml +++ b/.github/workflows/ci-docs.yml @@ -8,19 +8,12 @@ on: - '**.rs' - 'docs/**' - 'wiki/**' - - 'scripts/sync-wiki.py' - - 'scripts/check-wiki-consistency.py' - - 'scripts/validate-wiki-output.py' - - 'scripts/tests/test_sync_wiki.py' - - 'scripts/tests/test_check_wiki_consistency.py' + - 'scripts/**' - '.markdownlint.json' - '.markdown-link-check.json' - '.lychee.toml' - '.vale.ini' - '.vale/**' - - 'scripts/check-links.sh' - - 'scripts/verify-markdown-code.sh' - - 'scripts/check-code-fence-syntax.sh' - '.github/workflows/ci-docs.yml' pull_request: branches: [main] @@ -29,19 +22,12 @@ on: - '**.rs' - 'docs/**' - 'wiki/**' - - 'scripts/sync-wiki.py' - - 'scripts/check-wiki-consistency.py' - - 'scripts/validate-wiki-output.py' - - 'scripts/tests/test_sync_wiki.py' - - 'scripts/tests/test_check_wiki_consistency.py' + - 'scripts/**' - '.markdownlint.json' - '.markdown-link-check.json' - '.lychee.toml' - '.vale.ini' - '.vale/**' - - 'scripts/check-links.sh' - - 'scripts/verify-markdown-code.sh' - - 'scripts/check-code-fence-syntax.sh' - '.github/workflows/ci-docs.yml' workflow_dispatch: @@ -174,10 +160,10 @@ jobs: run: python scripts/check-wiki-consistency.py # ============================================================================ - # WIKI SYNC DRY-RUN - Validate wiki generation on PRs + # SCRIPT TESTS & WIKI DRY-RUN - Run all script tests and validate wiki generation # ============================================================================ wiki-sync-dry-run: - name: Wiki Sync Dry-Run + name: Script Tests & Wiki Dry-Run runs-on: ubuntu-latest steps: @@ -188,17 +174,11 @@ jobs: with: python-version: '3.12' - - name: Install pytest - run: pip install pytest + - name: Install test dependencies + run: pip install pytest pyyaml - - name: Run wiki sync tests - run: python -m pytest scripts/tests/test_sync_wiki.py -v - - - name: Run wiki validation tests - run: python -m pytest scripts/tests/test_validate_wiki_output.py -v - - - name: Run wiki consistency tests - run: python -m pytest scripts/tests/test_check_wiki_consistency.py -v + - name: Run script tests + run: python -m pytest scripts/tests/ -v - name: Generate wiki (dry-run) run: python scripts/sync-wiki.py --dest wiki-test-output diff --git a/.github/workflows/ci-rust.yml b/.github/workflows/ci-rust.yml index 632ecf5f..91992ba8 100644 --- a/.github/workflows/ci-rust.yml +++ b/.github/workflows/ci-rust.yml @@ -154,7 +154,7 @@ jobs: run: cargo fmt --check - name: Run clippy - run: cargo clippy --all-targets -- -D warnings + run: cargo clippy --all-targets --features tokio,json -- -D warnings # Miri undefined behavior check (cross-platform, optimized) # diff --git a/.github/workflows/ci-safety.yml b/.github/workflows/ci-safety.yml index 71e9a6a9..1e371adb 100644 --- a/.github/workflows/ci-safety.yml +++ b/.github/workflows/ci-safety.yml @@ -219,7 +219,7 @@ jobs: # Run clippy with nursery lints enabled # These are experimental but useful for catching additional issues # Lints from Cargo.toml are already applied; here we add nursery - cargo clippy --all-targets -- \ + cargo clippy --all-targets --features tokio,json -- \ -D warnings \ -W clippy::nursery \ -A clippy::significant_drop_tightening \ @@ -462,7 +462,7 @@ jobs: # - clippy::unimplemented: unimplemented!() macros # - clippy::unreachable: unreachable!() macros # - clippy::indexing_slicing: unchecked array/slice indexing - if cargo clippy --lib -- \ + if cargo clippy --lib --features tokio,json -- \ -D clippy::panic \ -D clippy::unwrap_used \ -D clippy::expect_used \ @@ -552,7 +552,7 @@ jobs: echo "- Review any warnings in the individual job logs" echo "- Replace \`.unwrap()\` with proper error handling where possible" echo "- Ensure all \`panic!\` calls are intentional and documented" - echo "- Run \`cargo clippy --all-targets\` locally before pushing" + echo "- Run \`cargo clippy --all-targets --features tokio,json\` locally before pushing" } >> "$GITHUB_STEP_SUMMARY" - name: Check for critical failures diff --git a/.llm/context.md b/.llm/context.md index 3e0e0fe5..be6635ff 100644 --- a/.llm/context.md +++ b/.llm/context.md @@ -17,7 +17,7 @@ ## Quick Commands ```bash -cargo fmt && cargo clippy --all-targets && cargo nextest run --no-capture # Pre-commit +cargo fmt && cargo clippy --all-targets --features tokio,json && cargo nextest run --no-capture # Pre-commit cargo c && cargo t # Aliases from .cargo/config.toml typos # Spell check (CI enforced) cargo test --features z3-verification -- --nocapture # Z3 proofs (slow) @@ -197,7 +197,7 @@ Consolidate integration tests into a single crate (`tests/it/main.rs`). Anti-pat ## Mandatory Linting -- **After Rust changes:** `cargo fmt && cargo clippy --all-targets` (or `cargo c`) +- **After Rust changes:** `cargo fmt && cargo clippy --all-targets --features tokio,json` (or `cargo c`) - **After workflow changes:** `actionlint` (no exceptions) - **After doc changes:** `cargo doc --no-deps` - **After markdown changes:** `npx markdownlint 'file.md' --config .markdownlint.json --fix` @@ -205,7 +205,7 @@ Consolidate integration tests into a single crate (`tests/it/main.rs`). Anti-pat - **Link validation:** `./scripts/check-links.sh` - **Spell check:** `typos` - **Vale (advisory):** `vale docs/` -- checks prose quality, non-blocking in CI -- **Full pre-commit:** `cargo fmt && cargo clippy --all-targets && cargo nextest run --no-capture` +- **Full pre-commit:** `cargo fmt && cargo clippy --all-targets --features tokio,json && cargo nextest run --no-capture` ## Skill Code Examples @@ -236,7 +236,7 @@ When changing public APIs, update: rustdoc comments (source of truth), README.md ## Quality Checklist - [ ] `cargo fmt` run -- [ ] `cargo clippy --all-targets` passes +- [ ] `cargo clippy --all-targets --features tokio,json` passes - [ ] All tests pass (`cargo nextest run`) - [ ] Tests for new functionality included - [ ] Rustdoc comments with examples @@ -248,7 +248,7 @@ When changing public APIs, update: rustdoc comments (source of truth), README.md ## For Agents -When spawning sub-agents or using Task tools: the sub-agent MUST run `cargo fmt` and verify `cargo clippy --all-targets` passes on any modified files. If the sub-agent cannot run these, the parent agent must run them after receiving changes. +When spawning sub-agents or using Task tools: the sub-agent MUST run `cargo fmt` and verify `cargo clippy --all-targets --features tokio,json` passes on any modified files. If the sub-agent cannot run these, the parent agent must run them after receiving changes. --- diff --git a/.llm/skills/async-rust.md b/.llm/skills/async-rust.md index c3700c5e..24739e09 100644 --- a/.llm/skills/async-rust.md +++ b/.llm/skills/async-rust.md @@ -83,6 +83,7 @@ Use bounded channels (`mpsc::channel(100)`) to prevent memory exhaustion. Never | Pitfall | Fix | |---------|-----| +| `#[track_caller]` on `async fn` | Not supported; extract a sync helper or remove the attribute | | `std::thread::sleep` in async | `tokio::time::sleep().await` | | Forgetting to `.await` futures | Futures are lazy -- nothing happens without await | | `std::sync::Mutex` held across `.await` | Use `tokio::sync::Mutex` or release before await | diff --git a/.llm/skills/ci-debugging.md b/.llm/skills/ci-debugging.md index e34e3310..ef119ca1 100644 --- a/.llm/skills/ci-debugging.md +++ b/.llm/skills/ci-debugging.md @@ -7,7 +7,7 @@ | Error Pattern | Category | Quick Fix | |---------------|----------|-----------| | `cargo fmt --check` fails | Formatting | `cargo fmt` | -| Clippy warnings | Linting | `cargo clippy --fix --allow-dirty` | +| Clippy warnings | Linting | `cargo clippy --all-targets --features tokio,json --fix --allow-dirty` | | Test assertion failures | Test logic | `RUST_BACKTRACE=1 cargo test name -- --nocapture` | | `VERIFICATION RESULT: FAILURE` | Kani | Verify assertion matches impl; add `#[kani::unwind(N)]` | | `linker cc not found` | Cross-compilation | Check Cross.toml, avoid unstable image tags | @@ -34,7 +34,7 @@ cat .github/workflows/ci-*.yml | grep "run:" # Common reproductions cargo fmt --check -cargo clippy --all-targets -- -D warnings +cargo clippy --all-targets --features tokio,json -- -D warnings RUSTDOCFLAGS="-D warnings" cargo doc --no-deps cargo nextest run test_name --no-capture cargo kani --harness proof_function_name diff --git a/.llm/skills/refactoring.md b/.llm/skills/refactoring.md index 90ace531..1656d1b6 100644 --- a/.llm/skills/refactoring.md +++ b/.llm/skills/refactoring.md @@ -23,13 +23,13 @@ ```bash # Pre-refactoring -cargo nextest run && cargo clippy --all-targets +cargo nextest run && cargo clippy --all-targets --features tokio,json # After each change cargo check && cargo nextest run # Post-refactoring -cargo fmt && cargo clippy --all-targets && cargo nextest run +cargo fmt && cargo clippy --all-targets --features tokio,json && cargo nextest run rg "unwrap\(\)|expect\(|panic!\(|todo!\(" src/ ``` @@ -77,7 +77,7 @@ rg "unwrap\(\)|expect\(|panic!\(|todo!\(" src/ ### Pre-Refactoring - [ ] Tests pass: `cargo nextest run` -- [ ] No clippy warnings: `cargo clippy --all-targets` +- [ ] No clippy warnings: `cargo clippy --all-targets --features tokio,json` - [ ] Git commit current changes ### Post-Refactoring diff --git a/.llm/skills/rust-pitfalls.md b/.llm/skills/rust-pitfalls.md index 9f58f440..2dc4f6a0 100644 --- a/.llm/skills/rust-pitfalls.md +++ b/.llm/skills/rust-pitfalls.md @@ -98,6 +98,10 @@ Use `ok_or_else(|| ...)` when error construction allocates or is expensive. Use - **`#[serde(default)]` on required fields:** Silently accepts missing data. - **Enum representation:** Default serializes as `{"Active": null}`. Use `#[serde(rename_all = "snake_case")]`. +## Async Pitfalls + +- **`#[track_caller]` on `async fn`:** Not supported by Rust. The attribute is silently ignored or triggers a clippy warning/error. Extract a sync helper that carries `#[track_caller]` and call it from the async fn, or remove the attribute entirely. A pre-commit grep hook catches this. + ## Testing Pitfalls - **Tests pass on panic:** Function panics before assertion is reached. Use `#[should_panic]` or `catch_unwind`. @@ -135,3 +139,4 @@ Proofs must verify what they claim. If name says "independent", actually test mo - [ ] `std::` types in loom tests - [ ] `ok_or` vs `ok_or_else` for allocating errors - [ ] Pattern matching: `match` not `if let` when fallback needs value +- [ ] `#[track_caller]` on async fn (not supported) diff --git a/.llm/skills/scripting.md b/.llm/skills/scripting.md index cef0f191..69e0bd0f 100644 --- a/.llm/skills/scripting.md +++ b/.llm/skills/scripting.md @@ -14,23 +14,159 @@ - **Meaningful exit codes** via `sys.exit(main())` - **Errors to stderr**: `print("ERROR: ...", file=sys.stderr)` - **No `shell=True`** in subprocess calls +- **UTF-8 for both stdout and stderr** -- When wrapping `sys.stdout` with `io.TextIOWrapper` for UTF-8, always wrap `sys.stderr` too (Check 8 enforces this) -### Exception Handling +### Error Reporting in Lint Scripts + +Lint hooks must use `{path}:{line_number}: {message}` format so editors +hyperlink to the correct location. When a violation spans multiple lines +(e.g., attribute on line A, target on line B), the `path:line` prefix +must point to the **violation site** (where the fix is needed), not the +trigger/detection line. Mention the trigger line in the message body: ```python -# WRONG: silent swallowing -try: - content = file.read_text() -except OSError: - pass +# Multi-line: prefix → async fn line, body → attribute line +f"{path}:{fn_line}: #[track_caller] (line {attr_line}) on async fn ..." +``` + +Never prefix issue lines with leading whitespace in `main()` summary +output -- leading spaces break the `path:line:` prefix that editors rely +on for hyperlinking. This includes both f-string whitespace (e.g., +`print(f" {issue}")`) and string concatenation (e.g., +`print(" " + f"{prefix} {issue}")`). **Check 1b** in +`check-hook-output-format.py` detects the concatenation variant. + +For file-level errors where no specific line number exists (e.g., cannot +read file), use a synthetic line number `:0:` to maintain the format: + +```python +# File-level error: use :0: as synthetic line number +f"{path}:0: cannot read file: {exc}" +``` + +### Path Handling in Lint Output + +**Rule**: All user-facing path output must use **relative paths** (relative to +project root). Absolute paths break `{path}:{line}:` parsing on Windows +(drive letter colon `C:\...:0:`). -# CORRECT: comment explains why +**glob/rglob/iterdir scripts** produce absolute `Path` objects -- convert first: + +```python try: - content = file.read_text() -except OSError: - pass # File read errors are non-fatal; treat link as valid + rel = filepath.relative_to(project_root) +except ValueError: + rel = filepath +``` + +**argv-based scripts** receive string paths -- use a `_display_path()` helper: + +```python +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path.""" + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) +``` + +**Check 6** in `check-hook-output-format.py` flags raw path variables (like +`{filepath}`) in f-string error output when the file uses glob/rglob/iterdir. +Safe variable names that pass Check 6: `rel`, `display_path`, `rel_path`, +`relative`, `rel_index`. Function calls like `{_display_path(filepath)}` +also pass because the regex only matches simple `{var_name}:` patterns. + +### Exception Handling + +#### read\_text() Exceptions + +`path.read_text(encoding="utf-8")` can raise both `OSError` (missing/locked file) +and `UnicodeDecodeError` (non-UTF-8 bytes). Always catch both: + +```python +# WRONG # CORRECT +except OSError as e: except (OSError, UnicodeDecodeError) as e: + ... ... +``` + +Alternative: `errors="replace"` for best-effort reading (grep-like hooks). + +#### Read Error Propagation + +Hooks that cannot read a file must **fail**, not silently pass. + +For **list-returning** hooks (`check_file() -> list[str]`), return the error +in the issues list so `main()` sees it. Do **not** also print -- `main()` +prints returned issues, so printing here causes duplicate output: + +```python +except (OSError, UnicodeDecodeError) as exc: + return [f"{path}:0: cannot read file: {exc}"] # NOT return [] -- that silently passes +``` + +For **fixer hooks** (`fix_file() -> bool | None`), print to stderr and return +`None` to signal an error (distinct from `False` = no change needed): + +```python +except (OSError, UnicodeDecodeError) as exc: + print(f"{path}:0: cannot read file: {exc}", file=sys.stderr) + return None # NOT return False -- that silently passes +``` + +#### Parse Error Line Numbers + +Extract the real line number from parse exceptions instead of hard-coding `:1:`: + +```python +line = getattr(e, "lineno", 1) or 1 # fallback to 1 +print(f"{path}:{line}: TOML error: {e}", file=sys.stderr) +``` + +Line number attributes: `tomllib.TOMLDecodeError.lineno`, +`json.JSONDecodeError.lineno`, `yaml.YAMLError.problem_mark.line` (0-based). + +#### Fixer Hook Pattern + +Fixer hooks modify files in-place. They must use `bool | None` return type +so `main()` can distinguish "unchanged" from "error": + +```python +def fix_file(filepath: str) -> bool | None: + """Fix something. Returns True if modified, False if unchanged, None on error.""" + try: + content = Path(filepath).read_bytes() + # ... fix logic ... + return True # or False + except OSError as exc: # Use (OSError, UnicodeDecodeError) for read_text() + print(f"{filepath}:0: cannot read file: {exc}", file=sys.stderr) + return None # NOT False -- False means "no change", None means "error" + +def main() -> int: + had_error = False + modified = False + for filepath in sys.argv[1:]: + result = fix_file(filepath) + if result is True: + modified = True + elif result is None: + had_error = True + return 1 if modified or had_error else 0 +``` + +### Regex Patterns for f-string Detection + +Handle both quote styles and the `r` prefix (the only prefix that combines with `f`): + +```python +# WRONG: only matches double-quoted f-strings +re.search(r'f"\{(\w+)\}: cannot read', line) + +# CORRECT: both quotes + optional r prefix (rf/fr) +re.search(r'''r?fr?["']\{(\w+)\}:\s+cannot\s+read''', line) ``` +The `check-hook-output-format.py` pre-commit hook enforces these patterns. + ### Subprocess Best Practices ```python @@ -55,9 +191,14 @@ Do NOT catch `FileNotFoundError` after `shutil.which()` already validated existe ```python #!/usr/bin/env python3 """Brief description. Works on Windows, macOS, and Linux.""" +import io import sys from pathlib import Path +# Wrap BOTH streams for cross-platform Unicode (CP1252 safety) +sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding="utf-8", errors="replace") +sys.stderr = io.TextIOWrapper(sys.stderr.buffer, encoding="utf-8", errors="replace") + def get_project_root() -> Path: return Path(__file__).parent.parent.resolve() @@ -94,21 +235,6 @@ Methods: `test_empty_input_returns_empty_string`, `test_unclosed_div_is_handled_ ## Shell Script Portability -### sed -i (Critical) - -The #1 cross-platform `sed` failure: - -```bash -# WRONG on macOS: -sed -i 's/old/new/g' file.txt - -# PORTABLE: use backup extension, then remove -sed -i.bak 's/old/new/g' file.txt && rm file.txt.bak - -# ALTERNATIVE: temp file pattern -sed 's/old/new/g' file.txt > file.txt.tmp && mv file.txt.tmp file.txt -``` - ### Portable Patterns Quick Reference | Task | Non-Portable | Portable | @@ -124,63 +250,18 @@ sed 's/old/new/g' file.txt > file.txt.tmp && mv file.txt.tmp file.txt | Canonical path | `readlink -f path` | `realpath path` | | Binary path | `/bin/sed 's/.../g'` | `sed 's/.../g'` (rely on PATH) | -### Backtick Escaping - -| Context | Backtick Handling | -|---------|-------------------| -| Single quotes `'...'` | Literal, no escaping needed | -| Double quotes `"..."` | Must escape: `\`cmd\`` | -| Heredoc `<< 'EOF'` | Literal, no escaping needed | -| Heredoc `<< EOF` | Executes -- avoid or escape | - ### GNU grep Extensions (Avoid) `--include`, `--exclude`, `-P` (Perl regex) are GNU-only. Use `find` + `grep` and `sed` instead. ### Best Practices -```bash -#!/bin/bash -set -euo pipefail # Strict mode - -# Check dependencies -command -v jq >/dev/null 2>&1 || { echo "Error: jq required" >&2; exit 1; } - -# Always quote variables -rm "$file" - -# Use $() not backticks -result=$(command) - -# Tool availability with fallback -if command -v sd &>/dev/null; then - sd 'pattern' 'replacement' file -else - sed -i.bak -E 's|pattern|replacement|g' file && rm -f file.bak -fi -``` - -### Platform Detection - -```bash -case "$(uname -s)" in - Linux*) OS=linux ;; - Darwin*) OS=macos ;; - MINGW*|CYGWIN*) OS=windows ;; - *) OS=unknown ;; -esac -``` - -### CI-Specific - -```yaml -# GitHub Actions: use bash explicitly -- name: Run script - shell: bash - run: | - set -euo pipefail - ./scripts/my-script.sh -``` +- `set -euo pipefail` at the top of every script +- `command -v tool >/dev/null 2>&1 || { echo "Error" >&2; exit 1; }` for deps +- Always quote variables: `rm "$file"` +- Use `$()` not backticks +- Platform detection: `case "$(uname -s)" in Linux*) ... ;; Darwin*) ... ;; esac` +- GitHub Actions: always set `shell: bash` and `set -euo pipefail` --- @@ -195,79 +276,25 @@ esac | Multi-tool install | `pip install a b c` | Install individually with fallback | | Layer cleanup | Separate `RUN rm ...` | Clean up in the same `RUN` layer | -### pip Cache - -Always pass `--no-cache-dir` to avoid storing wheel/sdist caches in the image: - -```dockerfile -# WRONG: leaves pip cache in the layer -RUN pip install requests - -# CORRECT: no cache stored -RUN pip install --no-cache-dir requests -``` - -### Output Suppression - -Use `>/dev/null 2>&1` for silent command detection, not `>&2`: - -```bash -# WRONG: sends stdout to stderr (still visible) -command -v tool >&2 +### Key Rules -# CORRECT: suppresses all output -command -v tool >/dev/null 2>&1 -``` - -### Resilient Multi-Tool Installs - -Install optional tools individually so one failure does not block the rest: +- `pip install --no-cache-dir` always (no wheel cache in image layers) +- `command -v tool >/dev/null 2>&1` for silent detection (not `>&2`) +- Install optional tools individually with `|| echo "failed"` fallback +- Guard `.bashrc` aliases/`eval` with `command -v` when tools are optional +- Clean up caches in the **same** `RUN` layer (otherwise bytes persist) ```dockerfile -# WRONG: one bad package fails the entire install -RUN pip install --no-cache-dir tool-a tool-b tool-c - -# CORRECT: each tool installed independently with fallback -RUN for tool in tool-a tool-b tool-c; do \ - pip install --no-cache-dir "$tool" \ - || echo "$tool: failed to install"; \ - done +# CORRECT: single layer, cleanup at the end +RUN apt-get update \ + && apt-get install -y --no-install-recommends curl \ + && rm -rf /var/lib/apt/lists/* ``` -### Guard Optional Tool Aliases in Shell Init - -When tools are installed with fallback (`|| echo "skipped"`), any -aliases or `eval` init in `.bashrc` **must** be guarded with -`command -v`. Unguarded aliases break the shell if the tool is missing: - ```bash -# WRONG: breaks ls if eza was not installed -alias ls="eza" -eval "$(zoxide init bash)" - -# CORRECT: only alias if tool exists +# Guard optional tool aliases (check-dockerfile enforces unguarded eval) command -v eza >/dev/null 2>&1 && alias ls="eza" command -v zoxide >/dev/null 2>&1 && eval "$(zoxide init bash)" ``` -The pre-commit hook `check-dockerfile` enforces unguarded `eval "$("` -detection. Unguarded aliases are caught by code review. Mandatory -apt-installed tools (e.g., `batcat`, `htop`) do not need guards -since `apt-get install` would fail the build. - -### Layer Hygiene - -Clean up caches, temp files, and package lists in the **same** `RUN` -layer that creates them -- otherwise deleted bytes still occupy space -in earlier layers: - -```dockerfile -# WRONG: cleanup in a separate layer does not reclaim space -RUN apt-get update && apt-get install -y curl -RUN rm -rf /var/lib/apt/lists/* - -# CORRECT: single layer, cleanup at the end -RUN apt-get update \ - && apt-get install -y --no-install-recommends curl \ - && rm -rf /var/lib/apt/lists/* -``` +Mandatory apt-installed tools do not need guards. diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ee93801f..d7744c94 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -82,6 +82,12 @@ repos: types: [rust] pass_filenames: false + - id: check-track-caller-async + name: check track_caller on async fn + entry: python scripts/hooks/check-track-caller-async.py + language: python + types: [rust] + - id: cargo-clippy name: cargo clippy entry: python scripts/run-cargo-clippy.py @@ -166,6 +172,16 @@ repos: files: '(Dockerfile|devcontainer\.json)$' pass_filenames: true + # ══════════════════════════════════════════════════════════════════════ + # Hook quality + # ══════════════════════════════════════════════════════════════════════ + - id: check-hook-output-format + name: check hook output format + entry: python scripts/hooks/check-hook-output-format.py + language: python + files: '^scripts/(hooks/)?[a-z][^/]*\.py$' + pass_filenames: true + # ══════════════════════════════════════════════════════════════════════ # CI validation (skip gracefully if tools not installed) # ══════════════════════════════════════════════════════════════════════ diff --git a/CLAUDE.md b/CLAUDE.md index 4d88a797..71176ecc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,7 +5,7 @@ ## Critical Reminders - **Zero-panic:** No `unwrap()`, `expect()`, `panic!()`, `todo!()` in production code -- **Pre-commit:** `cargo fmt && cargo clippy --all-targets && cargo nextest run --no-capture` (or `cargo c && cargo t`) +- **Pre-commit:** `cargo fmt && cargo clippy --all-targets --features tokio,json && cargo nextest run --no-capture` (or `cargo c && cargo t`) - **Kani:** Always add `#[kani::unwind(N)]` to proofs; CI uses `--default-unwind 8` via `--quick` mode - **Changelog:** Ask "Does this affect `pub` items or user-observable behavior?" — if yes, update `CHANGELOG.md` diff --git a/scripts/check-kani-coverage.py b/scripts/check-kani-coverage.py index 8b610b73..a4ec751d 100644 --- a/scripts/check-kani-coverage.py +++ b/scripts/check-kani-coverage.py @@ -26,6 +26,19 @@ def get_project_root() -> Path: return get_script_dir().parent +def _display_path(path: Path, project_root: Path) -> Path: + """Convert an absolute path to a project-relative display path. + + Unlike the CWD-based ``_display_path`` in argv-receiving hook scripts, + this variant takes an explicit *project_root* because the script discovers + files via ``rglob()`` and always knows its own root. + """ + try: + return path.relative_to(project_root) + except ValueError: + return path + + def find_source_proofs(project_root: Path) -> set[str]: """Find all proof function names in source code.""" proofs = set() @@ -54,7 +67,7 @@ def find_source_proofs(project_root: Path) -> set[str]: break except (OSError, UnicodeDecodeError) as e: - print(f"Warning: Could not read {rs_file}: {e}", file=sys.stderr) + print(f"{_display_path(rs_file, project_root)}:0: cannot read file: {e}", file=sys.stderr) return proofs @@ -65,7 +78,7 @@ def find_script_proofs(project_root: Path) -> set[str]: verify_script = project_root / "scripts" / "verify-kani.sh" if not verify_script.exists(): - print(f"Warning: {verify_script} not found", file=sys.stderr) + print(f"{_display_path(verify_script, project_root)}:0: file not found", file=sys.stderr) return proofs try: @@ -79,7 +92,7 @@ def find_script_proofs(project_root: Path) -> set[str]: proofs.add(match.group(1)) except (OSError, UnicodeDecodeError) as e: - print(f"Warning: Could not read {verify_script}: {e}", file=sys.stderr) + print(f"{_display_path(verify_script, project_root)}:0: cannot read file: {e}", file=sys.stderr) return proofs @@ -113,7 +126,7 @@ def find_script_proof_tiers(project_root: Path) -> dict[str, int]: proof_tiers[proof_match.group(1)] = tier except (OSError, UnicodeDecodeError) as e: - print(f"Warning: Could not read {verify_script}: {e}", file=sys.stderr) + print(f"{_display_path(verify_script, project_root)}:0: cannot read file: {e}", file=sys.stderr) return proof_tiers @@ -201,7 +214,7 @@ def check_unwind_attributes( advisories.append((fn_name, str(rel_path))) except (OSError, UnicodeDecodeError) as e: - print(f"Warning: Could not read {rs_file}: {e}", file=sys.stderr) + print(f"{_display_path(rs_file, project_root)}:0: cannot read file: {e}", file=sys.stderr) has_errors = False @@ -209,13 +222,15 @@ def check_unwind_attributes( has_errors = True print( f"\nERROR: {len(errors)} Tier 2/3 proof(s) missing required " - f"#[kani::unwind(N)]:" + f"#[kani::unwind(N)]:", + file=sys.stderr, ) for fn_name, file_path, tier in sorted(errors): print( f" ERROR: Tier {tier} proof '{fn_name}' in file '{file_path}' " f"has no #[kani::unwind(N)]. Tier 2/3 proofs MUST have explicit " - f"unwind bounds to prevent CI timeouts." + f"unwind bounds to prevent CI timeouts.", + file=sys.stderr, ) if advisories: @@ -228,7 +243,8 @@ def check_unwind_attributes( print( f" WARNING: proof '{fn_name}' in file '{file_path}' has no explicit " f"#[kani::unwind(N)]. CI uses --default-unwind 8; larger data " - f"structures may cause timeouts." + f"structures may cause timeouts.", + file=sys.stderr, ) else: print( @@ -260,16 +276,16 @@ def main() -> int: if missing_proofs: has_errors = True - print("ERROR: The following Kani proofs are NOT in verify-kani.sh:") + print("ERROR: The following Kani proofs are NOT in verify-kani.sh:", file=sys.stderr) for proof in sorted(missing_proofs): - print(f" - {proof}") - print("\nAdd them to one of the TIER*_PROOFS arrays in scripts/verify-kani.sh") + print(f" - {proof}", file=sys.stderr) + print("\nAdd them to one of the TIER*_PROOFS arrays in scripts/verify-kani.sh", file=sys.stderr) if extra_proofs: # This is a warning, not an error (could be commented out proofs) - print("\nWARNING: The following proofs are in verify-kani.sh but not in source:") + print("\nWARNING: The following proofs are in verify-kani.sh but not in source:", file=sys.stderr) for proof in sorted(extra_proofs): - print(f" - {proof}") + print(f" - {proof}", file=sys.stderr) if not has_errors: print(f"[OK] All {len(source_proofs)} Kani proofs are covered in verify-kani.sh") diff --git a/scripts/check-links.py b/scripts/check-links.py index af8c6a0c..d2ca2637 100644 --- a/scripts/check-links.py +++ b/scripts/check-links.py @@ -209,6 +209,14 @@ def is_wiki_file(source_file: Path, project_root: Path) -> bool: return False +def _rel(path: Path, root: Path) -> Path: + """Return *path* relative to *root*, falling back to *path* on ValueError.""" + try: + return path.relative_to(root) + except ValueError: + return path + + def check_markdown_link( source_file: Path, link_target: str, project_root: Path, verbose: bool = False ) -> tuple[bool, str]: @@ -229,9 +237,9 @@ def check_markdown_link( content = source_file.read_text(encoding="utf-8") anchors = extract_markdown_anchors(content) if anchor.lower() not in {a.lower() for a in anchors}: - return False, f"Anchor '{anchor}' not found in {source_file}" - except (OSError, UnicodeDecodeError): - pass # File read errors are non-fatal; treat link as valid + return False, f"Anchor '{anchor}' not found in {_rel(source_file, project_root)}" + except (OSError, UnicodeDecodeError) as exc: + return False, f"Cannot read file to validate anchor '{anchor}': {exc}" return True, "" # Handle anchor in link: file.md#anchor @@ -251,9 +259,9 @@ def check_markdown_link( if wiki_target_path.exists(): target_path = wiki_target_path else: - return False, f"Link target not found: {link_target} (from {source_file.relative_to(project_root)})" + return False, f"Link target not found: {link_target} (from {_rel(source_file, project_root)})" else: - return False, f"Link target not found: {link_target} (from {source_file.relative_to(project_root)})" + return False, f"Link target not found: {link_target} (from {_rel(source_file, project_root)})" # If there's an anchor, check it exists in target file if anchor and target_path.suffix.lower() == ".md": @@ -263,10 +271,13 @@ def check_markdown_link( if anchor.lower() not in {a.lower() for a in anchors}: return ( False, - f"Anchor '{anchor}' not found in {target_path.relative_to(project_root)}", + f"Anchor '{anchor}' not found in {_rel(target_path, project_root)}", ) - except (OSError, UnicodeDecodeError): - pass # File read errors are non-fatal; treat anchor as valid + except (OSError, UnicodeDecodeError) as exc: + return ( + False, + f"Cannot read {_rel(target_path, project_root)} to validate anchor '{anchor}': {exc}", + ) return True, "" @@ -282,7 +293,11 @@ def check_markdown_file( try: content = file_path.read_text(encoding="utf-8") except (OSError, UnicodeDecodeError) as e: - print(f"ERROR: Could not read {file_path}: {e}") + try: + rel_path = file_path.relative_to(project_root) + except ValueError: + rel_path = file_path + print(f"ERROR: Could not read {rel_path}: {e}", file=sys.stderr) return LinkCheckResult(errors=1, warnings=0, checked=0) # Find code fence ranges to skip @@ -315,8 +330,8 @@ def check_markdown_file( ) if not is_valid: errors += 1 - rel_path = file_path.relative_to(project_root) - print(f"ERROR: {rel_path}: {error_msg}") + rel_path = _rel(file_path, project_root) + print(f"ERROR: {rel_path}: {error_msg}", file=sys.stderr) return LinkCheckResult(errors=errors, warnings=warnings, checked=checked) diff --git a/scripts/check-rustdoc-links.py b/scripts/check-rustdoc-links.py index 2ddd0491..4af34d7c 100644 --- a/scripts/check-rustdoc-links.py +++ b/scripts/check-rustdoc-links.py @@ -54,20 +54,20 @@ def main() -> int: # If cargo doc failed (non-zero exit code), print stderr and fail if result.returncode != 0: - print("ERROR: cargo doc failed with rustdoc warnings/errors:") + print("ERROR: cargo doc failed with rustdoc warnings/errors:", file=sys.stderr) if result.stderr: - print(result.stderr) + print(result.stderr, file=sys.stderr) if result.stdout: - print(result.stdout) + print(result.stdout, file=sys.stderr) return 1 return 0 except FileNotFoundError: - print("ERROR: cargo not found. Is Rust installed?") + print("ERROR: cargo not found. Is Rust installed?", file=sys.stderr) return 1 except Exception as e: - print(f"ERROR: Failed to run cargo doc: {e}") + print(f"ERROR: Failed to run cargo doc: {e}", file=sys.stderr) return 1 diff --git a/scripts/check-wiki-consistency.py b/scripts/check-wiki-consistency.py index 8924402b..5c781b38 100644 --- a/scripts/check-wiki-consistency.py +++ b/scripts/check-wiki-consistency.py @@ -24,10 +24,12 @@ from pathlib import Path from typing import NamedTuple -# Ensure stdout uses UTF-8 encoding for Unicode symbols (✓, ✗, ⚠) +# Ensure stdout/stderr use UTF-8 encoding for Unicode symbols (✓, ✗, ⚠) # This fixes UnicodeEncodeError on Windows with CP1252 default encoding if sys.stdout.encoding != "utf-8": sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding="utf-8", errors="replace") +if sys.stderr.encoding != "utf-8": + sys.stderr = io.TextIOWrapper(sys.stderr.buffer, encoding="utf-8", errors="replace") class ValidationResult(NamedTuple): @@ -116,7 +118,7 @@ def parse_wiki_structure_from_sync_script(sync_script_path: Path) -> dict[str, s result[str(key.value)] = str(value.value) return result except (OSError, SyntaxError) as e: - print(red(f"ERROR: Could not parse {sync_script_path}: {e}")) + print(red(f"ERROR: Could not parse {sync_script_path}: {e}"), file=sys.stderr) return {} @@ -156,7 +158,7 @@ def parse_hardcoded_sidebar_from_sync_script(sync_script_path: Path) -> str | No if isinstance(stmt.value, ast.Constant): return str(stmt.value.value) except (OSError, SyntaxError) as e: - print(red(f"ERROR: Could not parse {sync_script_path}: {e}")) + print(red(f"ERROR: Could not parse {sync_script_path}: {e}"), file=sys.stderr) return None @@ -272,7 +274,8 @@ def validate_sync_script_sidebar_template( errors += 1 print( red("ERROR:") - + f" Could not extract hardcoded sidebar template from {sync_script_path}" + + f" Could not extract hardcoded sidebar template from {sync_script_path}", + file=sys.stderr, ) return ValidationResult(errors=errors, warnings=warnings) @@ -302,7 +305,8 @@ def validate_sync_script_sidebar_template( + f" {sync_script_path.name}:generate_sidebar(): " + f"Wiki-link [[{page_name}|{display_text}]] " + f"contains '{char}' in display text ({reason}). " - + f"Use standard markdown [Display](Page) syntax instead." + + f"Use standard markdown [Display](Page) syntax instead.", + file=sys.stderr, ) break # Only report first problematic character per link @@ -367,7 +371,7 @@ def parse_sidebar_wiki_links( content = sidebar_path.read_text(encoding="utf-8") return parse_wiki_links_from_string(content) except (OSError, UnicodeDecodeError) as e: - print(red(f"ERROR: Could not read {sidebar_path}: {e}")) + print(red(f"ERROR: Could not read {sidebar_path}: {e}"), file=sys.stderr) return [] @@ -433,7 +437,8 @@ def validate_wiki_link_display_text( red("ERROR:") + f" _Sidebar.md:{line_num}: Wiki-link [[{page_name}|{display_text}]] " + f"contains '{char}' in display text ({reason}). " - + f"Use standard markdown [Display](Page) syntax instead." + + f"Use standard markdown [Display](Page) syntax instead.", + file=sys.stderr, ) break # Only report first problematic character per link @@ -463,7 +468,8 @@ def validate_sidebar_links( print( red("ERROR:") + f" _Sidebar.md:{line_num}: Link to '{page_name}' " - + f"points to non-existent page '{page_name}.md'" + + f"points to non-existent page '{page_name}.md'", + file=sys.stderr, ) elif verbose: syntax_label = "wiki" if syntax_type == "wiki" else "md" @@ -495,7 +501,8 @@ def validate_wiki_structure_completeness( print( yellow("WARNING:") + f" docs/{unmapped} has no mapping in WIKI_STRUCTURE " - + "(scripts/sync-wiki.py)" + + "(scripts/sync-wiki.py)", + file=sys.stderr, ) # Also check for stale mappings (mapped files that no longer exist) @@ -506,7 +513,8 @@ def validate_wiki_structure_completeness( print( yellow("WARNING:") + f" WIKI_STRUCTURE contains mapping for '{stale}' " - + "which does not exist in docs/" + + "which does not exist in docs/", + file=sys.stderr, ) return ValidationResult(errors=errors, warnings=warnings) @@ -546,7 +554,8 @@ def validate_sidebar_completeness( warnings += 1 print( yellow("WARNING:") - + f" Wiki page '{missing}.md' has no entry in _Sidebar.md" + + f" Wiki page '{missing}.md' has no entry in _Sidebar.md", + file=sys.stderr, ) return ValidationResult(errors=errors, warnings=warnings) @@ -632,17 +641,19 @@ def validate_table_pipe_escaping( print( red("ERROR:") + f" {md_file.name}:{line_num}: Table cell contains " - + f"unescaped pipe in code span: `{code_content}`" + + f"unescaped pipe in code span: `{code_content}`", + file=sys.stderr, ) print( " " + yellow("Tip:") + " Escape the pipe with backslash: " - + f"`{code_content.replace('|', chr(92) + '|')}`" + + f"`{code_content.replace('|', chr(92) + '|')}`", + file=sys.stderr, ) except (OSError, UnicodeDecodeError) as e: - print(red(f"ERROR: Could not read {md_file}: {e}")) + print(red(f"ERROR: Could not read {md_file}: {e}"), file=sys.stderr) errors += 1 return ValidationResult(errors=errors, warnings=warnings) @@ -682,7 +693,8 @@ def validate_markdown_link_syntax(wiki_dir: Path, verbose: bool = False) -> Vali print( red("ERROR:") + f" {md_file.name}:{line_num}: Malformed link with space " - + f"after '[': {match.group()}" + + f"after '[': {match.group()}", + file=sys.stderr, ) # Check for space before closing bracket @@ -691,7 +703,8 @@ def validate_markdown_link_syntax(wiki_dir: Path, verbose: bool = False) -> Vali print( yellow("WARNING:") + f" {md_file.name}:{line_num}: Link has trailing space " - + f"before ']': {match.group()}" + + f"before ']': {match.group()}", + file=sys.stderr, ) # Check for empty link text @@ -700,11 +713,12 @@ def validate_markdown_link_syntax(wiki_dir: Path, verbose: bool = False) -> Vali print( yellow("WARNING:") + f" {md_file.name}:{line_num}: Empty link text: " - + f"{match.group()}" + + f"{match.group()}", + file=sys.stderr, ) except (OSError, UnicodeDecodeError) as e: - print(red(f"ERROR: Could not read {md_file}: {e}")) + print(red(f"ERROR: Could not read {md_file}: {e}"), file=sys.stderr) errors += 1 return ValidationResult(errors=errors, warnings=warnings) @@ -730,15 +744,15 @@ def main() -> int: # Check required files/directories exist if not wiki_dir.exists(): - print(red("ERROR:") + f" Wiki directory not found: {wiki_dir}") + print(red("ERROR:") + f" Wiki directory not found: {wiki_dir}", file=sys.stderr) return 1 if not sidebar_path.exists(): - print(red("ERROR:") + f" Sidebar not found: {sidebar_path}") + print(red("ERROR:") + f" Sidebar not found: {sidebar_path}", file=sys.stderr) return 1 if not sync_script_path.exists(): - print(red("ERROR:") + f" Sync script not found: {sync_script_path}") + print(red("ERROR:") + f" Sync script not found: {sync_script_path}", file=sys.stderr) return 1 # Get wiki pages and parse WIKI_STRUCTURE @@ -747,7 +761,7 @@ def main() -> int: docs_files = get_docs_source_files(docs_dir) if not wiki_structure: - print(red("ERROR:") + " Could not parse WIKI_STRUCTURE from sync-wiki.py") + print(red("ERROR:") + " Could not parse WIKI_STRUCTURE from sync-wiki.py", file=sys.stderr) return 1 if verbose: diff --git a/scripts/hooks/actionlint.py b/scripts/hooks/actionlint.py index 98c5af92..4319b8bb 100644 --- a/scripts/hooks/actionlint.py +++ b/scripts/hooks/actionlint.py @@ -43,14 +43,17 @@ def main() -> int: if not files: # No files provided, check all workflows - repo_root = Path(__file__).parent.parent.parent + # Use relative paths so actionlint output follows {path}:{line}: convention + repo_root = Path(__file__).resolve().parent.parent.parent workflows_dir = repo_root / ".github" / "workflows" if workflows_dir.exists(): - files = [ - str(f) - for f in workflows_dir.iterdir() - if f.suffix in (".yml", ".yaml") - ] + files = [] + for f in workflows_dir.iterdir(): + if f.suffix in (".yml", ".yaml"): + try: + files.append(str(f.relative_to(repo_root))) + except ValueError: + files.append(str(f)) if not files: return 0 # No workflow files to check diff --git a/scripts/hooks/check-dockerfile.py b/scripts/hooks/check-dockerfile.py index a0a01db6..ae03b676 100644 --- a/scripts/hooks/check-dockerfile.py +++ b/scripts/hooks/check-dockerfile.py @@ -15,18 +15,26 @@ from pathlib import Path -def check_file(filepath: Path) -> list[str]: +def check_file(filepath: Path, repo_root: Path | None = None) -> list[str]: """Check a single file for Dockerfile anti-patterns. Returns a list of issue descriptions (empty if no issues). + When repo_root is provided, paths in output are relative to it. """ issues: list[str] = [] + if repo_root is not None: + try: + display_path = filepath.relative_to(repo_root) + except ValueError: + display_path = filepath + else: + display_path = filepath + try: lines = filepath.read_text(encoding="utf-8").splitlines() except (OSError, UnicodeDecodeError) as exc: - print(f"Warning: skipping {filepath}: {exc}", file=sys.stderr) - return [] + return [f"{display_path}:0: cannot read file: {exc}"] is_dockerfile = filepath.name.startswith("Dockerfile") @@ -40,14 +48,14 @@ def check_file(filepath: Path) -> list[str]: # Check 1: pip install without --no-cache-dir if re.search(r"\bpip3?\s+install\b", stripped) and "--no-cache-dir" not in stripped: issues.append( - f"{filepath}:{line_num}: pip install without --no-cache-dir " + f"{display_path}:{line_num}: pip install without --no-cache-dir " f"(leaves pip cache in the image)" ) # Check 2: command -v with stderr redirect instead of /dev/null if re.search(r"\bcommand\s+-v\b", stripped) and re.search(r">&2", stripped): issues.append( - f"{filepath}:{line_num}: command -v output redirected to stderr " + f"{display_path}:{line_num}: command -v output redirected to stderr " f"instead of /dev/null (use >/dev/null 2>&1)" ) @@ -56,7 +64,7 @@ def check_file(filepath: Path) -> list[str]: r"\bcommand\s+-v\b", stripped ): issues.append( - f"{filepath}:{line_num}: eval \"$(...)\" without command -v guard " + f"{display_path}:{line_num}: eval \"$(...)\" without command -v guard " f"(use: command -v tool >/dev/null 2>&1 && eval \"$(tool init ...)\")" ) @@ -67,9 +75,10 @@ def main() -> int: """Check Dockerfiles and devcontainer.json for anti-patterns.""" files = sys.argv[1:] if len(sys.argv) > 1 else [] + repo_root = Path(__file__).resolve().parent.parent.parent + if not files: # No files provided, scan for all Dockerfiles and devcontainer.json - repo_root = Path(__file__).parent.parent.parent for path in repo_root.rglob("Dockerfile*"): files.append(str(path)) for path in repo_root.rglob("devcontainer.json"): @@ -90,14 +99,14 @@ def main() -> int: ): continue - issues = check_file(filepath) + issues = check_file(filepath, repo_root) all_issues.extend(issues) if all_issues: - print("Dockerfile anti-patterns detected:") + print("Dockerfile anti-patterns detected:", file=sys.stderr) for issue in all_issues: - print(f" {issue}") - print(f"\n{len(all_issues)} issue(s) found.") + print(issue, file=sys.stderr) + print(f"\n{len(all_issues)} issue(s) found.", file=sys.stderr) return 1 return 0 diff --git a/scripts/hooks/check-empty-tests.py b/scripts/hooks/check-empty-tests.py index 7c318a13..82da5779 100644 --- a/scripts/hooks/check-empty-tests.py +++ b/scripts/hooks/check-empty-tests.py @@ -15,6 +15,18 @@ from pathlib import Path +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + def is_empty_function(node: ast.FunctionDef | ast.AsyncFunctionDef) -> bool: """Check if a function is empty (has only pass, docstring, or ellipsis). @@ -58,11 +70,11 @@ def check_file(filepath: Path) -> list[str]: try: content = filepath.read_text(encoding="utf-8") tree = ast.parse(content, filename=str(filepath)) - except (SyntaxError, OSError, UnicodeDecodeError): - # SyntaxError: Let other tools catch syntax errors - # OSError: File not readable (permissions, deleted, etc.) - # UnicodeDecodeError: Non-UTF-8 files (rare for Python tests) + except SyntaxError: + # Let other tools catch syntax errors return [] + except (OSError, UnicodeDecodeError) as exc: + return [f"{_display_path(filepath)}:0: cannot read file: {exc}"] # Collect all functions that are inside classes (methods) methods_in_classes: set[int] = set() @@ -73,7 +85,7 @@ def check_file(filepath: Path) -> list[str]: methods_in_classes.add(id(item)) if item.name.startswith("test_") and is_empty_function(item): errors.append( - f"{filepath}:{item.lineno}: Empty test method " + f"{_display_path(filepath)}:{item.lineno}: Empty test method " f"'{node.name}.{item.name}'" ) @@ -84,7 +96,7 @@ def check_file(filepath: Path) -> list[str]: continue # Already checked as a class method if node.name.startswith("test_") and is_empty_function(node): errors.append( - f"{filepath}:{node.lineno}: Empty test function '{node.name}'" + f"{_display_path(filepath)}:{node.lineno}: Empty test function '{node.name}'" ) return errors @@ -112,10 +124,10 @@ def main() -> int: all_errors.extend(errors) if all_errors: - print("Empty test methods detected:") + print("Empty test methods detected:", file=sys.stderr) for error in all_errors: - print(f" {error}") - print("\nTest methods must have at least one assertion or meaningful code.") + print(error, file=sys.stderr) + print("\nTest methods must have at least one assertion or meaningful code.", file=sys.stderr) return 1 return 0 diff --git a/scripts/hooks/check-hook-output-format.py b/scripts/hooks/check-hook-output-format.py new file mode 100644 index 00000000..58871d7a --- /dev/null +++ b/scripts/hooks/check-hook-output-format.py @@ -0,0 +1,327 @@ +#!/usr/bin/env python3 +"""Check that lint hook scripts follow output format conventions. + +Validates: +- No leading whitespace on issue output lines (breaks editor hyperlinking) +- Error messages include line numbers in {path}:{line}: {message} format +- No Warning: prints that bypass the {path}:{line}: format convention +- No print() followed by return-in-list (causes duplicate output) +- No except-pass/except-return-fallback that swallows I/O errors (fail-open) +- No raw path variables in error output when file uses glob/rglob/iterdir + (absolute paths break {path}:{line}: parsing on Windows due to drive letter + colons -- use relative_to() or a display_path variable instead) +- No ERROR:/WARNING: diagnostic prints going to stdout (must use file=sys.stderr) +- sys.stdout wrapped with UTF-8 encoding but sys.stderr not wrapped (both + must be wrapped for cross-platform Unicode support) + +Cross-platform: Works on Linux, macOS, and Windows. +""" +from __future__ import annotations + +import re +import sys +from pathlib import Path + + +def check_file(filepath: Path, repo_root: Path | None = None) -> list[str]: + """Check a hook script for output format violations. + + Returns a list of issue descriptions (empty if no issues). + When repo_root is provided, paths in output are relative to it. + """ + issues: list[str] = [] + + if repo_root is not None: + try: + display_path = filepath.relative_to(repo_root) + except ValueError: + display_path = filepath + else: + display_path = filepath + + try: + content = filepath.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError) as exc: + return [f"{display_path}:0: cannot read file: {exc}"] + + lines = content.splitlines() + + for line_num, line in enumerate(lines, start=1): + stripped = line.strip() + + # Skip comments and blank lines + if stripped.startswith("#") or not stripped: + continue + + # Check 1: Indented issue printing in f-strings + # Detect print(f" {variable}") which adds leading whitespace to + # lint output, breaking editor hyperlinking on the path:line: prefix. + if re.search(r"""print\(r?fr?["']\s+\{""", stripped): + issues.append( + f"{display_path}:{line_num}: print() with leading whitespace " + f"in f-string breaks editor hyperlinking -- remove the " + f"leading spaces" + ) + + # Check 1b: Leading whitespace via string concatenation + # Detect print(" " + ...) or print(' ' + ...) which adds leading + # whitespace via concatenation, breaking editor hyperlinking. + if re.search(r"""print\(["']\s+["']\s*\+""", stripped): + issues.append( + f"{display_path}:{line_num}: print() with concatenated " + f"leading-whitespace string breaks editor hyperlinking " + f"-- remove the leading-space string" + ) + + # Check 2: Error messages missing line numbers + # Detect f"{path_var}: cannot read" that should be f"{path_var}:0: cannot read" + match = re.search(r'''r?fr?["']\{(\w+)\}:\s+cannot\s+read''', stripped) + if match: + var_name = match.group(1) + # Not already in {path}:{num}: or {path}:{var}: format + has_line_num = re.search( + r"\{" + re.escape(var_name) + r"\}:\d+:", stripped + ) + has_line_var = re.search( + r"\{" + re.escape(var_name) + r"\}:\{", stripped + ) + if not has_line_num and not has_line_var: + issues.append( + f"{display_path}:{line_num}: error message missing line " + f"number -- use {{path}}:0: for file-level errors" + ) + + # Check 3: Warning: prints that bypass lint format + # Detect print(f"Warning: ...") which produces output that doesn't + # follow {path}:{line}: {message} format and is often duplicative + # when check_file() also returns a formatted error. + if re.search(r'''print\(r?fr?["']Warning:\s''', stripped): + issues.append( + f"{display_path}:{line_num}: print() with Warning: prefix " + f"bypasses {{path}}:{{line}}: format -- return a " + f"formatted error instead" + ) + + # Check 4: Dual-output anti-pattern (print + return in list) + # Detect print(..., file=sys.stderr) followed by return [...] within + # 3 lines. This causes duplicate output: check_file() prints AND + # returns the message, then main() prints it again from the list. + if re.search(r"print\(.*file=sys\.stderr\)", stripped): + # Look ahead up to 3 lines for a return-list + for ahead in range(1, 4): + if line_num - 1 + ahead < len(lines): + next_line = lines[line_num - 1 + ahead].strip() + if re.search(r"return\s+\[(?!\])", next_line): + issues.append( + f"{display_path}:{line_num}: print() followed by " + f"return-in-list causes duplicate output -- " + f"remove the print() and let the caller print" + ) + break + + # Check 5: Fail-open anti-pattern (except + pass/return fallback) + # Detect except blocks for I/O errors that silently swallow the + # error via `pass` or by returning a fallback/default value. + # This allows hooks to exit 0 on unreadable files. + if re.search( + r"except\s+\(?\s*(?:OSError|UnicodeDecodeError|IOError)" + r"[\w\s,]*\)?\s*:", + stripped, + ): + # Look ahead up to 2 lines for `pass` or `return ` + for ahead in range(1, 3): + idx = line_num - 1 + ahead + if idx < len(lines): + next_line = lines[idx].strip() + # Skip blank lines + if not next_line: + continue + # Bare `pass` swallows the error entirely + if next_line == "pass" or next_line.startswith(("pass ", "pass\t")): + issues.append( + f"{display_path}:{line_num}: except block swallows " + f"I/O error with pass -- fail closed by " + f"returning an error or re-raising" + ) + break + # `return True` treats an unreadable file as valid + if re.search(r"return\s+True\b", next_line): + issues.append( + f"{display_path}:{line_num}: except block returns " + f"True on I/O error -- fail closed by " + f"returning False or raising" + ) + break + # `return []` or `return ""` or `return ''` treats + # an unreadable file as having no issues/content + if re.search(r"return\s+(\[\]|\"\"|\'\')(\s|$)", next_line): + issues.append( + f"{display_path}:{line_num}: except block returns " + f"empty value on I/O error -- fail closed by " + f"returning an error indicator" + ) + break + # Any other statement means the except block has real + # logic -- stop looking + break + + # Check 7: ERROR/WARNING prints going to stdout instead of stderr + # Detect print() calls containing ERROR: or WARNING: diagnostic + # prefixes that do not include file=sys.stderr. These messages + # must go to stderr per project conventions. + # + # Handles both single-line and multi-line cases: + # Single-line: print("ERROR: something") + # Multi-line: print( + # f" WARNING: proof ..." + # ) + # For multi-line, we check if we're inside an open print() call + # (tracked by in_print_call) and look for ERROR:/WARNING: on + # continuation lines. + if re.search( + r"""print\(.*(?:ERROR|WARNING)\s*:""", stripped + ) and "file=sys.stderr" not in stripped: + issues.append( + f"{display_path}:{line_num}: print() with ERROR:/WARNING: diagnostic goes to stdout -- add file=sys.stderr" + ) + + # Check 7b: Multi-line print() calls with ERROR:/WARNING: on + # continuation lines. Walk the file a second time, tracking open + # print( calls and scanning their bodies for diagnostic prefixes. + in_print = False + print_start_line = 0 + print_lines: list[str] = [] + paren_depth = 0 + for line_num, line in enumerate(lines, start=1): + stripped = line.strip() + if stripped.startswith("#") or not stripped: + if in_print: + # accumulate even blank/comment lines inside a print call + print_lines.append(stripped) + continue + + if not in_print: + # Detect start of a print( call + if re.match(r"print\(", stripped): + in_print = True + print_start_line = line_num + print_lines = [stripped] + paren_depth = stripped.count("(") - stripped.count(")") + if paren_depth <= 0: + # Single-line — already handled by Check 7 above + in_print = False + else: + print_lines.append(stripped) + paren_depth += stripped.count("(") - stripped.count(")") + if paren_depth <= 0: + # End of multi-line print call — analyse the joined body + joined = " ".join(print_lines) + has_diag = re.search(r"(?:ERROR|WARNING)\s*:", joined) + has_stderr = "file=sys.stderr" in joined + if has_diag and not has_stderr: + issues.append( + f"{display_path}:{print_start_line}: multi-line print() with ERROR:/WARNING: diagnostic goes to stdout -- add file=sys.stderr" + ) + in_print = False + + # Check 6: Raw path variables in error output when file uses glob/rglob + # When a script discovers files via glob(), rglob(), or iterdir(), the + # resulting Path objects are absolute. Using them directly in error + # output (e.g., f"{filepath}:0:") produces absolute paths that break + # the {path}:{line}: convention on Windows (drive letter colon, e.g., + # C:\...:0:). Scripts must convert to relative paths first. + uses_glob = any( + re.search(r"\.(rglob|glob|iterdir)\(", ln) + for ln in lines + if not ln.strip().startswith("#") + ) + if uses_glob: + # Allowed path variable names that indicate a relative/display path + safe_vars = {"rel", "display_path", "rel_path", "relative", "rel_index"} + for line_num, line in enumerate(lines, start=1): + stripped = line.strip() + if stripped.startswith("#") or not stripped: + continue + # Look for f-string path:line: patterns in error-producing lines + path_in_fstring = re.search( + r'''r?fr?["']\{(\w+)\}:\{?\w*\}?:''', stripped + ) + if path_in_fstring: + var_name = path_in_fstring.group(1) + if var_name not in safe_vars: + issues.append( + f"{display_path}:{line_num}: f-string uses " + f"{{{var_name}}} which may be absolute (file uses " + f"glob/rglob/iterdir) -- use relative_to() or a " + f"display_path variable" + ) + + # Check 8: sys.stdout wrapped with UTF-8 but sys.stderr not wrapped + # When a script wraps sys.stdout with io.TextIOWrapper for UTF-8 encoding + # (common for cross-platform Unicode support), sys.stderr must also be + # wrapped to avoid encoding errors when printing diagnostics that contain + # non-ASCII characters. + stdout_wrap_line = None + has_stderr_wrap = False + for line_num, line in enumerate(lines, start=1): + stripped = line.strip() + if stripped.startswith("#") or not stripped: + continue + if re.search( + r"sys\.stdout\s*=\s*io\.TextIOWrapper\s*\(\s*sys\.stdout\.buffer", + stripped, + ): + stdout_wrap_line = line_num + if re.search( + r"sys\.stderr\s*=\s*io\.TextIOWrapper\s*\(\s*sys\.stderr\.buffer", + stripped, + ): + has_stderr_wrap = True + if stdout_wrap_line is not None and not has_stderr_wrap: + issues.append( + f"{display_path}:{stdout_wrap_line}: sys.stdout wrapped with " + f"UTF-8 encoding but sys.stderr is not -- add matching " + f"sys.stderr wrapper for cross-platform Unicode support" + ) + + return issues + + +def main() -> int: + """Check lint hook scripts for output format violations.""" + files = sys.argv[1:] if len(sys.argv) > 1 else [] + repo_root = Path(__file__).resolve().parent.parent.parent + + if not files: + # Scan all check-*.py files in scripts/hooks/ + hooks_dir = Path(__file__).parent + for path in sorted(hooks_dir.glob("check-*.py")): + # Don't check ourselves + if path.name == "check-hook-output-format.py": + continue + files.append(str(path)) + + if not files: + return 0 + + all_issues: list[str] = [] + + for arg in files: + filepath = Path(arg) + if not filepath.name.endswith(".py"): + continue + issues = check_file(filepath, repo_root) + all_issues.extend(issues) + + if all_issues: + print("Hook output format violations detected:", file=sys.stderr) + for issue in all_issues: + print(issue, file=sys.stderr) + print(f"\n{len(all_issues)} violation(s) found.", file=sys.stderr) + return 1 + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/hooks/check-json.py b/scripts/hooks/check-json.py index 3daeb424..caf65e6d 100644 --- a/scripts/hooks/check-json.py +++ b/scripts/hooks/check-json.py @@ -2,7 +2,6 @@ """Validate JSON files. Cross-platform. Supports JSONC (JSON with Comments).""" import json -import re import sys from pathlib import Path @@ -67,6 +66,18 @@ def strip_jsonc_comments(content: str) -> str: return "".join(result) +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + def check_file(filepath: str) -> bool: """Check if JSON/JSONC file is valid. Returns True if valid.""" path = Path(filepath) @@ -83,10 +94,10 @@ def check_file(filepath: str) -> bool: json.loads(content) return True except json.JSONDecodeError as e: - print(f"JSON error in {filepath}: {e}") + print(f"{_display_path(filepath)}:{e.lineno}: JSON error: {e}", file=sys.stderr) return False - except OSError as e: - print(f"Cannot read {filepath}: {e}") + except (OSError, UnicodeDecodeError) as e: + print(f"{_display_path(filepath)}:0: cannot read file: {e}", file=sys.stderr) return False diff --git a/scripts/hooks/check-llm-line-limit.py b/scripts/hooks/check-llm-line-limit.py index 5c236715..ac32b835 100644 --- a/scripts/hooks/check-llm-line-limit.py +++ b/scripts/hooks/check-llm-line-limit.py @@ -19,22 +19,33 @@ def find_llm_md_files(repo_root: Path) -> list[Path]: return sorted(llm_dir.rglob("*.md")) -def check_file(filepath: Path, repo_root: Path) -> bool: - """Check if file exceeds the line limit. Returns True if within limit.""" +def check_file(filepath: Path, repo_root: Path | None = None) -> bool: + """Check if file exceeds the line limit. Returns True if within limit. + + When repo_root is provided, paths in output are relative to it. + """ + if repo_root is not None: + try: + display_path = filepath.relative_to(repo_root) + except ValueError: + display_path = filepath + else: + display_path = filepath + try: content = filepath.read_text(encoding="utf-8") line_count = len(content.splitlines()) if line_count > MAX_LINES: - rel = filepath.relative_to(repo_root) over = line_count - MAX_LINES print( - f"FAIL: {rel} has {line_count} lines " - f"({over} over the {MAX_LINES}-line limit)" + f"{display_path}:0: has {line_count} lines " + f"({over} over the {MAX_LINES}-line limit)", + file=sys.stderr, ) return False return True - except OSError as e: - print(f"Cannot read {filepath}: {e}") + except (OSError, UnicodeDecodeError) as e: + print(f"{display_path}:0: cannot read file: {e}", file=sys.stderr) return False @@ -52,7 +63,8 @@ def main() -> int: if not all_ok: print( - f"\nAll .md files under .llm/ must be {MAX_LINES} lines or fewer." + f"\nAll .md files under .llm/ must be {MAX_LINES} lines or fewer.", + file=sys.stderr, ) return 1 diff --git a/scripts/hooks/check-merge-conflict.py b/scripts/hooks/check-merge-conflict.py index cf4a6a24..ead606ba 100644 --- a/scripts/hooks/check-merge-conflict.py +++ b/scripts/hooks/check-merge-conflict.py @@ -5,6 +5,19 @@ import sys from pathlib import Path + +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + CONFLICT_PATTERNS = [ re.compile(rb"^<<<<<<<\s"), re.compile(rb"^>>>>>>>\s"), @@ -20,11 +33,12 @@ def check_file(filepath: str) -> bool: for i, line in enumerate(content.splitlines(), 1): for pattern in CONFLICT_PATTERNS: if pattern.match(line): - print(f"Merge conflict in {filepath}:{i}") + print(f"{_display_path(filepath)}:{i}: merge conflict marker found", file=sys.stderr) return False return True - except OSError: - return True # Skip files we can't read + except OSError as exc: + print(f"{_display_path(filepath)}:0: cannot read file: {exc}", file=sys.stderr) + return False # Fail if we can't read — don't let conflicts slip through def main() -> int: diff --git a/scripts/hooks/check-toml.py b/scripts/hooks/check-toml.py index 1b9be8a2..d11552ca 100644 --- a/scripts/hooks/check-toml.py +++ b/scripts/hooks/check-toml.py @@ -16,6 +16,18 @@ HAS_TOML = False +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + def check_file(filepath: str) -> bool: """Check if TOML file is valid. Returns True if valid.""" if not HAS_TOML: @@ -26,8 +38,15 @@ def check_file(filepath: str) -> bool: content = path.read_bytes() tomllib.loads(content.decode("utf-8")) return True - except Exception as e: - print(f"TOML error in {filepath}: {e}") + except UnicodeDecodeError as e: + print(f"{_display_path(filepath)}:0: cannot read file: {e}", file=sys.stderr) + return False + except ValueError as e: + line = getattr(e, "lineno", 1) or 1 + print(f"{_display_path(filepath)}:{line}: TOML error: {e}", file=sys.stderr) + return False + except OSError as e: + print(f"{_display_path(filepath)}:0: cannot read file: {e}", file=sys.stderr) return False @@ -36,7 +55,7 @@ def main() -> int: return 0 if not HAS_TOML: - print("Warning: tomllib/tomli not available, skipping TOML validation") + print("Skipping TOML validation: tomllib/tomli not available", file=sys.stderr) return 0 all_valid = True diff --git a/scripts/hooks/check-track-caller-async.py b/scripts/hooks/check-track-caller-async.py new file mode 100755 index 00000000..b47a006a --- /dev/null +++ b/scripts/hooks/check-track-caller-async.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +""" +Pre-commit hook: detect #[track_caller] on async fn. + +Rust does not support #[track_caller] on async functions (it is a no-op +or a compile error depending on the Rust version and lint configuration). +This fast grep-based check catches the mistake before a full compile. + +Known limitations (acceptable for a fast grep-based hook): + - ``#[cfg_attr(test, track_caller)]`` is not detected (rare in practice). + - ``#[track_caller]`` inside string literals may produce a false positive. + - Multi-line block comments are not tracked; a single-line block comment + containing the attribute (``/* #[track_caller] */``) is skipped, but + multi-line block comments are not fully parsed. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + + +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + +# Matches #[track_caller] (possibly with leading whitespace or other attrs on +# the same line) followed by an async fn within a few lines. +_TRACK_CALLER_RE = re.compile(r"#\[track_caller\]") +_ASYNC_FN_RE = re.compile(r"\basync\s+(?:unsafe\s+)?fn\b") + + +def check_file(path: Path) -> list[str]: + """Return a list of error messages for violations in *path*.""" + errors: list[str] = [] + try: + lines = path.read_text(encoding="utf-8", errors="replace").splitlines() + except OSError as exc: + return [f"{_display_path(path)}:0: cannot read file: {exc}"] + + for i, line in enumerate(lines): + stripped = line.strip() + # Skip comments and doc comments + if stripped.startswith("//"): + continue + # Skip single-line block comments + if stripped.startswith("/*") and stripped.endswith("*/"): + continue + if _TRACK_CALLER_RE.search(stripped): + # Check if async fn is on the same line as #[track_caller] + if _ASYNC_FN_RE.search(stripped): + errors.append( + f"{_display_path(path)}:{i + 1}: #[track_caller] on async fn " + f"is not supported by Rust" + ) + continue + # Look ahead up to 5 lines for an async fn + for j in range(i + 1, min(i + 6, len(lines))): + next_line = lines[j].strip() + if next_line.startswith("//"): + continue + # Skip single-line block comments in lookahead + if next_line.startswith("/*") and next_line.endswith("*/"): + continue + if _ASYNC_FN_RE.search(next_line): + errors.append( + f"{_display_path(path)}:{j + 1}: #[track_caller] (line {i + 1}) " + f"on async fn is not supported by Rust" + ) + break + # Stop lookahead if we hit a non-attribute, non-blank line + # that isn't an async fn + if next_line and not next_line.startswith("#["): + break + return errors + + +def main() -> int: + """Check all .rs files passed as arguments.""" + errors: list[str] = [] + for arg in sys.argv[1:]: + path = Path(arg) + if path.suffix == ".rs": + errors.extend(check_file(path)) + + if errors: + print("ERROR: #[track_caller] cannot be used on async fn:", file=sys.stderr) + for err in errors: + print(err, file=sys.stderr) + print( + "\nRust ignores or errors on #[track_caller] for async functions.", + file=sys.stderr, + ) + print("Remove the attribute or convert to a sync helper function.", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/hooks/check-yaml.py b/scripts/hooks/check-yaml.py index f6d968ef..cc594086 100644 --- a/scripts/hooks/check-yaml.py +++ b/scripts/hooks/check-yaml.py @@ -11,6 +11,18 @@ HAS_YAML = False +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + def check_file(filepath: str) -> bool: """Check if YAML file is valid. Returns True if valid.""" if not HAS_YAML: @@ -23,10 +35,11 @@ def check_file(filepath: str) -> bool: yaml.safe_load(content) return True except yaml.YAMLError as e: - print(f"YAML error in {filepath}: {e}") + line = (e.problem_mark.line + 1) if hasattr(e, 'problem_mark') and e.problem_mark else 1 + print(f"{_display_path(filepath)}:{line}: YAML error: {e}", file=sys.stderr) return False - except OSError as e: - print(f"Cannot read {filepath}: {e}") + except (OSError, UnicodeDecodeError) as e: + print(f"{_display_path(filepath)}:0: cannot read file: {e}", file=sys.stderr) return False @@ -35,7 +48,7 @@ def main() -> int: return 0 if not HAS_YAML: - print("Warning: PyYAML not installed, skipping YAML validation") + print("Skipping YAML validation: PyYAML not installed", file=sys.stderr) return 0 all_valid = True diff --git a/scripts/hooks/end-of-file-fixer.py b/scripts/hooks/end-of-file-fixer.py index ef5aea0f..16922bd4 100644 --- a/scripts/hooks/end-of-file-fixer.py +++ b/scripts/hooks/end-of-file-fixer.py @@ -5,8 +5,23 @@ from pathlib import Path -def fix_file(filepath: str) -> bool: - """Ensure file ends with exactly one newline. Returns True if modified.""" +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + +def fix_file(filepath: str) -> bool | None: + """Ensure file ends with exactly one newline. + + Returns True if modified, False if unchanged, None on error. + """ path = Path(filepath) try: content = path.read_bytes() @@ -19,24 +34,29 @@ def fix_file(filepath: str) -> bool: if fixed != content: path.write_bytes(fixed) - print(f"Fixed: {filepath}") + print(f"Fixed: {_display_path(filepath)}") return True return False - except OSError: - return False + except OSError as exc: + print(f"{_display_path(filepath)}:0: cannot read file: {exc}", file=sys.stderr) + return None def main() -> int: if len(sys.argv) < 2: return 0 + had_error = False modified = False for filepath in sys.argv[1:]: - if fix_file(filepath): + result = fix_file(filepath) + if result is True: modified = True + elif result is None: + had_error = True - return 1 if modified else 0 + return 1 if modified or had_error else 0 if __name__ == "__main__": diff --git a/scripts/hooks/markdownlint.py b/scripts/hooks/markdownlint.py index 618c075a..8574c0ac 100644 --- a/scripts/hooks/markdownlint.py +++ b/scripts/hooks/markdownlint.py @@ -26,8 +26,8 @@ def main() -> int: # Find markdownlint executable markdownlint = shutil.which("markdownlint") if markdownlint is None: - print("markdownlint not found. Install with: npm install -g markdownlint-cli") - print("Skipping markdown lint check.") + print("markdownlint not found. Install with: npm install -g markdownlint-cli", file=sys.stderr) + print("Skipping markdown lint check.", file=sys.stderr) return 0 # Don't fail if tool not available (Windows compatibility) # Build command with project configuration diff --git a/scripts/hooks/mixed-line-ending.py b/scripts/hooks/mixed-line-ending.py index 8ef25ed1..f27241f5 100644 --- a/scripts/hooks/mixed-line-ending.py +++ b/scripts/hooks/mixed-line-ending.py @@ -5,8 +5,23 @@ from pathlib import Path -def fix_file(filepath: str) -> bool: - """Convert all line endings to LF. Returns True if modified.""" +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + +def fix_file(filepath: str) -> bool | None: + """Convert all line endings to LF. + + Returns True if modified, False if unchanged, None on error. + """ path = Path(filepath) try: content = path.read_bytes() @@ -16,24 +31,29 @@ def fix_file(filepath: str) -> bool: if fixed != content: path.write_bytes(fixed) - print(f"Fixed line endings: {filepath}") + print(f"Fixed line endings: {_display_path(filepath)}") return True return False - except OSError: - return False + except OSError as exc: + print(f"{_display_path(filepath)}:0: cannot read file: {exc}", file=sys.stderr) + return None def main() -> int: if len(sys.argv) < 2: return 0 + had_error = False modified = False for filepath in sys.argv[1:]: - if fix_file(filepath): + result = fix_file(filepath) + if result is True: modified = True + elif result is None: + had_error = True - return 1 if modified else 0 + return 1 if modified or had_error else 0 if __name__ == "__main__": diff --git a/scripts/hooks/regenerate-skills-index.py b/scripts/hooks/regenerate-skills-index.py index 59b2c571..5692d08b 100644 --- a/scripts/hooks/regenerate-skills-index.py +++ b/scripts/hooks/regenerate-skills-index.py @@ -26,16 +26,28 @@ CATEGORY_RE = re.compile(r"") -def extract_metadata(filepath: Path) -> tuple[str, str]: +def extract_metadata( + filepath: Path, repo_root: Path | None = None +) -> tuple[str, str] | None: """Extract WHEN trigger and CATEGORY from a skill file. - Returns (category, when_trigger). Falls back to DEFAULT_CATEGORY - and the first heading line (or filename) when comments are absent. + Returns (category, when_trigger), or None on read error. + Falls back to DEFAULT_CATEGORY and the first heading line (or + filename) when metadata comments are absent. """ + if repo_root is not None: + try: + display_path = filepath.relative_to(repo_root) + except ValueError: + display_path = filepath + else: + display_path = filepath + try: content = filepath.read_text(encoding="utf-8") - except OSError: - return DEFAULT_CATEGORY, filepath.stem + except (OSError, UnicodeDecodeError) as exc: + print(f"{display_path}:0: cannot read file: {exc}", file=sys.stderr) + return None category = DEFAULT_CATEGORY when = "" @@ -62,8 +74,14 @@ def extract_metadata(filepath: Path) -> tuple[str, str]: return category, when -def build_index(skills_dir: Path) -> str: - """Build the index content from skill files.""" +def build_index( + skills_dir: Path, repo_root: Path | None = None +) -> tuple[str, bool]: + """Build the index content from skill files. + + Returns (index_content, had_error). had_error is True if any skill + file could not be read. + """ skill_files = sorted( f for f in skills_dir.glob("*.md") @@ -71,9 +89,14 @@ def build_index(skills_dir: Path) -> str: ) # Collect entries grouped by category + had_error = False categories: dict[str, list[tuple[str, str]]] = {} for filepath in skill_files: - category, when = extract_metadata(filepath) + result = extract_metadata(filepath, repo_root) + if result is None: + had_error = True + continue + category, when = result categories.setdefault(category, []).append((filepath.name, when)) # Sort categories alphabetically, but put Uncategorized last @@ -100,7 +123,7 @@ def build_index(skills_dir: Path) -> str: # Ensure trailing newline lines.append("") - return "\n".join(lines) + return "\n".join(lines), had_error def main() -> int: @@ -114,34 +137,44 @@ def main() -> int: # Nothing to index return 0 - new_content = build_index(skills_dir) + new_content, had_error = build_index(skills_dir, repo_root) + + if had_error: + print("Aborting: one or more skill files could not be read.", file=sys.stderr) + return 1 # Compare with existing index + try: + rel_index = index_path.relative_to(repo_root) + except ValueError: + rel_index = index_path existing = "" if index_path.is_file(): try: existing = index_path.read_text(encoding="utf-8") - except OSError: - pass + except (OSError, UnicodeDecodeError) as exc: + print(f"{rel_index}:0: cannot read file: {exc}", file=sys.stderr) + return 1 if new_content == existing: return 0 # Already up-to-date if check_only: print( - f"Skills index is out-of-date: {index_path.relative_to(repo_root)}" + f"Skills index is out-of-date: {rel_index}", + file=sys.stderr, ) - print("Run 'python scripts/hooks/regenerate-skills-index.py' to fix.") + print("Run 'python scripts/hooks/regenerate-skills-index.py' to fix.", file=sys.stderr) return 1 # Write updated index try: index_path.write_text(new_content, encoding="utf-8") except OSError as e: - print(f"Cannot write {index_path}: {e}") + print(f"{rel_index}:0: cannot write file: {e}", file=sys.stderr) return 1 - print(f"Updated {index_path.relative_to(repo_root)}") + print(f"Updated {rel_index}") return 1 # Exit 1 so pre-commit shows the auto-fix diff --git a/scripts/hooks/trailing-whitespace.py b/scripts/hooks/trailing-whitespace.py index 51cac60c..dd9f9933 100644 --- a/scripts/hooks/trailing-whitespace.py +++ b/scripts/hooks/trailing-whitespace.py @@ -5,8 +5,23 @@ from pathlib import Path -def fix_file(filepath: str) -> bool: - """Remove trailing whitespace from a file. Returns True if modified.""" +def _display_path(filepath: str | Path) -> str: + """Convert a file path to a relative display path. + + Pre-commit sets CWD to the repo root, so paths relative to CWD + are also relative to the project root. + """ + try: + return str(Path(filepath).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(filepath) + + +def fix_file(filepath: str) -> bool | None: + """Remove trailing whitespace from a file. + + Returns True if modified, False if unchanged, None on error. + """ path = Path(filepath) try: content = path.read_text(encoding="utf-8") @@ -34,23 +49,28 @@ def fix_file(filepath: str) -> bool: if modified: path.write_text("".join(fixed_lines), encoding="utf-8") - print(f"Fixed: {filepath}") + print(f"Fixed: {_display_path(filepath)}") return modified - except (OSError, UnicodeDecodeError): - return False + except (OSError, UnicodeDecodeError) as exc: + print(f"{_display_path(filepath)}:0: cannot read file: {exc}", file=sys.stderr) + return None def main() -> int: if len(sys.argv) < 2: return 0 + had_error = False modified = False for filepath in sys.argv[1:]: - if fix_file(filepath): + result = fix_file(filepath) + if result is True: modified = True + elif result is None: + had_error = True - return 1 if modified else 0 + return 1 if modified or had_error else 0 if __name__ == "__main__": diff --git a/scripts/run-cargo-clippy.py b/scripts/run-cargo-clippy.py index 5f58d5e1..4239a7d8 100644 --- a/scripts/run-cargo-clippy.py +++ b/scripts/run-cargo-clippy.py @@ -2,7 +2,7 @@ """ Cross-platform cargo clippy wrapper for pre-commit hooks. -Runs `cargo clippy --all-targets` with warnings as errors. +Runs `cargo clippy --all-targets --features tokio,json` with warnings as errors. Works on Windows (PowerShell/cmd), macOS, and Linux. """ @@ -23,22 +23,27 @@ def main() -> int: env.update(get_cargo_env()) # Run cargo clippy with warnings as errors + # Include tokio,json features so feature-gated code is also linted result = subprocess.run( - ["cargo", "clippy", "--all-targets", "--", "-D", "warnings"], + [ + "cargo", "clippy", "--all-targets", + "--features", "tokio,json", + "--", "-D", "warnings", + ], env=env, ) if result.returncode != 0: - print("\nERROR: Clippy found issues. Fix the warnings above.") + print("\nERROR: Clippy found issues. Fix the warnings above.", file=sys.stderr) return result.returncode except FileNotFoundError: - print("ERROR: cargo not found. Is Rust installed?") - print(" Install from: https://rustup.rs/") + print("ERROR: cargo not found. Is Rust installed?", file=sys.stderr) + print(" Install from: https://rustup.rs/", file=sys.stderr) return 1 except Exception as e: - print(f"ERROR: Failed to run cargo clippy: {e}") + print(f"ERROR: Failed to run cargo clippy: {e}", file=sys.stderr) return 1 diff --git a/scripts/run-cargo-fmt.py b/scripts/run-cargo-fmt.py index f879493b..9f118efa 100644 --- a/scripts/run-cargo-fmt.py +++ b/scripts/run-cargo-fmt.py @@ -71,7 +71,7 @@ def stage_modified_files(files_to_stage: list[str]) -> bool: capture_output=False, ) if stage_result.returncode != 0: - print("WARNING: Could not stage formatted files") + print("WARNING: Could not stage formatted files", file=sys.stderr) return False for f in files_to_stage: @@ -89,7 +89,7 @@ def main() -> int: # Run cargo fmt to fix formatting if not run_cargo_fmt(): - print("\nERROR: cargo fmt failed.") + print("\nERROR: cargo fmt failed.", file=sys.stderr) return 1 # Compare hashes to find files that cargo fmt actually modified @@ -101,7 +101,7 @@ def main() -> int: # Stage only the files that cargo fmt modified if not stage_modified_files(files_modified): - print("\nERROR: Failed to stage formatted files.") + print("\nERROR: Failed to stage formatted files.", file=sys.stderr) return 1 return 0 @@ -109,15 +109,15 @@ def main() -> int: except FileNotFoundError as e: cmd = str(e).split("'")[1] if "'" in str(e) else "command" if "cargo" in cmd.lower(): - print("ERROR: cargo not found. Is Rust installed?") - print(" Install from: https://rustup.rs/") + print("ERROR: cargo not found. Is Rust installed?", file=sys.stderr) + print(" Install from: https://rustup.rs/", file=sys.stderr) elif "git" in cmd.lower(): - print("ERROR: git not found. Is Git installed?") + print("ERROR: git not found. Is Git installed?", file=sys.stderr) else: - print(f"ERROR: {cmd} not found.") + print(f"ERROR: {cmd} not found.", file=sys.stderr) return 1 except Exception as e: - print(f"ERROR: Failed to run cargo fmt: {e}") + print(f"ERROR: Failed to run cargo fmt: {e}", file=sys.stderr) return 1 diff --git a/scripts/tests/test_check_dockerfile.py b/scripts/tests/test_check_dockerfile.py index 43f3285a..51a8c592 100644 --- a/scripts/tests/test_check_dockerfile.py +++ b/scripts/tests/test_check_dockerfile.py @@ -10,6 +10,7 @@ from __future__ import annotations import importlib.util +import re import sys from pathlib import Path @@ -253,10 +254,41 @@ def test_clean_dockerfile_returns_empty(self, tmp_path: Path) -> None: issues = check_file(f) assert issues == [] - def test_nonexistent_file_returns_empty(self, tmp_path: Path) -> None: - """Nonexistent file returns empty list with stderr warning.""" + def test_nonexistent_file_returns_error(self, tmp_path: Path) -> None: + """Nonexistent file returns a non-empty issues list.""" issues = check_file(tmp_path / "nonexistent") - assert issues == [] + assert len(issues) == 1 + assert "cannot read file" in issues[0] + + def test_nonexistent_file_causes_main_failure( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """main() returns 1 when a file does not exist.""" + nonexistent = tmp_path / "Dockerfile" + # Don't create the file -- it won't exist + monkeypatch.setattr( + sys, "argv", ["check-dockerfile.py", str(nonexistent)] + ) + assert check_dockerfile.main() == 1 + + def test_unreadable_file_causes_main_failure( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """main() returns 1 when a file exists but cannot be read.""" + unreadable = tmp_path / "Dockerfile" + unreadable.write_text("FROM ubuntu:22.04\n", encoding="utf-8") + unreadable.chmod(0o000) + try: + monkeypatch.setattr( + sys, "argv", ["check-dockerfile.py", str(unreadable)] + ) + assert check_dockerfile.main() == 1 + finally: + unreadable.chmod(0o644) def test_multiple_issues_in_one_file(self, tmp_path: Path) -> None: """Multiple anti-patterns in one file are all detected.""" @@ -277,5 +309,125 @@ def test_dockerfile_with_suffix(self, tmp_path: Path) -> None: assert len(issues) == 1 +class TestNoDuplicateOutput: + """Tests that read errors produce exactly one output line, not duplicates.""" + + def test_nonexistent_file_no_stderr_from_check_file( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """check_file() on a nonexistent file does not print to stderr itself. + + It only returns the issue in the list; main() is responsible for printing. + """ + issues = check_file(tmp_path / "Dockerfile") + captured = capsys.readouterr() + assert len(issues) == 1 + assert "cannot read file" in issues[0] + # check_file must NOT print -- the caller (main) prints + assert captured.err == "" + + def test_unreadable_file_no_stderr_from_check_file( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """check_file() on an unreadable file does not print to stderr itself.""" + f = _write(tmp_path, "Dockerfile", "FROM ubuntu\n") + f.chmod(0o000) + try: + issues = check_file(f) + captured = capsys.readouterr() + assert len(issues) == 1 + assert "cannot read file" in issues[0] + assert captured.err == "" + finally: + f.chmod(0o644) + + def test_main_prints_read_error_exactly_once( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + """main() prints the read-error message exactly once (no duplicates).""" + nonexistent = tmp_path / "Dockerfile" + monkeypatch.setattr( + sys, "argv", ["check-dockerfile.py", str(nonexistent)] + ) + check_dockerfile.main() + captured = capsys.readouterr() + error_lines = [ + line for line in captured.err.splitlines() + if "cannot read file" in line + ] + assert len(error_lines) == 1 + + +class TestOutputFormat: + """Tests that output follows {path}:{line_number}: {message} format.""" + + def test_issues_start_with_path_colon_line(self, tmp_path: Path) -> None: + """Each issue must start with path:line: (no leading whitespace).""" + f = _write(tmp_path, "Dockerfile", "RUN pip install requests\n") + issues = check_file(f) + assert len(issues) == 1 + # Must match path:line_number: pattern + assert re.match(r'^.+:\d+: ', issues[0]), f"Bad format: {issues[0]}" + # Must not start with whitespace + assert not issues[0].startswith(" "), f"Leading whitespace: {issues[0]}" + + def test_main_output_no_leading_whitespace( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + """main() prints issue lines without leading whitespace.""" + f = _write(tmp_path, "Dockerfile", "RUN pip install requests\n") + monkeypatch.setattr(sys, "argv", ["check-dockerfile.py", str(f)]) + # Import main from the module + check_dockerfile.main() + captured = capsys.readouterr() + # Each non-header, non-summary line should not start with spaces + for line in captured.err.splitlines(): + if line and not line.startswith(("Dockerfile anti-patterns", "\n")) and "issue(s) found" not in line: + assert not line.startswith(" "), f"Leading indent: {line!r}" + + +class TestRelativePaths: + """Tests that output paths are relative when repo_root is provided.""" + + def test_issues_use_relative_path_when_repo_root_provided( + self, tmp_path: Path + ) -> None: + """Issue output uses relative path when repo_root is given.""" + f = _write(tmp_path, "Dockerfile", "RUN pip install requests\n") + issues = check_file(f, repo_root=tmp_path) + assert len(issues) == 1 + assert str(tmp_path) not in issues[0] + assert issues[0].startswith("Dockerfile:") + + def test_read_error_uses_relative_path_when_repo_root_provided( + self, tmp_path: Path + ) -> None: + """Read-error output uses relative path when repo_root is given.""" + issues = check_file(tmp_path / "Dockerfile", repo_root=tmp_path) + assert len(issues) == 1 + # The display_path prefix must be relative (just the filename) + assert issues[0].startswith("Dockerfile:") + # The prefix before the line number must not contain the repo root + prefix = issues[0].split(":")[0] + assert str(tmp_path) not in prefix + + def test_issues_use_absolute_path_when_no_repo_root( + self, tmp_path: Path + ) -> None: + """Issue output uses full absolute path when repo_root is not given.""" + f = _write(tmp_path, "Dockerfile", "RUN pip install requests\n") + issues = check_file(f) + assert len(issues) == 1 + assert str(tmp_path) in issues[0] + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_empty_tests.py b/scripts/tests/test_check_empty_tests.py new file mode 100644 index 00000000..25da10a7 --- /dev/null +++ b/scripts/tests/test_check_empty_tests.py @@ -0,0 +1,208 @@ +#!/usr/bin/env python3 +""" +Unit tests for check-empty-tests.py hook. + +Verifies detection of empty test methods/functions and output format compliance. +""" + +from __future__ import annotations + +import importlib.util +import re +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "check_empty_tests", scripts_dir / "hooks" / "check-empty-tests.py" +) +check_empty_tests = importlib.util.module_from_spec(spec) +spec.loader.exec_module(check_empty_tests) + +check_file = check_empty_tests.check_file +main = check_empty_tests.main + + +def _write(directory: Path, name: str, content: str) -> Path: + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +class TestEmptyTestDetection: + """Tests for detecting empty test functions/methods.""" + + def test_empty_test_function_detected(self, tmp_path: Path) -> None: + f = _write(tmp_path, "test_example.py", "def test_something():\n pass\n") + errors = check_file(f) + assert len(errors) == 1 + assert "test_something" in errors[0] + + def test_empty_test_method_detected(self, tmp_path: Path) -> None: + content = ( + "class TestFoo:\n" + " def test_bar(self):\n" + " pass\n" + ) + f = _write(tmp_path, "test_example.py", content) + errors = check_file(f) + assert len(errors) == 1 + assert "test_bar" in errors[0] + + def test_nonempty_test_passes(self, tmp_path: Path) -> None: + content = ( + "def test_something():\n" + " assert True\n" + ) + f = _write(tmp_path, "test_example.py", content) + errors = check_file(f) + assert errors == [] + + def test_docstring_only_test_detected(self, tmp_path: Path) -> None: + content = ( + "def test_something():\n" + ' """This test does nothing."""\n' + ) + f = _write(tmp_path, "test_example.py", content) + errors = check_file(f) + assert len(errors) == 1 + + def test_ellipsis_only_test_detected(self, tmp_path: Path) -> None: + content = ( + "def test_something():\n" + " ...\n" + ) + f = _write(tmp_path, "test_example.py", content) + errors = check_file(f) + assert len(errors) == 1 + + def test_non_test_function_ignored(self, tmp_path: Path) -> None: + f = _write(tmp_path, "test_example.py", "def helper():\n pass\n") + errors = check_file(f) + assert errors == [] + + def test_syntax_error_file_returns_empty(self, tmp_path: Path) -> None: + f = _write(tmp_path, "test_example.py", "def test_foo(\n") + errors = check_file(f) + assert errors == [] + + +class TestOutputFormat: + """Tests that output follows {path}:{line_number}: {message} format.""" + + def test_issues_start_with_path_colon_line(self, tmp_path: Path) -> None: + f = _write(tmp_path, "test_example.py", "def test_foo():\n pass\n") + errors = check_file(f) + assert len(errors) == 1 + assert re.match(r'^.+:\d+: ', errors[0]), f"Bad format: {errors[0]}" + assert not errors[0].startswith(" "), f"Leading whitespace: {errors[0]}" + + def test_read_error_includes_line_number(self, tmp_path: Path) -> None: + path = tmp_path / "test_nonexistent.py" + errors = check_file(path) + assert len(errors) == 1 + assert ":0:" in errors[0], f"Missing :0: in read error: {errors[0]}" + assert re.match(r'^.+:\d+: ', errors[0]), f"Bad format: {errors[0]}" + + def test_main_output_no_leading_whitespace( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + f = _write(tmp_path, "test_example.py", "def test_foo():\n pass\n") + monkeypatch.setattr(sys, "argv", ["check-empty-tests.py", str(f)]) + main() + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line and not line.startswith(("Empty test", "Test methods")): + assert not line.startswith(" "), f"Leading indent: {line!r}" + + +class TestFileErrors: + """Tests for file read error handling.""" + + def test_nonexistent_file_no_warning_prefix( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str], + ) -> None: + """check_file on a non-existent file does not print a Warning: line. + + Only the formatted {path}:0: message should appear (via main's loop), + not a duplicate Warning: line from check_file itself. + """ + path = tmp_path / "test_nonexistent.py" + errors = check_file(path) + captured = capsys.readouterr() + assert len(errors) == 1 + assert ":0:" in errors[0] + assert "Warning:" not in captured.err + + +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(sys, "argv", ["check-empty-tests.py"]) + assert main() == 0 + + def test_main_clean_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "test_good.py", "def test_foo():\n assert 1 == 1\n") + monkeypatch.setattr(sys, "argv", ["check-empty-tests.py", str(f)]) + assert main() == 0 + + def test_main_violation_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "test_bad.py", "def test_foo():\n pass\n") + monkeypatch.setattr(sys, "argv", ["check-empty-tests.py", str(f)]) + assert main() == 1 + + def test_main_skips_non_test_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "helper.py", "def test_foo():\n pass\n") + monkeypatch.setattr(sys, "argv", ["check-empty-tests.py", str(f)]) + assert main() == 0 + + def test_main_checks_test_suffix_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "example_test.py", "def test_foo():\n pass\n") + monkeypatch.setattr(sys, "argv", ["check-empty-tests.py", str(f)]) + assert main() == 1 + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + sub = tmp_path / "sub" + sub.mkdir() + f = sub / "test_example.py" + f.write_text("def test_foo():\n pass\n", encoding="utf-8") + result = check_empty_tests._display_path(f) + assert result == str(Path("sub") / "test_example.py") + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = check_empty_tests._display_path(str(other / "test_file.py")) + assert result == str(other / "test_file.py") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_hook_output_format.py b/scripts/tests/test_check_hook_output_format.py new file mode 100644 index 00000000..5b32299a --- /dev/null +++ b/scripts/tests/test_check_hook_output_format.py @@ -0,0 +1,1167 @@ +#!/usr/bin/env python3 +""" +Unit tests for check-hook-output-format.py hook. + +Verifies that the hook output format checker correctly detects: +- Leading whitespace in print() f-strings (breaks editor hyperlinking) +- Error messages missing line numbers (should use :0: for file-level errors) +- Warning: prints that bypass {path}:{line}: format convention +- print() followed by return-in-list (causes duplicate output) +- ERROR:/WARNING: diagnostic prints going to stdout (must use file=sys.stderr) +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "check_hook_output_format", + scripts_dir / "hooks" / "check-hook-output-format.py", +) +check_hook_output_format = importlib.util.module_from_spec(spec) +spec.loader.exec_module(check_hook_output_format) + +check_file = check_hook_output_format.check_file + + +def _write(directory: Path, name: str, content: str) -> Path: + """Helper to create a file with given content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +class TestIndentedPrinting: + """Tests for leading whitespace in print() f-string detection.""" + + def test_indented_print_double_quote_detected(self, tmp_path: Path) -> None: + """print(f" {err}") is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def main():\n for err in errors:\n print(f" {err}")\n', + ) + issues = check_file(f) + assert len(issues) == 1 + assert "leading whitespace" in issues[0] + assert ":3:" in issues[0] + + def test_indented_print_single_quote_detected(self, tmp_path: Path) -> None: + """print(f' {err}') is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + "def main():\n for err in errors:\n print(f' {err}')\n", + ) + issues = check_file(f) + assert len(issues) == 1 + assert "leading whitespace" in issues[0] + + def test_print_without_indent_passes(self, tmp_path: Path) -> None: + """print(f"{err}") without leading spaces passes.""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n for err in errors:\n print(f"{err}")\n', + ) + issues = check_file(f) + assert issues == [] + + def test_regular_print_string_passes(self, tmp_path: Path) -> None: + """print(" Some header text") passes (not an f-string with variable).""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n print(" Some header text")\n', + ) + issues = check_file(f) + assert issues == [] + + def test_indented_print_rf_prefix_detected(self, tmp_path: Path) -> None: + """print(rf" {err}") with rf prefix is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def main():\n print(rf" {err}")\n', + ) + issues = check_file(f) + assert len(issues) == 1 + assert "leading whitespace" in issues[0] + + def test_indented_print_fr_prefix_detected(self, tmp_path: Path) -> None: + """print(fr" {err}") with fr prefix is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + "def main():\n print(fr\" {err}\")\n", + ) + issues = check_file(f) + assert len(issues) == 1 + assert "leading whitespace" in issues[0] + + def test_concat_leading_whitespace_double_quote_detected( + self, tmp_path: Path + ) -> None: + """print(" " + f"...") is flagged (Check 1b).""" + f = _write( + tmp_path, + "check-bad.py", + 'def main():\n print(" " + f"{prefix} {issue}")\n', + ) + issues = check_file(f) + assert any("leading" in i and "whitespace" in i for i in issues), ( + f"Expected leading-whitespace warning, got: {issues}" + ) + + def test_concat_leading_whitespace_single_quote_detected( + self, tmp_path: Path + ) -> None: + """print(' ' + ...) is flagged (Check 1b).""" + f = _write( + tmp_path, + "check-bad.py", + "def main():\n print(' ' + f'{prefix} {issue}')\n", + ) + issues = check_file(f) + assert any("leading" in i and "whitespace" in i for i in issues), ( + f"Expected leading-whitespace warning, got: {issues}" + ) + + def test_concat_no_leading_whitespace_passes(self, tmp_path: Path) -> None: + """print("prefix" + ...) without leading spaces passes (Check 1b).""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n print("prefix" + f" {issue}")\n', + ) + issues = check_file(f) + concat_issues = [i for i in issues if "concatenated" in i and "leading" in i] + assert not concat_issues + + def test_comment_with_pattern_skipped(self, tmp_path: Path) -> None: + """Comment lines are not checked.""" + f = _write( + tmp_path, + "check-good.py", + '# print(f" {err}")\n', + ) + issues = check_file(f) + assert issues == [] + + +class TestMissingLineNumber: + """Tests for error messages missing line numbers.""" + + def test_cannot_read_without_line_number_detected( + self, tmp_path: Path + ) -> None: + """f"{filepath}: cannot read" without :0: is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def check_file(filepath):\n return [f"{filepath}: cannot read"]\n', + ) + issues = check_file(f) + assert len(issues) == 1 + assert "missing line number" in issues[0] + assert ":2:" in issues[0] + + def test_cannot_read_with_zero_line_passes(self, tmp_path: Path) -> None: + """f"{filepath}:0: cannot read" passes.""" + f = _write( + tmp_path, + "check-good.py", + 'def check_file(filepath):\n return [f"{filepath}:0: cannot read file"]\n', + ) + issues = check_file(f) + assert issues == [] + + def test_cannot_read_with_variable_line_passes( + self, tmp_path: Path + ) -> None: + """f"{filepath}:{line}: cannot read" passes.""" + f = _write( + tmp_path, + "check-good.py", + 'def check_file(filepath):\n return [f"{filepath}:{line_num}: cannot read"]\n', + ) + issues = check_file(f) + assert issues == [] + + def test_path_var_name_detected(self, tmp_path: Path) -> None: + """f"{path}: cannot read" is detected regardless of variable name.""" + f = _write( + tmp_path, + "check-bad.py", + 'def check_file(path):\n return [f"{path}: cannot read file"]\n', + ) + issues = check_file(f) + assert len(issues) == 1 + assert "missing line number" in issues[0] + + def test_cannot_read_single_quote_detected(self, tmp_path: Path) -> None: + """f'{filepath}: cannot read' with single quotes is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + "def check_file(filepath):\n return [f'{filepath}: cannot read']\n", + ) + issues = check_file(f) + assert len(issues) == 1 + assert "missing line number" in issues[0] + + def test_cannot_read_rf_prefix_detected(self, tmp_path: Path) -> None: + """rf"{filepath}: cannot read" with rf prefix is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def check_file(filepath):\n return [rf"{filepath}: cannot read"]\n', + ) + issues = check_file(f) + assert len(issues) == 1 + assert "missing line number" in issues[0] + + def test_cannot_read_fr_prefix_detected(self, tmp_path: Path) -> None: + """fr"{filepath}: cannot read" with fr prefix is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def check_file(filepath):\n return [fr"{filepath}: cannot read"]\n', + ) + issues = check_file(f) + assert len(issues) == 1 + assert "missing line number" in issues[0] + + def test_comment_with_pattern_skipped(self, tmp_path: Path) -> None: + """Comment lines are not checked.""" + f = _write( + tmp_path, + "check-good.py", + '# f"{path}: cannot read"\n', + ) + issues = check_file(f) + assert issues == [] + + +class TestWarningPrefix: + """Tests for Warning: print detection.""" + + def test_warning_print_double_quote_detected(self, tmp_path: Path) -> None: + """print(f"Warning: ...") is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def check_file(path):\n print(f"Warning: cannot read {path}")\n', + ) + issues = check_file(f) + assert len(issues) == 1 + assert "Warning:" in issues[0] + assert ":2:" in issues[0] + + def test_warning_print_single_quote_detected(self, tmp_path: Path) -> None: + """print(f'Warning: ...') is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + "def check_file(path):\n print(f'Warning: cannot read {path}')\n", + ) + issues = check_file(f) + assert len(issues) == 1 + assert "Warning:" in issues[0] + + def test_warning_without_f_prefix_passes(self, tmp_path: Path) -> None: + """print("Warning: ...") without f-prefix passes (static text is OK).""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n print("Warning: tomllib not available")\n', + ) + issues = check_file(f) + assert issues == [] + + def test_formatted_error_passes(self, tmp_path: Path) -> None: + """print(f"{path}:0: cannot read") passes (correct format).""" + f = _write( + tmp_path, + "check-good.py", + 'def check_file(path):\n print(f"{path}:0: cannot read file")\n', + ) + issues = check_file(f) + assert issues == [] + + def test_comment_with_warning_pattern_skipped(self, tmp_path: Path) -> None: + """Comment lines are not checked.""" + f = _write( + tmp_path, + "check-good.py", + '# print(f"Warning: cannot read {path}")\n', + ) + issues = check_file(f) + assert issues == [] + + +class TestDualOutputDetection: + """Tests for Check 4: print() followed by return-in-list detection.""" + + def test_print_then_return_list_detected(self, tmp_path: Path) -> None: + """print(file=sys.stderr) followed by return [...] is flagged.""" + content = ( + "def check_file(filepath):\n" + " except OSError as exc:\n" + " print(f'{filepath}:0: error: {exc}', file=sys.stderr)\n" + " return [f'{filepath}:0: error: {exc}']\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("duplicate output" in i for i in issues), f"Expected dual-output warning, got: {issues}" + + def test_print_then_return_list_with_gap_detected( + self, tmp_path: Path + ) -> None: + """print(file=sys.stderr) with 1-2 blank lines before return [...] is flagged.""" + content = ( + "def check_file(filepath):\n" + " except OSError as exc:\n" + " msg = f'{filepath}:0: error'\n" + " print(msg, file=sys.stderr)\n" + "\n" + " return [msg]\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("duplicate output" in i for i in issues), f"Expected dual-output warning, got: {issues}" + + def test_print_then_return_bool_passes(self, tmp_path: Path) -> None: + """print(file=sys.stderr) followed by return False passes (no list).""" + content = ( + "def check_file(filepath):\n" + " except OSError as exc:\n" + " print(f'{filepath}:0: error: {exc}', file=sys.stderr)\n" + " return False\n" + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("duplicate output" in i for i in issues) + + def test_print_in_main_loop_passes(self, tmp_path: Path) -> None: + """print() in main() loop that iterates issues passes (no return-list nearby).""" + content = ( + "def main():\n" + " for issue in issues:\n" + " print(issue, file=sys.stderr)\n" + " return 1\n" + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("duplicate output" in i for i in issues) + + def test_print_then_return_empty_list_passes(self, tmp_path: Path) -> None: + """print(file=sys.stderr) followed by return [] passes (empty list is not dual-output).""" + content = ( + "def check_file(filepath):\n" + " except OSError as exc:\n" + " print(f'{filepath}:0: error: {exc}', file=sys.stderr)\n" + " return []\n" + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("duplicate output" in i for i in issues) + + def test_print_far_from_return_list_passes(self, tmp_path: Path) -> None: + """print(file=sys.stderr) more than 3 lines before return [...] passes.""" + content = ( + "def check_file(filepath):\n" + " print(f'{filepath}:0: error', file=sys.stderr)\n" + " do_something()\n" + " do_more()\n" + " yet_more()\n" + " even_more()\n" + " return [f'{filepath}:0: error']\n" + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("duplicate output" in i for i in issues) + + +class TestNoDuplicateOutput: + """Tests that check_file() read errors don't produce duplicate stderr output.""" + + def test_nonexistent_file_no_stderr_from_check_file( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """check_file() on a nonexistent file does not print to stderr itself.""" + issues = check_file(tmp_path / "nonexistent.py") + captured = capsys.readouterr() + assert len(issues) == 1 + assert "cannot read file" in issues[0] + # check_file must NOT print -- the caller (main) prints + assert captured.err == "" + + def test_main_prints_read_error_exactly_once( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + """main() prints the read-error message exactly once (no duplicates).""" + nonexistent = tmp_path / "check-missing.py" + monkeypatch.setattr( + sys, "argv", ["check-hook-output-format.py", str(nonexistent)] + ) + check_hook_output_format.main() + captured = capsys.readouterr() + error_lines = [ + line for line in captured.err.splitlines() + if "cannot read file" in line + ] + assert len(error_lines) == 1 + + +class TestFileHandling: + """Tests for file handling edge cases.""" + + def test_clean_file_returns_empty(self, tmp_path: Path) -> None: + """A clean hook script returns no issues.""" + content = ( + "#!/usr/bin/env python3\n" + "import sys\n" + "\n" + "def check_file(filepath):\n" + ' return [f"{filepath}:0: cannot read"]\n' + "\n" + "def main():\n" + " for issue in issues:\n" + " print(issue, file=sys.stderr)\n" + " return 0\n" + ) + f = _write(tmp_path, "check-clean.py", content) + issues = check_file(f) + assert issues == [] + + def test_nonexistent_file_fails_closed(self, tmp_path: Path) -> None: + """Nonexistent file returns error issue (fail-closed).""" + issues = check_file(tmp_path / "nonexistent.py") + assert len(issues) == 1 + assert "cannot read file" in issues[0] + assert ":0:" in issues[0] + + def test_unreadable_file_fails_closed(self, tmp_path: Path) -> None: + """Unreadable file returns error issue (fail-closed).""" + f = _write(tmp_path, "check-unreadable.py", "content") + f.chmod(0o000) + try: + issues = check_file(f) + assert len(issues) == 1 + assert "cannot read file" in issues[0] + assert ":0:" in issues[0] + finally: + f.chmod(0o644) # Restore for cleanup + + def test_binary_file_fails_closed(self, tmp_path: Path) -> None: + """Binary (non-UTF-8) file returns error issue (fail-closed).""" + f = tmp_path / "check-binary.py" + f.write_bytes(b"\xff\xfe\x00\x01") + issues = check_file(f) + assert len(issues) == 1 + assert "cannot read file" in issues[0] + assert ":0:" in issues[0] + + def test_multiple_violations_detected(self, tmp_path: Path) -> None: + """Multiple violations in one file are all reported.""" + content = ( + 'def main():\n' + ' print(f" {err}")\n' + ' print(f" {issue}")\n' + ) + f = _write(tmp_path, "check-multi.py", content) + issues = check_file(f) + assert len(issues) == 2 + + def test_blank_lines_skipped(self, tmp_path: Path) -> None: + """Blank lines do not cause issues.""" + f = _write(tmp_path, "check-blank.py", "\n\n\n") + issues = check_file(f) + assert issues == [] + + def test_issue_format_has_path_and_line(self, tmp_path: Path) -> None: + """Issue output uses {path}:{line}: {message} format.""" + f = _write( + tmp_path, + "check-bad.py", + 'print(f" {err}")\n', + ) + issues = check_file(f) + assert len(issues) == 1 + # Verify format: path:line: message + parts = issues[0].split(":") + assert len(parts) >= 3 + assert parts[1].strip().isdigit() + + +class TestFailOpenDetection: + """Tests for Check 5: except-pass/return-fallback fail-open detection.""" + + def test_except_oserror_pass_detected(self, tmp_path: Path) -> None: + """except OSError followed by pass is flagged.""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError:\n" + " pass\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("swallows" in i and "pass" in i for i in issues), ( + f"Expected fail-open warning, got: {issues}" + ) + + def test_except_unicode_error_pass_detected(self, tmp_path: Path) -> None: + """except UnicodeDecodeError followed by pass is flagged.""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except UnicodeDecodeError:\n" + " pass\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("swallows" in i and "pass" in i for i in issues) + + def test_except_combined_errors_pass_detected(self, tmp_path: Path) -> None: + """except (OSError, UnicodeDecodeError) followed by pass is flagged.""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except (OSError, UnicodeDecodeError):\n" + " pass\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("swallows" in i for i in issues) + + def test_except_oserror_return_true_detected(self, tmp_path: Path) -> None: + """except OSError followed by return True is flagged.""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError:\n" + " return True\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("returns True" in i for i in issues), ( + f"Expected fail-open warning, got: {issues}" + ) + + def test_except_oserror_return_false_passes(self, tmp_path: Path) -> None: + """except OSError followed by return False passes (fail-closed).""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError:\n" + " return False\n" + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("swallows" in i or "returns True" in i for i in issues) + + def test_except_oserror_return_none_passes(self, tmp_path: Path) -> None: + """except OSError followed by return None passes (fail-closed fixer).""" + content = ( + "def fix_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError as exc:\n" + ' print(f"{filepath}:0: cannot read file: {exc}", file=sys.stderr)\n' + " return None\n" + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("swallows" in i or "returns True" in i for i in issues) + + def test_except_with_print_and_return_false_passes( + self, tmp_path: Path + ) -> None: + """except with print then return False passes (proper error handling).""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except (OSError, UnicodeDecodeError) as e:\n" + ' print(f"{filepath}:0: cannot read file: {e}", file=sys.stderr)\n' + " return False\n" + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("swallows" in i or "returns True" in i for i in issues) + + def test_except_with_named_var_pass_detected(self, tmp_path: Path) -> None: + """except OSError as exc followed by pass is flagged.""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError as exc:\n" + " pass\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("swallows" in i for i in issues) + + def test_except_valueerror_not_detected(self, tmp_path: Path) -> None: + """except ValueError with pass is not flagged (not an I/O error).""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " int(value)\n" + " except ValueError:\n" + " pass\n" + ) + f = _write(tmp_path, "check-ok.py", content) + issues = check_file(f) + assert not any("swallows" in i or "returns True" in i for i in issues) + + def test_pass_with_comment_still_detected(self, tmp_path: Path) -> None: + """pass with an explanatory comment is still flagged.""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except (OSError, UnicodeDecodeError):\n" + " pass # File read errors are non-fatal\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("swallows" in i for i in issues) + + def test_except_ioerror_pass_detected(self, tmp_path: Path) -> None: + """except IOError followed by pass is flagged.""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except IOError:\n" + " pass\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("swallows" in i for i in issues) + + def test_except_oserror_return_empty_list_detected( + self, tmp_path: Path + ) -> None: + """except OSError followed by return [] is flagged (fail-open).""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError:\n" + " return []\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("empty value" in i for i in issues), ( + f"Expected empty-value warning, got: {issues}" + ) + + def test_except_oserror_return_empty_string_detected( + self, tmp_path: Path + ) -> None: + """except OSError followed by return '' is flagged (fail-open).""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError:\n" + " return ''\n" + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("empty value" in i for i in issues) + + def test_except_oserror_return_empty_dquote_string_detected( + self, tmp_path: Path + ) -> None: + """except OSError followed by return "" is flagged (fail-open).""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError:\n" + ' return ""\n' + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("empty value" in i for i in issues) + + def test_except_oserror_return_nonempty_list_passes( + self, tmp_path: Path + ) -> None: + """except OSError followed by return [error_msg] passes (fail-closed).""" + content = ( + "def check_file(filepath):\n" + " try:\n" + " content = filepath.read_text()\n" + " except OSError as exc:\n" + ' return [f"{filepath}:0: cannot read: {exc}"]\n' + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + fail_open_issues = [ + i for i in issues + if "swallows" in i or "returns True" in i or "empty value" in i + ] + assert not fail_open_issues + + +class TestRelativePaths: + """Tests that check_file() uses relative paths when repo_root is provided.""" + + def test_issues_use_relative_path_when_repo_root_provided( + self, tmp_path: Path + ) -> None: + """Issue paths are relative to repo_root, not absolute.""" + f = _write( + tmp_path, + "check-bad.py", + 'def main():\n print(f" {err}")\n', + ) + issues = check_file(f, repo_root=tmp_path) + assert len(issues) == 1 + assert str(tmp_path) not in issues[0] + assert "check-bad.py:2:" in issues[0] + + def test_read_error_uses_relative_path(self, tmp_path: Path) -> None: + """Read error path prefix is relative to repo_root.""" + nonexistent = tmp_path / "check-missing.py" + issues = check_file(nonexistent, repo_root=tmp_path) + assert len(issues) == 1 + assert "check-missing.py:0:" in issues[0] + # The path prefix (before ": cannot read") should be relative; + # the exception message itself may still contain the absolute path. + prefix = issues[0].split(": cannot read")[0] + assert str(tmp_path) not in prefix + + +class TestRawPathInGlobScript: + """Tests for Check 6: raw path variables in error output when file uses glob/rglob/iterdir.""" + + def test_raw_filepath_with_glob_detected(self, tmp_path: Path) -> None: + """f"{filepath}:0:" in a script using .glob( is flagged.""" + content = ( + "def check_all():\n" + " for p in path.glob('*.py'):\n" + ' issues.append(f"{filepath}:0: error")\n' + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("may be absolute" in i for i in issues), ( + f"Expected raw-path warning, got: {issues}" + ) + assert any("{filepath}" in i for i in issues) + + def test_raw_filepath_with_rglob_detected(self, tmp_path: Path) -> None: + """f"{filepath}:0:" in a script using .rglob( is flagged.""" + content = ( + "def check_all():\n" + " for p in path.rglob('*.py'):\n" + ' issues.append(f"{filepath}:0: error")\n' + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("may be absolute" in i for i in issues), ( + f"Expected raw-path warning, got: {issues}" + ) + + def test_raw_path_with_iterdir_detected(self, tmp_path: Path) -> None: + """f"{filepath}:0:" in a script using .iterdir() is flagged.""" + content = ( + "def check_all():\n" + " for p in path.iterdir():\n" + ' issues.append(f"{filepath}:0: error")\n' + ) + f = _write(tmp_path, "check-bad.py", content) + issues = check_file(f) + assert any("may be absolute" in i for i in issues), ( + f"Expected raw-path warning, got: {issues}" + ) + + def test_display_path_with_glob_passes(self, tmp_path: Path) -> None: + """f"{display_path}:0:" in a script using .glob( passes (safe variable).""" + content = ( + "def check_all():\n" + " for p in path.glob('*.py'):\n" + " display_path = p.relative_to(root)\n" + ' issues.append(f"{display_path}:0: error")\n' + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("may be absolute" in i for i in issues) + + def test_rel_var_with_glob_passes(self, tmp_path: Path) -> None: + """f"{rel}:0:" in a script using .glob( passes (safe variable).""" + content = ( + "def check_all():\n" + " for p in path.glob('*.py'):\n" + " rel = p.relative_to(root)\n" + ' issues.append(f"{rel}:0: error")\n' + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("may be absolute" in i for i in issues) + + def test_raw_filepath_without_glob_passes(self, tmp_path: Path) -> None: + """f"{filepath}:0:" without glob/rglob/iterdir passes (argv paths are relative).""" + content = ( + "def check_file(filepath):\n" + ' issues.append(f"{filepath}:0: error")\n' + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("may be absolute" in i for i in issues) + + def test_comment_glob_not_detected(self, tmp_path: Path) -> None: + """A commented-out .glob( line does not trigger Check 6. + + The hook skips comment lines when scanning for .glob(/.rglob(/.iterdir( + to avoid false positives on scripts that only mention glob in comments. + """ + content = ( + "def check_file(filepath):\n" + " # for p in path.glob('*.py'):\n" + ' issues.append(f"{filepath}:0: error")\n' + ) + f = _write(tmp_path, "check-good.py", content) + issues = check_file(f) + assert not any("may be absolute" in i for i in issues) + + +class TestStderrErrorPrints: + """Tests for Check 7: ERROR/WARNING prints going to stdout instead of stderr.""" + + def test_error_print_without_stderr_flagged(self, tmp_path: Path) -> None: + """print("ERROR: something") without file=sys.stderr is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def main():\n print("ERROR: something went wrong")\n', + ) + issues = check_file(f) + assert len(issues) == 1 + assert "stdout" in issues[0] + assert ":2:" in issues[0] + + def test_error_print_with_stderr_ok(self, tmp_path: Path) -> None: + """print("ERROR: something", file=sys.stderr) is not flagged.""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n print("ERROR: something went wrong", file=sys.stderr)\n', + ) + issues = check_file(f) + assert not any("stdout" in i for i in issues) + + def test_fstring_error_without_stderr_flagged(self, tmp_path: Path) -> None: + """print(f"ERROR: {var}") without file=sys.stderr is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def main():\n print(f"ERROR: {detail}")\n', + ) + issues = check_file(f) + assert any("stdout" in i for i in issues), ( + f"Expected stdout warning, got: {issues}" + ) + + def test_red_error_without_stderr_flagged(self, tmp_path: Path) -> None: + """print(red("ERROR:") + " msg") without file=sys.stderr is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def main():\n print(red("ERROR:") + " something failed")\n', + ) + issues = check_file(f) + assert any("stdout" in i for i in issues), ( + f"Expected stdout warning, got: {issues}" + ) + + def test_red_error_with_stderr_ok(self, tmp_path: Path) -> None: + """print(red("ERROR:") + " msg", file=sys.stderr) is not flagged.""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n print(red("ERROR:") + " something failed", file=sys.stderr)\n', + ) + issues = check_file(f) + assert not any("stdout" in i for i in issues) + + def test_warning_print_without_stderr_flagged(self, tmp_path: Path) -> None: + """print("WARNING: something") without file=sys.stderr is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + 'def main():\n print("WARNING: check skipped")\n', + ) + issues = check_file(f) + assert any("stdout" in i for i in issues), ( + f"Expected stdout warning, got: {issues}" + ) + + def test_warning_print_with_stderr_ok(self, tmp_path: Path) -> None: + """print("WARNING: something", file=sys.stderr) is not flagged.""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n print("WARNING: check skipped", file=sys.stderr)\n', + ) + issues = check_file(f) + assert not any("stdout" in i for i in issues) + + def test_comment_line_not_flagged(self, tmp_path: Path) -> None: + """# print("ERROR: something") is not flagged (comment line).""" + f = _write( + tmp_path, + "check-good.py", + '# print("ERROR: something went wrong")\n', + ) + issues = check_file(f) + assert issues == [] + + def test_ok_summary_print_not_flagged(self, tmp_path: Path) -> None: + """print("[OK] All good") is not flagged (not an error print).""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n print("[OK] All good")\n', + ) + issues = check_file(f) + assert not any("stdout" in i for i in issues) + + def test_single_quoted_error_without_stderr_flagged( + self, tmp_path: Path + ) -> None: + """print('ERROR: something') with single quotes is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + "def main():\n print('ERROR: something went wrong')\n", + ) + issues = check_file(f) + assert any("stdout" in i for i in issues), ( + f"Expected stdout warning, got: {issues}" + ) + + def test_single_quoted_fstring_error_without_stderr_flagged( + self, tmp_path: Path + ) -> None: + """print(f'ERROR: {var}') with single-quoted f-string is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + "def main():\n print(f'ERROR: {detail}')\n", + ) + issues = check_file(f) + assert any("stdout" in i for i in issues), ( + f"Expected stdout warning, got: {issues}" + ) + + def test_single_quoted_warning_without_stderr_flagged( + self, tmp_path: Path + ) -> None: + """print('WARNING: something') with single quotes is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + "def main():\n print('WARNING: check skipped')\n", + ) + issues = check_file(f) + assert any("stdout" in i for i in issues), ( + f"Expected stdout warning, got: {issues}" + ) + + def test_single_quoted_fstring_warning_without_stderr_flagged( + self, tmp_path: Path + ) -> None: + """print(f'WARNING: {var}') with single-quoted f-string is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + "def main():\n print(f'WARNING: {detail}')\n", + ) + issues = check_file(f) + assert any("stdout" in i for i in issues), ( + f"Expected stdout warning, got: {issues}" + ) + + def test_informational_print_not_flagged(self, tmp_path: Path) -> None: + """print("Running cargo fmt...") is not flagged (informational).""" + f = _write( + tmp_path, + "check-good.py", + 'def main():\n print("Running cargo fmt...")\n', + ) + issues = check_file(f) + assert not any("stdout" in i for i in issues) + + +class TestMultiLineStderrPrints: + """Tests for Check 7b: multi-line print() with ERROR:/WARNING: on continuation lines.""" + + def test_multiline_warning_without_stderr_flagged(self, tmp_path: Path) -> None: + """Multi-line print with WARNING: on a continuation line is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + ( + "def main():\n" + " print(\n" + ' f" WARNING: proof missing unwind"\n' + " )\n" + ), + ) + issues = check_file(f) + assert any("multi-line" in i and "stdout" in i for i in issues), ( + f"Expected multi-line stdout warning, got: {issues}" + ) + + def test_multiline_error_without_stderr_flagged(self, tmp_path: Path) -> None: + """Multi-line print with ERROR: on a continuation line is flagged.""" + f = _write( + tmp_path, + "check-bad.py", + ( + "def main():\n" + " print(\n" + ' red("ERROR:")\n' + ' + f" something failed"\n' + " )\n" + ), + ) + issues = check_file(f) + assert any("multi-line" in i and "stdout" in i for i in issues), ( + f"Expected multi-line stdout warning, got: {issues}" + ) + + def test_multiline_with_stderr_ok(self, tmp_path: Path) -> None: + """Multi-line print with file=sys.stderr is not flagged.""" + f = _write( + tmp_path, + "check-good.py", + ( + "def main():\n" + " print(\n" + ' f" WARNING: proof missing unwind",\n' + " file=sys.stderr,\n" + " )\n" + ), + ) + issues = check_file(f) + assert not any("multi-line" in i for i in issues) + + def test_multiline_informational_not_flagged(self, tmp_path: Path) -> None: + """Multi-line informational print is not flagged.""" + f = _write( + tmp_path, + "check-good.py", + ( + "def main():\n" + " print(\n" + ' f"Found {count} proofs"\n' + " )\n" + ), + ) + issues = check_file(f) + assert not any("multi-line" in i for i in issues) + + +class TestSelfCompliance: + """Verify that check-hook-output-format.py passes its own checks.""" + + def test_self_passes_check6(self) -> None: + """check-hook-output-format.py itself passes Check 6 (uses display_path, not filepath).""" + hook_path = scripts_dir / "hooks" / "check-hook-output-format.py" + issues = check_file(hook_path) + check6_issues = [i for i in issues if "may be absolute" in i] + assert not check6_issues, ( + f"check-hook-output-format.py fails its own Check 6: {check6_issues}" + ) + + +class TestCheck8StderrEncoding: + """Check 8: sys.stdout wrapped with UTF-8 but sys.stderr not wrapped.""" + + def test_stdout_wrapped_stderr_not_wrapped(self, tmp_path: Path) -> None: + """Script wraps stdout but not stderr -- should flag.""" + f = _write( + tmp_path, + "check-bad.py", + ( + "import io\n" + "import sys\n" + 'sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding="utf-8")\n' + ), + ) + issues = check_file(f) + assert any("sys.stdout wrapped" in i and "sys.stderr" in i for i in issues), ( + f"Expected stderr-wrapping warning, got: {issues}" + ) + + def test_both_wrapped(self, tmp_path: Path) -> None: + """Script wraps both stdout and stderr -- should pass.""" + f = _write( + tmp_path, + "check-good.py", + ( + "import io\n" + "import sys\n" + 'sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding="utf-8")\n' + 'sys.stderr = io.TextIOWrapper(sys.stderr.buffer, encoding="utf-8")\n' + ), + ) + issues = check_file(f) + assert not any("sys.stdout wrapped" in i for i in issues) + + def test_neither_wrapped(self, tmp_path: Path) -> None: + """Script wraps neither -- should pass.""" + f = _write( + tmp_path, + "check-good.py", + ( + "import sys\n" + 'print("hello")\n' + ), + ) + issues = check_file(f) + assert not any("sys.stdout wrapped" in i for i in issues) + + def test_only_stderr_wrapped(self, tmp_path: Path) -> None: + """Script wraps only stderr -- should pass.""" + f = _write( + tmp_path, + "check-good.py", + ( + "import io\n" + "import sys\n" + 'sys.stderr = io.TextIOWrapper(sys.stderr.buffer, encoding="utf-8")\n' + ), + ) + issues = check_file(f) + assert not any("sys.stdout wrapped" in i for i in issues) + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_json.py b/scripts/tests/test_check_json.py new file mode 100644 index 00000000..cd2c3c77 --- /dev/null +++ b/scripts/tests/test_check_json.py @@ -0,0 +1,245 @@ +#!/usr/bin/env python3 +""" +Unit tests for check-json.py hook. + +Verifies that the JSON/JSONC validator correctly detects invalid JSON files, +handles JSONC comments, and that output follows the {path}:{line}: {message} format. +""" + +from __future__ import annotations + +import importlib.util +import re +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "check_json", scripts_dir / "hooks" / "check-json.py" +) +check_json = importlib.util.module_from_spec(spec) +spec.loader.exec_module(check_json) + +check_file = check_json.check_file +main = check_json.main +strip_jsonc_comments = check_json.strip_jsonc_comments + + +def _write(directory: Path, name: str, content: str) -> Path: + """Helper to create a file with given content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +class TestJsonValidation: + """Tests for JSON validation.""" + + def test_valid_json_passes(self, tmp_path: Path) -> None: + """A valid JSON file passes.""" + f = _write(tmp_path, "data.json", '{"key": "value", "num": 42}\n') + assert check_file(str(f)) is True + + def test_empty_object_passes(self, tmp_path: Path) -> None: + """An empty JSON object passes.""" + f = _write(tmp_path, "empty.json", "{}\n") + assert check_file(str(f)) is True + + def test_empty_array_passes(self, tmp_path: Path) -> None: + """An empty JSON array passes.""" + f = _write(tmp_path, "array.json", "[]\n") + assert check_file(str(f)) is True + + def test_invalid_json_fails(self, tmp_path: Path) -> None: + """An invalid JSON file fails.""" + f = _write(tmp_path, "bad.json", '{"key": value}\n') + assert check_file(str(f)) is False + + def test_trailing_comma_fails(self, tmp_path: Path) -> None: + """JSON with trailing comma fails (not valid JSON, only JSONC comments are stripped).""" + f = _write(tmp_path, "trailing.json", '{"key": "value",}\n') + assert check_file(str(f)) is False + + def test_jsonc_single_line_comment_passes(self, tmp_path: Path) -> None: + """JSONC with single-line comments passes after stripping.""" + content = '// comment\n{"key": "value"}\n' + f = _write(tmp_path, "config.json", content) + assert check_file(str(f)) is True + + def test_jsonc_multi_line_comment_passes(self, tmp_path: Path) -> None: + """JSONC with multi-line comments passes after stripping.""" + content = '/* comment */\n{"key": "value"}\n' + f = _write(tmp_path, "config.json", content) + assert check_file(str(f)) is True + + def test_bom_stripped(self, tmp_path: Path) -> None: + """BOM at start of file is stripped.""" + content = '\ufeff{"key": "value"}\n' + f = _write(tmp_path, "bom.json", content) + assert check_file(str(f)) is True + + +class TestStripJsoncComments: + """Tests for the strip_jsonc_comments function.""" + + def test_no_comments_unchanged(self) -> None: + """Content without comments is unchanged.""" + content = '{"key": "value"}' + assert strip_jsonc_comments(content) == content + + def test_single_line_comment_stripped(self) -> None: + """Single-line comments are removed.""" + content = '// comment\n{"key": "value"}' + result = strip_jsonc_comments(content) + assert "//" not in result + assert '"key"' in result + + def test_multi_line_comment_stripped(self) -> None: + """Multi-line comments are removed.""" + content = '/* comment */{"key": "value"}' + result = strip_jsonc_comments(content) + assert "/*" not in result + assert '"key"' in result + + def test_comment_inside_string_preserved(self) -> None: + """// and /* inside strings are preserved.""" + content = '{"url": "https://example.com"}' + assert strip_jsonc_comments(content) == content + + def test_escaped_quote_in_string(self) -> None: + """Escaped quotes inside strings are handled correctly.""" + content = '{"key": "value with \\"quote\\""}' + assert strip_jsonc_comments(content) == content + + def test_inline_comment_stripped(self) -> None: + """Inline single-line comment after value is stripped.""" + content = '{"key": "value"} // inline comment\n' + result = strip_jsonc_comments(content) + assert "inline comment" not in result + assert '"key"' in result + + +class TestOutputFormat: + """Tests that output follows {path}:{line_number}: {message} format.""" + + def test_output_starts_with_path_colon_line( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output must start with path:line: (no leading whitespace).""" + f = _write(tmp_path, "bad.json", '{"key": value}\n') + check_file(str(f)) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + assert not line.startswith(" "), f"Leading whitespace: {line}" + + def test_output_contains_json_error( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output contains 'JSON error'.""" + f = _write(tmp_path, "bad.json", '{"key": value}\n') + check_file(str(f)) + captured = capsys.readouterr() + assert "JSON error" in captured.err + + def test_correct_line_number_reported( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Line number from JSONDecodeError is reported.""" + f = _write(tmp_path, "bad.json", '{\n "key": value\n}\n') + check_file(str(f)) + captured = capsys.readouterr() + # The error should be on line 2 where 'value' is unquoted + assert ":2:" in captured.err + + def test_read_error_uses_zero_line_number( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Read error message must include :0: synthetic line number.""" + path = tmp_path / "nonexistent.json" + check_file(str(path)) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert ":0:" in line, f"Missing :0: in read error: {line}" + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + + def test_unicode_decode_error_handled( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Invalid UTF-8 file returns False and outputs :0: format.""" + path = tmp_path / "bad_encoding.json" + path.write_bytes(b"\x80\x81\x82 invalid utf8") + assert check_file(str(path)) is False + captured = capsys.readouterr() + assert ":0:" in captured.err + + +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys, "argv", ["check-json.py"]) + assert main() == 0 + + def test_main_valid_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "good.json", '{"key": "value"}\n') + monkeypatch.setattr(sys, "argv", ["check-json.py", str(f)]) + assert main() == 0 + + def test_main_invalid_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "bad.json", '{"key": value}\n') + monkeypatch.setattr(sys, "argv", ["check-json.py", str(f)]) + assert main() == 1 + + def test_main_multiple_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns 1 if any file is invalid.""" + good = _write(tmp_path, "good.json", '{"key": "value"}\n') + bad = _write(tmp_path, "bad.json", '{"key": value}\n') + monkeypatch.setattr( + sys, "argv", ["check-json.py", str(good), str(bad)] + ) + assert main() == 1 + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + f = tmp_path / "data.json" + f.write_text('{"key": "value"}\n', encoding="utf-8") + result = check_json._display_path(str(f)) + assert result == "data.json" + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = check_json._display_path(str(other / "data.json")) + assert result == str(other / "data.json") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_links.py b/scripts/tests/test_check_links.py index eefa0833..588515b1 100644 --- a/scripts/tests/test_check_links.py +++ b/scripts/tests/test_check_links.py @@ -259,5 +259,146 @@ def test_unclosed_fence_extends_to_end(self) -> None: assert ranges[0][1] == len(content) +check_markdown_link = check_links.check_markdown_link +check_markdown_file = check_links.check_markdown_file + + +class TestFailClosedAnchorValidation: + """Tests that file read errors during anchor validation fail closed.""" + + def test_same_file_anchor_unreadable_returns_error( + self, tmp_path: Path + ) -> None: + """Anchor validation on an unreadable file returns an error, not True.""" + f = tmp_path / "test.md" + f.write_text("# Heading\n[link](#anchor)\n", encoding="utf-8") + f.chmod(0o000) + try: + is_valid, error_msg = check_markdown_link( + f, "#anchor", tmp_path + ) + assert not is_valid + assert "Cannot read file" in error_msg + finally: + f.chmod(0o644) + + def test_same_file_anchor_binary_returns_error( + self, tmp_path: Path + ) -> None: + """Anchor validation on a binary file returns an error, not True.""" + f = tmp_path / "test.md" + f.write_bytes(b"\xff\xfe\x00\x01") + is_valid, error_msg = check_markdown_link( + f, "#heading", tmp_path + ) + assert not is_valid + assert "Cannot read file" in error_msg + + def test_cross_file_anchor_unreadable_returns_error( + self, tmp_path: Path + ) -> None: + """Cross-file anchor validation on unreadable target returns error.""" + source = tmp_path / "source.md" + source.write_text("[link](target.md#heading)\n", encoding="utf-8") + target = tmp_path / "target.md" + target.write_text("# Heading\n", encoding="utf-8") + target.chmod(0o000) + try: + is_valid, error_msg = check_markdown_link( + source, "target.md#heading", tmp_path + ) + assert not is_valid + assert "Cannot read" in error_msg + assert "anchor" in error_msg + finally: + target.chmod(0o644) + + def test_cross_file_anchor_binary_target_returns_error( + self, tmp_path: Path + ) -> None: + """Cross-file anchor validation on binary target returns error.""" + source = tmp_path / "source.md" + source.write_text("[link](target.md#heading)\n", encoding="utf-8") + target = tmp_path / "target.md" + target.write_bytes(b"\xff\xfe\x00\x01") + is_valid, error_msg = check_markdown_link( + source, "target.md#heading", tmp_path + ) + assert not is_valid + assert "Cannot read" in error_msg + + def test_check_markdown_file_counts_anchor_read_errors( + self, tmp_path: Path + ) -> None: + """check_markdown_file counts anchor read errors as errors.""" + # Create a source file that links to an unreadable target + target = tmp_path / "target.md" + target.write_text("# Heading\n", encoding="utf-8") + target.chmod(0o000) + try: + source = tmp_path / "source.md" + source.write_text( + "[link](target.md#heading)\n", encoding="utf-8" + ) + result = check_markdown_file(source, tmp_path) + assert result.errors > 0 + finally: + target.chmod(0o644) + + +class TestRelativePaths: + """Tests that _rel() converts absolute paths to relative.""" + + def test_rel_converts_absolute_to_relative(self, tmp_path: Path) -> None: + """Absolute path under root is converted to relative.""" + sub = tmp_path / "docs" + sub.mkdir() + f = sub / "guide.md" + f.write_text("# Guide\n", encoding="utf-8") + result = check_links._rel(f, tmp_path) + assert result == Path("docs") / "guide.md" + + def test_rel_fallback_when_outside_root(self, tmp_path: Path) -> None: + """Path outside root falls back to original path.""" + other = tmp_path / "other" + other.mkdir() + root = tmp_path / "root" + root.mkdir() + target = other / "file.md" + target.write_text("# File\n", encoding="utf-8") + result = check_links._rel(target, root) + assert result == target + + +class TestErrorsGoToStderr: + """Tests that ERROR messages go to stderr, not stdout.""" + + def test_file_read_error_prints_to_stderr( + self, tmp_path: Path, capsys + ) -> None: + """File read errors produce ERROR on stderr, not stdout.""" + f = tmp_path / "unreadable.md" + f.write_text("# Heading\n", encoding="utf-8") + f.chmod(0o000) + try: + check_markdown_file(f, tmp_path) + captured = capsys.readouterr() + assert "ERROR:" in captured.err + assert "ERROR:" not in captured.out + finally: + f.chmod(0o644) + + def test_broken_link_error_prints_to_stderr( + self, tmp_path: Path, capsys + ) -> None: + """Broken link errors produce ERROR on stderr, not stdout.""" + f = tmp_path / "broken.md" + f.write_text("[text](nonexistent.md)\n", encoding="utf-8") + check_markdown_file(f, tmp_path) + captured = capsys.readouterr() + assert "ERROR:" in captured.err + assert "ERROR:" not in captured.out + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_llm_line_limit.py b/scripts/tests/test_check_llm_line_limit.py index ab63f56f..45351459 100644 --- a/scripts/tests/test_check_llm_line_limit.py +++ b/scripts/tests/test_check_llm_line_limit.py @@ -8,6 +8,7 @@ from __future__ import annotations +import re import sys from pathlib import Path @@ -145,9 +146,8 @@ def test_fail_prints_message(self, tmp_path: Path, capsys: pytest.CaptureFixture assert result is False captured = capsys.readouterr() - assert "FAIL" in captured.out - assert "305" in captured.out - assert "big.md" in captured.out + assert "big.md:0:" in captured.err + assert "305" in captured.err def test_pass_prints_nothing(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: """Passing file produces no output.""" @@ -165,7 +165,19 @@ def test_oserror_returns_false(self, tmp_path: Path, capsys: pytest.CaptureFixtu assert result is False captured = capsys.readouterr() - assert "Cannot read" in captured.out + assert "cannot read file" in captured.err + + def test_unicode_decode_error_returns_false( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Invalid UTF-8 file triggers UnicodeDecodeError and returns False.""" + filepath = tmp_path / "bad_encoding.md" + filepath.write_bytes(b"\x80\x81\x82 invalid utf8") + result = check_file(filepath, tmp_path) + + assert result is False + captured = capsys.readouterr() + assert "cannot read file" in captured.err class TestMain: @@ -202,7 +214,7 @@ def test_failure_prints_summary_message(self, tmp_path: Path, capsys: pytest.Cap _run_main_with_root(tmp_path, print_summary=True) captured = capsys.readouterr() - assert "must be 300 lines or fewer" in captured.out + assert "must be 300 lines or fewer" in captured.err def _run_main_with_root(repo_root: Path, *, print_summary: bool = False) -> int: @@ -220,7 +232,7 @@ def _run_main_with_root(repo_root: Path, *, print_summary: bool = False) -> int: all_ok = False if not all_ok: if print_summary: - print(f"\nAll .md files under .llm/ must be {MAX_LINES} lines or fewer.") + print(f"\nAll .md files under .llm/ must be {MAX_LINES} lines or fewer.", file=sys.stderr) return 1 return 0 @@ -281,5 +293,93 @@ def test_nested_files_checked(self, tmp_path: Path) -> None: assert check_file(md_files[0], tmp_path) is False +class TestOutputFormat: + """Tests that output follows {path}:{line_number}: {message} format.""" + + def test_fail_output_starts_with_path_colon_line( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Failure message must start with path:line: (no leading whitespace).""" + filepath = _make_md_file(tmp_path, "big.md", 305) + check_file(filepath, tmp_path) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + assert not line.startswith(" "), f"Leading whitespace: {line}" + + def test_oserror_output_includes_line_number( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Read error message must include :0: synthetic line number.""" + nonexistent = tmp_path / "does_not_exist.md" + check_file(nonexistent, tmp_path) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert ":0:" in line, f"Missing :0: in read error: {line}" + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + + def test_oserror_output_uses_relative_path( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """OSError output must use a relative path, not an absolute path.""" + nonexistent = tmp_path / "does_not_exist.md" + check_file(nonexistent, tmp_path) + captured = capsys.readouterr() + err = captured.err.strip() + assert not err.startswith(str(tmp_path)), ( + f"Error output should not start with absolute path: {err}" + ) + assert err.startswith("does_not_exist.md"), ( + f"Error output should start with relative filename: {err}" + ) + + def test_over_limit_output_uses_relative_path( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Over-limit output must use a relative path, not an absolute path.""" + llm_dir = tmp_path / ".llm" + filepath = _make_md_file(llm_dir, "big.md", 305) + check_file(filepath, tmp_path) + captured = capsys.readouterr() + err = captured.err.strip() + assert str(tmp_path) not in err, ( + f"Error output should not contain absolute path: {err}" + ) + assert err.startswith(".llm/"), ( + f"Error output should start with relative .llm/ path: {err}" + ) + + def test_unicode_error_output_uses_relative_path( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """UnicodeDecodeError output must use a relative path, not absolute.""" + filepath = tmp_path / "bad_encoding.md" + filepath.write_bytes(b"\x80\x81\x82 invalid utf8") + check_file(filepath, tmp_path) + captured = capsys.readouterr() + err = captured.err.strip() + assert not err.startswith(str(tmp_path)), ( + f"Error output should not start with absolute path: {err}" + ) + assert err.startswith("bad_encoding.md"), ( + f"Error output should start with relative filename: {err}" + ) + + def test_main_output_no_leading_whitespace( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """main()-equivalent prints issue lines without leading whitespace.""" + llm_dir = tmp_path / ".llm" + _make_md_file(llm_dir, "bad.md", 301) + rc = _run_main_with_root(tmp_path, print_summary=True) + assert rc == 1 + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert not line.startswith(" "), f"Leading indent: {line!r}" + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_merge_conflict.py b/scripts/tests/test_check_merge_conflict.py new file mode 100644 index 00000000..7bf50ef5 --- /dev/null +++ b/scripts/tests/test_check_merge_conflict.py @@ -0,0 +1,195 @@ +#!/usr/bin/env python3 +""" +Unit tests for check-merge-conflict.py hook. + +Verifies that the merge conflict marker checker correctly detects +conflict markers (HEAD, separator, branch) in files, and that output +follows the {path}:{line}: {message} format. +""" + +from __future__ import annotations + +import importlib.util +import re +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "check_merge_conflict", scripts_dir / "hooks" / "check-merge-conflict.py" +) +check_merge_conflict = importlib.util.module_from_spec(spec) +spec.loader.exec_module(check_merge_conflict) + +check_file = check_merge_conflict.check_file +main = check_merge_conflict.main + + +def _write(directory: Path, name: str, content: str) -> Path: + """Helper to create a file with given content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +class TestMergeConflictDetection: + """Tests for detecting merge conflict markers.""" + + def test_head_marker_detected(self, tmp_path: Path) -> None: + """<<<<<<< HEAD marker is detected.""" + f = _write(tmp_path, "file.txt", "<<<<<<< HEAD\nsome content\n") + assert check_file(str(f)) is False + + def test_separator_marker_detected(self, tmp_path: Path) -> None: + """======= marker is detected.""" + f = _write(tmp_path, "file.txt", "line one\n=======\nline two\n") + assert check_file(str(f)) is False + + def test_branch_marker_detected(self, tmp_path: Path) -> None: + """>>>>>>> branch marker is detected.""" + f = _write(tmp_path, "file.txt", ">>>>>>> feature-branch\n") + assert check_file(str(f)) is False + + def test_clean_file_passes(self, tmp_path: Path) -> None: + """A file without conflict markers passes.""" + f = _write(tmp_path, "file.txt", "normal content\nnothing special\n") + assert check_file(str(f)) is True + + def test_empty_file_passes(self, tmp_path: Path) -> None: + """An empty file passes.""" + f = _write(tmp_path, "file.txt", "") + assert check_file(str(f)) is True + + def test_partial_markers_not_detected(self, tmp_path: Path) -> None: + """Markers without trailing space/newline are not detected.""" + f = _write(tmp_path, "file.txt", "<<<<< None: + """Full conflict block is detected (stops at first marker).""" + content = ( + "before\n" + "<<<<<<< HEAD\n" + "our version\n" + "=======\n" + "their version\n" + ">>>>>>> feature\n" + "after\n" + ) + f = _write(tmp_path, "file.txt", content) + assert check_file(str(f)) is False + + +class TestOutputFormat: + """Tests that output follows {path}:{line_number}: {message} format.""" + + def test_output_starts_with_path_colon_line( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output must start with path:line: (no leading whitespace).""" + f = _write(tmp_path, "file.txt", "<<<<<<< HEAD\n") + check_file(str(f)) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + assert not line.startswith(" "), f"Leading whitespace: {line}" + + def test_output_contains_merge_conflict_message( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output contains 'merge conflict marker found'.""" + f = _write(tmp_path, "file.txt", "<<<<<<< HEAD\n") + check_file(str(f)) + captured = capsys.readouterr() + assert "merge conflict marker found" in captured.err + + def test_correct_line_number_reported( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Line number in output matches the line with the marker.""" + f = _write(tmp_path, "file.txt", "clean\nclean\n<<<<<<< HEAD\n") + check_file(str(f)) + captured = capsys.readouterr() + assert ":3:" in captured.err + + def test_read_error_uses_zero_line_number( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Read error message must include :0: synthetic line number.""" + path = tmp_path / "nonexistent.txt" + check_file(str(path)) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert ":0:" in line, f"Missing :0: in read error: {line}" + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + + +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys, "argv", ["check-merge-conflict.py"]) + assert main() == 0 + + def test_main_clean_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "file.txt", "clean content\n") + monkeypatch.setattr(sys, "argv", ["check-merge-conflict.py", str(f)]) + assert main() == 0 + + def test_main_conflict_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "file.txt", "<<<<<<< HEAD\n") + monkeypatch.setattr(sys, "argv", ["check-merge-conflict.py", str(f)]) + assert main() == 1 + + def test_main_multiple_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns 1 if any file has conflicts.""" + clean = _write(tmp_path, "clean.txt", "ok\n") + dirty = _write(tmp_path, "dirty.txt", "<<<<<<< HEAD\n") + monkeypatch.setattr( + sys, "argv", ["check-merge-conflict.py", str(clean), str(dirty)] + ) + assert main() == 1 + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + f = tmp_path / "file.txt" + f.write_text("content\n", encoding="utf-8") + result = check_merge_conflict._display_path(str(f)) + assert result == "file.txt" + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = check_merge_conflict._display_path(str(other / "file.txt")) + assert result == str(other / "file.txt") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_toml.py b/scripts/tests/test_check_toml.py new file mode 100644 index 00000000..09794021 --- /dev/null +++ b/scripts/tests/test_check_toml.py @@ -0,0 +1,217 @@ +#!/usr/bin/env python3 +""" +Unit tests for check-toml.py hook. + +Verifies that the TOML validator correctly detects invalid TOML files +and that output follows the {path}:{line}: {message} format. +""" + +from __future__ import annotations + +import importlib.util +import re +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "check_toml", scripts_dir / "hooks" / "check-toml.py" +) +check_toml = importlib.util.module_from_spec(spec) +spec.loader.exec_module(check_toml) + +check_file = check_toml.check_file +main = check_toml.main +HAS_TOML = check_toml.HAS_TOML + + +def _write(directory: Path, name: str, content: str) -> Path: + """Helper to create a file with given content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +@pytest.mark.skipif(not HAS_TOML, reason="tomllib/tomli not available") +class TestTomlValidation: + """Tests for TOML validation.""" + + def test_valid_toml_passes(self, tmp_path: Path) -> None: + """A valid TOML file passes.""" + content = '[section]\nkey = "value"\nnumber = 42\n' + f = _write(tmp_path, "config.toml", content) + assert check_file(str(f)) is True + + def test_empty_toml_passes(self, tmp_path: Path) -> None: + """An empty TOML file passes.""" + f = _write(tmp_path, "empty.toml", "") + assert check_file(str(f)) is True + + def test_invalid_toml_fails(self, tmp_path: Path) -> None: + """An invalid TOML file fails.""" + f = _write(tmp_path, "bad.toml", "[section\nkey = value without quotes\n") + assert check_file(str(f)) is False + + def test_valid_toml_with_arrays(self, tmp_path: Path) -> None: + """TOML with arrays passes.""" + content = 'list = [1, 2, 3]\n\n[[items]]\nname = "a"\n\n[[items]]\nname = "b"\n' + f = _write(tmp_path, "arrays.toml", content) + assert check_file(str(f)) is True + + def test_duplicate_keys_fails(self, tmp_path: Path) -> None: + """TOML with duplicate keys fails.""" + content = 'key = "first"\nkey = "second"\n' + f = _write(tmp_path, "dup.toml", content) + assert check_file(str(f)) is False + + def test_error_line_number_is_accurate( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error line number uses lineno from exception when available. + + Python's built-in tomllib.TOMLDecodeError does not expose a lineno + attribute, so getattr falls back to 1. If the underlying library + (e.g. tomli) ever does expose it, the hook will use the real value. + This test verifies the output format is correct and the hook does + not crash. + """ + # Line 1-2 are valid, line 3 has the error (unclosed bracket) + content = '[section]\nkey = "value"\n[broken\n' + f = _write(tmp_path, "lineno.toml", content) + assert check_file(str(f)) is False + captured = capsys.readouterr() + first_line = captured.err.splitlines()[0] + # Must match path:line: format and contain TOML error + assert re.match(r'^.+:\d+: TOML error:', first_line), ( + f"Bad format: {first_line}" + ) + + +@pytest.mark.skipif(not HAS_TOML, reason="tomllib/tomli not available") +class TestUnicodeError: + """Tests that UnicodeDecodeError is treated as a read error, not TOML error.""" + + def test_binary_file_treated_as_read_error( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """A binary/non-UTF-8 file is reported as a read error with :0:.""" + binary_file = tmp_path / "binary.toml" + binary_file.write_bytes(b"\x80\x81\x82\xff\xfe") + assert check_file(str(binary_file)) is False + captured = capsys.readouterr() + first_line = captured.err.splitlines()[0] + assert ":0:" in first_line, f"Expected :0: in read error: {first_line}" + assert "cannot read file" in first_line, ( + f"Expected 'cannot read file' in: {first_line}" + ) + assert "TOML error" not in first_line, ( + f"Should not say 'TOML error' for encoding issue: {first_line}" + ) + + +@pytest.mark.skipif(not HAS_TOML, reason="tomllib/tomli not available") +class TestOutputFormat: + """Tests that output follows {path}:{line_number}: {message} format.""" + + def test_output_starts_with_path_colon_line( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output must start with path:line: (no leading whitespace).""" + f = _write(tmp_path, "bad.toml", "[section\nbroken\n") + check_file(str(f)) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + assert not line.startswith(" "), f"Leading whitespace: {line}" + + def test_output_contains_toml_error( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output contains 'TOML error'.""" + f = _write(tmp_path, "bad.toml", "[section\nbroken\n") + check_file(str(f)) + captured = capsys.readouterr() + assert "TOML error" in captured.err + + def test_read_error_uses_zero_line_number( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Read error message must include :0: synthetic line number.""" + path = tmp_path / "nonexistent.toml" + check_file(str(path)) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert ":0:" in line, f"Missing :0: in read error: {line}" + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + + +@pytest.mark.skipif(not HAS_TOML, reason="tomllib/tomli not available") +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys, "argv", ["check-toml.py"]) + assert main() == 0 + + def test_main_valid_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "good.toml", 'key = "value"\n') + monkeypatch.setattr(sys, "argv", ["check-toml.py", str(f)]) + assert main() == 0 + + def test_main_invalid_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "bad.toml", "[section\nbroken\n") + monkeypatch.setattr(sys, "argv", ["check-toml.py", str(f)]) + assert main() == 1 + + def test_main_multiple_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns 1 if any file is invalid.""" + good = _write(tmp_path, "good.toml", 'key = "value"\n') + bad = _write(tmp_path, "bad.toml", "[section\nbroken\n") + monkeypatch.setattr( + sys, "argv", ["check-toml.py", str(good), str(bad)] + ) + assert main() == 1 + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + f = tmp_path / "config.toml" + f.write_text('key = "value"\n', encoding="utf-8") + result = check_toml._display_path(str(f)) + assert result == "config.toml" + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = check_toml._display_path(str(other / "config.toml")) + assert result == str(other / "config.toml") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_track_caller_async.py b/scripts/tests/test_check_track_caller_async.py new file mode 100644 index 00000000..c314b659 --- /dev/null +++ b/scripts/tests/test_check_track_caller_async.py @@ -0,0 +1,635 @@ +#!/usr/bin/env python3 +""" +Unit tests for check-track-caller-async.py hook. + +Verifies that the #[track_caller] on async fn checker correctly detects +violations when the attribute and async fn appear on the same line, on +adjacent lines, or within the 5-line lookahead window, and that valid +usage does not produce false positives. +""" + +from __future__ import annotations + +import importlib.util +import re +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "check_track_caller_async", + scripts_dir / "hooks" / "check-track-caller-async.py", +) +check_track_caller_async = importlib.util.module_from_spec(spec) +spec.loader.exec_module(check_track_caller_async) + +check_file = check_track_caller_async.check_file +main = check_track_caller_async.main + + +def _write(directory: Path, name: str, content: str) -> Path: + """Helper to create a file with given content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +class TestSameLineDetection: + """Tests for #[track_caller] and async fn on the same line (bug fix).""" + + def test_track_caller_async_fn_same_line(self, tmp_path: Path) -> None: + """#[track_caller] async fn on same line is detected.""" + f = _write(tmp_path, "lib.rs", "#[track_caller] async fn foo() {}\n") + errors = check_file(f) + assert len(errors) == 1 + assert f"{f}:1:" in errors[0] + assert "#[track_caller] on async fn" in errors[0] + + def test_track_caller_pub_async_fn_same_line(self, tmp_path: Path) -> None: + """#[track_caller] pub async fn on same line is detected.""" + f = _write( + tmp_path, "lib.rs", "#[track_caller] pub async fn foo() {}\n" + ) + errors = check_file(f) + assert len(errors) == 1 + assert f"{f}:1:" in errors[0] + + def test_track_caller_pub_crate_async_fn_same_line( + self, tmp_path: Path + ) -> None: + """#[track_caller] pub(crate) async fn on same line is detected.""" + f = _write( + tmp_path, + "lib.rs", + "#[track_caller] pub(crate) async fn foo() {}\n", + ) + errors = check_file(f) + assert len(errors) == 1 + assert f"{f}:1:" in errors[0] + + def test_inline_attribute_before_track_caller_async( + self, tmp_path: Path + ) -> None: + """#[inline] #[track_caller] async fn on same line is detected.""" + f = _write( + tmp_path, + "lib.rs", + "#[inline] #[track_caller] async fn foo() {}\n", + ) + errors = check_file(f) + assert len(errors) == 1 + assert f"{f}:1:" in errors[0] + + def test_track_caller_unsafe_async_fn_same_line( + self, tmp_path: Path + ) -> None: + """#[track_caller] unsafe async fn on same line is detected.""" + f = _write( + tmp_path, + "lib.rs", + "#[track_caller] unsafe async fn foo() {}\n", + ) + errors = check_file(f) + assert len(errors) == 1 + assert f"{f}:1:" in errors[0] + + def test_track_caller_async_unsafe_fn_same_line( + self, tmp_path: Path + ) -> None: + """#[track_caller] async unsafe fn on same line is detected.""" + f = _write( + tmp_path, + "lib.rs", + "#[track_caller] async unsafe fn foo() {}\n", + ) + errors = check_file(f) + assert len(errors) == 1 + assert f"{f}:1:" in errors[0] + + def test_track_caller_pub_async_unsafe_fn( + self, tmp_path: Path + ) -> None: + """#[track_caller] pub async unsafe fn on same line is detected.""" + f = _write( + tmp_path, + "lib.rs", + "#[track_caller] pub async unsafe fn foo() {}\n", + ) + errors = check_file(f) + assert len(errors) == 1 + assert f"{f}:1:" in errors[0] + + +class TestMultiLineDetection: + """Tests for existing lookahead behavior across multiple lines.""" + + def test_track_caller_next_line_async_fn(self, tmp_path: Path) -> None: + """Standard 2-line case: #[track_caller] then async fn.""" + content = "#[track_caller]\nasync fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 2) + assert f"{f}:2:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + def test_track_caller_with_other_attrs_then_async( + self, tmp_path: Path + ) -> None: + """#[track_caller] then #[inline] then async fn is detected.""" + content = "#[track_caller]\n#[inline]\nasync fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 3) + assert f"{f}:3:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + def test_track_caller_with_comment_between(self, tmp_path: Path) -> None: + """Comment between #[track_caller] and async fn doesn't break detection.""" + content = "#[track_caller]\n// some comment\nasync fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 3) + assert f"{f}:3:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + def test_track_caller_with_block_comment_between( + self, tmp_path: Path + ) -> None: + """Single-line block comment between doesn't break detection.""" + content = "#[track_caller]\n/* some comment */\nasync fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 3) + assert f"{f}:3:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + def test_track_caller_stops_at_non_attr_line( + self, tmp_path: Path + ) -> None: + """A sync fn between stops lookahead; async fn after is not flagged.""" + content = ( + "#[track_caller]\n" + "fn sync_foo() {}\n" + "async fn bar() {}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 0 + + def test_track_caller_five_line_lookahead_limit( + self, tmp_path: Path + ) -> None: + """Async fn beyond the 5-line lookahead window is NOT detected.""" + # #[track_caller] on line 1, then 5 blank lines, async fn on line 7 + # Lookahead checks lines 2-6 (range i+1 to i+6), line 7 is outside + content = ( + "#[track_caller]\n" + "\n" + "\n" + "\n" + "\n" + "\n" + "async fn foo() {}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 0 + + def test_track_caller_unsafe_async_fn_multi_line( + self, tmp_path: Path + ) -> None: + """#[track_caller] then unsafe async fn on next line is detected.""" + content = "#[track_caller]\nunsafe async fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 2) + assert f"{f}:2:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + def test_track_caller_async_unsafe_fn_multi_line( + self, tmp_path: Path + ) -> None: + """#[track_caller] then async unsafe fn on next line is detected.""" + content = "#[track_caller]\nasync unsafe fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 2) + assert f"{f}:2:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + def test_track_caller_exactly_five_lines_detected( + self, tmp_path: Path + ) -> None: + """Async fn exactly at the 5-line lookahead boundary IS detected.""" + # #[track_caller] on line 1, then 4 blank lines, async fn on line 6 + # Lookahead checks lines 2-6 (range i+1 to i+6), line 6 is included + content = ( + "#[track_caller]\n" + "\n" + "\n" + "\n" + "\n" + "async fn foo() {}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 6) + assert f"{f}:6:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + +class TestValidUsage: + """Tests that valid code does not produce false positives.""" + + def test_track_caller_on_sync_fn(self, tmp_path: Path) -> None: + """#[track_caller] fn foo() is valid and should not be flagged.""" + f = _write(tmp_path, "lib.rs", "#[track_caller] fn foo() {}\n") + errors = check_file(f) + assert errors == [] + + def test_track_caller_on_sync_pub_fn(self, tmp_path: Path) -> None: + """#[track_caller] then pub fn on next line is valid.""" + content = "#[track_caller]\npub fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert errors == [] + + def test_async_fn_without_track_caller(self, tmp_path: Path) -> None: + """async fn without #[track_caller] is valid.""" + f = _write(tmp_path, "lib.rs", "async fn foo() {}\n") + errors = check_file(f) + assert errors == [] + + def test_empty_file(self, tmp_path: Path) -> None: + """Empty file produces no errors.""" + f = _write(tmp_path, "lib.rs", "") + errors = check_file(f) + assert errors == [] + + def test_no_rust_code(self, tmp_path: Path) -> None: + """Non-Rust content produces no errors.""" + f = _write(tmp_path, "lib.rs", "hello world\nfoo bar baz\n") + errors = check_file(f) + assert errors == [] + + def test_track_caller_in_comment(self, tmp_path: Path) -> None: + """// #[track_caller] before async fn is not flagged.""" + content = "// #[track_caller]\nasync fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert errors == [] + + def test_track_caller_in_single_line_block_comment( + self, tmp_path: Path + ) -> None: + """/* #[track_caller] */ before async fn is not flagged.""" + content = "/* #[track_caller] */\nasync fn foo() {}\n" + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert errors == [] + + def test_indented_track_caller_on_sync_fn(self, tmp_path: Path) -> None: + """Indented #[track_caller] on sync fn is valid (inside impl block).""" + content = ( + "impl Foo {\n" + " #[track_caller]\n" + " fn bar() {}\n" + "}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert errors == [] + + +class TestMultipleViolations: + """Tests for files with multiple violations.""" + + def test_multiple_violations_all_detected(self, tmp_path: Path) -> None: + """Multiple violations in one file are all detected.""" + content = ( + "#[track_caller]\n" + "async fn foo() {}\n" + "\n" + "#[track_caller]\n" + "async fn bar() {}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 2 + # First: async fn on line 2, #[track_caller] on line 1 + assert f"{f}:2:" in errors[0] + assert "(line 1)" in errors[0] + # Second: async fn on line 5, #[track_caller] on line 4 + assert f"{f}:5:" in errors[1] + assert "(line 4)" in errors[1] + + def test_mix_valid_and_invalid(self, tmp_path: Path) -> None: + """Only invalid usage is flagged when mixed with valid usage.""" + content = ( + "#[track_caller]\n" + "fn good() {}\n" + "\n" + "#[track_caller]\n" + "async fn bad() {}\n" + "\n" + "async fn also_good() {}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 5) + assert f"{f}:5:" in errors[0] + # message body mentions the #[track_caller] attribute (line 4) + assert "(line 4)" in errors[0] + + def test_same_line_and_multi_line_violations( + self, tmp_path: Path + ) -> None: + """Both same-line and multi-line violations are detected.""" + content = ( + "#[track_caller] async fn foo() {}\n" + "\n" + "#[track_caller]\n" + "async fn bar() {}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 2 + # First error: same-line on line 1 + assert f"{f}:1:" in errors[0] + assert "#[track_caller] on async fn" in errors[0] + # Second error: multi-line, async fn on line 4, #[track_caller] on line 3 + assert f"{f}:4:" in errors[1] + assert "(line 3)" in errors[1] + + +class TestEdgeCases: + """Tests for edge cases and boundary conditions.""" + + def test_nonexistent_file_reports_error(self, tmp_path: Path) -> None: + """Nonexistent file returns an error about read failure.""" + path = tmp_path / "nonexistent.rs" + errors = check_file(path) + assert len(errors) == 1 + assert "nonexistent.rs" in errors[0] + + def test_nonexistent_file_no_warning_prefix_on_stderr( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Nonexistent file does not print a Warning: line to stderr. + + check_file returns the error in {path}:0: format; it should not + also print a separate Warning: line. + """ + path = tmp_path / "nonexistent.rs" + errors = check_file(path) + captured = capsys.readouterr() + assert len(errors) == 1 + assert ":0: cannot read file" in errors[0] + assert "Warning:" not in captured.err + + def test_unreadable_file_reports_error(self, tmp_path: Path) -> None: + """Unreadable file returns an error about read failure.""" + f = _write(tmp_path, "lib.rs", "#[track_caller]\nasync fn foo() {}\n") + f.chmod(0o000) + try: + errors = check_file(f) + assert len(errors) == 1 + assert str(f) in errors[0] + finally: + f.chmod(0o644) # Restore permissions for cleanup + + def test_track_caller_at_end_of_file(self, tmp_path: Path) -> None: + """#[track_caller] on last line with no async fn after is fine.""" + f = _write(tmp_path, "lib.rs", "fn foo() {}\n#[track_caller]\n") + errors = check_file(f) + assert errors == [] + + def test_doc_comment_between_track_caller_and_async( + self, tmp_path: Path + ) -> None: + """/// doc comment between should continue scanning.""" + content = ( + "#[track_caller]\n" + "/// This is a doc comment\n" + "async fn foo() {}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 3) + assert f"{f}:3:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + def test_indented_track_caller_on_async_fn_detected( + self, tmp_path: Path + ) -> None: + """Indented #[track_caller] on async fn inside impl block is detected.""" + content = ( + "impl Foo {\n" + " #[track_caller]\n" + " async fn bar() {}\n" + "}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 3) + assert f"{f}:3:" in errors[0] + # message body mentions the #[track_caller] attribute (line 2) + assert "(line 2)" in errors[0] + + def test_track_caller_with_blank_lines(self, tmp_path: Path) -> None: + """Blank lines between #[track_caller] and async fn are skipped.""" + content = ( + "#[track_caller]\n" + "\n" + "\n" + "async fn foo() {}\n" + ) + f = _write(tmp_path, "lib.rs", content) + errors = check_file(f) + assert len(errors) == 1 + # path:line prefix points to the async fn (line 4) + assert f"{f}:4:" in errors[0] + # message body mentions the #[track_caller] attribute (line 1) + assert "(line 1)" in errors[0] + + +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero(self, monkeypatch: pytest.MonkeyPatch) -> None: + """No args means no errors, return 0.""" + monkeypatch.setattr(sys, "argv", ["check-track-caller-async.py"]) + assert main() == 0 + + def test_main_clean_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Clean .rs file returns 0.""" + f = _write(tmp_path, "clean.rs", "#[track_caller]\nfn foo() {}\n") + monkeypatch.setattr( + sys, "argv", ["check-track-caller-async.py", str(f)] + ) + assert main() == 0 + + def test_main_violation_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """File with violation returns 1.""" + f = _write( + tmp_path, "bad.rs", "#[track_caller]\nasync fn foo() {}\n" + ) + monkeypatch.setattr( + sys, "argv", ["check-track-caller-async.py", str(f)] + ) + assert main() == 1 + + def test_main_prints_error_details( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + """Verify the output format includes error details on stderr.""" + f = _write( + tmp_path, "bad.rs", "#[track_caller]\nasync fn foo() {}\n" + ) + monkeypatch.setattr( + sys, "argv", ["check-track-caller-async.py", str(f)] + ) + main() + captured = capsys.readouterr() + assert "ERROR: #[track_caller] cannot be used on async fn:" in captured.err + assert "#[track_caller]" in captured.err + assert "on async fn" in captured.err + assert "not supported by Rust" in captured.err + + def test_main_skips_non_rs_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Non-.rs files are ignored by main() even if they contain violations.""" + f = _write( + tmp_path, + "notes.txt", + "#[track_caller]\nasync fn foo() {}\n", + ) + monkeypatch.setattr( + sys, "argv", ["check-track-caller-async.py", str(f)] + ) + assert main() == 0 + + def test_main_nonexistent_rs_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when given a non-existent .rs file. + + Missing .rs files must not be silently skipped. + """ + missing = tmp_path / "missing.rs" + monkeypatch.setattr( + sys, "argv", ["check-track-caller-async.py", str(missing)] + ) + assert main() == 1 + + def test_main_multiple_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() checks all .rs files passed as arguments.""" + good = _write( + tmp_path, "good.rs", "#[track_caller]\nfn foo() {}\n" + ) + bad = _write( + tmp_path, "bad.rs", "#[track_caller]\nasync fn bar() {}\n" + ) + monkeypatch.setattr( + sys, + "argv", + ["check-track-caller-async.py", str(good), str(bad)], + ) + assert main() == 1 + + +class TestOutputFormat: + """Tests that output follows {path}:{line_number}: {message} format.""" + + def test_issues_start_with_path_colon_line(self, tmp_path: Path) -> None: + """Each issue must start with path:line: (no leading whitespace).""" + f = _write(tmp_path, "lib.rs", "#[track_caller]\nasync fn foo() {}\n") + errors = check_file(f) + assert len(errors) == 1 + assert re.match(r'^.+:\d+: ', errors[0]), f"Bad format: {errors[0]}" + assert not errors[0].startswith(" "), f"Leading whitespace: {errors[0]}" + + def test_read_error_includes_line_number(self, tmp_path: Path) -> None: + """Read error message must include :0: synthetic line number.""" + path = tmp_path / "nonexistent.rs" + errors = check_file(path) + assert len(errors) == 1 + assert ":0:" in errors[0], f"Missing :0: in read error: {errors[0]}" + assert re.match(r'^.+:\d+: ', errors[0]), f"Bad format: {errors[0]}" + + def test_main_output_no_leading_whitespace( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + """main() prints issue lines without leading whitespace.""" + f = _write(tmp_path, "bad.rs", "#[track_caller]\nasync fn foo() {}\n") + monkeypatch.setattr(sys, "argv", ["check-track-caller-async.py", str(f)]) + main() + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line and not line.startswith(("ERROR:", "Rust", "Remove")): + assert not line.startswith(" "), f"Leading indent: {line!r}" + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + sub = tmp_path / "src" + sub.mkdir() + f = sub / "lib.rs" + f.write_text("fn main() {}\n", encoding="utf-8") + result = check_track_caller_async._display_path(f) + assert result == str(Path("src") / "lib.rs") + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = check_track_caller_async._display_path(str(other / "lib.rs")) + assert result == str(other / "lib.rs") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_check_yaml.py b/scripts/tests/test_check_yaml.py new file mode 100644 index 00000000..e5f11bdd --- /dev/null +++ b/scripts/tests/test_check_yaml.py @@ -0,0 +1,181 @@ +#!/usr/bin/env python3 +""" +Unit tests for check-yaml.py hook. + +Verifies that the YAML validator correctly detects invalid YAML files +and that output follows the {path}:{line}: {message} format. +""" + +from __future__ import annotations + +import importlib.util +import re +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "check_yaml", scripts_dir / "hooks" / "check-yaml.py" +) +check_yaml = importlib.util.module_from_spec(spec) +spec.loader.exec_module(check_yaml) + +check_file = check_yaml.check_file +main = check_yaml.main +HAS_YAML = check_yaml.HAS_YAML + + +def _write(directory: Path, name: str, content: str) -> Path: + """Helper to create a file with given content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +@pytest.mark.skipif(not HAS_YAML, reason="PyYAML not installed") +class TestYamlValidation: + """Tests for YAML validation.""" + + def test_valid_yaml_passes(self, tmp_path: Path) -> None: + """A valid YAML file passes.""" + f = _write(tmp_path, "config.yaml", "key: value\nlist:\n - item1\n - item2\n") + assert check_file(str(f)) is True + + def test_empty_yaml_passes(self, tmp_path: Path) -> None: + """An empty YAML file passes.""" + f = _write(tmp_path, "empty.yaml", "") + assert check_file(str(f)) is True + + def test_invalid_yaml_fails(self, tmp_path: Path) -> None: + """An invalid YAML file fails.""" + f = _write(tmp_path, "bad.yaml", "key: value\n bad_indent: oops\n") + assert check_file(str(f)) is False + + def test_invalid_yaml_with_tabs_fails(self, tmp_path: Path) -> None: + """YAML with tab indentation fails.""" + f = _write(tmp_path, "tabs.yaml", "key:\n\t- value\n") + assert check_file(str(f)) is False + + def test_valid_yaml_with_anchors_passes(self, tmp_path: Path) -> None: + """YAML with anchors/aliases passes.""" + content = "defaults: &defaults\n key: value\nconfig:\n <<: *defaults\n" + f = _write(tmp_path, "anchors.yaml", content) + assert check_file(str(f)) is True + + +@pytest.mark.skipif(not HAS_YAML, reason="PyYAML not installed") +class TestOutputFormat: + """Tests that output follows {path}:{line_number}: {message} format.""" + + def test_output_starts_with_path_colon_line( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output first line must start with path:line: (no leading whitespace).""" + f = _write(tmp_path, "bad.yaml", "key: value\n bad_indent: oops\n") + check_file(str(f)) + captured = capsys.readouterr() + # Only check the first line -- YAML exceptions produce multiline str(e) + first_line = captured.err.splitlines()[0] + assert re.match(r'^.+:\d+: ', first_line), f"Bad format: {first_line}" + assert not first_line.startswith(" "), f"Leading whitespace: {first_line}" + + def test_output_contains_yaml_error( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output contains 'YAML error'.""" + f = _write(tmp_path, "bad.yaml", "key: value\n bad_indent: oops\n") + check_file(str(f)) + captured = capsys.readouterr() + assert "YAML error" in captured.err + + def test_read_error_uses_zero_line_number( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Read error message must include :0: synthetic line number.""" + path = tmp_path / "nonexistent.yaml" + check_file(str(path)) + captured = capsys.readouterr() + for line in captured.err.splitlines(): + if line: + assert ":0:" in line, f"Missing :0: in read error: {line}" + assert re.match(r'^.+:\d+: ', line), f"Bad format: {line}" + + def test_unicode_decode_error_handled( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Invalid UTF-8 file returns False and outputs :0: format.""" + path = tmp_path / "bad_encoding.yaml" + path.write_bytes(b"\x80\x81\x82 invalid utf8") + assert check_file(str(path)) is False + captured = capsys.readouterr() + assert ":0:" in captured.err + + +@pytest.mark.skipif(not HAS_YAML, reason="PyYAML not installed") +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys, "argv", ["check-yaml.py"]) + assert main() == 0 + + def test_main_valid_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "good.yaml", "key: value\n") + monkeypatch.setattr(sys, "argv", ["check-yaml.py", str(f)]) + assert main() == 0 + + def test_main_invalid_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "bad.yaml", "key: value\n bad: indent\n") + monkeypatch.setattr(sys, "argv", ["check-yaml.py", str(f)]) + assert main() == 1 + + def test_main_multiple_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns 1 if any file is invalid.""" + good = _write(tmp_path, "good.yaml", "key: value\n") + bad = _write(tmp_path, "bad.yaml", "key: value\n bad: indent\n") + monkeypatch.setattr( + sys, "argv", ["check-yaml.py", str(good), str(bad)] + ) + assert main() == 1 + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + f = tmp_path / "config.yaml" + f.write_text("key: value\n", encoding="utf-8") + result = check_yaml._display_path(str(f)) + assert result == "config.yaml" + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = check_yaml._display_path(str(other / "config.yaml")) + assert result == str(other / "config.yaml") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_end_of_file_fixer.py b/scripts/tests/test_end_of_file_fixer.py new file mode 100644 index 00000000..8a30d3ee --- /dev/null +++ b/scripts/tests/test_end_of_file_fixer.py @@ -0,0 +1,223 @@ +#!/usr/bin/env python3 +""" +Unit tests for end-of-file-fixer.py hook. + +Verifies that the end-of-file fixer correctly ensures files end with a single +newline, handles errors, and that output follows conventions. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "end_of_file_fixer", scripts_dir / "hooks" / "end-of-file-fixer.py" +) +end_of_file_fixer = importlib.util.module_from_spec(spec) +spec.loader.exec_module(end_of_file_fixer) + +fix_file = end_of_file_fixer.fix_file +main = end_of_file_fixer.main + + +def _write_bytes(directory: Path, name: str, content: bytes) -> Path: + """Helper to create a file with given byte content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_bytes(content) + return filepath + + +class TestFixFile: + """Tests for fix_file() functionality.""" + + def test_correct_ending_unchanged(self, tmp_path: Path) -> None: + """A file ending with exactly one newline is not modified.""" + f = _write_bytes(tmp_path, "good.txt", b"hello\nworld\n") + assert fix_file(str(f)) is False + assert f.read_bytes() == b"hello\nworld\n" + + def test_missing_newline_added(self, tmp_path: Path) -> None: + """A file missing a trailing newline gets one added.""" + f = _write_bytes(tmp_path, "no_nl.txt", b"hello\nworld") + assert fix_file(str(f)) is True + assert f.read_bytes() == b"hello\nworld\n" + + def test_extra_newlines_removed(self, tmp_path: Path) -> None: + """Extra trailing newlines are reduced to one.""" + f = _write_bytes(tmp_path, "extra.txt", b"hello\nworld\n\n\n") + assert fix_file(str(f)) is True + assert f.read_bytes() == b"hello\nworld\n" + + def test_trailing_spaces_removed(self, tmp_path: Path) -> None: + """Trailing whitespace (spaces/tabs) at end of file is removed.""" + f = _write_bytes(tmp_path, "ws.txt", b"hello\nworld\n \t\n") + assert fix_file(str(f)) is True + assert f.read_bytes() == b"hello\nworld\n" + + def test_crlf_trailing_removed(self, tmp_path: Path) -> None: + """Trailing CRLF endings are normalized.""" + f = _write_bytes(tmp_path, "crlf.txt", b"hello\r\n\r\n") + assert fix_file(str(f)) is True + assert f.read_bytes() == b"hello\n" + + def test_empty_file_unchanged(self, tmp_path: Path) -> None: + """An empty file is not modified.""" + f = _write_bytes(tmp_path, "empty.txt", b"") + assert fix_file(str(f)) is False + + def test_prints_fixed_message_on_modification( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Prints 'Fixed: ' to stdout when file is modified.""" + f = _write_bytes(tmp_path, "no_nl.txt", b"hello") + fix_file(str(f)) + captured = capsys.readouterr() + assert f"Fixed: {f}" in captured.out + + def test_no_output_when_unchanged( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """No output when file is unchanged.""" + f = _write_bytes(tmp_path, "good.txt", b"hello\n") + fix_file(str(f)) + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + + +class TestErrorHandling: + """Tests that read errors cause non-zero exit (fail-closed).""" + + def test_nonexistent_file_returns_none(self, tmp_path: Path) -> None: + """Nonexistent file returns None (error), not False.""" + result = fix_file(str(tmp_path / "nonexistent.txt")) + assert result is None + + def test_nonexistent_file_prints_to_stderr( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Nonexistent file prints error to stderr with :0: format.""" + fix_file(str(tmp_path / "nonexistent.txt")) + captured = capsys.readouterr() + assert ":0:" in captured.err + assert "cannot read file" in captured.err + + def test_unreadable_file_returns_none(self, tmp_path: Path) -> None: + """Unreadable file returns None (error), not False.""" + f = _write_bytes(tmp_path, "noperm.txt", b"hello") + f.chmod(0o000) + try: + result = fix_file(str(f)) + assert result is None + finally: + f.chmod(0o644) + + def test_main_nonexistent_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when a file cannot be read (fail-closed).""" + nonexistent = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["end-of-file-fixer.py", str(nonexistent)] + ) + assert main() == 1 + + def test_main_error_and_clean_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when any file has an error, even if others are clean.""" + clean = _write_bytes(tmp_path, "clean.txt", b"hello\n") + missing = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["end-of-file-fixer.py", str(clean), str(missing)] + ) + assert main() == 1 + + def test_main_error_and_modified_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when one file errors and another is modified.""" + dirty = _write_bytes(tmp_path, "dirty.txt", b"hello") + missing = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["end-of-file-fixer.py", str(dirty), str(missing)] + ) + assert main() == 1 + # The dirty file should still have been fixed + assert dirty.read_bytes() == b"hello\n" + + +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys, "argv", ["end-of-file-fixer.py"]) + assert main() == 0 + + def test_main_clean_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write_bytes(tmp_path, "good.txt", b"hello\n") + monkeypatch.setattr( + sys, "argv", ["end-of-file-fixer.py", str(f)] + ) + assert main() == 0 + + def test_main_modified_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write_bytes(tmp_path, "no_nl.txt", b"hello") + monkeypatch.setattr( + sys, "argv", ["end-of-file-fixer.py", str(f)] + ) + assert main() == 1 + + def test_main_multiple_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns 1 if any file is modified.""" + clean = _write_bytes(tmp_path, "good.txt", b"hello\n") + dirty = _write_bytes(tmp_path, "no_nl.txt", b"hello") + monkeypatch.setattr( + sys, "argv", ["end-of-file-fixer.py", str(clean), str(dirty)] + ) + assert main() == 1 + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + f = tmp_path / "file.txt" + f.write_text("content\n", encoding="utf-8") + result = end_of_file_fixer._display_path(str(f)) + assert result == "file.txt" + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = end_of_file_fixer._display_path(str(other / "file.txt")) + assert result == str(other / "file.txt") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_mixed_line_ending.py b/scripts/tests/test_mixed_line_ending.py new file mode 100644 index 00000000..46107c2e --- /dev/null +++ b/scripts/tests/test_mixed_line_ending.py @@ -0,0 +1,217 @@ +#!/usr/bin/env python3 +""" +Unit tests for mixed-line-ending.py hook. + +Verifies that the line ending fixer correctly converts CRLF/CR to LF, +handles errors, and that output follows conventions. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "mixed_line_ending", scripts_dir / "hooks" / "mixed-line-ending.py" +) +mixed_line_ending = importlib.util.module_from_spec(spec) +spec.loader.exec_module(mixed_line_ending) + +fix_file = mixed_line_ending.fix_file +main = mixed_line_ending.main + + +def _write_bytes(directory: Path, name: str, content: bytes) -> Path: + """Helper to create a file with given byte content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_bytes(content) + return filepath + + +class TestFixFile: + """Tests for fix_file() functionality.""" + + def test_lf_only_unchanged(self, tmp_path: Path) -> None: + """A file with only LF endings is not modified.""" + f = _write_bytes(tmp_path, "lf.txt", b"hello\nworld\n") + assert fix_file(str(f)) is False + assert f.read_bytes() == b"hello\nworld\n" + + def test_crlf_converted_to_lf(self, tmp_path: Path) -> None: + """CRLF line endings are converted to LF.""" + f = _write_bytes(tmp_path, "crlf.txt", b"hello\r\nworld\r\n") + assert fix_file(str(f)) is True + assert f.read_bytes() == b"hello\nworld\n" + + def test_cr_converted_to_lf(self, tmp_path: Path) -> None: + """CR-only line endings are converted to LF.""" + f = _write_bytes(tmp_path, "cr.txt", b"hello\rworld\r") + assert fix_file(str(f)) is True + assert f.read_bytes() == b"hello\nworld\n" + + def test_mixed_endings_converted(self, tmp_path: Path) -> None: + """Mixed line endings are all converted to LF.""" + f = _write_bytes(tmp_path, "mixed.txt", b"hello\r\nworld\rend\n") + assert fix_file(str(f)) is True + assert f.read_bytes() == b"hello\nworld\nend\n" + + def test_empty_file_unchanged(self, tmp_path: Path) -> None: + """An empty file is not modified.""" + f = _write_bytes(tmp_path, "empty.txt", b"") + assert fix_file(str(f)) is False + + def test_prints_fixed_message_on_modification( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Prints 'Fixed line endings: ' to stdout when file is modified.""" + f = _write_bytes(tmp_path, "crlf.txt", b"hello\r\nworld\r\n") + fix_file(str(f)) + captured = capsys.readouterr() + assert f"Fixed line endings: {f}" in captured.out + + def test_no_output_when_unchanged( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """No output when file is unchanged.""" + f = _write_bytes(tmp_path, "lf.txt", b"hello\nworld\n") + fix_file(str(f)) + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + + +class TestErrorHandling: + """Tests that read errors cause non-zero exit (fail-closed).""" + + def test_nonexistent_file_returns_none(self, tmp_path: Path) -> None: + """Nonexistent file returns None (error), not False.""" + result = fix_file(str(tmp_path / "nonexistent.txt")) + assert result is None + + def test_nonexistent_file_prints_to_stderr( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Nonexistent file prints error to stderr with :0: format.""" + fix_file(str(tmp_path / "nonexistent.txt")) + captured = capsys.readouterr() + assert ":0:" in captured.err + assert "cannot read file" in captured.err + + def test_unreadable_file_returns_none(self, tmp_path: Path) -> None: + """Unreadable file returns None (error), not False.""" + f = _write_bytes(tmp_path, "noperm.txt", b"hello\r\n") + f.chmod(0o000) + try: + result = fix_file(str(f)) + assert result is None + finally: + f.chmod(0o644) + + def test_main_nonexistent_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when a file cannot be read (fail-closed).""" + nonexistent = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["mixed-line-ending.py", str(nonexistent)] + ) + assert main() == 1 + + def test_main_error_and_clean_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when any file has an error, even if others are clean.""" + clean = _write_bytes(tmp_path, "clean.txt", b"hello\n") + missing = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["mixed-line-ending.py", str(clean), str(missing)] + ) + assert main() == 1 + + def test_main_error_and_modified_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when one file errors and another is modified.""" + dirty = _write_bytes(tmp_path, "dirty.txt", b"hello\r\n") + missing = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["mixed-line-ending.py", str(dirty), str(missing)] + ) + assert main() == 1 + # The dirty file should still have been fixed + assert dirty.read_bytes() == b"hello\n" + + +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys, "argv", ["mixed-line-ending.py"]) + assert main() == 0 + + def test_main_clean_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write_bytes(tmp_path, "clean.txt", b"hello\n") + monkeypatch.setattr( + sys, "argv", ["mixed-line-ending.py", str(f)] + ) + assert main() == 0 + + def test_main_modified_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write_bytes(tmp_path, "crlf.txt", b"hello\r\n") + monkeypatch.setattr( + sys, "argv", ["mixed-line-ending.py", str(f)] + ) + assert main() == 1 + + def test_main_multiple_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns 1 if any file is modified.""" + clean = _write_bytes(tmp_path, "clean.txt", b"hello\n") + dirty = _write_bytes(tmp_path, "dirty.txt", b"hello\r\n") + monkeypatch.setattr( + sys, "argv", ["mixed-line-ending.py", str(clean), str(dirty)] + ) + assert main() == 1 + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + f = tmp_path / "file.txt" + f.write_text("content\n", encoding="utf-8") + result = mixed_line_ending._display_path(str(f)) + assert result == "file.txt" + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = mixed_line_ending._display_path(str(other / "file.txt")) + assert result == str(other / "file.txt") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_regenerate_skills_index.py b/scripts/tests/test_regenerate_skills_index.py new file mode 100644 index 00000000..058405e5 --- /dev/null +++ b/scripts/tests/test_regenerate_skills_index.py @@ -0,0 +1,291 @@ +#!/usr/bin/env python3 +""" +Unit tests for regenerate-skills-index.py hook. + +Verifies that the skills index generator correctly handles metadata +extraction, error propagation, and fail-closed behavior on unreadable files. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "regenerate_skills_index", + scripts_dir / "hooks" / "regenerate-skills-index.py", +) +regenerate_skills_index = importlib.util.module_from_spec(spec) +spec.loader.exec_module(regenerate_skills_index) + +extract_metadata = regenerate_skills_index.extract_metadata +build_index = regenerate_skills_index.build_index +main = regenerate_skills_index.main + + +def _write(directory: Path, name: str, content: str) -> Path: + """Helper to create a file with given content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +class TestExtractMetadata: + """Tests for extract_metadata() functionality.""" + + def test_extracts_category_and_when(self, tmp_path: Path) -> None: + """Extracts CATEGORY and WHEN from metadata comments.""" + f = _write( + tmp_path, + "test-skill.md", + "\n\n# Test Skill\n", + ) + result = extract_metadata(f) + assert result is not None + category, when = result + assert category == "Testing" + assert when == "Writing tests" + + def test_fallback_to_heading_when_no_when_comment(self, tmp_path: Path) -> None: + """Falls back to first heading when WHEN comment is absent.""" + f = _write( + tmp_path, + "test-skill.md", + "\n# My Skill Guide\n", + ) + result = extract_metadata(f) + assert result is not None + category, when = result + assert category == "Tools" + assert when == "My Skill Guide" + + def test_fallback_to_filename_when_no_heading(self, tmp_path: Path) -> None: + """Falls back to filename stem when no heading exists.""" + f = _write( + tmp_path, + "my-tool.md", + "\nSome content without heading.\n", + ) + result = extract_metadata(f) + assert result is not None + category, when = result + assert category == "Tools" + assert when == "my tool" + + def test_fallback_to_defaults_when_no_metadata(self, tmp_path: Path) -> None: + """Falls back to Uncategorized and heading when no comments exist.""" + f = _write( + tmp_path, + "basic.md", + "# Basic Guide\nSome content.\n", + ) + result = extract_metadata(f) + assert result is not None + category, when = result + assert category == "Uncategorized" + assert when == "Basic Guide" + + def test_nonexistent_file_returns_none(self, tmp_path: Path) -> None: + """Nonexistent file returns None (error), not fallback metadata.""" + result = extract_metadata(tmp_path / "nonexistent.md") + assert result is None + + def test_nonexistent_file_prints_to_stderr( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Nonexistent file prints error to stderr with :0: format.""" + extract_metadata(tmp_path / "nonexistent.md") + captured = capsys.readouterr() + assert ":0:" in captured.err + assert "cannot read file" in captured.err + + def test_unreadable_file_returns_none(self, tmp_path: Path) -> None: + """Unreadable file returns None (error), not fallback metadata.""" + f = _write(tmp_path, "noperm.md", "# Skill\n") + f.chmod(0o000) + try: + result = extract_metadata(f) + assert result is None + finally: + f.chmod(0o644) + + def test_binary_file_returns_none(self, tmp_path: Path) -> None: + """Binary (non-UTF-8) file returns None (error), not fallback metadata.""" + f = tmp_path / "binary.md" + f.write_bytes(b"\xff\xfe\x00\x01") + result = extract_metadata(f) + assert result is None + + +class TestBuildIndex: + """Tests for build_index() error tracking.""" + + def test_valid_skills_no_error(self, tmp_path: Path) -> None: + """Valid skill files produce index content with no error.""" + _write( + tmp_path, + "skill-a.md", + "\n\n# A\n", + ) + content, had_error = build_index(tmp_path) + assert not had_error + assert "skill-a.md" in content + + def test_unreadable_skill_sets_had_error(self, tmp_path: Path) -> None: + """Unreadable skill file sets had_error flag.""" + good = _write( + tmp_path, + "good.md", + "\n\n# Good\n", + ) + bad = _write(tmp_path, "bad.md", "# Bad\n") + bad.chmod(0o000) + try: + content, had_error = build_index(tmp_path) + assert had_error + # Good file should still be in index + assert "good.md" in content + # Bad file should NOT be in index (skipped due to error) + assert "bad.md" not in content + finally: + bad.chmod(0o644) + + def test_binary_skill_sets_had_error(self, tmp_path: Path) -> None: + """Binary (non-UTF-8) skill file sets had_error flag.""" + _write( + tmp_path, + "valid.md", + "\n\n# Valid\n", + ) + binary = tmp_path / "binary.md" + binary.write_bytes(b"\xff\xfe\x00\x01") + _content, had_error = build_index(tmp_path) + assert had_error + + def test_ignores_index_file(self, tmp_path: Path) -> None: + """_index.md is not treated as a skill file.""" + _write(tmp_path, "_index.md", "# Old Index\n") + _write( + tmp_path, + "real.md", + "\n\n# Real\n", + ) + content, had_error = build_index(tmp_path) + assert not had_error + assert "real.md" in content + assert "_index.md" not in content + + +class TestRelativePaths: + """Tests for relative-path error output when repo_root is provided.""" + + def test_extract_metadata_error_uses_relative_path( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output uses relative path when repo_root is provided.""" + nonexistent = tmp_path / "nonexistent.md" + extract_metadata(nonexistent, repo_root=tmp_path) + captured = capsys.readouterr() + assert captured.err.startswith("nonexistent.md:") + assert not captured.err.startswith(str(tmp_path)) + + def test_extract_metadata_error_uses_absolute_when_no_repo_root( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Error output uses full path when repo_root is not provided.""" + nonexistent = tmp_path / "nonexistent.md" + extract_metadata(nonexistent) + captured = capsys.readouterr() + assert str(tmp_path) in captured.err + + def test_build_index_unreadable_uses_relative_path( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """build_index() error output uses relative path when repo_root is provided.""" + skills_dir = tmp_path / "skills" + skills_dir.mkdir() + bad = _write(skills_dir, "bad.md", "# Bad\n") + bad.chmod(0o000) + try: + _content, had_error = build_index(skills_dir, repo_root=tmp_path) + assert had_error + captured = capsys.readouterr() + assert "skills/bad.md" in captured.err + assert not captured.err.startswith(str(tmp_path)) + finally: + bad.chmod(0o644) + + +class TestMain: + """Tests for main() entry point fail-closed behavior.""" + + def test_up_to_date_index_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns 0 when index is already up-to-date.""" + skills_dir = tmp_path / ".llm" / "skills" + skills_dir.mkdir(parents=True) + _write( + skills_dir, + "test.md", + "\n\n# Test\n", + ) + # Generate index, write it, then run main() which should find up-to-date + content, _had_error = build_index(skills_dir) + (skills_dir / "_index.md").write_text(content, encoding="utf-8") + # Patch __file__ so main() resolves to our tmp structure + fake_script = tmp_path / "scripts" / "hooks" / "regenerate-skills-index.py" + fake_script.parent.mkdir(parents=True, exist_ok=True) + fake_script.write_text("", encoding="utf-8") + monkeypatch.setattr( + regenerate_skills_index, "__file__", str(fake_script) + ) + monkeypatch.setattr(sys, "argv", ["regenerate-skills-index.py"]) + assert main() == 0 + + def test_unreadable_skill_causes_nonzero_exit( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when a skill file cannot be read (fail-closed).""" + skills_dir = tmp_path / ".llm" / "skills" + skills_dir.mkdir(parents=True) + bad = _write(skills_dir, "bad.md", "# Bad\n") + bad.chmod(0o000) + try: + fake_script = tmp_path / "scripts" / "hooks" / "regenerate-skills-index.py" + fake_script.parent.mkdir(parents=True, exist_ok=True) + fake_script.write_text("", encoding="utf-8") + monkeypatch.setattr( + regenerate_skills_index, "__file__", str(fake_script) + ) + monkeypatch.setattr(sys, "argv", ["regenerate-skills-index.py"]) + assert main() == 1 + finally: + bad.chmod(0o644) + + def test_binary_skill_causes_nonzero_exit( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when a skill file is binary (fail-closed).""" + skills_dir = tmp_path / ".llm" / "skills" + skills_dir.mkdir(parents=True) + binary = skills_dir / "binary.md" + binary.write_bytes(b"\xff\xfe\x00\x01") + fake_script = tmp_path / "scripts" / "hooks" / "regenerate-skills-index.py" + fake_script.parent.mkdir(parents=True, exist_ok=True) + fake_script.write_text("", encoding="utf-8") + monkeypatch.setattr( + regenerate_skills_index, "__file__", str(fake_script) + ) + monkeypatch.setattr(sys, "argv", ["regenerate-skills-index.py"]) + assert main() == 1 + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/tests/test_trailing_whitespace.py b/scripts/tests/test_trailing_whitespace.py new file mode 100644 index 00000000..3c9cc26d --- /dev/null +++ b/scripts/tests/test_trailing_whitespace.py @@ -0,0 +1,235 @@ +#!/usr/bin/env python3 +""" +Unit tests for trailing-whitespace.py hook. + +Verifies that the trailing whitespace fixer correctly removes trailing +whitespace, handles errors, and that output follows conventions. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +# Import the hook module (hyphenated filename requires importlib) +scripts_dir = Path(__file__).parent.parent +spec = importlib.util.spec_from_file_location( + "trailing_whitespace", scripts_dir / "hooks" / "trailing-whitespace.py" +) +trailing_whitespace = importlib.util.module_from_spec(spec) +spec.loader.exec_module(trailing_whitespace) + +fix_file = trailing_whitespace.fix_file +main = trailing_whitespace.main + + +def _write(directory: Path, name: str, content: str) -> Path: + """Helper to create a file with given content.""" + filepath = directory / name + filepath.parent.mkdir(parents=True, exist_ok=True) + filepath.write_text(content, encoding="utf-8") + return filepath + + +class TestFixFile: + """Tests for fix_file() functionality.""" + + def test_no_trailing_whitespace_unchanged(self, tmp_path: Path) -> None: + """A file without trailing whitespace is not modified.""" + f = _write(tmp_path, "clean.txt", "hello\nworld\n") + assert fix_file(str(f)) is False + assert f.read_text(encoding="utf-8") == "hello\nworld\n" + + def test_trailing_spaces_removed(self, tmp_path: Path) -> None: + """Trailing spaces are removed.""" + f = _write(tmp_path, "spaces.txt", "hello \nworld \n") + assert fix_file(str(f)) is True + assert f.read_text(encoding="utf-8") == "hello\nworld\n" + + def test_trailing_tabs_removed(self, tmp_path: Path) -> None: + """Trailing tabs are removed.""" + f = _write(tmp_path, "tabs.txt", "hello\t\nworld\t\t\n") + assert fix_file(str(f)) is True + assert f.read_text(encoding="utf-8") == "hello\nworld\n" + + def test_preserves_line_endings_lf(self, tmp_path: Path) -> None: + """LF line endings are preserved.""" + f = _write(tmp_path, "lf.txt", "hello \nworld \n") + fix_file(str(f)) + assert f.read_text(encoding="utf-8") == "hello\nworld\n" + + def test_empty_file_unchanged(self, tmp_path: Path) -> None: + """An empty file is not modified.""" + f = _write(tmp_path, "empty.txt", "") + assert fix_file(str(f)) is False + + def test_prints_fixed_message_on_modification( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Prints 'Fixed: ' to stdout when file is modified.""" + f = _write(tmp_path, "ws.txt", "hello \n") + fix_file(str(f)) + captured = capsys.readouterr() + assert f"Fixed: {f}" in captured.out + + def test_no_output_when_unchanged( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """No output when file is unchanged.""" + f = _write(tmp_path, "clean.txt", "hello\n") + fix_file(str(f)) + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + + +class TestErrorHandling: + """Tests that read errors cause non-zero exit (fail-closed).""" + + def test_nonexistent_file_returns_none(self, tmp_path: Path) -> None: + """Nonexistent file returns None (error), not False.""" + result = fix_file(str(tmp_path / "nonexistent.txt")) + assert result is None + + def test_nonexistent_file_prints_to_stderr( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Nonexistent file prints error to stderr with :0: format.""" + fix_file(str(tmp_path / "nonexistent.txt")) + captured = capsys.readouterr() + assert ":0:" in captured.err + assert "cannot read file" in captured.err + + def test_unreadable_file_returns_none(self, tmp_path: Path) -> None: + """Unreadable file returns None (error), not False.""" + f = _write(tmp_path, "noperm.txt", "hello \n") + f.chmod(0o000) + try: + result = fix_file(str(f)) + assert result is None + finally: + f.chmod(0o644) + + def test_binary_file_returns_none(self, tmp_path: Path) -> None: + """Binary (non-UTF-8) file returns None (error), not False.""" + f = tmp_path / "binary.txt" + f.write_bytes(b"\xff\xfe\x00\x01") + result = fix_file(str(f)) + assert result is None + + def test_main_nonexistent_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when a file cannot be read (fail-closed).""" + nonexistent = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["trailing-whitespace.py", str(nonexistent)] + ) + assert main() == 1 + + def test_main_binary_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when a binary file cannot be decoded.""" + f = tmp_path / "binary.txt" + f.write_bytes(b"\xff\xfe\x00\x01") + monkeypatch.setattr( + sys, "argv", ["trailing-whitespace.py", str(f)] + ) + assert main() == 1 + + def test_main_error_and_clean_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when any file has an error, even if others are clean.""" + clean = _write(tmp_path, "clean.txt", "hello\n") + missing = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["trailing-whitespace.py", str(clean), str(missing)] + ) + assert main() == 1 + + def test_main_error_and_modified_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """main() returns 1 when one file errors and another is modified.""" + dirty = _write(tmp_path, "dirty.txt", "hello \n") + missing = tmp_path / "missing.txt" + monkeypatch.setattr( + sys, "argv", ["trailing-whitespace.py", str(dirty), str(missing)] + ) + assert main() == 1 + # The dirty file should still have been fixed + assert dirty.read_text(encoding="utf-8") == "hello\n" + + +class TestMain: + """Tests for the main() entry point.""" + + def test_main_no_args_returns_zero( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(sys, "argv", ["trailing-whitespace.py"]) + assert main() == 0 + + def test_main_clean_file_returns_zero( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "clean.txt", "hello\n") + monkeypatch.setattr( + sys, "argv", ["trailing-whitespace.py", str(f)] + ) + assert main() == 0 + + def test_main_modified_file_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + f = _write(tmp_path, "ws.txt", "hello \n") + monkeypatch.setattr( + sys, "argv", ["trailing-whitespace.py", str(f)] + ) + assert main() == 1 + + def test_main_multiple_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns 1 if any file is modified.""" + clean = _write(tmp_path, "clean.txt", "hello\n") + dirty = _write(tmp_path, "dirty.txt", "hello \n") + monkeypatch.setattr( + sys, "argv", ["trailing-whitespace.py", str(clean), str(dirty)] + ) + assert main() == 1 + + +class TestRelativePaths: + """Tests that _display_path() converts absolute paths to relative.""" + + def test_display_path_converts_absolute_to_relative( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absolute path under CWD is converted to relative.""" + monkeypatch.chdir(tmp_path) + f = tmp_path / "file.txt" + f.write_text("content\n", encoding="utf-8") + result = trailing_whitespace._display_path(str(f)) + assert result == "file.txt" + + def test_display_path_fallback_when_outside_cwd( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Path outside CWD falls back to original string.""" + other = tmp_path / "other" + other.mkdir() + cwd_dir = tmp_path / "cwd_dir" + cwd_dir.mkdir() + monkeypatch.chdir(cwd_dir) + result = trailing_whitespace._display_path(str(other / "file.txt")) + assert result == str(other / "file.txt") + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/scripts/validate-wiki-output.py b/scripts/validate-wiki-output.py index 8e7efbbc..73fc57d1 100644 --- a/scripts/validate-wiki-output.py +++ b/scripts/validate-wiki-output.py @@ -26,9 +26,11 @@ from pathlib import Path from typing import NamedTuple -# Ensure stdout uses UTF-8 encoding for Unicode symbols +# Ensure stdout/stderr use UTF-8 encoding for Unicode symbols (cross-platform) if sys.stdout.encoding != "utf-8": sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding="utf-8", errors="replace") +if sys.stderr.encoding != "utf-8": + sys.stderr = io.TextIOWrapper(sys.stderr.buffer, encoding="utf-8", errors="replace") class Issue(NamedTuple): @@ -321,7 +323,7 @@ def check_broken_wiki_links(content: str, filename: str, wiki_pages: set[str]) - def validate_wiki(wiki_dir: Path, strict: bool = False) -> int: """Validate all wiki files and return exit code.""" if not wiki_dir.exists(): - print(red(f"ERROR: Wiki directory not found: {wiki_dir}")) + print(red(f"ERROR: Wiki directory not found: {wiki_dir}"), file=sys.stderr) return 1 # Get all wiki pages @@ -363,15 +365,15 @@ def validate_wiki(wiki_dir: Path, strict: bool = False) -> int: else: prefix = yellow("WARNING") - print(f" {prefix} {issue.file}:{issue.line}: {issue.message}") + print(f"{issue.file}:{issue.line}: {prefix}: {issue.message}", file=sys.stderr) print() # Summary if errors: - print(red(f"✗ Found {len(errors)} error(s) and {len(warnings)} warning(s)")) + print(red(f"✗ Found {len(errors)} error(s) and {len(warnings)} warning(s)"), file=sys.stderr) elif warnings: - print(yellow(f"⚠ Found {len(warnings)} warning(s)")) + print(yellow(f"⚠ Found {len(warnings)} warning(s)"), file=sys.stderr) else: print(green("✓ Wiki validation passed")) diff --git a/src/network/tokio_socket.rs b/src/network/tokio_socket.rs index 130da54a..c731892a 100644 --- a/src/network/tokio_socket.rs +++ b/src/network/tokio_socket.rs @@ -593,7 +593,7 @@ mod tests { // Helper function to wait for messages with retry logic using the async recv_all method. // This is necessary because UDP packet delivery timing can vary across platforms. - #[track_caller] + // Note: #[track_caller] is a no-op on async fn, so it is intentionally omitted here. async fn wait_for_messages( socket: &mut TokioUdpSocket, expected_count: usize,