Skip to content

fix(lint): add first-class Biome support to rtk lint - #3150

Open
guyoron1 wants to merge 1 commit into
rtk-ai:developfrom
guyoron1:fix/biome-lint-detection
Open

guyoron1 wants to merge 1 commit into
rtk-ai:developfrom
guyoron1:fix/biome-lint-detection

Conversation

@guyoron1

@guyoron1 guyoron1 commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #3117

rtk lint runs eslint when the args name no linter, and the hook rewrite peels the tool name off as a strippable prefix. For eslint that is harmless. For Biome it is fatal: biome check src/ rewrote to rtk lint check src/, and detect_linter then tried to run check as the linter:

rtk: Failed to run check. Is it installed?

So Biome output never reached a Biome-aware filter through the hook, only if you typed rtk lint biome check ... by hand. Thanks pszymkowiak for catching that: the earlier version of this PR only had the filter half.

Routing. In rewrite_segment_inner, when the matched rtk lint prefix names biome, the tool name is kept, so biome check src/, npx biome check src/, pnpm biome ..., pnpm exec biome ... and npm run biome ... all land on rtk lint biome .... eslint is still stripped, unchanged, because eslint is the default. It reuses the existing tool_portion helper (8 added lines).

Filtering. A "biome" arm in the format-flag match (Biome has its own subcommands and needs no -f json) and filter_biome_output, which groups diagnostics by rule and file and falls back to the generic filter when there is nothing recognizable. The regexes are LazyLock statics: the previous push used lazy_static, which isn't a dependency, and that is what turned the clippy job red.

End to end, against a stand-in biome printing a real 1110-byte report:

$ rtk rewrite 'biome check src/'
rtk lint biome check src/
$ rtk lint biome check src/          # exit 1, as biome exited
Biome: 4 issues in 2 files
Rules:
  lint/suspicious/noDoubleEquals (2x)
  lint/correctness/noUnusedVariables (1x)
  lint/style/useConst (1x)

Files:
  App.tsx
  helpers.ts

1110 bytes -> 251 bytes, exit code preserved. On develop the same rewrite (rtk lint check src/) ends in npm error could not determine executable to run.

tests/biome_hook_roundtrip_test.rs pins the whole chain (rewrite, run, filter, exit code) and test_rewrite_biome_keeps_the_linter_name pins every invocation spelling; both fail on develop's routing.

Related open PRs: #2378 routes direct biome invocations to a separate rtk biome command rather than rtk lint biome, so it is a competing destination. This PR and #2860 both edit the test_rewrite_lint list in registry.rs; the conflict is confined to that one test and resolves to the eslint-only list.

Test plan

  • cargo fmt --all -- --check
  • cargo clippy --all-targets (0 warnings)
  • cargo test --all (all pass)
  • end to end: biome check src/ -> rewrite -> biome filter applied, exit code preserved

@pszymkowiak

Copy link
Copy Markdown
Collaborator

The filter itself is genuinely nice — I tested it against real Biome (v2.5.5) output and rtk lint biome check src/ compresses a 8620-byte report down to 242 bytes (~97%), grouping cleanly by rule and file:

Biome: 12 issues in 2 files
Rules:
  lint/suspicious/noDoubleEquals (4x)
  lint/style/useConst (4x)
  lint/correctness/noUnusedVariables (2x)
Files: App.ts, utils.ts

But there's a gap that stops it from working through the hook, which is the normal path. The rewrite drops the biome name (it's treated as a strippable prefix in rules.rs, same as eslint):

biome check src/  → rtk lint check src/     # "biome" gone
eslint src/       → rtk lint src/           # fine — eslint is rtk lint's default

The difference is fatal: eslint is the default, biome isn't. So the command the hook actually produces, rtk lint check src/, fails at runtime — detect_linter(["check", "src/"]) no longer sees biome and tries to run check as the linter:

rtk: Failed to run check. Is it installed?

So filter_biome_output only runs if someone manually types rtk lint biome check … (keeping the name) — never via the hook. The PR description says biome check src/ rewrites to rtk lint biome check src/, but on current develop it rewrites to rtk lint check src/ (verified on a compiled build).

To make this work end-to-end, the routing needs to preserve the linter name for biome (delegate to rtk lint biome check …) rather than stripping it — that's in rules.rs/the lint rewrite path, outside this PR's current diff. An end-to-end test (biome check → rewrite → biome filter applied) would catch it. The filter is solid; it just needs the routing half to land with it. Happy to help wire that up.

@guyoron1
guyoron1 force-pushed the fix/biome-lint-detection branch 2 times, most recently from 6897ce7 to e71581e Compare August 15, 2026 07:23
@CLAassistant

CLAassistant commented Aug 15, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@guyoron1
guyoron1 force-pushed the fix/biome-lint-detection branch from e71581e to 6d07d2c Compare August 16, 2026 10:34
`rtk lint` treats eslint as its default linter, and the hook rewrite
peels the tool name off as a strippable prefix. For eslint that is
harmless, for biome it is fatal: `biome check src/` became
`rtk lint check src/`, and `detect_linter` then tried to run `check` as
the linter. Biome output never reached a Biome-aware filter through the
hook — only by typing `rtk lint biome check …` by hand.

Routing: keep the linter name when the matched rewrite prefix names a
tool that is not `rtk lint`'s default, so every spelling
(`biome`, `npx biome`, `pnpm biome`, `pnpm exec biome`, …) lands on
`rtk lint biome …`. eslint keeps being stripped exactly as before.

Filtering: add a `"biome"` arm to the format-flag match (Biome has its
own subcommands and needs no `-f json`) and a `filter_biome_output`
that groups diagnostics by rule and file, falling back to the generic
lint filter when the output carries no recognizable diagnostics.

Tests: a rewrite test per invocation form, four filter tests including a
token-savings floor, and an end-to-end test that rewrites
`biome check src/` and runs the result against a stubbed biome,
asserting the compressed report and the preserved exit code.

Fixes rtk-ai#3117
@guyoron1

Copy link
Copy Markdown
Contributor Author

@pszymkowiak thanks for catching the routing gap, that was the missing half. It's closed in this push:

  • rewrite_segment_inner now keeps the tool name when the matched rtk lint prefix is biome, so biome check src/ becomes rtk lint biome check src/ (also npx biome ..., pnpm biome ..., pnpm exec biome ..., npm run biome ...). eslint is still stripped since it's the default. It's 8 added lines reusing tool_portion.
  • tests/biome_hook_roundtrip_test.rs is the end-to-end test you suggested: rewrite, then run exactly that command against a stand-in biome printing a real 1110-byte report, asserting the grouped summary, more than 2x compression and that biome's exit code 1 survives. It fails on develop with your exact finding.
  • The red clippy job was mine: lazy_static isn't a dependency. The three regexes are LazyLock statics now.
  • Rebased on current develop, and the description no longer claims a rewrite that didn't happen.

One question, since you offered to help wire this up: #2378 routes direct biome invocations to a separate rtk biome command instead of rtk lint biome. Which destination do you want? This PR follows what rtk lint already does for the other linters.

Also, this and #2860 both edit the test_rewrite_lint list in registry.rs; the conflict is confined to that one test.

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.

rtk hook claude rewrites Biome commands to ESLint and can mask execution failures

3 participants