-
-
Notifications
You must be signed in to change notification settings - Fork 893
feat(organizeImports): add sortBareImports option
#9384
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
Changes from 8 commits
cadf73d
91f1118
a94d874
ab9f0bb
b9060b0
7316afb
7b8e6a0
2e5bc87
f03d531
1210dff
78ac541
fa981cf
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,17 @@ | ||
| --- | ||
| "@biomejs/biome": minor | ||
| --- | ||
|
|
||
| Added the `sortBareImports` option to [`organizeImports`](https://biomejs.dev/assist/actions/organize-imports/), | ||
| which allows bare imports to be sorted within other imports when set to `true`. | ||
|
|
||
| ```diff | ||
| /* `sortBareImports` set to `true */ | ||
| - import "b"; | ||
| import "a"; | ||
| + import "b"; | ||
| import { A } from "a"; | ||
| + import "./file"; | ||
| import { Local } from "./file"; | ||
| - import "./file"; | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,8 @@ | ||
| pub mod import_key; | ||
| pub mod specifiers_attributes; | ||
| mod util; | ||
|
|
||
| use crate::{JsRuleAction, assist::source::organize_imports::import_key::ImportStatementKind}; | ||
| use biome_analyze::{ | ||
| ActionCategory, Ast, FixKind, Rule, RuleDiagnostic, RuleSource, SourceActionKind, | ||
| context::RuleContext, declare_source_rule, | ||
|
|
@@ -17,20 +22,15 @@ use specifiers_attributes::{ | |
| are_import_attributes_sorted, merge_export_specifiers, merge_import_specifiers, | ||
| sort_attributes, sort_export_specifiers, sort_import_specifiers, | ||
| }; | ||
|
|
||
| use crate::JsRuleAction; | ||
| use util::{attached_trivia, detached_trivia, has_detached_leading_comment, leading_newlines}; | ||
|
|
||
| pub mod import_key; | ||
| pub mod specifiers_attributes; | ||
| mod util; | ||
|
|
||
| declare_source_rule! { | ||
| /// Provides a code action to sort the imports and exports in the file using a built-in or custom order. | ||
| /// | ||
| /// Imports and exports are first separated into chunks, before being sorted. | ||
| /// Imports or exports of a chunk are then grouped according to the user-defined groups. | ||
| /// Within a group, imports are sorted using a built-in order that depends on the import/export kind, whether the import/export has attributes and the source being imported from. | ||
| /// Within a group, imports are sorted using a built-in order that depends on the import/export kind, | ||
| /// whether the import/export has attributes and the source being imported from. | ||
| /// **source** is also often called **specifier** in the JavaScript ecosystem. | ||
| /// | ||
| /// ```js,ignore | ||
|
|
@@ -62,8 +62,9 @@ declare_source_rule! { | |
| /// export { F } from "f"; | ||
| /// ``` | ||
| /// | ||
| /// Chunks also end as soon as a statement or a **side-effect import** (also called _bare import_) is encountered. | ||
| /// Every side-effect import forms an independent chunk. | ||
| /// By default, chunks also end as soon as a statement or a **bare import** (also called _side-effect import_) is encountered. | ||
| /// Bare imports can be sorted with other imports by setting the `sortBareImports` option to `true`. | ||
| /// when `sortBareImports` is unset or `false`, every bare import forms an independent chunk. | ||
Conaclos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// The following example contains six chunks: | ||
| /// | ||
| /// ```js,ignore | ||
|
|
@@ -84,9 +85,9 @@ declare_source_rule! { | |
| /// export { F } from "f"; | ||
| /// ``` | ||
| /// | ||
| /// 1. The first chunk contains the two first `import` and ends with the appearance of the first side-effect import `import "x"`. | ||
| /// 2. The second chunk contains only the side-effect import `import "x"`. | ||
| /// 3. The third chunk contains only the side-effect import `import "y"`. | ||
| /// 1. The first chunk contains the two first `import` and ends with the appearance of the first bare import `import "x"`. | ||
| /// 2. The second chunk contains only the bare import `import "x"`. | ||
| /// 3. The third chunk contains only the bare import `import "y"`. | ||
| /// 4. The fourth chunk contains a single `import`; The first `export` ends it. | ||
| /// 5. The fifth chunk contains the first `export`; The function declaration ends it. | ||
| /// 6. The sixth chunk contains the last two `export`. | ||
|
|
@@ -111,8 +112,10 @@ declare_source_rule! { | |
| /// The line `import { C } from "c"` forms the second chunk. | ||
| /// The blank line between the first two imports is ignored so they form a single chunk. | ||
| /// | ||
| /// The sorter ensures that chunks are separated from each other with blank lines. | ||
| /// Only side-effect imports adjacent to a chunk of imports are not separated by a blank line. | ||
| /// The sorter ensures that a chunk of imports and a chunk of exports | ||
| /// are separated from each other with blank lines. | ||
| /// Also, it ensures that a chunk that starts with a detached comment | ||
| /// is separated from the previous chunk with a blank line. | ||
| /// The following code... | ||
| /// | ||
| /// ```js,ignore | ||
|
|
@@ -198,16 +201,18 @@ declare_source_rule! { | |
| /// import sibling from "./file.js"; | ||
| /// ``` | ||
| /// | ||
| /// If two imports or exports share the same source and are in the same chunk, then they are ordered according to their kind as follows: | ||
| /// If two imports or exports share the same source and are in the same chunk, | ||
| /// then they are ordered according to their kind as follows: | ||
| /// | ||
| /// 1. Namespace type import / Namespace type export | ||
| /// 2. Default type import | ||
| /// 3. Named type import / Named type export | ||
| /// 4. Namespace import / Namespace export | ||
| /// 5. Combined default and namespace import | ||
| /// 6. Default import | ||
| /// 7. Combined default and named import | ||
| /// 8. Named import / Named export | ||
| /// 1. Bare imports | ||
| /// 2. Namespace type import / Namespace type export | ||
| /// 3. Default type import | ||
| /// 4. Named type import / Named type export | ||
| /// 5. Namespace import / Namespace export | ||
| /// 6. Combined default and namespace import | ||
| /// 7. Default import | ||
| /// 8. Combined default and named import | ||
| /// 9. Named import / Named export | ||
| /// | ||
| /// Imports and exports with attributes are always placed first. | ||
| /// For example, the following code... | ||
|
|
@@ -570,6 +575,8 @@ declare_source_rule! { | |
| /// import D2, { A, B } from "package"; | ||
| /// ``` | ||
| /// | ||
| /// Also, note that bare imports are never merged with other imports | ||
| /// even if you set `sortBareImports` to `true`. | ||
| /// | ||
| /// ## Named imports, named exports and attributes sorting | ||
| /// | ||
|
|
@@ -661,8 +668,11 @@ declare_source_rule! { | |
| /// } | ||
| /// ``` | ||
| /// | ||
| /// ## Change the sorting of import identifiers to lexicographic sorting | ||
| /// This only applies to the named import/exports and not the source itself. | ||
| /// ## Change the sorting of import and export identifiers | ||
| /// | ||
| /// By default, attributes, imported and exported names are sorted with a `natural` sort. | ||
| /// Yo ucan opt for a `lexicographic` sort (sometimes referred as _binary_ sort) by | ||
Conaclos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// setting the `identifierOrder` option. | ||
| /// | ||
| /// ```json,options | ||
| /// { | ||
|
|
@@ -671,31 +681,43 @@ declare_source_rule! { | |
| /// } | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// ```js,use_options,expect_diagnostic | ||
| /// import { var1, var2, var21, var11, var12, var22 } from 'my-package' | ||
| /// import { var1, var2, var21, var11, var12, var22 } from "my-package"; | ||
| /// ``` | ||
| /// | ||
| /// ## Change the sorting of import identifiers to logical sorting | ||
| /// This is the default behavior in case you do not override. This only applies to the named import/exports and not the source itself. | ||
| /// Note that this order doesn't change how import and export sources are sorted. | ||
| /// | ||
| /// ## Sorting bare imports | ||
| /// | ||
| /// By default, bare imports are not sorted within other imports. | ||
| /// Setting `sortBareImports` to `true`, allow sorting them with other imports. | ||
| /// Note that this can lead to issues because bare imports often signal the presence of side-effects. | ||
Conaclos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// Thus changing their order can change the behavior of your code. | ||
| /// | ||
| /// ```json,options | ||
| /// { | ||
| /// "options": { | ||
| /// "identifierOrder": "natural" | ||
| /// "sortBareImports": true | ||
| /// } | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// ```js,use_options,expect_diagnostic | ||
| /// import { var1, var2, var21, var11, var12, var22 } from 'my-package' | ||
| /// import "./file"; | ||
| /// import { A } from "my-package"; | ||
| /// ``` | ||
| /// | ||
| pub OrganizeImports { | ||
| version: "1.0.0", | ||
| name: "organizeImports", | ||
| language: "js", | ||
| recommended: true, | ||
| fix_kind: FixKind::Safe, | ||
| sources: &[RuleSource::Eslint("sort-imports").inspired(), RuleSource::Eslint("no-duplicate-imports").inspired(), RuleSource::EslintImport("order").inspired()], | ||
| sources: &[ | ||
| RuleSource::Eslint("sort-imports").inspired(), | ||
| RuleSource::Eslint("no-duplicate-imports").inspired(), | ||
| RuleSource::EslintImport("order").inspired() | ||
| ], | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -757,18 +779,29 @@ impl Rule for OrganizeImports { | |
| let options = ctx.options(); | ||
| let groups = options.groups.as_ref(); | ||
| let sort_order = options.identifier_order.unwrap_or_default(); | ||
| let sort_bare_imports = options.sort_bare_imports.unwrap_or_default(); | ||
| let mut chunk: Option<ChunkBuilder> = None; | ||
| let mut prev_kind: Option<JsSyntaxKind> = None; | ||
| let mut prev_is_bare_import = false; | ||
|
Member
Author
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. I am not fond of this new variable. However, the alternative approach I have in mind is not necessarily better: The alternative implementation is to modify Thus, I decided to keep the current approach. |
||
| let mut prev_group = 0; | ||
| for item in root.items() { | ||
| let current_kind = item.syntax().kind(); | ||
| if let Some((info, specifiers, attributes)) = ImportInfo::from_module_item(&item) { | ||
| let prev_is_distinct = prev_kind.is_some_and(|kind| kind != item.syntax().kind()); | ||
| // A detached comment marks the start of a new chunk | ||
| if prev_is_distinct || has_detached_leading_comment(item.syntax()) { | ||
| let prev_is_distinct = prev_kind.is_some_and(|kind| kind != current_kind); | ||
| // switching of kind (import/statement/export) marks the start of a new chunk. | ||
| if prev_is_distinct | ||
| // A detached comment marks the start of a new chunk | ||
| || has_detached_leading_comment(item.syntax()) | ||
| // bare imports marks the start of a new chunk if they are ignored in the sort. | ||
| || (!sort_bare_imports && ( | ||
| prev_is_bare_import || info.kind == ImportStatementKind::Bare | ||
| )) | ||
| { | ||
| // The chunk ends, here | ||
| report_unsorted_chunk(chunk.take(), &mut result); | ||
| prev_group = 0; | ||
| } | ||
| prev_is_bare_import = info.kind == ImportStatementKind::Bare; | ||
| let key = ImportKey::new(info, groups); | ||
| let blank_line_separated_groups = groups | ||
| .is_some_and(|groups| groups.separated_by_blank_line(prev_group, key.group)); | ||
|
|
@@ -782,7 +815,7 @@ impl Rule for OrganizeImports { | |
| }); | ||
| let newline_issue = if leading_newline_count == 1 | ||
| // A chunk must start with a blank line (two newlines) | ||
| // if an export or a statement precedes it. | ||
| // if a distinct kind (import/statement/export) precedes it. | ||
| && ((starts_chunk && prev_is_distinct) || | ||
| // Some groups must be separated by a blank line | ||
| blank_line_separated_groups) | ||
|
|
@@ -826,12 +859,8 @@ impl Rule for OrganizeImports { | |
| chunk = Some(ChunkBuilder::new(key)); | ||
| } | ||
| } else if chunk.is_some() { | ||
| // This is either | ||
| // - a bare (side-effect) import | ||
| // - a buggy import or export | ||
| // a statement | ||
| // | ||
| // In any case, the chunk ends here | ||
| // This is either a buggy import/export or a statement. | ||
| // So, the chunk ends here. | ||
| report_unsorted_chunk(chunk.take(), &mut result); | ||
| prev_group = 0; | ||
| // A statement must be separated of a chunk with a blank line | ||
|
|
@@ -842,8 +871,9 @@ impl Rule for OrganizeImports { | |
| slot_index: statement.syntax().index() as u32, | ||
| }); | ||
| } | ||
| prev_is_bare_import = false; | ||
| } | ||
| prev_kind = Some(item.syntax().kind()); | ||
| prev_kind = Some(current_kind); | ||
| } | ||
| // Report the last chunk | ||
| report_unsorted_chunk(chunk.take(), &mut result); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| /* should not generate diagnostics */ | ||
|
Member
Author
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. This is a non-regression test: While I was implementing the feature I introduced a bug that was not covered by the test suite. This test covers it. |
||
|
|
||
| import "b"; | ||
| import { A } from "a"; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: chunk-because-of-bare-imports.js | ||
| --- | ||
| # Input | ||
| ```js | ||
| /* should not generate diagnostics */ | ||
|
|
||
| import "b"; | ||
| import { A } from "a"; | ||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| import * as path from "node:path"; | ||
| import "node:path"; | ||
| import "./file.js"; | ||
| import { A } from "./file.js"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: custom-order-with-bare-imports.js | ||
| --- | ||
| # Input | ||
| ```js | ||
| import * as path from "node:path"; | ||
| import "node:path"; | ||
| import "./file.js"; | ||
| import { A } from "./file.js"; | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| custom-order-with-bare-imports.js:1:1 assist/source/organizeImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i The imports and exports are not sorted. | ||
|
|
||
| > 1 │ import * as path from "node:path"; | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 2 │ import "node:path"; | ||
| 3 │ import "./file.js"; | ||
|
|
||
| i Safe fix: Organize Imports (Biome) | ||
|
|
||
| 1 │ - import·*·as·path·from·"node:path"; | ||
| 2 │ - import·"node:path"; | ||
| 3 │ - import·"./file.js"; | ||
| 4 │ - import·{·A·}·from·"./file.js"; | ||
| 1 │ + import·"./file.js"; | ||
| 2 │ + import·{·A·}·from·"./file.js"; | ||
| 3 │ + import·"node:path"; | ||
| 4 │ + import·*·as·path·from·"node:path"; | ||
| 5 5 │ | ||
|
|
||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| { | ||
| "$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json", | ||
| "assist": { | ||
| "actions": { | ||
| "source": { | ||
| "organizeImports": { | ||
| "level": "on", | ||
| "options": { | ||
| "groups": [ | ||
| ":PATH:", | ||
| ":NODE:" | ||
| ], | ||
| "sortBareImports": true | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| import "mod" with { att2: "", att1: "" }; |
Uh oh!
There was an error while loading. Please reload this page.