diff --git a/crates/biome_js_analyze/src/assists/correctness/organize_imports.rs b/crates/biome_js_analyze/src/assists/correctness/organize_imports.rs index 6bcb6b838fcf..0ce1ecdaf142 100644 --- a/crates/biome_js_analyze/src/assists/correctness/organize_imports.rs +++ b/crates/biome_js_analyze/src/assists/correctness/organize_imports.rs @@ -1,10 +1,4 @@ -use std::{ - cell::Cell, - cmp::Ordering, - collections::{btree_map::Entry, BTreeMap}, - iter, - mem::take, -}; +use std::{cell::Cell, cmp::Ordering, collections::BTreeMap, iter, mem::take}; use biome_analyze::{ context::RuleContext, declare_rule, ActionCategory, Ast, FixKind, Rule, SourceActionKind, @@ -18,7 +12,7 @@ use biome_js_syntax::{ }; use biome_rowan::{ chain_trivia_pieces, syntax::SyntaxTrivia, AstNode, AstNodeExt, AstNodeList, AstSeparatedList, - BatchMutationExt, SyntaxTriviaPiece, TokenText, TriviaPiece, + BatchMutationExt, SyntaxTriviaPiece, TextSize, TokenText, TriviaPiece, }; use crate::JsRuleAction; @@ -63,31 +57,24 @@ impl Rule for OrganizeImports { fn run(ctx: &RuleContext) -> Option { let root = ctx.query(); - let mut groups = Vec::new(); - let mut first_node = None; let mut nodes = BTreeMap::new(); for item in root.items() { - let import = match item { - AnyJsModuleItem::JsImport(import) => import, - AnyJsModuleItem::AnyJsStatement(_) | AnyJsModuleItem::JsExport(_) => { - // If we have pending nodes and encounter a non-import node, append the nodes to a new group - if let Some(first_node) = first_node.take() { - groups.push(ImportGroup { - first_node, - nodes: take(&mut nodes), - }); - } - continue; + let AnyJsModuleItem::JsImport(import) = item else { + // If we have pending nodes and encounter a non-import node, append the nodes to a new group + if let Some(first_node) = first_node.take() { + groups.push(ImportGroup { + first_node, + nodes: take(&mut nodes), + }); } + continue; }; - let first_token = import.import_token().ok()?; - // If this is not the first import in the group, check for a group break - if has_empty_line(&first_token.leading_trivia()) { + if has_empty_line(&import.import_token().ok()?.leading_trivia()) { if let Some(first_node) = first_node.take() { groups.push(ImportGroup { first_node, @@ -96,28 +83,16 @@ impl Rule for OrganizeImports { } } - let source = match import.import_clause().ok()? { - AnyJsImportClause::JsImportBareClause(clause) => clause.source().ok()?, - AnyJsImportClause::JsImportDefaultClause(clause) => clause.source().ok()?, - AnyJsImportClause::JsImportNamedClause(clause) => clause.source().ok()?, - AnyJsImportClause::JsImportNamespaceClause(clause) => clause.source().ok()?, - }; - // If this is the first import in the group save the leading trivia // and slot index if first_node.is_none() { first_node = Some(import.clone()); } - let key = source.inner_string_text().ok()?; - match nodes.entry(ImportKey(key)) { - Entry::Vacant(entry) => { - entry.insert(vec![ImportNode::from(import)]); - } - Entry::Occupied(mut entry) => { - entry.get_mut().push(ImportNode::from(import)); - } - } + nodes + .entry(ImportKey(import.source_text().ok()?)) + .or_default() + .push(ImportNode::from(import)); } // Flush the remaining nodes @@ -287,35 +262,16 @@ struct ImportGroup { impl ImportGroup { /// Returns true if the nodes in the group are already sorted in the file fn is_sorted(&self) -> bool { - // The imports are sorted if the start of each node in the `BTreeMap` - // (sorted in natural order) is higher or equal to the previous item in + // The imports are sorted if the text position of each node in the `BTreeMap` + // (sorted in natural order) is higher than the previous item in // the sequence let mut iter = self.nodes.values().flat_map(|nodes| nodes.iter()); - - let Some(import_node) = iter.next() else { - return true; - }; - - if !import_node.is_sorted() { - return false; - } - - let mut last = import_node.node.syntax().text_range().start(); + let mut previous = TextSize::from(0); iter.all(|import_node| { - let start = import_node.node.syntax().text_range().start(); - if start < last { - return false; - } - - // Only return false if the node has been fully inspected and was - // found to be unsorted, but continue visiting the remaining - // imports if the node was sorted or contained syntax errors - if !import_node.is_sorted() { - return false; - } - - last = start; - true + let current = import_node.node.syntax().text_range().end(); + let is_sorted = previous < current && import_node.is_sorted(); + previous = current; + is_sorted }) } } @@ -337,11 +293,8 @@ impl From for ImportNode { let mut separator_count = 0; let specifiers = import_clause.and_then(|import_clause| { - let import_named_clause = match import_clause { - AnyJsImportClause::JsImportBareClause(_) - | AnyJsImportClause::JsImportDefaultClause(_) - | AnyJsImportClause::JsImportNamespaceClause(_) => return None, - AnyJsImportClause::JsImportNamedClause(import_clause) => import_clause, + let AnyJsImportClause::JsImportNamedClause(import_named_clause) = import_clause else { + return None; }; let named_import = import_named_clause.named_import().ok()?; @@ -354,7 +307,7 @@ impl From for ImportNode { for element in named_import_specifiers.specifiers().elements() { let node = element.node.ok()?; - let key = import_specifier_name(&node)?; + let key = node.local_name()?.token_text_trimmed(); let trailing_separator = element.trailing_separator.ok()?; separator_count += usize::from(trailing_separator.is_some()); @@ -376,22 +329,12 @@ impl From for ImportNode { impl ImportNode { /// Returns `true` if the named import specifiers of this import node are sorted fn is_sorted(&self) -> bool { - let mut iter = self - .specifiers - .values() - .map(|(node, _)| node.syntax().text_range().start()); - - let Some(mut last) = iter.next() else { - return true; - }; - - iter.all(|value| { - if value < last { - return false; - } - - last = value; - true + let mut previous = TextSize::from(0); + self.specifiers.values().all(|(node, _)| { + let current = node.syntax().text_range().end(); + let is_sorted = previous < current; + previous = current; + is_sorted }) } @@ -400,20 +343,14 @@ impl ImportNode { let import = self.node.clone().detach(); let import_clause = import.import_clause(); - let import_named_clause = - if let Ok(AnyJsImportClause::JsImportNamedClause(node)) = import_clause { - node - } else { - return import; - }; + let Ok(AnyJsImportClause::JsImportNamedClause(import_named_clause)) = import_clause else { + return import; + }; let named_import = import_named_clause.named_import(); - let old_specifiers = - if let Ok(AnyJsNamedImport::JsNamedImportSpecifiers(node)) = named_import { - node - } else { - return import; - }; + let Ok(AnyJsNamedImport::JsNamedImportSpecifiers(old_specifiers)) = named_import else { + return import; + }; let element_count = self.specifiers.len(); let last_element = element_count.saturating_sub(1); @@ -428,9 +365,8 @@ impl ImportNode { let is_last = index == last_element; let mut node = node.clone().detach(); - let prev_token = match node.syntax().last_token() { - Some(last_token) => last_token, - None => return node, + let Some(prev_token) = node.syntax().last_token() else { + return node; }; if let Some(sep) = sep { @@ -546,29 +482,6 @@ impl ImportNode { } } -/// Return the name associated with a given named import specifier -/// -/// Currently named import specifiers are sorted using their import name, -/// not the local name they get imported as -fn import_specifier_name(import_specifier: &AnyJsNamedImportSpecifier) -> Option { - let token = match import_specifier { - AnyJsNamedImportSpecifier::JsNamedImportSpecifier(import_specifier) => { - import_specifier.name().ok()?.value().ok()? - } - AnyJsNamedImportSpecifier::JsShorthandNamedImportSpecifier(import_specifier) => { - import_specifier - .local_name() - .ok()? - .as_js_identifier_binding()? - .name_token() - .ok()? - } - AnyJsNamedImportSpecifier::JsBogusNamedImportSpecifier(_) => return None, - }; - - Some(token.token_text_trimmed()) -} - /// Return a clone of `prev_token` with a newline trivia piece prepended to its /// leading trivia if it didn't have one already. This function will try to copy /// the newline trivia piece from the leading trivia of `newline_source` if its set @@ -668,12 +581,16 @@ impl PartialEq for ImportKey { /// are listed before imports closer to the source file. #[derive(Eq, Ord, PartialEq, PartialOrd)] enum ImportCategory { + /// Anything with an explicit `bun:` prefix. + Bun, /// Anything 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, + /// Absolute file imports `/`. + Absolute, /// Imports from an absolute URL such as supported by browsers. Url, /// Anything without explicit protocol specifier is assumed to be a library @@ -681,7 +598,10 @@ enum ImportCategory { /// may (incorrectly) include source imports through custom import mappings /// as well. Library, - /// Relative file imports. + /// Node allows specifying an import map with name prefixed with `#`. + /// See https://nodejs.org/api/packages.html#subpath-imports + SharpImport, + /// Relative file imports `./`. Relative, /// Any unrecognized protocols are grouped here. These may include custom /// protocols such as supported by bundlers. @@ -725,18 +645,25 @@ const NODE_BUILTINS: &[&str] = &[ 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 + match value.chars().next().unwrap_or_default() { + '#' => Self::SharpImport, + '/' => Self::Absolute, + '.' => Self::Relative, + _ => { + if let Some((protocol, _)) = value.split_once(':') { + match protocol { + "bun" => Self::Bun, + "http" | "https" => Self::Url, + "node" => Self::NodeBuiltin, + "npm" => Self::Npm, + _ => Self::Other, + } + } else if NODE_BUILTINS.contains(&value) { + Self::NodeBuiltin + } else { + Self::Library + } + } } } } diff --git a/crates/biome_js_analyze/src/react.rs b/crates/biome_js_analyze/src/react.rs index 64ecd5100cfd..d0e3a232afac 100644 --- a/crates/biome_js_analyze/src/react.rs +++ b/crates/biome_js_analyze/src/react.rs @@ -269,9 +269,8 @@ fn is_react_export(binding: &Binding, lib: ReactLibrary) -> bool { binding .syntax() .ancestors() - .find_map(|ancestor| JsImport::cast_ref(&ancestor)) - .and_then(|js_import| js_import.source_is(lib.import_name()).ok()) - .unwrap_or(false) + .find_map(|ancestor| JsImport::cast(ancestor)?.source_text().ok()) + .is_some_and(|source| source.text() == lib.import_name()) } fn is_named_react_export(binding: &Binding, lib: ReactLibrary, name: &str) -> Option { @@ -296,5 +295,5 @@ fn is_named_react_export(binding: &Binding, lib: ReactLibrary, name: &str) -> Op let import_clause = import_specifiers.parent::()?; let import = import_clause.parent::()?; - import.source_is(lib.import_name()).ok() + Some(import.source_text().ok()?.text() == lib.import_name()) } diff --git a/crates/biome_js_analyze/tests/specs/correctness/organizeImports/groups.js b/crates/biome_js_analyze/tests/specs/correctness/organizeImports/groups.js index 398ed07bf293..63ff2c9ae2d8 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/organizeImports/groups.js +++ b/crates/biome_js_analyze/tests/specs/correctness/organizeImports/groups.js @@ -6,3 +6,6 @@ import assert from "node:assert"; import aunt from "../aunt"; import { VERSION } from "https://deno.land/std/version.ts"; import { mock, test } from "node:test"; +import { expect } from "bun:test"; +import { internal } from "#internal"; +import { secret } from "/absolute/path"; \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/correctness/organizeImports/groups.js.snap b/crates/biome_js_analyze/tests/specs/correctness/organizeImports/groups.js.snap index 0687a36e3eb4..7773a37b0019 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/organizeImports/groups.js.snap +++ b/crates/biome_js_analyze/tests/specs/correctness/organizeImports/groups.js.snap @@ -12,25 +12,35 @@ import assert from "node:assert"; import aunt from "../aunt"; import { VERSION } from "https://deno.land/std/version.ts"; import { mock, test } from "node:test"; - +import { expect } from "bun:test"; +import { internal } from "#internal"; +import { secret } from "/absolute/path"; ``` # Actions ```diff -@@ -1,8 +1,8 @@ +@@ -1,11 +1,11 @@ +-import uncle from "../uncle"; +-import sibling from "./sibling"; ++import { expect } from "bun:test"; +import assert from "node:assert"; +import { mock, test } from "node:test"; -+import express from "npm:express"; -+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:express"; - import imageUrl from "url:./image.png"; + import express from "npm:express"; +-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 { secret } from "/absolute/path"; + import { VERSION } from "https://deno.land/std/version.ts"; -import { mock, test } from "node:test"; +-import { expect } from "bun:test"; + import { internal } from "#internal"; +-import { secret } from "/absolute/path"; +\ No newline at end of file ++import aunt from "../aunt"; ++import uncle from "../uncle"; ++import sibling from "./sibling"; ++import imageUrl from "url:./image.png"; +\ No newline at end of file ``` diff --git a/crates/biome_js_syntax/src/import_ext.rs b/crates/biome_js_syntax/src/import_ext.rs index 18c23d7fadcc..e89715573ef6 100644 --- a/crates/biome_js_syntax/src/import_ext.rs +++ b/crates/biome_js_syntax/src/import_ext.rs @@ -1,4 +1,7 @@ -use crate::{inner_string_text, AnyJsImportClause, JsImport, JsModuleSource}; +use crate::{ + inner_string_text, AnyJsImportClause, AnyJsNamedImportSpecifier, JsImport, JsModuleSource, + JsSyntaxToken, +}; use biome_rowan::{SyntaxResult, TokenText}; impl JsImport { @@ -7,25 +10,70 @@ impl JsImport { /// ## Examples /// /// ``` - /// use biome_js_factory::make::{js_reference_identifier, ident, js_module_source, js_import_default_clause, token, js_identifier_binding, js_import}; - /// use biome_js_syntax::{AnyJsBinding, AnyJsImportClause, T}; - /// let source = js_module_source(ident("react")); - /// let binding = js_identifier_binding(ident("React")); - /// let clause = js_import_default_clause(AnyJsBinding::JsIdentifierBinding(binding), token(T![from]), source).build(); - /// let import = js_import(token(T![import]), AnyJsImportClause::JsImportDefaultClause(clause)).build(); - /// assert_eq!(import.source_is("react"), Ok(true)); - /// assert_eq!(import.source_is("React"), Ok(false)); - /// ``` - pub fn source_is(&self, source_to_check: &str) -> SyntaxResult { - let clause = self.import_clause()?; - let source = match clause { - AnyJsImportClause::JsImportBareClause(node) => node.source(), - AnyJsImportClause::JsImportDefaultClause(node) => node.source(), - AnyJsImportClause::JsImportNamedClause(node) => node.source(), - AnyJsImportClause::JsImportNamespaceClause(node) => node.source(), - }?; + /// use biome_js_factory::make; + /// use biome_js_syntax::T; + /// + /// let source = make::js_module_source(make::js_string_literal("react")); + /// let binding = make::js_identifier_binding(make::ident("React")); + /// let clause = make::js_import_default_clause(binding.into(), make::token(T![from]), source).build(); + /// let import = make::js_import(make::token(T![import]), clause.into()).build(); + /// + /// assert_eq!(import.source_text().unwrap().text(), "react"); + /// ``` + pub fn source_text(&self) -> SyntaxResult { + self.import_clause()?.source()?.inner_string_text() + } +} + +impl AnyJsImportClause { + /// Source of this import clause. + /// + /// ``` + /// use biome_js_factory::make; + /// use biome_js_syntax::T; + /// + /// let source = make::js_module_source(make::js_string_literal("react")); + /// let binding = make::js_identifier_binding(make::ident("React")); + /// let clause = make::js_import_default_clause(binding.into(), make::token(T![from]), source).build(); + /// + /// assert_eq!(clause.source().unwrap().inner_string_text().unwrap().text(), "react"); + /// ``` + pub fn source(&self) -> SyntaxResult { + match self { + Self::JsImportBareClause(node) => node.source(), + Self::JsImportDefaultClause(node) => node.source(), + Self::JsImportNamedClause(node) => node.source(), + Self::JsImportNamespaceClause(node) => node.source(), + } + } +} - Ok(source.inner_string_text()?.text() == source_to_check) +impl AnyJsNamedImportSpecifier { + /// LOcal name of this import specifier + /// + /// ``` + /// use biome_js_factory::make; + /// use biome_js_syntax::{AnyJsNamedImportSpecifier, T}; + /// + /// let binding = make::js_identifier_binding(make::ident("React")); + /// let specifier = make::js_shorthand_named_import_specifier(binding.into()).build(); + /// let specifier = AnyJsNamedImportSpecifier::JsShorthandNamedImportSpecifier(specifier); + /// + /// assert_eq!(specifier.local_name().unwrap().text_trimmed(), "React"); + /// ``` + pub fn local_name(&self) -> Option { + match self { + AnyJsNamedImportSpecifier::JsNamedImportSpecifier(specifier) => { + specifier.name().ok()?.value().ok() + } + AnyJsNamedImportSpecifier::JsShorthandNamedImportSpecifier(specifier) => specifier + .local_name() + .ok()? + .as_js_identifier_binding()? + .name_token() + .ok(), + AnyJsNamedImportSpecifier::JsBogusNamedImportSpecifier(_) => None, + } } } @@ -34,12 +82,14 @@ impl JsModuleSource { /// ## Examples /// /// ``` - /// use biome_js_factory::make::{ident, js_module_source}; - /// use biome_js_syntax::{AnyJsBinding, AnyJsImportClause, T}; + /// use biome_js_factory::make; /// use biome_rowan::TriviaPieceKind; - /// let source = js_module_source(ident("react").with_leading_trivia(vec![(TriviaPieceKind::Whitespace, " ")])); - /// let text = source.inner_string_text().unwrap(); - /// assert_eq!(text.text(), "react"); + /// + /// let source_token = make::js_string_literal("react") + /// .with_leading_trivia(vec![(TriviaPieceKind::Whitespace, " ")]); + /// let source = make::js_module_source(source_token); + /// + /// assert_eq!(source.inner_string_text().unwrap().text(), "react"); /// ``` pub fn inner_string_text(&self) -> SyntaxResult { Ok(inner_string_text(&self.value_token()?)) diff --git a/website/src/content/docs/analyzer/index.mdx b/website/src/content/docs/analyzer/index.mdx index 982671cd820a..47b8e746897f 100644 --- a/website/src/content/docs/analyzer/index.mdx +++ b/website/src/content/docs/analyzer/index.mdx @@ -23,12 +23,15 @@ This feature is enabled by default but can be opted-in/out via configuration: Import statements are sorted by "distance". Modules that are "farther" from the user are put on the top, modules "closer" to the user are put on the bottom: -1. built-in Node.js modules that are explicitly imported using the `node:` protocol; -2. modules imported via `npm:` protocol. This is a valid syntax when writing code run by Deno, for example; -3. modules imported via URL; -4. modules imported from libraries; -5. modules imported via relative imports; -6. modules that couldn't be identified by the previous criteria; +1. modules imported via `bun:` protocol. This is a valid syntax when writing code run by Bun; +2. built-in Node.js modules that are explicitly imported using the `node:` protocol; +3. modules imported via `npm:` protocol. This is a valid syntax when writing code run by Deno, for example; +4. modules imported via absolute imports; +5. modules imported via URL; +6. modules imported from libraries; +7. modules imported from a name prefixed by `#`. This is a valid syntax when [writing code run by Node](https://nodejs.org/api/packages.html#subpath-imports); +8. modules imported via relative imports; +9. modules that couldn't be identified by the previous criteria; For example, given the following code: @@ -41,15 +44,23 @@ import assert from "node:assert"; import aunt from "../aunt"; import { VERSION } from "https://deno.land/std/version.ts"; import { mock, test } from "node:test"; +import { expect } from "bun:test"; +import { internal } from "#internal"; +import { secret } from "/absolute/path"; +import React from "react"; ``` They will be sorted like this: ```ts + import { expect } from "bun:test"; import assert from "node:assert"; import { mock, test } from "node:test"; import express from "npm:express"; + import { secret } from "/absolute/path"; import { VERSION } from "https://deno.land/std/version.ts"; + import React from "react"; + import { internal } from "#internal"; import aunt from "../aunt"; import uncle from "../uncle"; import sibling from "./sibling";