Skip to content

fix(lint): a bare path is read as a linter name, turning the failure into a lint verdict - #3567

Open
WoldenMn wants to merge 1 commit into
rtk-ai:developfrom
WoldenMn:fix/lint-bare-path-treated-as-linter
Open

WoldenMn wants to merge 1 commit into
rtk-ai:developfrom
WoldenMn:fix/lint-bare-path-treated-as-linter

Conversation

@WoldenMn

Copy link
Copy Markdown

Problem

rtk lint src reports lint results for a run that never happened. Two shapes, depending on the package manager:

# npm — clean project, ESLint exits 0 with no findings
$ npx eslint src
$ echo $?
0

$ rtk lint src
Lint: 2 errors, 0 warnings
npm error could not determine executable to run
# pnpm — same command, opposite lie
$ rtk lint src
Lint: No issues found

That second one is the dangerous one: the only line printed says the project is clean, while ESLint never started.

Cause

Three steps line up:

  1. The rewrite rule maps npx eslint src to rtk lint src, dropping the linter name.
  2. detect_linter treats the first argument as a linter name unless it starts with - or contains . or /. A bare directory — src, app, lib, tests — matches none of those, so src becomes the linter and rtk runs npx -- src.
  3. That command fails, and filter_generic_lint scores the failure as lint output. It counts lines containing error or warning:
    • npm prints two npm error … lines → Lint: 2 errors.
    • pnpm prints ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command "src" not found, which contains neither word → both counters stay at 0 → Lint: No issues found.

On a non-English system the shell's own message is localized as well ('src' n'est pas reconnu en tant que commande interne), so a wording-based heuristic has nothing to latch onto there either.

The exit code is propagated correctly in both cases — but it is invisible to whoever reads the summary, and the summary is what an agent quotes.

Fix

detect_linter: an argument that exists on disk is a path. rtk lint src then lints src with ESLint rather than inventing a linter named after the directory. A name that is not a path keeps its linter meaning, so rtk lint biome check and rtk lint ruff are unaffected. The ambiguity is real — the rewrite discards the information — so it is resolved in the only direction that cannot fabricate a linter run.

filter_generic_lint: never certify a failed run as clean. It now takes the exit status and returns the raw output instead of Lint: No issues found when the command failed. The filter recognizes issues by the words error and warning, which means it cannot distinguish a clean run from one that never started; the exit status can. Real findings are still summarized as before.

Verified with a build of this branch:

Case Before After
rtk lint src, clean project, npm Lint: 2 errors, 0 warnings ESLint: No issues found
rtk lint src, ESLint absent, pnpm Lint: No issues found ESLint output (JSON parse failed…) + the pnpm error
rtk lint <unknown-linter> Lint: No issues found raw failure output

Tests

5 new tests, 20 passing in the module. Three are regression guards, all confirmed non-vacuous by mutation (inverting each guard makes them fail, restoring makes them pass):

  • detect_linter_treats_an_existing_directory_as_a_path
  • generic_lint_never_reports_clean_on_a_failed_run — fixture is the verbatim pnpm failure, localized line included
  • generic_lint_reports_clean_on_a_successful_run

cargo fmt -- --check clean, cargo clippy --all-targets no warnings.

cargo test on the full crate: 2577 passed; 3 failed. The three failures are in hooks::rewrite_cmd::tests::unattestable_passthrough and are pre-existing on develop (6f0b0ca), unrelated to this change — same ones noted in #3566.

Environment: Windows 11, stable-x86_64-pc-windows-msvc, npm and pnpm 10.34.4.

`npx eslint src` is rewritten to `rtk lint src`, which drops the linter
name. detect_linter then treated `src` as the linter, because a bare
directory name carries no `.`, `/` or leading `-`. rtk ran `npx -- src`,
the package manager failed, and filter_generic_lint turned that failure
into a lint verdict:

- npm prints two `npm error` lines, so a clean project was reported as
  "Lint: 2 errors, 0 warnings" while `npx eslint src` exits 0.
- pnpm prints `ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command "src" not
  found`, which contains neither "error" nor "warning", so both counters
  stayed at zero and the run was summarized as "Lint: No issues found" —
  a clean bill of health for a linter that never started. On a
  non-English system the first line is localized too, so the wording a
  reader would recognize is not there either.

Only the exit code carried the truth, and it is invisible to whoever
reads the summary.

Two changes:

- detect_linter treats an argument that exists on disk as a path, so
  `rtk lint src` lints `src` with ESLint instead of inventing a linter
  named after the directory. A name that is not a path keeps its linter
  meaning, so `rtk lint biome check` is unaffected.
- filter_generic_lint takes the exit status and refuses to emit "No
  issues found" for a command that failed, returning the raw output
  instead. It recognizes issues by the words "error" and "warning", so
  it cannot tell a clean run from one that never happened — the exit
  status can.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants