feat(lint): add nursery rule useImportsFirst#9272
feat(lint): add nursery rule useImportsFirst#9272terror wants to merge 1 commit intobiomejs:mainfrom
useImportsFirst#9272Conversation
|
github-actions
bot
added
A-Project
WalkthroughA new linting rule Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.js (1)
1-5: Optional: add a directive-focused valid fixture.Given the rule docs explicitly allow directives, a tiny
valid_with_directive.js(e.g."use strict";then imports) would lock that behaviour in and guard regressions.Based on learnings: Add tests for all code changes - lint rules need snapshot tests in
tests/specs/{group}/{rule}/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.js` around lines 1 - 5, Add a new valid fixture file (e.g., valid_with_directive.js) under tests/specs/nursery/useImportsFirst/ that begins with a directive like "use strict"; followed by the same import lines (import { foo } from "foo"; import { bar } from "bar"; import { baz } from "baz"; const qux = 1;) and the same top comment ("should not generate diagnostics") so the test suite captures that directives are allowed by the useImportsFirst rule; ensure the file follows the existing snapshot test pattern used in tests/specs/{group}/{rule}/ so it is picked up by the rule tests.
🤖 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_rule_options/src/use_imports_first.rs`:
- Line 6: Add a rustdoc comment above the public struct UseImportsFirstOptions
explaining what this options type configures (options for the
"use-imports-first" rule/assist), what each field would control (if currently
empty, state that it has no configurable fields yet and is reserved for future
settings), and include example usage or default behavior; ensure the doc uses
standard /// rustdoc format placed immediately before the declaration of
UseImportsFirstOptions.
---
Nitpick comments:
In `@crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.js`:
- Around line 1-5: Add a new valid fixture file (e.g., valid_with_directive.js)
under tests/specs/nursery/useImportsFirst/ that begins with a directive like
"use strict"; followed by the same import lines (import { foo } from "foo";
import { bar } from "bar"; import { baz } from "baz"; const qux = 1;) and the
same top comment ("should not generate diagnostics") so the test suite captures
that directives are allowed by the useImportsFirst rule; ensure the file follows
the existing snapshot test pattern used in tests/specs/{group}/{rule}/ so it is
picked up by the rule tests.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/invalid_mixed.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid_with_export.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
crates/biome_js_analyze/src/lint/nursery/use_imports_first.rscrates/biome_js_analyze/tests/specs/nursery/useImportsFirst/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useImportsFirst/invalid_mixed.jscrates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.jscrates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid_with_export.jscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_imports_first.rs
| #[derive(Default, Clone, Debug, Deserialize, Deserializable, Merge, Eq, PartialEq, Serialize)] | ||
| #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields, default)] | ||
| pub struct UseImportsFirstOptions {} |
There was a problem hiding this comment.
Please add rustdoc for the public options type.
Line 6 introduces UseImportsFirstOptions without inline docs; a brief rustdoc block keeps generated docs clear for users.
Suggested patch
+/// Options for the `useImportsFirst` rule.
+///
+/// This rule currently has no configurable options.
pub struct UseImportsFirstOptions {}As per coding guidelines: **/*.rs: Use inline rustdoc documentation for rules, assists, and their options.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub struct UseImportsFirstOptions {} | |
| /// Options for the `useImportsFirst` rule. | |
| /// | |
| /// This rule currently has no configurable options. | |
| pub struct UseImportsFirstOptions {} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_rule_options/src/use_imports_first.rs` at line 6, Add a rustdoc
comment above the public struct UseImportsFirstOptions explaining what this
options type configures (options for the "use-imports-first" rule/assist), what
each field would control (if currently empty, state that it has no configurable
fields yet and is reserved for future settings), and include example usage or
default behavior; ensure the doc uses standard /// rustdoc format placed
immediately before the declaration of UseImportsFirstOptions.
1 participant
Resolves #9265
This diff implements the
useImportsFirstlint rule, which enforces that all import statements appear before any non-import statements in a module. This corresponds to the ESLint import/first rule (see #9265). The rule iterates over the module item list and flags anyJsImportthat appears after a non-import item.