Conversation
🦋 Changeset detectedLatest commit: 6fe9e9b The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will not alter performance
Comparing Footnotes
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new nursery JavaScript lint rule Possibly related PRs
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)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/biome_rule_options/src/use_vars_on_top.rs (1)
3-6: Avoid shipping an empty options type.This rule has no fields, so a dedicated
{}options struct just adds schema/codegen surface for no extra behaviour. I’d switch the rule totype Options = ()and drop this module until there’s an actual setting.As per coding guidelines,
Use type Options = () if a rule does not have additional configuration options.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_rule_options/src/use_vars_on_top.rs` around lines 3 - 6, The file defines an empty options struct UseVarsOnTopOptions which should be replaced with a unit-type alias; change the exported options symbol to `type Options = ();` and remove the empty struct and its derives (serde/schemars/Merge/etc.), and update any references or re-exports that mentioned UseVarsOnTopOptions to use the Options alias instead (or remove the module entirely if unused) so no empty `{}` options type is shipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/fresh-coins-do.md:
- Line 7: Update the lead-in sentence "For example, the following code now
triggers the rule:" in .changeset/fresh-coins-do.md to end with a period instead
of a colon (i.e., change the trailing ":" to "."), ensuring the sentence follows
the guideline "End every sentence in changeset descriptions with a full stop
(period)."
In `@crates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rs`:
- Around line 171-217: The leading-scan helpers is_top_statement_in_list and
is_top_module_item_in_list currently only treat JsVariableStatement with
declaration().is_var() as allowed, which is stricter than ESLint's vars-on-top
semantics; update both helpers to (1) treat any variable declaration as allowed
by accepting declaration().is_ok() (so let/const/var are permitted), (2) also
allow directive prologue statements and imports at module scope (e.g., recognize
string-literal expression statements as directives and keep allowing
AnyJsModuleItem::JsImport(_)), and (3) add unit/fixture tests demonstrating
examples like `let x; var y;` and leading directives so the rule matches ESLint
behavior; adjust references in the code to the functions
is_top_statement_in_list, is_top_module_item_in_list,
AnyJsStatement/AnyJsModuleItem, and JsVariableStatement when making these
changes.
---
Nitpick comments:
In `@crates/biome_rule_options/src/use_vars_on_top.rs`:
- Around line 3-6: The file defines an empty options struct UseVarsOnTopOptions
which should be replaced with a unit-type alias; change the exported options
symbol to `type Options = ();` and remove the empty struct and its derives
(serde/schemars/Merge/etc.), and update any references or re-exports that
mentioned UseVarsOnTopOptions to use the Options alias instead (or remove the
module entirely if unused) so no empty `{}` options type is shipped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aef591b6-e8fa-4f08-ab46-05d54b5f99a4
⛔ Files ignored due to path filters (8)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
.changeset/fresh-coins-do.md.claude/skills/lint-rule-development/SKILL.mdcrates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rscrates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.jscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_vars_on_top.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.claude/skills/lint-rule-development/SKILL.md (1)
357-361: Consider simplifying the “Bad” example wording for readability.The embedded
<Emphasis>markup inside quoted prose makes the anti-pattern example a bit noisy. Plain text would keep the point clearer for contributors.Suggested tweak
-3. "At module scope, imports and leading "<Emphasis>"export var"</Emphasis>" declarations may appear before other statements." // Doesn't explain the action, just gives a superfluous detail about module scope. +3. "At module scope, imports and leading export var declarations may appear before other statements." // Doesn't explain the action, just gives a superfluous detail about module scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/lint-rule-development/SKILL.md around lines 357 - 361, The quoted "Bad" example is noisy due to embedded <Emphasis> markup and awkward ordering; simplify it by removing the inline <Emphasis> tags (use plain text "export var"), reorder the three lines so they state the problem first, then the corrective action, then any allowed exceptions, and rephrase the third line to be a concise exception note about module scope (e.g., "At module scope, imports and leading export var declarations may appear before other statements.") to keep the example clear and readable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/skills/lint-rule-development/SKILL.md:
- Around line 357-361: The quoted "Bad" example is noisy due to embedded
<Emphasis> markup and awkward ordering; simplify it by removing the inline
<Emphasis> tags (use plain text "export var"), reorder the three lines so they
state the problem first, then the corrective action, then any allowed
exceptions, and rephrase the third line to be a concise exception note about
module scope (e.g., "At module scope, imports and leading export var
declarations may appear before other statements.") to keep the example clear and
readable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a7122634-aeb5-46c0-9b76-683dd6e780cb
⛔ Files ignored due to path filters (8)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
.changeset/fresh-coins-do.md.claude/skills/lint-rule-development/SKILL.mdcrates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rscrates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.jscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_vars_on_top.rs
✅ Files skipped from review due to trivial changes (4)
- .changeset/fresh-coins-do.md
- crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.js
- crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.js
- crates/biome_rule_options/src/use_vars_on_top.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.claude/skills/lint-rule-development/SKILL.md (1)
357-361: Tiny clarity tweak in the “Bad” exampleThis section works, but the inline note “Good start” inside a block labelled “Bad” is a little mixed-signal. Consider neutral wording so readers don’t momentarily parse it as partially recommended.
Suggested wording tweak
-1. "This var declaration is not at the top of its containing scope." // Good start, explains what the problem is +1. "This var declaration is not at the top of its containing scope." // States the problem, but the overall sequence below is still incorrect🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/lint-rule-development/SKILL.md around lines 357 - 361, Update the "Bad" example block so the inline comments use neutral critique rather than mixed praise; specifically edit the three comment lines that begin with "Good start," "Doesn't explain why," and "Doesn't explain the action" to neutral phrasing that identifies the issue only (e.g., "Explains the problem but not the rationale", "Tells what to do but not why", "Includes unnecessary module-scope detail"). Locate and modify the exact strings shown in the diff (the three quoted comment lines) to use concise, neutral feedback without positive wording inside a section labeled "Bad".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/skills/lint-rule-development/SKILL.md:
- Around line 357-361: Update the "Bad" example block so the inline comments use
neutral critique rather than mixed praise; specifically edit the three comment
lines that begin with "Good start," "Doesn't explain why," and "Doesn't explain
the action" to neutral phrasing that identifies the issue only (e.g., "Explains
the problem but not the rationale", "Tells what to do but not why", "Includes
unnecessary module-scope detail"). Locate and modify the exact strings shown in
the diff (the three quoted comment lines) to use concise, neutral feedback
without positive wording inside a section labeled "Bad".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1e8a0bd-2f64-4dbd-8e59-8b2ca1088c74
⛔ Files ignored due to path filters (8)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
.changeset/fresh-coins-do.md.claude/skills/lint-rule-development/SKILL.mdcrates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rscrates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.jscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_vars_on_top.rs
✅ Files skipped from review due to trivial changes (4)
- .changeset/fresh-coins-do.md
- crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.js
- crates/biome_rule_options/src/use_vars_on_top.rs
- crates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.js
ematipico
approved these changes
Apr 13, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rs`:
- Around line 23-25: Update the doc comments in use_vars_on_top.rs (the
top-level rule comment and the note later) to reflect that the implementation
allows a leading run of variable declarations (not just `var`) and adjust
wording to say "variable declarations (e.g., var/let/const)" or similar; also
when porting the ESLint source, use RuleSource::Eslint("use-vars-on-top").same()
to indicate identical behavior. Ensure you modify the doc block at the top of
use_vars_on_top.rs and the explanatory note (the two places referenced) and
replace any “only `var`” phrasing with the broader, accurate description.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4411b0d7-123d-449b-989d-a8e8fd72fbd5
⛔ Files ignored due to path filters (8)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
.changeset/fresh-coins-do.md.claude/skills/lint-rule-development/SKILL.mdcrates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rscrates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.jscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_vars_on_top.rs
✅ Files skipped from review due to trivial changes (5)
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/invalid.js
- crates/biome_rule_options/src/use_vars_on_top.rs
- crates/biome_js_analyze/tests/specs/nursery/useVarsOnTop/valid.js
- .changeset/fresh-coins-do.md
Comment on lines
+23
to
+25
| /// This rule only allows leading standalone `var` statements. At module | ||
| /// scope, leading `export var` declarations are allowed too. Directives and | ||
| /// imports may appear before them. |
Contributor
There was a problem hiding this comment.
Docs/message drift from actual rule behaviour.
The implementation permits a leading run of variable declarations (not just var), but the doc block and note currently read as “only var before other statements”. Please align wording with behaviour to avoid confusing users.
✏️ Suggested wording update
- /// This rule only allows leading standalone `var` statements. At module
- /// scope, leading `export var` declarations are allowed too. Directives and
- /// imports may appear before them.
+ /// This rule allows a leading run of variable declarations (`var`, `let`,
+ /// `const`). At module scope, leading variable exports are also allowed.
+ /// Directives and imports may appear before that leading run.
...
- "Move this "<Emphasis>"var"</Emphasis>" declaration before other statements in the same scope. At module scope, imports may remain before it."
+ "Move this "<Emphasis>"var"</Emphasis>" declaration before non-declaration statements in the same scope. At module scope, imports may remain before it."Based on learnings: Use RuleSource::Eslint("rule-name").same() when porting an ESLint rule with matching behavior.
Also applies to: 89-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/nursery/use_vars_on_top.rs` around lines 23
- 25, Update the doc comments in use_vars_on_top.rs (the top-level rule comment
and the note later) to reflect that the implementation allows a leading run of
variable declarations (not just `var`) and adjust wording to say "variable
declarations (e.g., var/let/const)" or similar; also when porting the ESLint
source, use RuleSource::Eslint("use-vars-on-top").same() to indicate identical
behavior. Ensure you modify the doc block at the top of use_vars_on_top.rs and
the explanatory note (the two places referenced) and replace any “only `var`”
phrasing with the broader, accurate description.
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
2 participants
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.
Summary
This PR adds
useVarsOnTopwhich is a port of https://eslint.org/docs/latest/rules/vars-on-topThis is a pretty legacy rule, I don't imagine it getting much usage. Still, porting it for completion.
Generated by gpt 5.4
Test Plan
snapshots
Docs