Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Group organized imports by distance #4725

Merged
merged 18 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1538,11 +1538,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());

Expand Down Expand Up @@ -1737,11 +1737,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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 │


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 │


Expand Down
104 changes: 92 additions & 12 deletions crates/rome_js_analyze/src/assists/correctness/organize_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
Original file line number Diff line number Diff line change
@@ -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";

```


8 changes: 4 additions & 4 deletions editors/vscode/src/commands/syntaxTree.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
import { SyntaxTreeParams, syntaxTreeRequest } from "../lsp_requests";
import { Command, Session } from "../session";
import { isRomeEditor } from "../utils";
import { SyntaxTreeDocument } from "./syntaxTreeDocument";
import {
CancellationToken,
Disposable,
Expand All @@ -19,6 +15,10 @@ import {
window,
workspace,
} from "vscode";
import { SyntaxTreeParams, syntaxTreeRequest } from "../lsp_requests";
import { Command, Session } from "../session";
import { isRomeEditor } from "../utils";
import { SyntaxTreeDocument } from "./syntaxTreeDocument";

type FilePath = string;

Expand Down
10 changes: 5 additions & 5 deletions editors/vscode/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
import { Commands } from "./commands";
import { syntaxTree } from "./commands/syntaxTree";
import { Session } from "./session";
import { StatusBar } from "./statusBar";
import { setContextValue } from "./utils";
import { type ChildProcess, spawn } from "child_process";
import { type Socket, connect } from "net";
import { isAbsolute } from "path";
Expand All @@ -23,6 +18,11 @@ import {
ServerOptions,
StreamInfo,
} from "vscode-languageclient/node";
import { Commands } from "./commands";
import { syntaxTree } from "./commands/syntaxTree";
import { Session } from "./session";
import { StatusBar } from "./statusBar";
import { setContextValue } from "./utils";

import resolveImpl = require("resolve/async");
import type * as resolve from "resolve";
Expand Down
4 changes: 2 additions & 2 deletions editors/vscode/src/session.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Commands } from "./commands";
import { RomeEditor, isRomeEditor } from "./utils";
import { Disposable, ExtensionContext, commands, window } from "vscode";
import { LanguageClient } from "vscode-languageclient/node";
import { Commands } from "./commands";
import { RomeEditor, isRomeEditor } from "./utils";

export type Command = (...args: unknown[]) => unknown;

Expand Down
2 changes: 1 addition & 1 deletion editors/vscode/src/statusBar.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Commands } from "./commands";
import { StatusBarAlignment, StatusBarItem, ThemeColor, window } from "vscode";
import { State } from "vscode-languageclient";
import { LanguageClient } from "vscode-languageclient/node";
import { Commands } from "./commands";

/**
* Enumeration of all the status the extension can display
Expand Down
2 changes: 1 addition & 1 deletion npm/js-api/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { Distribution, WasmModule, loadModule, wrapError } from "./wasm";
import type {
Configuration,
Diagnostic,
PullDiagnosticsResult,
RomePath,
Workspace,
} from "@rometools/wasm-nodejs";
import { Distribution, WasmModule, loadModule, wrapError } from "./wasm";

// Re-export of some useful types for users
export type { Configuration, Diagnostic };
Expand Down
2 changes: 1 addition & 1 deletion npm/js-api/tests/formatContent.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Distribution, Rome } from "../dist";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { Distribution, Rome } from "../dist";

describe("Rome WebAssembly formatContent", () => {
let rome: Rome;
Expand Down
2 changes: 1 addition & 1 deletion npm/js-api/tests/lintContent.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Distribution, Rome } from "../dist";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { Distribution, Rome } from "../dist";

describe("Rome WebAssembly lintContent", () => {
let rome: Rome;
Expand Down
2 changes: 1 addition & 1 deletion npm/js-api/tests/printDiagnostics.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Distribution, Rome } from "../dist";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { Distribution, Rome } from "../dist";

describe("Rome WebAssembly DiagnosticPrinter", () => {
let rome: Rome;
Expand Down
4 changes: 2 additions & 2 deletions website/astro.config.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import fs from "node:fs/promises";
import path from "node:path";
import mdx from "@astrojs/mdx";
import react from "@astrojs/react";
import type { AstroIntegration } from "astro";
import compress from "astro-compress";
import { defineConfig } from "astro/config";
import { globby } from "globby";
import fs from "node:fs/promises";
import path from "node:path";
import rehypeAutolinkHeadings from "rehype-autolink-headings";
import rehypeSlug from "rehype-slug";
import remarkToc from "remark-toc";
Expand Down
2 changes: 1 addition & 1 deletion website/src/frontend-scripts/docsearch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { matchesDark, setCurrentTheme } from "./util";
import docsearch from "@docsearch/js";
import { matchesDark, setCurrentTheme } from "./util";

const docsearchContainer = document.querySelector(
"#docsearch-target",
Expand Down
2 changes: 1 addition & 1 deletion website/src/playground/CodeMirror.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { useTheme } from "./utils";
import type { Diagnostic as CodeMirrorDiagnostic } from "@codemirror/lint";
import { lintGutter, setDiagnostics } from "@codemirror/lint";
import type { Extension } from "@codemirror/state";
Expand All @@ -10,6 +9,7 @@ import type {
} from "@uiw/react-codemirror";
import RealCodeMirror from "@uiw/react-codemirror";
import { forwardRef, useEffect, useMemo, useState } from "react";
import { useTheme } from "./utils";

export type RomeExtension = Extension;

Expand Down
Loading