Fix map_kind for generated rules so renamed kinds keep deps/data - #67
Conversation
Using `# gazelle:map_kind` to rename a kind this extension generates (e.g.
`map_kind jest_test vitest_test //build:vitest_test.bzl`) produced a rule with
no deps, no data, and no load() statement, breaking dependency management.
Self-maps (X X //file) worked, which is why this went unnoticed.
Root cause: the extension pre-applied map_kind itself at generation time via a
getKind() helper, writing the mapped kind to disk directly. That bypassed
gazelle core's rename handling:
- Core never saw a builtin rule to record/remember, so it never injected the
load() for the mapped kind.
- Core's inverseMapKindResolver resets a renamed rule back to its builtin kind
before calling Imports/Resolve, but resolve.go compared r.Kind() against the
mapped name (getKind), so the comparison failed and the dep/data block was
skipped.
Let gazelle core own map_kind instead:
- generate.go: emit builtin kinds at rule creation; add unmapKind() (the
inverse of KindMap) and use it only where the code inspects on-disk kinds
(readExistingRules, pruneManagedRules, the collect-all collision check) so a
rule already renamed on disk is still recognized, merged, and pruned across
re-runs.
- resolve.go: compare r.Kind() against builtin literals (consistent with the
existing ts_project checks).
Add a golden fixture (tests/map_kind_test) covering both a fresh rename and a
re-run over an existing renamed rule. Document map_kind on the test rule in the
README.
Backward-compatible: with no map_kind (or a self-map), KindMap is empty/identity
and output is byte-identical, so the existing golden suite is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
deb3840 to
70257fa
Compare
|
Hi @ewianda, would you mind taking a look at this? |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the JS Gazelle extension so that # gazelle:map_kind can rename generated rule kinds (notably jest_test → vitest_test) without losing automatic deps/data population or required load() statements, and adds regression coverage to prevent churn on reruns.
Changes:
- Generate builtin rule kinds (e.g.
jest_test,js_library) and rely on Gazelle’smap_kindmachinery to rename them consistently. - Add
unmapKindand use it when interpreting on-disk rules so previously-renamed kinds are still recognized as managed. - Add a new
tests/map_kind_testfixture to validate correct outputs and rerun behavior; document the recommendedmap_kindusage inREADME.md.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| gazelle/generate.go | Adds unmapKind and uses it while pruning/reading managed rules so mapped kinds remain managed. |
| gazelle/resolve.go | Updates kind comparisons after shifting generation to builtin kinds (needs follow-up fixes to preserve map_kind behavior). |
| README.md | Documents using map_kind to emit vitest_test while retaining automatic dependency management. |
| tests/BUILD | Registers the new map_kind_test test case. |
| tests/map_kind_test/** | Adds fixtures to validate correct map_kind outputs and stability on reruns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
unmapKind reversed only a single map_kind step, so a chained mapping (A->B->C) left an on-disk kind resolving to the intermediate kind rather than the builtin, which would break managed-rule detection for chained maps. Follow the chain back to the builtin, mirroring gazelle's forward resolution, with a visited-set guard against cycles and self-maps. Add a unit test covering single/chained/self-map/unmapped/cyclic cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Hi @ewianda I don't think the macos 6.5.0 CI test failure is related to this change, is there anything else to address to get this PR moving along? |
Yes, it is not, will remove the check so we can unblock and merge. Thanks for your contribution |
I'm adding Vitest into my Bazel project, but gazelle hardcodes the generated test target to
jest_testwhich is kinda misleading since we don't use Jest. Triedmap_kind# gazelle:map_kind jest_test vitest_test //build:vitest_test.bzlbut the renamed rule comes out with no
deps, nodata, and noload(). Getting by with a self-map (# gazelle:map_kind jest_test jest_test //build:vitest_test.bzlandjest_test = vitest_testinbuild/vitest_test.bzl) for now, but would like a cleaner fix that doesn't leave the BUILD files readingjest_test.This change has the plugin emit builtin kinds and lets gazelle do the renaming, and adds an
unmapKindhelper for the few spots that read existing on-disk rules so a rule already renamed on a previous run is still recognized and not churned