-
-
Notifications
You must be signed in to change notification settings - Fork 879
feat(lint): add nursery rule useImportsFirst
#9272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
c6d9702
e5f5245
e4594c0
bb4718f
2a6ad5c
70dec32
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added the nursery rule [`useImportsFirst`](https://biomejs.dev/linter/rules/use-imports-first/) that enforces all import statements appear before any non-import statements in a module. Inspired by the ESLint [`import/first`](https://github.com/import-js/eslint-plugin-import/blob/HEAD/docs/rules/first.md) rule. | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| use biome_analyze::{ | ||
| Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule, | ||
| }; | ||
| use biome_console::markup; | ||
| use biome_js_syntax::{AnyJsModuleItem, JsModuleItemList}; | ||
| use biome_rowan::{AstNode, AstNodeList, TextRange}; | ||
|
|
||
| declare_lint_rule! { | ||
| /// Enforce that all imports appear at the top of the module. | ||
| /// | ||
| /// Import statements that appear after non-import statements are harder to | ||
| /// find and may indicate disorganized code. Keeping all imports together at | ||
| /// the top makes dependencies immediately visible. | ||
| /// | ||
| /// Note that directives such as `"use strict"` are always allowed before | ||
| /// imports, since they are parsed separately from module items. | ||
| /// | ||
| /// ## Examples | ||
| /// | ||
| /// ### Invalid | ||
| /// | ||
| /// ```js,expect_diagnostic | ||
| /// import { foo } from "foo"; | ||
| /// const bar = 1; | ||
| /// import { baz } from "baz"; | ||
| /// ``` | ||
| /// | ||
| /// ### Valid | ||
| /// | ||
| /// ```js | ||
| /// import { foo } from "foo"; | ||
| /// import { bar } from "bar"; | ||
| /// const baz = 1; | ||
| /// ``` | ||
| /// | ||
| /// ```js | ||
| /// "use strict"; | ||
| /// import { foo } from "foo"; | ||
| /// ``` | ||
| /// | ||
| pub UseImportsFirst { | ||
| version: "next", | ||
| name: "useImportsFirst", | ||
| language: "js", | ||
| recommended: false, | ||
| sources: &[RuleSource::EslintImport("first").same()], | ||
| } | ||
| } | ||
|
|
||
| impl Rule for UseImportsFirst { | ||
| type Query = Ast<JsModuleItemList>; | ||
| type State = TextRange; | ||
| type Signals = Vec<Self::State>; | ||
| type Options = (); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In another PR we are denying Options to be empty, might be good to comply here already |
||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let items = ctx.query(); | ||
| let mut seen_non_import = false; | ||
| let mut signals = Vec::new(); | ||
|
|
||
| for item in items.iter() { | ||
| match item { | ||
| AnyJsModuleItem::JsImport(_) => { | ||
| if seen_non_import { | ||
| signals.push(item.range()); | ||
| } | ||
| } | ||
| _ => { | ||
| seen_non_import = true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| signals | ||
| } | ||
|
|
||
| fn diagnostic(_ctx: &RuleContext<Self>, range: &Self::State) -> Option<RuleDiagnostic> { | ||
| Some( | ||
| RuleDiagnostic::new( | ||
| rule_category!(), | ||
| range, | ||
| markup! { | ||
| "This import should appear at the top of the module." | ||
| }, | ||
| ) | ||
| .note(markup! { | ||
| "Move all import statements before any other statements." | ||
| }), | ||
| ) | ||
terror marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import { foo } from "foo"; | ||
| const bar = 1; | ||
| import { baz } from "baz"; | ||
|
|
||
| import { qux } from "qux"; | ||
terror marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: invalid.js | ||
| --- | ||
| # Input | ||
| ```js | ||
| import { foo } from "foo"; | ||
| const bar = 1; | ||
| import { baz } from "baz"; | ||
| import { qux } from "qux"; | ||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| invalid.js:3:1 lint/nursery/useImportsFirst ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
| i This import should appear at the top of the module. | ||
| 1 │ import { foo } from "foo"; | ||
| 2 │ const bar = 1; | ||
| > 3 │ import { baz } from "baz"; | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 4 │ | ||
| 5 │ import { qux } from "qux"; | ||
| i Move all import statements before any other statements. | ||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
| ``` | ||
|
|
||
| ``` | ||
| invalid.js:5:1 lint/nursery/useImportsFirst ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
| i This import should appear at the top of the module. | ||
| 3 │ import { baz } from "baz"; | ||
| 4 │ | ||
| > 5 │ import { qux } from "qux"; | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 6 │ | ||
| i Move all import statements before any other statements. | ||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import foo from "foo"; | ||
| foo.init(); | ||
| import bar from "bar"; | ||
| export { bar }; | ||
| import baz from "baz"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: invalid_mixed.js | ||
| --- | ||
| # Input | ||
| ```js | ||
| import foo from "foo"; | ||
| foo.init(); | ||
| import bar from "bar"; | ||
| export { bar }; | ||
| import baz from "baz"; | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| invalid_mixed.js:3:1 lint/nursery/useImportsFirst ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i This import should appear at the top of the module. | ||
|
|
||
| 1 │ import foo from "foo"; | ||
| 2 │ foo.init(); | ||
| > 3 │ import bar from "bar"; | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^ | ||
| 4 │ export { bar }; | ||
| 5 │ import baz from "baz"; | ||
|
|
||
| i Move all import statements before any other statements. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid_mixed.js:5:1 lint/nursery/useImportsFirst ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i This import should appear at the top of the module. | ||
|
|
||
| 3 │ import bar from "bar"; | ||
| 4 │ export { bar }; | ||
| > 5 │ import baz from "baz"; | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^ | ||
| 6 │ | ||
|
|
||
| i Move all import statements before any other statements. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| /* should not generate diagnostics */ | ||
| import { foo } from "foo"; | ||
| import { bar } from "bar"; | ||
| import { baz } from "baz"; | ||
| const qux = 1; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: valid.js | ||
| --- | ||
| # Input | ||
| ```js | ||
| /* should not generate diagnostics */ | ||
| import { foo } from "foo"; | ||
| import { bar } from "bar"; | ||
| import { baz } from "baz"; | ||
| const qux = 1; | ||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| /* should not generate diagnostics */ | ||
| import { foo } from "foo"; | ||
| export { foo }; | ||
| const bar = 1; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: valid_with_export.js | ||
| --- | ||
| # Input | ||
| ```js | ||
| /* should not generate diagnostics */ | ||
| import { foo } from "foo"; | ||
| export { foo }; | ||
| const bar = 1; | ||
|
|
||
| ``` |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused, but will be with other comment |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| use biome_deserialize_macros::{Deserializable, Merge}; | ||
| use serde::{Deserialize, Serialize}; | ||
| #[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 {} | ||
dyc3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current kind of suggests the original rule came from Eslint itself