rome · ematipico · Jul 31, 2023 · Jul 14, 2023 · Jul 15, 2023 · Jul 15, 2023
diff --git a/Cargo.toml b/Cargo.toml
@@ -61,10 +61,10 @@ rome_parser                 = { version = "0.0.1", path = "./crates/rome_parser"
 rome_rowan                  = { version = "0.0.1", path = "./crates/rome_rowan" }
 rome_service                = { path = "./crates/rome_service" }
 rome_suppression            = { version = "0.0.1", path = "./crates/rome_suppression" }
+rome_test_utils             = { path = "./crates/rome_test_utils" }
 rome_text_edit              = { version = "0.0.1", path = "./crates/rome_text_edit" }
 rome_text_size              = { version = "0.0.1", path = "./crates/rome_text_size" }
 tests_macros                = { path = "./crates/tests_macros" }
-rome_test_utils                = { path = "./crates/rome_test_utils" }
 
 # Crates needed in the workspace
 bitflags          = "2.3.1"

diff --git a/crates/rome_cli/tests/commands/check.rs b/crates/rome_cli/tests/commands/check.rs
@@ -1493,11 +1493,11 @@ fn applies_organize_imports() {
     fs.insert(file_path.into(), config.as_bytes());
 
     let file_path = Path::new("check.js");
-    let content = r#"import { lorem, foom, bar } from "foo";
-import * as something from "../something";
+    let content = r#"import * as something from "../something";
+import { lorem, foom, bar } from "foo";
 "#;
-    let expected = r#"import * as something from "../something";
-import { bar, foom, lorem } from "foo";
+    let expected = r#"import { bar, foom, lorem } from "foo";
+import * as something from "../something";
 "#;
     fs.insert(file_path.into(), content.as_bytes());
 
@@ -1692,11 +1692,11 @@ fn applies_organize_imports_from_cli() {
     let mut console = BufferConsole::default();
 
     let file_path = Path::new("check.js");
-    let content = r#"import { lorem, foom, bar } from "foo";
-import * as something from "../something";
+    let content = r#"import * as something from "../something";
+import { lorem, foom, bar } from "foo";
 "#;
-    let expected = r#"import * as something from "../something";
-import { bar, foom, lorem } from "foo";
+    let expected = r#"import { bar, foom, lorem } from "foo";
+import * as something from "../something";
 "#;
 
     fs.insert(file_path.into(), content.as_bytes());

diff --git a/crates/rome_cli/tests/snapshots/main_commands_check/applies_organize_imports.snap b/crates/rome_cli/tests/snapshots/main_commands_check/applies_organize_imports.snap
@@ -11,8 +11,8 @@ expression: content
 ## `check.js`
 
 ```js
-import * as something from "../something";
 import { bar, foom, lorem } from "foo";
+import * as something from "../something";
 
 ```
 

diff --git a/crates/rome_cli/tests/snapshots/main_commands_check/applies_organize_imports_from_cli.snap b/crates/rome_cli/tests/snapshots/main_commands_check/applies_organize_imports_from_cli.snap
@@ -5,8 +5,8 @@ expression: content
 ## `check.js`
 
 ```js
-import * as something from "../something";
 import { bar, foom, lorem } from "foo";
+import * as something from "../something";
 
 ```
 

diff --git a/...es/rome_cli/tests/snapshots/main_commands_check/shows_organize_imports_diff_on_check.snap b/...es/rome_cli/tests/snapshots/main_commands_check/shows_organize_imports_diff_on_check.snap
@@ -35,9 +35,8 @@ check.js organizeImports ━━━━━━━━━━━━━━━━━━
   i Import statements could be sorted:
 
     1   │ - import·{·lorem,·foom,·bar·}·from·"foo";
-    2   │ - import·*·as·something·from·"../something";
-      1 │ + import·*·as·something·from·"../something";
-      2 │ + import·{·bar,·foom,·lorem·}·from·"foo";
+      1 │ + import·{·bar,·foom,·lorem·}·from·"foo";
+    2 2 │   import * as something from "../something";
     3 3 │   
 
 

diff --git a/...e_cli/tests/snapshots/main_commands_check/shows_organize_imports_diff_on_check_apply.snap b/...e_cli/tests/snapshots/main_commands_check/shows_organize_imports_diff_on_check_apply.snap
@@ -35,9 +35,8 @@ check.js organizeImports ━━━━━━━━━━━━━━━━━━
   i Import statements could be sorted:
 
     1   │ - import·{·lorem,·foom,·bar·}·from·"foo";
-    2   │ - import·*·as·something·from·"../something";
-      1 │ + import·*·as·something·from·"../something";
-      2 │ + import·{·bar,·foom,·lorem·}·from·"foo";
+      1 │ + import·{·bar,·foom,·lorem·}·from·"foo";
+    2 2 │   import * as something from "../something";
     3 3 │   
 
 

diff --git a/crates/rome_css_parser/Cargo.toml b/crates/rome_css_parser/Cargo.toml
@@ -12,8 +12,8 @@ version              = "0.0.1"
 
 [dependencies]
 rome_console          = { workspace = true }
-rome_css_syntax       = { workspace = true }
 rome_css_factory      = { workspace = true }
+rome_css_syntax       = { workspace = true }
 rome_diagnostics      = { workspace = true }
 rome_js_unicode_table = { workspace = true }
 rome_parser           = { workspace = true }

diff --git a/crates/rome_js_analyze/Cargo.toml b/crates/rome_js_analyze/Cargo.toml
@@ -31,12 +31,12 @@ serde_json            = { workspace = true }
 smallvec              = { workspace = true }
 
 [dev-dependencies]
-countme          = { workspace = true, features = ["enable"] }
-insta            = { workspace = true, features = ["glob"] }
-rome_js_parser   = { workspace = true, features = ["tests"] }
-rome_text_edit   = { workspace = true }
-tests_macros     = { workspace = true }
-rome_test_utils =  { workspace = true }
+countme         = { workspace = true, features = ["enable"] }
+insta           = { workspace = true, features = ["glob"] }
+rome_js_parser  = { workspace = true, features = ["tests"] }
+rome_test_utils = { workspace = true }
+rome_text_edit  = { workspace = true }
+tests_macros    = { workspace = true }
 
 [features]
 schema = ["schemars", "rome_deserialize/schema"]
diff --git a/crates/rome_js_analyze/src/assists/correctness/organize_imports.rs b/crates/rome_js_analyze/src/assists/correctness/organize_imports.rs
@@ -25,7 +25,7 @@ use crate::JsRuleAction;
 
 declare_rule! {
     /// Provides a whole-source code action to sort the imports in the file
-    /// using natural ordering
+    /// using import groups and natural ordering.
     ///
     /// ## Examples
     ///
@@ -291,9 +291,8 @@ impl ImportGroup {
         // the sequence
         let mut iter = self.nodes.values().flat_map(|nodes| nodes.iter());
 
-        let import_node = match iter.next() {
-            Some(node) => node,
-            None => return true,
+        let Some(import_node) = iter.next() else {
+            return true;
         };
 
         if !import_node.is_sorted() {
@@ -345,11 +344,8 @@ impl From<JsImport> for ImportNode {
             };
 
             let named_import = import_named_clause.named_import().ok()?;
-            let named_import_specifiers = match named_import {
-                AnyJsNamedImport::JsNamespaceImportSpecifier(_) => return None,
-                AnyJsNamedImport::JsNamedImportSpecifiers(named_import_specifiers) => {
-                    named_import_specifiers
-                }
+            let AnyJsNamedImport::JsNamedImportSpecifiers(named_import_specifiers) = named_import else {
+                return None;
             };
 
             let mut result = BTreeMap::new();
@@ -383,9 +379,8 @@ impl ImportNode {
             .values()
             .map(|(node, _)| node.syntax().text_range().start());
 
-        let mut last = match iter.next() {
-            Some(last) => last,
-            None => return true,
+        let Some(mut last) = iter.next() else {
+            return true;
         };
 
         iter.all(|value| {
@@ -640,6 +635,12 @@ struct ImportKey(TokenText);
 
 impl Ord for ImportKey {
     fn cmp(&self, other: &Self) -> Ordering {
+        let own_category = ImportCategory::from(self.0.text());
+        let other_category = ImportCategory::from(other.0.text());
+        if own_category != other_category {
+            return own_category.cmp(&other_category);
+        }
+
         // Sort imports using natural ordering
         natord::compare(&self.0, &other.0)
     }
@@ -659,6 +660,85 @@ impl PartialEq for ImportKey {
     }
 }
 
+/// Imports get sorted by categories before being sorted on natural order.
+///
+/// The rationale for this is that imports "further away" from the source file
+/// are listed before imports closer to the source file.
+#[derive(Eq, Ord, PartialEq, PartialOrd)]
+enum ImportCategory {
+    /// Anythign with an explicit `node:` prefix, or one of the recognized
+    /// Node built-ins, such `"fs"`, `"child_process"`, etc..
+    NodeBuiltin,
+    /// NPM dependencies with an explicit `npm:` prefix, such as supported by
+    /// Deno.
+    Npm,
+    /// Imports from an absolute URL such as supported by browsers.
+    Url,
+    /// Anything without explicit protocol specifier is assumed to be a library
+    /// import. Because we currently do not have configuration for this, this
+    /// may (incorrectly) include source imports through custom import mappings
+    /// as well.
+    Library,
+    /// Relative file imports.
+    Relative,
+    /// Any unrecognized protocols are grouped here. These may include custom
+    /// protocols such as supported by bundlers.
+    Other,
+}
+
+const NODE_BUILTINS: &[&str] = &[
+    "assert",
+    "buffer",
+    "child_process",
+    "cluster",
+    "console",
+    "constants",
+    "crypto",
+    "dgram",
+    "dns",
+    "domain",
+    "events",
+    "fs",
+    "http",
+    "https",
+    "module",
+    "net",
+    "os",
+    "path",
+    "punycode",
+    "querystring",
+    "readline",
+    "repl",
+    "stream",
+    "string_decoder",
+    "sys",
+    "timers",
+    "tls",
+    "tty",
+    "url",
+    "util",
+    "vm",
+    "zlib",
+];
+
+impl From<&str> for ImportCategory {
+    fn from(value: &str) -> Self {
+        if value.starts_with("node:") || NODE_BUILTINS.contains(&value) {
+            Self::NodeBuiltin
+        } else if value.starts_with("npm:") {
+            Self::Npm
+        } else if value.starts_with("http:") || value.starts_with("https:") {
+            Self::Url
+        } else if value.starts_with('.') {
+            Self::Relative
+        } else if !value.contains(':') {
+            Self::Library
+        } else {
+            Self::Other
+        }
+    }
+}
+
 /// Returns true is this trivia piece is "ASCII whitespace" (newline or whitespace)
 fn is_ascii_whitespace(piece: &SyntaxTriviaPiece<JsLanguage>) -> bool {
     piece.is_newline() || piece.is_whitespace()

diff --git a/crates/rome_js_analyze/tests/specs/correctness/organizeImports/groups.js b/crates/rome_js_analyze/tests/specs/correctness/organizeImports/groups.js
@@ -0,0 +1,8 @@
+import uncle from "../uncle";
+import sibling from "./sibling";
+import express from "npm:epxress";
+import imageUrl from "url:./image.png";
+import assert from "node:assert";
+import aunt from "../aunt";
+import { VERSION } from "https://deno.land/std/version.ts";
+import { mock, test } from "node:test";
diff --git a/crates/rome_js_analyze/tests/specs/correctness/organizeImports/groups.js.snap b/crates/rome_js_analyze/tests/specs/correctness/organizeImports/groups.js.snap
@@ -0,0 +1,37 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: groups.js
+---
+# Input
+```js
+import uncle from "../uncle";
+import sibling from "./sibling";
+import express from "npm:epxress";
+import imageUrl from "url:./image.png";
+import assert from "node:assert";
+import aunt from "../aunt";
+import { VERSION } from "https://deno.land/std/version.ts";
+import { mock, test } from "node:test";
+
+```
+
+# Actions
+```diff
+@@ -1,8 +1,8 @@
++import assert from "node:assert";
++import { mock, test } from "node:test";
++import express from "npm:epxress";
++import { VERSION } from "https://deno.land/std/version.ts";
++import aunt from "../aunt";
+ import uncle from "../uncle";
+ import sibling from "./sibling";
+-import express from "npm:epxress";
+ import imageUrl from "url:./image.png";
+-import assert from "node:assert";
+-import aunt from "../aunt";
+-import { VERSION } from "https://deno.land/std/version.ts";
+-import { mock, test } from "node:test";
+
+```
+
+
diff --git a/crates/rome_js_transform/Cargo.toml b/crates/rome_js_transform/Cargo.toml
@@ -11,17 +11,17 @@ version              = "0.1.0"
 [dependencies]
 lazy_static      = { workspace = true }
 rome_analyze     = { workspace = true }
+rome_console     = { workspace = true }
 rome_diagnostics = { workspace = true }
 rome_js_factory  = { workspace = true }
 rome_js_syntax   = { workspace = true }
 rome_rowan       = { workspace = true }
-rome_console = {workspace= true}
 
 
 [dev-dependencies]
+insta             = { workspace = true }
+rome_analyze      = { workspace = true }
 rome_js_formatter = { workspace = true }
 rome_js_parser    = { workspace = true }
-rome_analyze =  { workspace = true }
-rome_test_utils = { workspace = true }
-tests_macros= { workspace = true }
-insta = { workspace = true }
+rome_test_utils   = { workspace = true }
+tests_macros      = { workspace = true }
diff --git a/crates/rome_json_analyze/Cargo.toml b/crates/rome_json_analyze/Cargo.toml
@@ -21,5 +21,5 @@ insta             = { workspace = true, features = ["glob"] }
 rome_json_factory = { workspace = true }
 rome_json_parser  = { workspace = true }
 rome_service      = { workspace = true }
+rome_test_utils   = { workspace = true }
 tests_macros      = { workspace = true }
-rome_test_utils = { workspace = true }
diff --git a/crates/rome_test_utils/Cargo.toml b/crates/rome_test_utils/Cargo.toml
@@ -1,25 +1,24 @@
 [package]
-name = "rome_test_utils"
-version = "0.1.0"
 edition = "2021"
+name    = "rome_test_utils"
 publish = false
+version = "0.1.0"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-rome_js_parser = {workspace = true }
-rome_json_parser = {workspace = true }
-rome_js_syntax = {workspace = true }
-rome_json_syntax = {workspace = true }
-rome_analyze = {workspace = true }
-rome_rowan = {workspace = true }
-rome_diagnostics = {workspace = true }
-rome_service = {workspace = true }
-rome_console = {workspace = true }
-rome_deserialize = {workspace = true }
-countme = { workspace = true, features = ["enable"] }
-similar = { version = "2.2.1" }
+countme          = { workspace = true, features = ["enable"] }
 json_comments    = "0.2.1"
-serde = { workspace = true }
-serde_json = { workspace = true }
-
+rome_analyze     = { workspace = true }
+rome_console     = { workspace = true }
+rome_deserialize = { workspace = true }
+rome_diagnostics = { workspace = true }
+rome_js_parser   = { workspace = true }
+rome_js_syntax   = { workspace = true }
+rome_json_parser = { workspace = true }
+rome_json_syntax = { workspace = true }
+rome_rowan       = { workspace = true }
+rome_service     = { workspace = true }
+serde            = { workspace = true }
+serde_json       = { workspace = true }
+similar          = { version = "2.2.1" }