fix(codegen): remove duplicate preamble in generated analyzer files#8993
fix(codegen): remove duplicate preamble in generated analyzer files#8993ematipico merged 5 commits intobiomejs:mainfrom
Conversation
The codegen for analyzer crates was calling `reformat()` twice on the
same content — once to format the `quote!{}` output, then again before
writing. Since `reformat()` always prepends the generated file preamble,
this produced a duplicate `//! Generated file` header in 41 files.
Remove the redundant second `reformat()` call from `generate_category`,
`generate_group`, `update_json_registry_builder`, and
`update_css_registry_builder`, making them consistent with the other
registry builder functions that already used a single call. Also fix 4
orphaned files (options.rs/assists.rs) that had the same duplicate from
a since-removed `generate_options` function.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The codegen for analyzer crates was calling `reformat()` twice on the
same content — once to format the `quote!{}` output, then again before
writing. Since `reformat()` always prepends the generated file preamble,
this produced a duplicate `//! Generated file` header in 41 files.
Remove the redundant second `reformat()` call from `generate_category`,
`generate_group`, `update_json_registry_builder`, and
`update_css_registry_builder`, making them consistent with the other
registry builder functions that already used a single call. Also fix 4
orphaned files (options.rs/assists.rs) that had the same duplicate from
a since-removed `generate_options` function.
Add an assertion in `prepend_generated_preamble()` to prevent regression
— it now panics if content already contains the preamble.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace five near-identical `generate_*_analyzer` functions with a shared `generate_analyzer_crate` helper that takes the crate name, category list, and registry update function. Also remove dead `_category_comment` code and a pointless `let key = name` alias. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
WalkthroughRemoved duplicated top-of-file "Generated file, do not edit by hand, see Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dyc3
left a comment
There was a problem hiding this comment.
I don't mind the premise behind this change, but the actual diff on the codegen side is a bit large for what I would expect for this fix.
Good point! The actual fix is:
Re. the codegen side. The bug existed because of the duplication. Five near-identical functions, and the reformat() pattern was applied inconsistently across them, some had double calls, some didn't. The copy-paste made the inconsistency invisible. Deduplicating into a single function means the pattern is defined once, so it can't drift between copies again. This is in the third commit, I can remove it if you would prefer to keep the diff smaller. |
Verify that prepend_generated_preamble adds exactly one preamble and panics when called on content that already contains one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ematipico
left a comment
There was a problem hiding this comment.
If the PR description contained the contents of your second comment, we wouldn't have needed to understand why "so many changes". The description emitted by the AI is full of a slop, and misses the very reason of the bug.
Food for thought.
Merging this PR will not alter performance
Comparing Footnotes
|
Inline generate_categories into generate_analyzer_crate to avoid the complex return type that triggered clippy::type_complexity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
xtask/codegen/src/generate_analyzer.rs (1)
26-28:⚠️ Potential issue | 🟡 MinorPre-existing nit:
#crate_namein doc comment renders literally.Line 27's
//! Build script for#crate_name.won't interpolate —quote!doesn't substitute inside doc comments. The generatedbuild.rsfiles will literally say#crate_name. Sincegenerate_build_scriptdoesn't even receive a crate name parameter, this was presumably always the case. Not introduced by this PR, but worth a quick fix while you're in the neighbourhood.
🧹 Nitpick comments (1)
xtask/codegen/src/generate_analyzer.rs (1)
246-339: Optional: the fiveupdate_*_registry_builderfunctions are still near-identical.These differ only in the crate path and the
Languagetype. Since the language type is a generic parameter inRegistryVisitor<L>, you could potentially collapse these into a single function that takes the crate name and aquote!-generated language token. Not urgent — the current state is perfectly readable and the PR already did the heavy lifting on dedup. Just flagging for a potential follow-up.
@ematipico Thank you and you're right. I updated the description. I'll be more careful going forward. |
|
@mldangelo just FYI, the command you want to use for linting is |
Summary
The analyzer codegen had five near-identical
generate_*_analyzerfunctions, and thereformat()pattern was applied inconsistently across them — four called it twice (producing a duplicate//! Generated filepreamble), one called it once correctly. The copy-paste duplication made the inconsistency invisible.This PR removes the redundant
reformat()calls, deduplicates the five functions into a singlegenerate_analyzer_cratehelper so the pattern is defined once, adds an assertion inprepend_generated_preamble()to catch any future double-call, and fixes 4 orphaned files that had the same duplicate from the since-removedgenerate_optionsfunction (#5543).Test plan
cargo checkandcargo clippypasscargo test -p xtask_glue)AI assistance disclosure
This PR was written primarily by Claude Code.