Repository: denoland/deno
Pull request: feat(lsp): Add support for `source.organizeImports`

<!--
Before submitting a PR, please read https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(std/http): Fix race condition in server
    - docs(console): Update docstrings
    - feat(doc): Handle nested reexports

  Examples of bad title:
    - fix #7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
7. Open as a draft PR if your work is still in progress. The CI won't run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench' label.
-->
The removal of unused imports is made conditional based on the presence of existing diagnostics (errors) in the file.
This approach is modeled after the behavior found in the [typescript-language-server](https://github.com/typescript-language-server/typescript-language-server) to prevent unwanted import removal when a file is in an unstable or error state.

Closes #30840

Changed files:
filename: cli/lsp/capabilities.rs
patch: @@ -20,8 +20,11 @@ fn code_action_capabilities(
     .and_then(|it| it.code_action.as_ref())
     .and_then(|it| it.code_action_literal_support.as_ref())
     .map(|_| {
-      let mut code_action_kinds =
-        vec![CodeActionKind::QUICKFIX, CodeActionKind::REFACTOR];
+      let mut code_action_kinds = vec![
+        CodeActionKind::QUICKFIX,
+        CodeActionKind::REFACTOR,
+        CodeActionKind::SOURCE_ORGANIZE_IMPORTS,
+      ];
       code_action_kinds.extend(
         ALL_KNOWN_REFACTOR_ACTION_KINDS
           .iter()

filename: cli/lsp/language_server.rs
patch: @@ -2289,12 +2289,12 @@ impl Inner {
     code_actions.set_preferred_fixes();
     all_actions.extend(code_actions.get_response());
 
+    let kinds = params.context.only.unwrap_or_default();
+
     // Refactor
-    let only = params
-      .context
-      .only
-      .as_ref()
-      .and_then(|values| values.first().map(|v| v.as_str().to_owned()))
+    let only = kinds
+      .first()
+      .map(|v| v.as_str().to_owned())
       .unwrap_or_default();
     let refactor_infos = self
       .ts_server
@@ -2340,6 +2340,60 @@ impl Inner {
         .map(CodeActionOrCommand::CodeAction),
     );
 
+    // Organize imports
+    if kinds.is_empty()
+      || kinds.contains(&CodeActionKind::SOURCE_ORGANIZE_IMPORTS)
+    {
+      let document_has_errors = params.context.diagnostics.iter().any(|d| {
+        // Assume diagnostics without a severity are errors
+        d.severity.is_none_or(|s| s == DiagnosticSeverity::ERROR)
+      });
+      let organize_imports_edit = self
+        .ts_server
+        .organize_imports(self.snapshot(), &module, document_has_errors, token)
+        .await
+        .map_err(|err| {
+          if token.is_cancelled() {
+            LspError::request_cancelled()
+          } else {
+            error!(
+              "Unable to get organize imports edit from TypeScript: {:#}",
+              err
+            );
+            LspError::internal_error()
+          }
+        })?;
+
+      if !organize_imports_edit.is_empty() {
+        let mut changes_with_modules = IndexMap::new();
+        changes_with_modules.extend(
+          fix_ts_import_changes(&organize_imports_edit, &module, self, token)
+            .map_err(|err| {
+              if token.is_cancelled() {
+                LspError::request_cancelled()
+              } else {
+                error!("Unable to fix import changes: {:#}", err);
+                LspError::internal_error()
+              }
+            })?
+            .into_iter()
+            .map(|c| (c, module.clone())),
+        );
+
+        all_actions.push(CodeActionOrCommand::CodeAction(CodeAction {
+          title: "Organize imports".to_string(),
+          kind: Some(CodeActionKind::SOURCE_ORGANIZE_IMPORTS),
+          edit: file_text_changes_to_workspace_edit(
+            &changes_with_modules,
+            self,
+            token,
+          )?,
+          data: Some(json!({ "uri": params.text_document.uri})),
+          ..Default::default()
+        }));
+      }
+    }
+
     let code_action_disabled_capable =
       self.config.code_action_disabled_capable();
     let actions: Vec<CodeActionOrCommand> = all_actions.into_iter().filter(|ca| {

filename: cli/lsp/tsc.rs
patch: @@ -1492,6 +1492,60 @@ impl TsServer {
       .await
   }
 
+  #[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all))]
+  pub async fn organize_imports(
+    &self,
+    snapshot: Arc<StateSnapshot>,
+    module: &DocumentModule,
+    document_has_errors: bool,
+    token: &CancellationToken,
+  ) -> Result<Vec<FileTextChanges>, AnyError> {
+    let req = TscRequest::OrganizeImports((
+      OrganizeImportsArgs {
+        scope: CombinedCodeFixScope {
+          r#type: "file",
+          file_name: self
+            .specifier_map
+            .denormalize(&module.specifier, module.media_type),
+        },
+        mode: if document_has_errors {
+          Some(OrganizeImportsMode::SortAndCombine)
+        } else {
+          Some(OrganizeImportsMode::All)
+        },
+      },
+      (&snapshot
+        .config
+        .tree
+        .fmt_config_for_specifier(&module.specifier)
+        .options)
+        .into(),
+      Some(UserPreferences::from_config_for_specifier(
+        &snapshot.config,
+        &module.specifier,
+      )),
+    ));
+    self
+      .request::<Vec<FileTextChanges>>(
+        snapshot,
+        req,
+        &module.compiler_options_key,
+        module.scope.as_ref(),
+        module.notebook_uri.as_ref(),
+        token,
+      )
+      .await
+      .and_then(|mut changes| {
+        for file_change in &mut changes {
+          if token.is_cancelled() {
+            return Err(anyhow!("request cancelled"));
+          }
+          file_change.normalize(&self.specifier_map)?;
+        }
+        Ok(changes)
+      })
+  }
+
   async fn request<R>(
     &self,
     snapshot: Arc<StateSnapshot>,
@@ -5771,6 +5825,21 @@ pub struct CombinedCodeFixScope {
   file_name: String,
 }
 
+#[derive(Debug, Serialize, Clone)]
+pub enum OrganizeImportsMode {
+  All,
+  SortAndCombine,
+}
+
+#[derive(Debug, Serialize, Clone)]
+#[serde(rename_all = "camelCase")]
+pub struct OrganizeImportsArgs {
+  #[serde(flatten)]
+  pub scope: CombinedCodeFixScope,
+  #[serde(skip_serializing_if = "Option::is_none")]
+  pub mode: Option<OrganizeImportsMode>,
+}
+
 #[derive(Serialize, Clone, Copy)]
 #[allow(dead_code)]
 pub struct JsNull;
@@ -5883,6 +5952,14 @@ enum TscRequest {
   GetNavigateToItems((String, Option<u32>, Option<String>)),
   // https://github.com/denoland/deno/blob/v1.37.1/cli/tsc/dts/typescript.d.ts#L6239
   ProvideInlayHints((String, TextSpan, UserPreferences)),
+  // https://github.com/denoland/deno/blob/v2.5.2/cli/tsc/dts/typescript.d.ts#L6769
+  OrganizeImports(
+    (
+      OrganizeImportsArgs,
+      FormatCodeSettings,
+      Option<UserPreferences>,
+    ),
+  ),
 }
 
 impl TscRequest {
@@ -5984,6 +6061,9 @@ impl TscRequest {
       TscRequest::ProvideInlayHints(args) => {
         ("provideInlayHints", Some(serde_v8::to_v8(scope, args)?))
       }
+      TscRequest::OrganizeImports(args) => {
+        ("organizeImports", Some(serde_v8::to_v8(scope, args)?))
+      }
       TscRequest::CleanupSemanticCache => ("$cleanupSemanticCache", None),
     };
 
@@ -6030,6 +6110,7 @@ impl TscRequest {
       TscRequest::GetSignatureHelpItems(_) => "getSignatureHelpItems",
       TscRequest::GetNavigateToItems(_) => "getNavigateToItems",
       TscRequest::ProvideInlayHints(_) => "provideInlayHints",
+      TscRequest::OrganizeImports(_) => "organizeImports",
     }
   }
 }

filename: tests/integration/lsp_tests.rs
patch: @@ -7690,6 +7690,241 @@ fn lsp_quote_style_from_workspace_settings() {
   client.shutdown();
 }
 
+#[test]
+#[timeout(300_000)]
+fn lsp_code_actions_organize_imports() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+  let file = temp_dir.source_file(
+    "file.ts",
+    r#"import { z, y } from "./z.ts";
+import { c, a, b } from "./b.ts";
+import { d } from "./a.ts";
+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();
+  client.initialize_default();
+  client.did_open_file(&file);
+
+  // Request "Organize Imports" action
+  let res = client.write_request(
+    "textDocument/codeAction",
+    json!({
+      "textDocument": { "uri": uri },
+      "range": {
+        "start": { "line": 0, "character": 0 },
+        "end": { "line": 0, "character": 0 }
+      },
+      "context": {
+        "diagnostics": [],
+        "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 }
+                },
+                // 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 }
+                },
+                // Second line removed (replaced by organized imports above)
+                "newText": ""
+              },
+              {
+                "range": {
+                  "start": { "line": 2, "character": 0 },
+                  "end": { "line": 3, "character": 0 }
+                },
+                // Third line removed (replaced by organized imports above)
+                "newText": ""
+              },
+              {
+                "range": {
+                  "start": { "line": 3, "character": 0 },
+                  "end": { "line": 4, "character": 0 }
+                },
+                // Unused import from "./c.ts" is removed
+                "newText": ""
+              }
+            ]
+          }
+        ]
+      },
+      "data": {
+        "uri": uri
+      }
+    }
+  ]);
+
+  assert_eq!(res, expected);
+  client.shutdown();
+}
+
+#[test]
+#[timeout(300_000)]
+fn lsp_code_actions_organize_imports_already_organized() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let tmp_dir = context.temp_dir();
+  let file = tmp_dir.source_file(
+    "file.ts",
+    r#"import { a, b, c } from "./b.ts";
+
+console.log(a, b, c);
+"#,
+  );
+  let uri = file.uri();
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  client.did_open_file(&file);
+
+  // Request "Organize Imports" action
+  let res = client.write_request(
+    "textDocument/codeAction",
+    json!({
+      "textDocument": { "uri": uri },
+      "range": {
+        "start": { "line": 0, "character": 0 },
+        "end": { "line": 0, "character": 0 }
+      },
+      "context": {
+        "diagnostics": [],
+        "only": ["source.organizeImports"]
+      }
+    }),
+  );
+
+  assert_eq!(res, json!(null));
+  client.shutdown();
+}
+
+#[test]
+#[timeout(300_000)]
+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();
+}
+
 #[test]
 #[timeout(300_000)]
 fn lsp_code_actions_refactor_no_disabled_support() {