From 111c437798ee36d7900ae1d53d283430839ee142 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Thu, 12 Mar 2026 22:03:14 +0000 Subject: [PATCH] fix(lsp): align organize imports output with tsgo --- cli/lsp/code_lens.rs | 23 ++- cli/lsp/ts_server.rs | 75 ++++------ cli/lsp/tsc.rs | 102 +++++++------ cli/lsp/tsgo.rs | 2 +- tests/integration/lsp_tests.rs | 262 +++++++++------------------------ 5 files changed, 161 insertions(+), 303 deletions(-) diff --git a/cli/lsp/code_lens.rs b/cli/lsp/code_lens.rs index b2b7cf3244d5cc..59c11ae20771c7 100644 --- a/cli/lsp/code_lens.rs +++ b/cli/lsp/code_lens.rs @@ -3,7 +3,6 @@ use std::cell::RefCell; use std::collections::HashSet; use std::rc::Rc; -use std::sync::Arc; use deno_ast::ParsedSource; use deno_ast::SourceRange; @@ -399,7 +398,7 @@ pub fn collect_test( pub fn collect_tsc( uri: &Uri, code_lens_settings: &CodeLensSettings, - line_index: Arc, + line_index: &LineIndex, navigation_tree: &NavigationTree, token: &CancellationToken, ) -> Result, AnyError> { @@ -412,7 +411,7 @@ pub fn collect_tsc( let source = CodeLensSource::Implementations; match i.kind { tsc::ScriptElementKind::InterfaceElement => { - code_lenses.push(i.to_code_lens(line_index.clone(), uri, source)); + code_lenses.push(i.to_code_lens(line_index, uri, source)); } tsc::ScriptElementKind::ClassElement | tsc::ScriptElementKind::MemberFunctionElement @@ -420,7 +419,7 @@ pub fn collect_tsc( | tsc::ScriptElementKind::MemberGetAccessorElement | tsc::ScriptElementKind::MemberSetAccessorElement => { if ABSTRACT_MODIFIER.is_match(&i.kind_modifiers) { - code_lenses.push(i.to_code_lens(line_index.clone(), uri, source)); + code_lenses.push(i.to_code_lens(line_index, uri, source)); } } _ => (), @@ -433,30 +432,30 @@ pub fn collect_tsc( if let Some(parent) = &mp && parent.kind == tsc::ScriptElementKind::EnumElement { - code_lenses.push(i.to_code_lens(line_index.clone(), uri, source)); + code_lenses.push(i.to_code_lens(line_index, uri, source)); } match i.kind { tsc::ScriptElementKind::FunctionElement => { if code_lens_settings.references_all_functions { - code_lenses.push(i.to_code_lens(line_index.clone(), uri, source)); + code_lenses.push(i.to_code_lens(line_index, uri, source)); } } tsc::ScriptElementKind::ConstElement | tsc::ScriptElementKind::LetElement | tsc::ScriptElementKind::VariableElement => { if EXPORT_MODIFIER.is_match(&i.kind_modifiers) { - code_lenses.push(i.to_code_lens(line_index.clone(), uri, source)); + code_lenses.push(i.to_code_lens(line_index, uri, source)); } } tsc::ScriptElementKind::ClassElement => { if i.text != "" { - code_lenses.push(i.to_code_lens(line_index.clone(), uri, source)); + code_lenses.push(i.to_code_lens(line_index, uri, source)); } } tsc::ScriptElementKind::InterfaceElement | tsc::ScriptElementKind::TypeElement | tsc::ScriptElementKind::EnumElement => { - code_lenses.push(i.to_code_lens(line_index.clone(), uri, source)); + code_lenses.push(i.to_code_lens(line_index, uri, source)); } tsc::ScriptElementKind::LocalFunctionElement | tsc::ScriptElementKind::MemberFunctionElement @@ -471,11 +470,7 @@ pub fn collect_tsc( tsc::ScriptElementKind::ClassElement | tsc::ScriptElementKind::InterfaceElement | tsc::ScriptElementKind::TypeElement => { - code_lenses.push(i.to_code_lens( - line_index.clone(), - uri, - source, - )); + code_lenses.push(i.to_code_lens(line_index, uri, source)); } _ => (), } diff --git a/cli/lsp/ts_server.rs b/cli/lsp/ts_server.rs index 429ee5ed73f911..3e766880c5893a 100644 --- a/cli/lsp/ts_server.rs +++ b/cli/lsp/ts_server.rs @@ -6,7 +6,6 @@ use std::sync::Arc; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; use deno_core::futures::future::Shared; -use deno_core::serde_json::json; use deno_resolver::deno_json::CompilerOptionsKey; use indexmap::IndexMap; use indexmap::IndexSet; @@ -269,7 +268,7 @@ impl TsServer { let code_lenses = crate::lsp::code_lens::collect_tsc( &module.uri, settings, - module.line_index.clone(), + &module.line_index, &navigation_tree, token, )?; @@ -310,7 +309,7 @@ impl TsServer { return Err(anyhow!("request cancelled")); } item.collect_document_symbols( - module.line_index.clone(), + &module.line_index, &mut document_symbols, ); } @@ -492,18 +491,8 @@ impl TsServer { o.contains(&lsp::CodeActionKind::SOURCE_ORGANIZE_IMPORTS) }) { - let document_has_errors = context.diagnostics.iter().any(|d| { - // Assume diagnostics without a severity are errors - d.severity - .is_none_or(|s| s == lsp::DiagnosticSeverity::ERROR) - }); let organize_imports_edit = ts_server - .organize_imports( - snapshot.clone(), - module, - document_has_errors, - token, - ) + .organize_imports(snapshot.clone(), module, token) .await .map_err(|err| { anyhow!( @@ -511,21 +500,26 @@ impl TsServer { err ) })?; - if !organize_imports_edit.is_empty() { - let changes_with_modules = organize_imports_edit + let text_edits = organize_imports_edit.first().map(|c| { + c.text_changes .iter() - .map(|c| (c, module)) - .collect::>(); + .map(|c| c.as_text_edit(&module.line_index)) + .collect::>() + }); + if let Some(text_edits) = text_edits + && !text_edits.is_empty() + { actions.push(lsp::CodeActionOrCommand::CodeAction( lsp::CodeAction { - title: "Organize imports".to_string(), + title: "Organize Imports".to_string(), kind: Some(lsp::CodeActionKind::SOURCE_ORGANIZE_IMPORTS), - edit: file_text_changes_to_workspace_edit( - changes_with_modules, - &snapshot, - token, - )?, - data: Some(json!({ "uri": &module.uri})), + edit: Some(lsp::WorkspaceEdit { + changes: Some( + std::iter::once((module.uri.as_ref().clone(), text_edits)) + .collect(), + ), + ..Default::default() + }), ..Default::default() }, )); @@ -564,11 +558,9 @@ impl TsServer { highlights .into_iter() .map(|dh| { - dh.to_highlight(module.line_index.clone(), token).map_err( - |err| { - anyhow!("Unable to convert document highlights: {:#}", err) - }, - ) + dh.to_highlight(&module.line_index, token).map_err(|err| { + anyhow!("Unable to convert document highlights: {:#}", err) + }) }) .collect::, _>>() .map(|s| s.into_iter().flatten().collect()) @@ -691,7 +683,7 @@ impl TsServer { .map(|completion_info| { completion_info .as_completion_response( - module.line_index.clone(), + &module.line_index, &snapshot .config .language_settings_for_specifier(&module.specifier) @@ -860,7 +852,7 @@ impl TsServer { return Err(anyhow!("request cancelled")); } Ok(span.to_folding_range( - module.line_index.clone(), + &module.line_index, module.text.as_bytes(), snapshot.config.line_folding_only_capable(), )) @@ -1112,9 +1104,8 @@ impl TsServer { token, ) .await?; - selection_ranges.push( - selection_range.to_selection_range(module.line_index.clone()), - ); + selection_ranges + .push(selection_range.to_selection_range(&module.line_index)); } Ok(Some(selection_ranges)) } @@ -1144,7 +1135,7 @@ impl TsServer { token, ) .await? - .to_semantic_tokens(module.line_index.clone(), token), + .to_semantic_tokens(&module.line_index, token), // TODO(nayeemrmn): Fix when tsgo supports semantic tokens. Self::Go(_) => Ok(Default::default()), } @@ -1184,7 +1175,7 @@ impl TsServer { token, ) .await? - .to_semantic_tokens(module.line_index.clone(), token)?, + .to_semantic_tokens(&module.line_index, token)?, // TODO(nayeemrmn): Fix when tsgo supports semantic tokens. Self::Go(_) => Default::default(), }; @@ -1325,12 +1316,10 @@ impl TsServer { ) -> Result>, AnyError> { match self { Self::Js(ts_server) => { - let text_span = - tsc::TextSpan::from_range(range, module.line_index.clone()).map_err( - |err| { - anyhow!("Failed to convert range to tsc text span: {:#}", err) - }, - )?; + let text_span = tsc::TextSpan::from_range(range, &module.line_index) + .map_err(|err| { + anyhow!("Failed to convert range to tsc text span: {:#}", err) + })?; let mut inlay_hints = ts_server .provide_inlay_hints(snapshot.clone(), module, text_span, token) .await; diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 5a331ed3655edb..e0b9483bf0ab16 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -1512,7 +1512,6 @@ impl TsJsServer { &self, snapshot: Arc, module: &DocumentModule, - document_has_errors: bool, token: &CancellationToken, ) -> Result, AnyError> { let req = TscRequest::OrganizeImports(( @@ -1523,11 +1522,7 @@ impl TsJsServer { .specifier_map .denormalize(&module.specifier, module.media_type), }, - mode: if document_has_errors { - Some(OrganizeImportsMode::SortAndCombine) - } else { - Some(OrganizeImportsMode::All) - }, + mode: Some(OrganizeImportsMode::All), }, (&snapshot .config @@ -1938,14 +1933,14 @@ pub struct TextSpan { impl TextSpan { pub fn from_range( range: lsp::Range, - line_index: Arc, + line_index: &LineIndex, ) -> Result { let start = line_index.offset_tsc(range.start)?; let length = line_index.offset_tsc(range.end)? - start; Ok(Self { start, length }) } - pub fn to_range(&self, line_index: Arc) -> lsp::Range { + pub fn to_range(&self, line_index: &LineIndex) -> lsp::Range { lsp::Range { start: line_index.position_utf16(self.start.into()), end: line_index.position_utf16(TextSize::from(self.start + self.length)), @@ -2154,7 +2149,7 @@ impl QuickInfo { kind: lsp::MarkupKind::Markdown, value, }), - range: Some(self.text_span.to_range(module.line_index.clone())), + range: Some(self.text_span.to_range(&module.line_index)), } } } @@ -2195,21 +2190,21 @@ impl DocumentSpan { let (target_range, target_selection_range) = if let Some(context_span) = &self.context_span { ( - context_span.to_range(target_module.line_index.clone()), - self.text_span.to_range(target_module.line_index.clone()), + context_span.to_range(&target_module.line_index), + self.text_span.to_range(&target_module.line_index), ) } else { ( - self.text_span.to_range(target_module.line_index.clone()), - self.text_span.to_range(target_module.line_index.clone()), + self.text_span.to_range(&target_module.line_index), + self.text_span.to_range(&target_module.line_index), ) }; let origin_selection_range = if let Some(original_context_span) = &self.original_context_span { - Some(original_context_span.to_range(origin_module.line_index.clone())) + Some(original_context_span.to_range(&origin_module.line_index)) } else { self.original_text_span.as_ref().map(|original_text_span| { - original_text_span.to_range(origin_module.line_index.clone()) + original_text_span.to_range(&origin_module.line_index) }) }; let link = lsp::LocationLink { @@ -2235,7 +2230,7 @@ impl DocumentSpan { module.scope.as_deref(), Some(&module.compiler_options_key), )?; - let range = self.text_span.to_range(target_module.line_index.clone()); + let range = self.text_span.to_range(&target_module.line_index); let mut target = uri_to_url(&target_module.uri); target.set_fragment(Some(&format!( "{},{}-{},{}", @@ -2318,7 +2313,7 @@ impl NavigateToItem { scope, Some(compiler_options_key), )?; - let range = self.text_span.to_range(target_module.line_index.clone()); + let range = self.text_span.to_range(&target_module.line_index); let location = lsp::Location { uri: target_module.uri.as_ref().clone(), range, @@ -2373,7 +2368,7 @@ impl InlayHintDisplayPart { let range = self .span .as_ref() - .map(|s| s.to_range(target_module.line_index.clone())) + .map(|s| s.to_range(&target_module.line_index)) .unwrap_or_else(|| { lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 0)) }); @@ -2461,7 +2456,7 @@ pub struct NavigationTree { impl NavigationTree { pub fn to_code_lens( &self, - line_index: Arc, + line_index: &LineIndex, uri: &Uri, source: code_lens::CodeLensSource, ) -> lsp::CodeLens { @@ -2485,7 +2480,7 @@ impl NavigationTree { pub fn collect_document_symbols( &self, - line_index: Arc, + line_index: &LineIndex, document_symbols: &mut Vec, ) -> bool { let mut should_include = self.should_include_entry(); @@ -2515,8 +2510,8 @@ impl NavigationTree { }) .any(|child_range| range.intersect(child_range).is_some()); if should_traverse_child { - let included_child = child - .collect_document_symbols(line_index.clone(), &mut symbol_children); + let included_child = + child.collect_document_symbols(line_index, &mut symbol_children); should_include = should_include || included_child; } } @@ -2560,8 +2555,8 @@ impl NavigationTree { document_symbols.push(lsp::DocumentSymbol { name, kind: self.kind.clone().into(), - range: span.to_range(line_index.clone()), - selection_range: selection_span.to_range(line_index.clone()), + range: span.to_range(line_index), + selection_range: selection_span.to_range(line_index), tags, children, detail: None, @@ -2707,7 +2702,7 @@ impl RenameLocation { range: location .document_span .text_span - .to_range(target_module.line_index.clone()), + .to_range(&target_module.line_index), new_text, }); } @@ -2796,7 +2791,7 @@ pub struct DocumentHighlights { impl DocumentHighlights { pub fn to_highlight( &self, - line_index: Arc, + line_index: &LineIndex, token: &CancellationToken, ) -> Result, AnyError> { let mut highlights = Vec::with_capacity(self.highlight_spans.len()); @@ -2805,7 +2800,7 @@ impl DocumentHighlights { return Err(anyhow!("request cancelled")); } highlights.push(lsp::DocumentHighlight { - range: hs.text_span.to_range(line_index.clone()), + range: hs.text_span.to_range(line_index), kind: match hs.kind { HighlightSpanKind::WrittenReference => { Some(lsp::DocumentHighlightKind::WRITE) @@ -2826,7 +2821,7 @@ pub struct TextChange { } impl TextChange { - pub fn as_text_edit(&self, line_index: Arc) -> lsp::TextEdit { + pub fn as_text_edit(&self, line_index: &LineIndex) -> lsp::TextEdit { lsp::TextEdit { range: self.span.to_range(line_index), new_text: self.new_text.clone(), @@ -2835,7 +2830,7 @@ impl TextChange { pub fn as_text_or_annotated_text_edit( &self, - line_index: Arc, + line_index: &LineIndex, ) -> lsp::OneOf { lsp::OneOf::Left(lsp::TextEdit { range: self.span.to_range(line_index), @@ -2884,12 +2879,12 @@ impl FileTextChanges { .or_else(|| url_to_uri(&target_specifier).ok().map(Arc::new))?; let line_index = target_module .as_ref() - .map(|m| m.line_index.clone()) - .unwrap_or_else(|| Arc::new(LineIndex::new(""))); + .map(|m| Cow::Borrowed(m.line_index.as_ref())) + .unwrap_or_else(|| Cow::Owned(LineIndex::new(""))); let edits = self .text_changes .iter() - .map(|tc| tc.as_text_edit(line_index.clone())) + .map(|tc| tc.as_text_edit(&line_index)) .collect(); Some((target_uri.as_ref().clone(), edits)) } @@ -2917,8 +2912,8 @@ impl FileTextChanges { .or_else(|| url_to_uri(&target_specifier).ok().map(Arc::new))?; let line_index = target_module .as_ref() - .map(|m| m.line_index.clone()) - .unwrap_or_else(|| Arc::new(LineIndex::new(""))); + .map(|m| Cow::Borrowed(m.line_index.as_ref())) + .unwrap_or_else(|| Cow::Owned(LineIndex::new(""))); if is_new_file { ops.push(lsp::DocumentChangeOperation::Op(lsp::ResourceOp::Create( @@ -2936,7 +2931,7 @@ impl FileTextChanges { let edits = self .text_changes .iter() - .map(|tc| tc.as_text_or_annotated_text_edit(line_index.clone())) + .map(|tc| tc.as_text_or_annotated_text_edit(&line_index)) .collect(); ops.push(lsp::DocumentChangeOperation::Edit(lsp::TextDocumentEdit { text_document: lsp::OptionalVersionedTextDocumentIdentifier { @@ -2962,7 +2957,7 @@ pub struct Classifications { impl Classifications { pub fn to_semantic_tokens( &self, - line_index: Arc, + line_index: &LineIndex, token: &CancellationToken, ) -> Result { // https://github.com/microsoft/vscode/blob/1.89.0/extensions/typescript-language-features/src/languageFeatures/semanticTokens.ts#L89-L115 @@ -3373,7 +3368,7 @@ impl ReferenceEntry { range: self .document_span .text_span - .to_range(target_module.line_index.clone()), + .to_range(&target_module.line_index), }) } } @@ -3449,10 +3444,10 @@ impl CallHierarchyItem { uri: target_module.uri.as_ref().clone(), detail: self.container_name.clone(), kind: self.kind.clone().into(), - range: self.span.to_range(target_module.line_index.clone()), + range: self.span.to_range(&target_module.line_index), selection_range: self .selection_span - .to_range(target_module.line_index.clone()), + .to_range(&target_module.line_index), data: None, }, target_module, @@ -3494,7 +3489,7 @@ impl CallHierarchyIncomingCall { from_ranges: self .from_spans .iter() - .map(|span| span.to_range(target_module.line_index.clone())) + .map(|span| span.to_range(&target_module.line_index)) .collect(), }) } @@ -3527,7 +3522,7 @@ impl CallHierarchyOutgoingCall { from_ranges: self .from_spans .iter() - .map(|span| span.to_range(module.line_index.clone())) + .map(|span| span.to_range(&module.line_index)) .collect(), }) } @@ -3551,7 +3546,7 @@ fn parse_code_actions( for change in &ts_action.changes { if module.specifier.as_str() == change.file_name { additional_text_edits.extend(change.text_changes.iter().map(|tc| { - let mut text_edit = tc.as_text_edit(module.line_index.clone()); + let mut text_edit = tc.as_text_edit(&module.line_index); if let Some(specifier_rewrite) = &data.specifier_rewrite { let specifier_index = text_edit .new_text @@ -3860,7 +3855,7 @@ impl CompletionInfo { #[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all, fields(entries = %self.entries.len())))] pub fn as_completion_response( &self, - line_index: Arc, + line_index: &LineIndex, settings: &config::CompletionSettings, module: &DocumentModule, position: u32, @@ -3877,7 +3872,7 @@ impl CompletionInfo { return Err(anyhow!("request cancelled")); } if let Some(item) = entry.as_completion_item( - line_index.clone(), + line_index, self, settings, module, @@ -4125,7 +4120,7 @@ impl CompletionEntry { #[allow(clippy::too_many_arguments)] fn as_completion_item( &self, - line_index: Arc, + line_index: &LineIndex, info: &CompletionInfo, settings: &config::CompletionSettings, module: &DocumentModule, @@ -4349,11 +4344,11 @@ const FOLD_END_PAIR_CHARACTERS: &[u8] = b"}])`"; impl OutliningSpan { pub fn to_folding_range( &self, - line_index: Arc, + line_index: &LineIndex, content: &[u8], line_folding_only: bool, ) -> lsp::FoldingRange { - let range = self.text_span.to_range(line_index.clone()); + let range = self.text_span.to_range(line_index); lsp::FoldingRange { start_line: range.start.line, start_character: if line_folding_only { @@ -4380,7 +4375,7 @@ impl OutliningSpan { fn adjust_folding_end_line( &self, range: &lsp::Range, - line_index: Arc, + line_index: &LineIndex, content: &[u8], line_folding_only: bool, ) -> u32 { @@ -4552,10 +4547,10 @@ pub struct SelectionRange { impl SelectionRange { pub fn to_selection_range( &self, - line_index: Arc, + line_index: &LineIndex, ) -> lsp::SelectionRange { lsp::SelectionRange { - range: self.text_span.to_range(line_index.clone()), + range: self.text_span.to_range(line_index), parent: self.parent.as_ref().map(|parent_selection| { Box::new(parent_selection.to_selection_range(line_index)) }), @@ -5793,7 +5788,10 @@ pub struct CombinedCodeFixScope { #[derive(Debug, Serialize, Clone)] pub enum OrganizeImportsMode { All, + #[allow(unused)] SortAndCombine, + #[allow(unused)] + RemoveUnused, } #[derive(Debug, Serialize, Clone)] @@ -6941,12 +6939,12 @@ mod tests { #[test] fn test_classification_to_semantic_tokens_multiline_tokens() { - let line_index = Arc::new(LineIndex::new(" to\nken \n")); + let line_index = LineIndex::new(" to\nken \n"); let classifications = Classifications { spans: vec![2, 6, 2057], }; let semantic_tokens = classifications - .to_semantic_tokens(line_index, &Default::default()) + .to_semantic_tokens(&line_index, &Default::default()) .unwrap(); assert_eq!( &semantic_tokens.data, diff --git a/cli/lsp/tsgo.rs b/cli/lsp/tsgo.rs index 359bdea40f0b18..fc56f7edff2f08 100644 --- a/cli/lsp/tsgo.rs +++ b/cli/lsp/tsgo.rs @@ -2055,7 +2055,7 @@ fn normalize_code_action_response( code_action .kind .as_ref() - .is_none_or(|k| k.as_str() != "source.organizeImports") + .is_none_or(|k| *k != lsp::CodeActionKind::SOURCE_ORGANIZE_IMPORTS) }); } for item in response { diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 515a01a7086558..77f18e143e8482 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -7917,10 +7917,8 @@ fn lsp_quote_style_from_workspace_settings() { client.shutdown(); } -// TODO(nayeemrmn): Enable for tsgo when the upstream commit lands in our fork: -// https://github.com/microsoft/typescript-go/commit/ffa96d57ad5af333fe66f7cb1b7a4f3041000d8e -#[test(timeout = 300)] -fn lsp_code_actions_organize_imports() { +#[test(timeout = 300, fork_with_suffix = "_tsgo")] +fn lsp_code_actions_organize_imports(use_tsgo: bool) { let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); let file = temp_dir.source_file( @@ -7933,8 +7931,7 @@ import unused from "./c.ts"; console.log(b, a, c, d, y, z); "#, ); - let uri = file.uri(); - let mut client = context.new_lsp_command().build(); + let mut client = context.new_lsp_command().set_use_tsgo(use_tsgo).build(); client.initialize_default(); client.did_open_file(&file); @@ -7942,7 +7939,7 @@ console.log(b, a, c, d, y, z); let res = client.write_request( "textDocument/codeAction", json!({ - "textDocument": { "uri": uri }, + "textDocument": { "uri": file.uri() }, "range": { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 } @@ -7956,61 +7953,52 @@ console.log(b, a, c, d, y, z); let expected = json!([ { - "title": "Organize imports", + "title": "Organize Imports", "kind": "source.organizeImports", "edit": { - "documentChanges": [ - { - "textDocument": { - "uri": uri, - "version": 1 + "changes": { + file.uri().as_str(): [ + { + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 1, "character": 0 }, + }, + // All organized imports in the first replacement: + // files sorted alphabetically, imports within files sorted alphabetically + "newText": concat!( + "import { d } from \"./a.ts\";\n", + "import { a, b, c } from \"./b.ts\";\n", + "import { y, z } from \"./z.ts\";\n", + ), }, - "edits": [ - { - "range": { - "start": { "line": 0, "character": 0 }, - "end": { "line": 1, "character": 0 } - }, - // All organized imports in the first replacement: - // files sorted alphabetically, imports within files sorted alphabetically - "newText": concat!( - "import { d } from \"./a.ts\";\n", - "import { a, b, c } from \"./b.ts\";\n", - "import { y, z } from \"./z.ts\";\n", - ) + { + "range": { + "start": { "line": 1, "character": 0 }, + "end": { "line": 2, "character": 0 }, }, - { - "range": { - "start": { "line": 1, "character": 0 }, - "end": { "line": 2, "character": 0 } - }, - // Second line removed (replaced by organized imports above) - "newText": "" + // Second line removed (replaced by organized imports above) + "newText": "", + }, + { + "range": { + "start": { "line": 2, "character": 0 }, + "end": { "line": 3, "character": 0 }, }, - { - "range": { - "start": { "line": 2, "character": 0 }, - "end": { "line": 3, "character": 0 } - }, - // Third line removed (replaced by organized imports above) - "newText": "" + // Third line removed (replaced by organized imports above) + "newText": "", + }, + { + "range": { + "start": { "line": 3, "character": 0 }, + "end": { "line": 4, "character": 0 }, }, - { - "range": { - "start": { "line": 3, "character": 0 }, - "end": { "line": 4, "character": 0 } - }, - // Unused import from "./c.ts" is removed - "newText": "" - } - ] - } - ] + // Unused import from "./c.ts" is removed + "newText": "", + }, + ], + }, }, - "data": { - "uri": uri - } - } + }, ]); assert_eq!(res, expected); @@ -8078,110 +8066,8 @@ console.log(a, b, c); client.shutdown(); } -// TODO(nayeemrmn): Enable for tsgo when the upstream commit lands in our fork: -// https://github.com/microsoft/typescript-go/commit/ffa96d57ad5af333fe66f7cb1b7a4f3041000d8e -#[test(timeout = 300)] -fn lsp_code_actions_organize_imports_with_diagnostics() { - let context = TestContextBuilder::new().use_temp_cwd().build(); - let temp_dir = context.temp_dir(); - // File with unordered imports and a type error - let file = temp_dir.source_file( - "file.ts", - r#"import { b } from "./b.ts"; -import { a } from "./a.ts"; -import unused from "./c.ts"; - -// Type error: using undeclared variable -console.log(undeclaredVariable); -"#, - ); - let uri = file.uri(); - let mut client = context.new_lsp_command().build(); - client.initialize_default(); - client.did_open_file(&file); - - // Request "Organize Imports" action with diagnostics indicating an error - let res = client.write_request( - "textDocument/codeAction", - json!({ - "textDocument": { "uri": uri }, - "range": { - "start": { "line": 0, "character": 0 }, - "end": { "line": 0, "character": 0 } - }, - "context": { - "diagnostics": [ - { - "range": { - "start": { "line": 5, "character": 12 }, - "end": { "line": 5, "character": 29 } - }, - "severity": 1, - "message": "Cannot find name 'undeclaredVariable'.", - "source": "deno-ts" - } - ], - "only": ["source.organizeImports"] - } - }), - ); - - let expected = json!([ - { - "title": "Organize imports", - "kind": "source.organizeImports", - "edit": { - "documentChanges": [ - { - "textDocument": { - "uri": uri, - "version": 1 - }, - "edits": [ - { - "range": { - "start": { "line": 0, "character": 0 }, - "end": { "line": 1, "character": 0 } - }, - // Imports sorted alphabetically, but unused imports NOT removed due to error - "newText": concat!( - "import { a } from \"./a.ts\";\n", - "import { b } from \"./b.ts\";\n", - "import unused from \"./c.ts\";\n", - ) - }, - { - "range": { - "start": { "line": 1, "character": 0 }, - "end": { "line": 2, "character": 0 } - }, - "newText": "" - }, - { - "range": { - "start": { "line": 2, "character": 0 }, - "end": { "line": 3, "character": 0 } - }, - "newText": "" - } - ] - } - ] - }, - "data": { - "uri": uri - } - } - ]); - - assert_eq!(res, expected); - client.shutdown(); -} - -// TODO(nayeemrmn): Enable for tsgo when the upstream commit lands in our fork: -// https://github.com/microsoft/typescript-go/commit/ffa96d57ad5af333fe66f7cb1b7a4f3041000d8e -#[test(timeout = 300)] -fn lsp_code_actions_organize_imports_in_a_workspace() { +#[test(timeout = 300, fork_with_suffix = "_tsgo")] +fn lsp_code_actions_organize_imports_in_a_workspace(use_tsgo: bool) { let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( @@ -8222,8 +8108,7 @@ console.log(other, submodule); .to_string(), ); temp_dir.source_file("other/other.ts", r#"export const other = 0;"#); - let uri = file.uri(); - let mut client = context.new_lsp_command().build(); + let mut client = context.new_lsp_command().set_use_tsgo(use_tsgo).build(); client.initialize_default(); client.did_open_file(&file); @@ -8231,7 +8116,7 @@ console.log(other, submodule); let res = client.write_request( "textDocument/codeAction", json!({ - "textDocument": { "uri": uri }, + "textDocument": { "uri": file.uri() }, "range": { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 } @@ -8245,42 +8130,33 @@ console.log(other, submodule); let expected = json!([ { - "title": "Organize imports", + "title": "Organize Imports", "kind": "source.organizeImports", "edit": { - "documentChanges": [ - { - "textDocument": { - "uri": uri, - "version": 1 - }, + "changes": { + file.uri().as_str(): [ // Relative imports come after scoped imports. - "edits": [ - { - "range": { - "start": { "line": 0, "character": 0 }, - "end": { "line": 1, "character": 0 } - }, - "newText": concat!( - "import { other } from \"@scope/other\";\n", - "import { submodule } from \"./submodule.ts\";\n", - ) + { + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 1, "character": 0 }, }, - { - "range": { - "start": { "line": 1, "character": 0 }, - "end": { "line": 2, "character": 0 } - }, - "newText": "" - } - ] - } - ] + "newText": concat!( + "import { other } from \"@scope/other\";\n", + "import { submodule } from \"./submodule.ts\";\n", + ), + }, + { + "range": { + "start": { "line": 1, "character": 0 }, + "end": { "line": 2, "character": 0 }, + }, + "newText": "", + }, + ], + }, }, - "data": { - "uri": uri - } - } + }, ]); assert_eq!(res, expected);