diff --git a/.changeset/neat-papers-think.md b/.changeset/neat-papers-think.md new file mode 100644 index 000000000000..f6126b6bc78c --- /dev/null +++ b/.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. +This fix doesn't merge adjacent bare exports. + +```diff +- export { b, a }; ++ export { a, b }; +``` diff --git a/crates/biome_js_analyze/src/assist/source/organize_imports.rs b/crates/biome_js_analyze/src/assist/source/organize_imports.rs index e5916b4e4555..406f948f210b 100644 --- a/crates/biome_js_analyze/src/assist/source/organize_imports.rs +++ 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: false, + 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) { @@ -1148,11 +1174,11 @@ fn merge( let clause2 = clause2.as_js_export_named_from_clause()?; let specifiers1 = clause1.specifiers(); let specifiers2 = clause2.specifiers(); - if let Some(meregd_specifiers) = - merge_export_specifiers(&specifiers1, &specifiers2, sort_order) + if let Some(merged_specifiers) = + 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()); + let merged_specifiers = clause1.with_specifiers(merged_specifiers); + let merged_item = item2.clone().with_export_clause(merged_specifiers.into()); let item1_leading_trivia = item1.syntax().first_leading_trivia()?; let merged_item = if item1_leading_trivia.is_empty() { @@ -1215,10 +1241,10 @@ fn merge( return None; }; let specifiers2 = clause2.named_specifiers().ok()?; - if let Some(meregd_specifiers) = + if let Some(merged_specifiers) = merge_import_specifiers(specifiers1, &specifiers2, sort_order) { - let merged_clause = clause1.with_specifier(meregd_specifiers.into()); + let merged_clause = clause1.with_specifier(merged_specifiers.into()); let merged_item = item2.clone().with_import_clause(merged_clause.into()); let item1_leading_trivia = item1.syntax().first_leading_trivia()?; @@ -1238,10 +1264,10 @@ fn merge( ) => { let specifiers1 = clause1.named_specifiers().ok()?; let specifiers2 = clause2.named_specifiers().ok()?; - if let Some(meregd_specifiers) = + if let Some(merged_specifiers) = merge_import_specifiers(specifiers1, &specifiers2, sort_order) { - let merged_clause = clause1.with_named_specifiers(meregd_specifiers); + let merged_clause = clause1.with_named_specifiers(merged_specifiers); let merged_item = item2.clone().with_import_clause(merged_clause.into()); let item1_leading_trivia = item1.syntax().first_leading_trivia()?; let merged_item = if item1_leading_trivia.is_empty() { diff --git a/crates/biome_js_analyze/src/assist/source/organize_imports/specifiers_attributes.rs b/crates/biome_js_analyze/src/assist/source/organize_imports/specifiers_attributes.rs index 67100e53d01f..fb8235df136a 100644 --- a/crates/biome_js_analyze/src/assist/source/organize_imports/specifiers_attributes.rs +++ b/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,15 +12,19 @@ use std::cmp::Ordering; pub enum JsNamedSpecifiers { JsNamedImportSpecifiers(JsNamedImportSpecifiers), JsExportNamedFromSpecifierList(JsExportNamedFromSpecifierList), + JsExportNamedSpecifierList(JsExportNamedSpecifierList), } impl JsNamedSpecifiers { pub fn are_sorted(&self, sort_order: SortOrder) -> bool { match self { - Self::JsNamedImportSpecifiers(specifeirs) => { - are_import_specifiers_sorted(specifeirs, sort_order) + Self::JsNamedImportSpecifiers(specifiers) => { + are_import_specifiers_sorted(specifiers, sort_order) } - Self::JsExportNamedFromSpecifierList(specifeirs) => { - are_export_specifiers_sorted(specifeirs, sort_order) + Self::JsExportNamedFromSpecifierList(specifiers) => { + are_export_from_specifiers_sorted(specifiers, sort_order) + } + Self::JsExportNamedSpecifierList(specifiers) => { + are_export_specifiers_sorted(specifiers, sort_order) } } // Assume the import is already sorted if there are any bogus nodes, otherwise the `--write` @@ -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 { @@ -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 { @@ -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,53 @@ 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 { + 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 { + 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 are_import_attributes_sorted( attributes: &JsImportAssertion, sort_order: SortOrder, diff --git a/crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-from-less-export.js b/crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-from-less-export.js new file mode 100644 index 000000000000..f7aa2368adba --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-from-less-export.js @@ -0,0 +1 @@ +export { b, a } diff --git a/crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-from-less-export.js.snap b/crates/biome_js_analyze/tests/specs/source/organizeImports/unsorted-from-less-export.js.snap new file mode 100644 index 000000000000..d2f45d15e6ca --- /dev/null +++ b/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 │ + + +``` diff --git a/crates/biome_lsp/src/server.tests.rs b/crates/biome_lsp/src/server.tests.rs index 40675ba98428..4c47a18a7e15 100644 --- a/crates/biome_lsp/src/server.tests.rs +++ b/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) {} "#,