fix(organizeImports): sort specifiers in bare exports

Author: Conaclos

.changeset/neat-papers-think.md

@@ -0,0 +1,13 @@
+---
+"@biomejs/biome": patch
+---
+
+Partially fix [#7583](https://github.com/biomejs/biome/issues/7583).
+[`organizeImports`](https://biomejs.dev/assist/actions/organize-imports/) now
+sorts named specifiers inside bare exports.
+
+```diff
+- export { b, a };
++ export { a, b };
+```
+

crates/biome_js_analyze/src/assist/source/organize_imports.rs

@@ -1,3 +1,8 @@
+pub mod import_key;
+pub mod specifiers_attributes;
+mod util;
+
+use crate::JsRuleAction;
 use biome_analyze::{
     ActionCategory, Ast, FixKind, Rule, RuleDiagnostic, RuleSource, SourceActionKind,
     context::RuleContext, declare_source_rule,
@@ -14,17 +19,12 @@ use biome_rule_options::{organize_imports::OrganizeImportsOptions, sort_order::S
 use import_key::{ImportInfo, ImportKey};
 use rustc_hash::FxHashMap;
 use specifiers_attributes::{
-    are_import_attributes_sorted, merge_export_specifiers, merge_import_specifiers,
-    sort_attributes, sort_export_specifiers, sort_import_specifiers,
+    JsNamedSpecifiers, are_import_attributes_sorted, merge_export_from_specifiers,
+    merge_import_specifiers, sort_attributes, sort_export_from_specifiers, 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.
     ///
@@ -825,22 +825,43 @@ impl Rule for OrganizeImports {
                     prev_group = key.group;
                     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
-                report_unsorted_chunk(chunk.take(), &mut result);
-                prev_group = 0;
-                // A statement must be separated of a chunk with a blank line
-                if let AnyJsModuleItem::AnyJsStatement(statement) = &item
-                    && leading_newlines(statement.syntax()).count() == 1
+            } else {
+                if chunk.is_some() {
+                    // This is either
+                    // - a bare (side-effect) import
+                    // - an export without `from` clause
+                    // - a buggy import or export
+                    // - a statement
+                    //
+                    // In any case, 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
+                    if let AnyJsModuleItem::AnyJsStatement(statement) = &item
+                        && leading_newlines(statement.syntax()).count() == 1
+                    {
+                        result.push(Issue::AddLeadingNewline {
+                            slot_index: statement.syntax().index() as u32,
+                        });
+                    }
+                }
+                if let AnyJsModuleItem::JsExport(js_export) = &item
+                    && let Ok(AnyJsExportClause::JsExportNamedClause(clause)) =
+                        js_export.export_clause()
                 {
-                    result.push(Issue::AddLeadingNewline {
-                        slot_index: statement.syntax().index() as u32,
-                    });
+                    let specifiers =
+                        JsNamedSpecifiers::JsExportNamedSpecifierList(clause.specifiers());
+                    let are_specifiers_unsorted = !specifiers.are_sorted(sort_order);
+                    if are_specifiers_unsorted {
+                        // Report the violation of one of the previous requirement
+                        result.push(Issue::UnorganizedItem {
+                            slot_index: item.syntax().index() as u32,
+                            are_specifiers_unsorted,
+                            // An export without `from` clause has no attributes.
+                            are_attributes_unsorted: true,
+                            newline_issue: NewLineIssue::None,
+                        });
+                    }
                 }
             }
             prev_kind = Some(item.syntax().kind());
@@ -902,6 +923,11 @@ impl Rule for OrganizeImports {
                             if *are_specifiers_unsorted {
                                 // Sort named specifiers
                                 if let AnyJsExportClause::JsExportNamedFromClause(cast) = &clause
+                                    && let Some(sorted_specifiers) =
+                                        sort_export_from_specifiers(&cast.specifiers(), sort_order)
+                                {
+                                    clause = cast.clone().with_specifiers(sorted_specifiers).into();
+                                } else if let AnyJsExportClause::JsExportNamedClause(cast) = &clause
                                     && let Some(sorted_specifiers) =
                                         sort_export_specifiers(&cast.specifiers(), sort_order)
                                 {
@@ -1149,7 +1175,7 @@ fn merge(
             let specifiers1 = clause1.specifiers();
             let specifiers2 = clause2.specifiers();
             if let Some(meregd_specifiers) =
-                merge_export_specifiers(&specifiers1, &specifiers2, sort_order)
+                merge_export_from_specifiers(&specifiers1, &specifiers2, sort_order)
             {
                 let meregd_clause = clause1.with_specifiers(meregd_specifiers);
                 let merged_item = item2.clone().with_export_clause(meregd_clause.into());

crates/biome_js_analyze/src/assist/source/organize_imports/specifiers_attributes.rs

@@ -1,8 +1,8 @@
 use biome_analyze::utils::{is_separated_list_sorted_by, sorted_separated_list_by};
 use biome_js_factory::make;
 use biome_js_syntax::{
-    AnyJsBinding, AnyJsImportAssertionEntry, JsExportNamedFromSpecifierList, JsImportAssertion,
-    JsNamedImportSpecifiers, T, inner_string_text,
+    AnyJsBinding, AnyJsImportAssertionEntry, JsExportNamedFromSpecifierList,
+    JsExportNamedSpecifierList, JsImportAssertion, JsNamedImportSpecifiers, T, inner_string_text,
 };
 use biome_rowan::{AstNode, AstSeparatedElement, AstSeparatedList, TriviaPieceKind};
 use biome_rule_options::organize_imports::SortOrder;
@@ -12,6 +12,7 @@ use std::cmp::Ordering;
 pub enum JsNamedSpecifiers {
     JsNamedImportSpecifiers(JsNamedImportSpecifiers),
     JsExportNamedFromSpecifierList(JsExportNamedFromSpecifierList),
+    JsExportNamedSpecifierList(JsExportNamedSpecifierList),
 }
 impl JsNamedSpecifiers {
     pub fn are_sorted(&self, sort_order: SortOrder) -> bool {
@@ -20,6 +21,9 @@ impl JsNamedSpecifiers {
                 are_import_specifiers_sorted(specifeirs, sort_order)
             }
             Self::JsExportNamedFromSpecifierList(specifeirs) => {
+                are_export_from_specifiers_sorted(specifeirs, sort_order)
+            }
+            Self::JsExportNamedSpecifierList(specifeirs) => {
                 are_export_specifiers_sorted(specifeirs, sort_order)
             }
         }
@@ -111,7 +115,7 @@ pub fn merge_import_specifiers(
     sort_import_specifiers(named_specifiers1.with_specifiers(new_list), sort_order)
 }
 
-pub fn are_export_specifiers_sorted(
+pub fn are_export_from_specifiers_sorted(
     specifiers: &JsExportNamedFromSpecifierList,
     sort_order: SortOrder,
 ) -> Option<bool> {
@@ -131,7 +135,7 @@ pub fn are_export_specifiers_sorted(
     .ok()
 }
 
-pub fn sort_export_specifiers(
+pub fn sort_export_from_specifiers(
     named_specifiers: &JsExportNamedFromSpecifierList,
     sort_order: SortOrder,
 ) -> Option<JsExportNamedFromSpecifierList> {
@@ -152,7 +156,7 @@ pub fn sort_export_specifiers(
     Some(new_list)
 }
 
-pub fn merge_export_specifiers(
+pub fn merge_export_from_specifiers(
     specifiers1: &JsExportNamedFromSpecifierList,
     specifiers2: &JsExportNamedFromSpecifierList,
     sort_order: SortOrder,
@@ -185,12 +189,92 @@ pub fn merge_export_specifiers(
             separators.push(separator);
         }
     }
-    sort_export_specifiers(
+    sort_export_from_specifiers(
         &make::js_export_named_from_specifier_list(nodes, separators),
         sort_order,
     )
 }
 
+pub fn are_export_specifiers_sorted(
+    specifiers: &JsExportNamedSpecifierList,
+    sort_order: SortOrder,
+) -> Option<bool> {
+    let comparator = get_comparator(sort_order);
+
+    is_separated_list_sorted_by(
+        specifiers,
+        |node| {
+            node.local_name()
+                .ok()?
+                .name()
+                .ok()
+                .map(ComparableToken::new)
+        },
+        comparator,
+    )
+    .ok()
+}
+
+pub fn sort_export_specifiers(
+    named_specifiers: &JsExportNamedSpecifierList,
+    sort_order: SortOrder,
+) -> Option<JsExportNamedSpecifierList> {
+    let comparator = get_comparator(sort_order);
+    let new_list = sorted_separated_list_by(
+        named_specifiers,
+        |node| {
+            node.local_name()
+                .ok()?
+                .name()
+                .ok()
+                .map(ComparableToken::new)
+        },
+        || make::token(T![,]).with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]),
+        comparator,
+    )
+    .ok()?;
+    Some(new_list)
+}
+
+pub fn merge_export_specifiers(
+    specifiers1: &JsExportNamedSpecifierList,
+    specifiers2: &JsExportNamedSpecifierList,
+    sort_order: SortOrder,
+) -> Option<JsExportNamedSpecifierList> {
+    let mut nodes = Vec::with_capacity(specifiers1.len() + specifiers2.len());
+    let mut separators = Vec::with_capacity(specifiers1.len() + specifiers2.len());
+    for AstSeparatedElement {
+        node,
+        trailing_separator,
+    } in specifiers1.elements()
+    {
+        let separator = trailing_separator.ok()?;
+        let mut node = node.ok()?;
+        if separator.is_none() {
+            node = node.trim_trailing_trivia()?;
+        }
+        let separator = separator.unwrap_or_else(|| {
+            make::token(T![,]).with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")])
+        });
+        nodes.push(node);
+        separators.push(separator);
+    }
+    for AstSeparatedElement {
+        node,
+        trailing_separator,
+    } in specifiers2.elements()
+    {
+        nodes.push(node.ok()?);
+        if let Some(separator) = trailing_separator.ok()? {
+            separators.push(separator);
+        }
+    }
+    sort_export_specifiers(
+        &make::js_export_named_specifier_list(nodes, separators),
+        sort_order,
+    )
+}
+
 pub fn are_import_attributes_sorted(
     attributes: &JsImportAssertion,
     sort_order: SortOrder,

crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-from-less-export.js

@@ -0,0 +1 @@
+export { b, a }

crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-from-less-export.js.snap

@@ -0,0 +1,28 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: unsorted-from-less-export.js
+---
+# Input
+```js
+export { b, a }
+
+```
+
+# Diagnostics
+```
+unsorted-from-less-export.js:1:1 assist/source/organizeImports  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  i The imports and exports are not sorted.
+  
+  > 1 │ export { b, a }
+      │ ^^^^^^^^^^^^^^^
+    2 │ 
+  
+  i Safe fix: Organize Imports (Biome)
+  
+    1   │ - export·{·b,·a·}
+      1 │ + export·{·a,·b·}
+    2 2 │   
+  
+
+```

crates/biome_lsp/src/server.tests.rs

@@ -2016,7 +2016,7 @@ async fn pull_code_actions_with_import_sorting() -> Result<()> {
 import z from "zod";
 import { test } from "./test";
 import { describe } from "node:test";
-export { z, test, describe };
+export { describe, test, z };
 
 if(a === -0) {}
 "#,