Fix map_kind for generated rules so renamed kinds keep deps/data#67
Open
firestreaker wants to merge 2 commits into
Open
Fix map_kind for generated rules so renamed kinds keep deps/data#67firestreaker wants to merge 2 commits into
firestreaker wants to merge 2 commits into
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>
firestreaker
force-pushed
the
fix-map-kind-generated-rules
branch
from
June 24, 2026 02:39
deb3840 to
70257fa
Compare
Author
|
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.
Comment on lines
114
to
118
| // modules can be resolved via the directory containing them | ||
| if (isBarrel || jsConfig.CollectAll) && r.Kind() != getKind(c, "jest_test") { | ||
| if (isBarrel || jsConfig.CollectAll) && r.Kind() != "jest_test" { | ||
| importSpecs = append(importSpecs, resolve.ImportSpec{ | ||
| Lang: lang.Name(), | ||
| Imp: f.Pkg, |
Author
There was a problem hiding this comment.
Gazelle sets the mapped kind on the rule in the file, but for resolution it wraps the language resolver in an inverseMapKindResolver that resets the rule's kind back to the builtin before calling Imports/Resolve. So in these methods r.Kind() is jest_test, not vitest_test, which is why the literal matches (and why the existing r.Kind() == "ts_project" checks work the same way).
Comment on lines
258
to
262
| // Add in additional jest dependencies | ||
| if r.Kind() == getKind(c, "jest_test") { | ||
| if r.Kind() == "jest_test" { | ||
| // All deps are also data for jest_test rules. | ||
| for name := range depSet { | ||
| dataSet[name] = true |
Author
There was a problem hiding this comment.
See comment on the above for resolve.go
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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