fix(organizeImport): ignore side effect imports (#2563)

CHANGELOG.md

@@ -9,8 +9,49 @@ New entries must be placed in a section entitled `Unreleased`.
 Read
 our [guidelines for writing a good changelog entry](https://github.com/biomejs/biome/blob/main/CONTRIBUTING.md#changelog).
 
-## 1.7.1 (2024-04-22)
+## Unreleased
+
+### Analyzer
+
+#### Bug fixes
+
+- Import sorting now ignores side effect imports ([#817](https://github.com/biomejs/biome/issues/817)).
+
+  A side effect import consists now in its own group.
+  This ensures that side effect imports are not reordered.
+
+  Here is an example of how imports are now sorted:
+
+  ```diff
+    import "z"
+    - import { D } from "d";
+    import { C } from "c";
+    + import { D } from "d";
+    import "y"
+    import "x"
+    - import { B } from "b";
+    import { A } from "a";
+    + import { B } from "b";
+    import "w"
+  ```
+
+  Contributed by @Conaclos
+
+### CLI
+
+### Configuration
+
+### Editors
+
+### Formatter
 
+### JavaScript APIs
+
+### Linter
+
+### Parser
+
+## 1.7.1 (2024-04-22)
 
 ### Editors
 

crates/biome_cli/tests/commands/check.rs

@@ -2204,7 +2204,7 @@ fn check_stdin_apply_unsafe_successfully() {
     let mut console = BufferConsole::default();
 
     console.in_buffer.push(
-        "import 'zod'; import 'lodash'; function f() {var x = 1; return{x}} class Foo {}"
+        "import zod from 'zod'; import _ from 'lodash'; function f() {var x = 1; return{x}} class Foo {}"
             .to_string(),
     );
 
@@ -2236,7 +2236,7 @@ fn check_stdin_apply_unsafe_successfully() {
 
     assert_eq!(
         content,
-        "import \"lodash\";\nimport \"zod\";\nfunction f() {\n\tconst x = 1;\n\treturn { x };\n}\nclass Foo {}\n"
+        "import _ from \"lodash\";\nimport zod from \"zod\";\nfunction f() {\n\tconst x = 1;\n\treturn { x };\n}\nclass Foo {}\n"
     );
 
     assert_cli_snapshot(SnapshotPayload::new(
@@ -2253,9 +2253,10 @@ fn check_stdin_apply_unsafe_only_organize_imports() {
     let mut fs = MemoryFileSystem::default();
     let mut console = BufferConsole::default();
 
-    console
-        .in_buffer
-        .push("import 'zod'; import 'lodash'; function f() {return{}} class Foo {}".to_string());
+    console.in_buffer.push(
+        "import zod from 'zod'; import _ from 'lodash'; function f() {return{}} class Foo {}"
+            .to_string(),
+    );
 
     let result = run_cli(
         DynRef::Borrowed(&mut fs),
@@ -2287,7 +2288,7 @@ fn check_stdin_apply_unsafe_only_organize_imports() {
 
     assert_eq!(
         content,
-        "import 'lodash'; import 'zod'; function f() {return{}} class Foo {}"
+        "import _ from 'lodash'; import zod from 'zod'; function f() {return{}} class Foo {}"
     );
 
     assert_cli_snapshot(SnapshotPayload::new(

crates/biome_cli/tests/snapshots/main_commands_check/check_stdin_apply_unsafe_only_organize_imports.snap

@@ -5,13 +5,11 @@ expression: content
 # Input messages
 
 ```block
-import 'zod'; import 'lodash'; function f() {return{}} class Foo {}
+import zod from 'zod'; import _ from 'lodash'; function f() {return{}} class Foo {}
 ```
 
 # Emitted Messages
 
 ```block
-import 'lodash'; import 'zod'; function f() {return{}} class Foo {}
+import _ from 'lodash'; import zod from 'zod'; function f() {return{}} class Foo {}
 ```
-
-

crates/biome_cli/tests/snapshots/main_commands_check/check_stdin_apply_unsafe_successfully.snap

@@ -5,20 +5,18 @@ expression: content
 # Input messages
 
 ```block
-import 'zod'; import 'lodash'; function f() {var x = 1; return{x}} class Foo {}
+import zod from 'zod'; import _ from 'lodash'; function f() {var x = 1; return{x}} class Foo {}
 ```
 
 # Emitted Messages
 
 ```block
-import "lodash";
-import "zod";
+import _ from "lodash";
+import zod from "zod";
 function f() {
 	const x = 1;
 	return { x };
 }
 class Foo {}
 
 ```
-
-

crates/biome_js_analyze/src/assists/correctness/organize_imports.rs

@@ -73,6 +73,30 @@ impl Rule for OrganizeImports {
                 continue;
             };
 
+            let is_side_effect_import = matches!(
+                import.import_clause(),
+                Ok(AnyJsImportClause::JsImportBareClause(_))
+            );
+            if is_side_effect_import {
+                if let Some(first_node) = first_node.take() {
+                    groups.push(ImportGroup {
+                        first_node,
+                        nodes: take(&mut nodes),
+                    });
+                }
+                // A side effect import creates its own import group
+                let mut nodes = BTreeMap::new();
+                nodes.insert(
+                    ImportKey(import.source_text().ok()?),
+                    vec![ImportNode::from(import.clone())],
+                );
+                groups.push(ImportGroup {
+                    first_node: import.clone(),
+                    nodes,
+                });
+                continue;
+            }
+
             // If this is not the first import in the group, check for a group break
             if has_empty_line(&import.import_token().ok()?.leading_trivia()) {
                 if let Some(first_node) = first_node.take() {

crates/biome_js_analyze/tests/specs/correctness/organizeImports/side-effect-imports.js

@@ -0,0 +1,8 @@
+import "z"
+import { D } from "d";
+import { C } from "c";
+import "y"
+import "x"
+import { B } from "b";
+import { A } from "a";
+import "w"

crates/biome_js_analyze/tests/specs/correctness/organizeImports/side-effect-imports.js.snap

@@ -0,0 +1,32 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: side-effect-imports.js
+---
+# Input
+```jsx
+import "z"
+import { D } from "d";
+import { C } from "c";
+import "y"
+import "x"
+import { B } from "b";
+import { A } from "a";
+import "w"
+```
+
+# Actions
+```diff
+@@ -1,8 +1,8 @@
+ import "z"
++import { C } from "c";
+ import { D } from "d";
+-import { C } from "c";
+ import "y"
+ import "x"
++import { A } from "a";
+ import { B } from "b";
+-import { A } from "a";
+ import "w"
+\ No newline at end of file
+
+```