Repository: denoland/deno
Pull request: fix(lsp): `organizeImports` without resolving specifiers

Organize imports action should move, group, or delete imports but not rewrite the imported modules.

The current behavior is causing a bug with workspaces, rewriting relative imports to scoped imports if the workspace members are loaded in the server. The two specifiers are not functionally equivalent, at least due to https://github.com/jsr-io/jsr/issues/1220.

This change removes the rewriting and keeps the imports as they are sent from tsc.

Fixes #31141

Changed files:
filename: cli/lsp/language_server.rs
patch: @@ -2347,21 +2347,10 @@ impl Inner {
         })?;
 
       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())),
-        );
-
+        let changes_with_modules = organize_imports_edit
+          .into_iter()
+          .map(|c| (c, module.clone()))
+          .collect::<IndexMap<_, _>>();
         all_actions.push(CodeActionOrCommand::CodeAction(CodeAction {
           title: "Organize imports".to_string(),
           kind: Some(CodeActionKind::SOURCE_ORGANIZE_IMPORTS),

filename: tests/integration/lsp_tests.rs
patch: @@ -7925,6 +7925,114 @@ console.log(undeclaredVariable);
   client.shutdown();
 }
 
+#[test]
+#[timeout(300_000)]
+fn lsp_code_actions_organize_imports_in_a_workspace() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write(
+    "deno.json",
+    json!({
+      "workspace": ["./member", "./other"]
+    })
+    .to_string(),
+  );
+  temp_dir.write(
+    "member/deno.json",
+    json!({
+      "name": "@scope/member",
+      "version": "1.0.0",
+      "exports": {
+        ".": "./member.ts",
+        "./submodule": "./submodule.ts"
+      }
+    })
+    .to_string(),
+  );
+  let file = temp_dir.source_file(
+    "member/member.ts",
+    r#"import { submodule } from "./submodule.ts";
+import { other } from "@scope/other";
+export const member = 0;
+console.log(other, submodule);
+"#,
+  );
+  temp_dir.source_file("member/submodule.ts", r#"export const submodule = 0;"#);
+  temp_dir.write(
+    "other/deno.json",
+    json!({
+      "name": "@scope/other",
+      "version": "1.0.0",
+      "exports": "./other.ts"
+    })
+    .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();
+  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
+            },
+            // 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": 1, "character": 0 },
+                  "end": { "line": 2, "character": 0 }
+                },
+                "newText": ""
+              }
+            ]
+          }
+        ]
+      },
+      "data": {
+        "uri": uri
+      }
+    }
+  ]);
+
+  assert_eq!(res, expected);
+  client.shutdown();
+}
+
 #[test]
 #[timeout(300_000)]
 fn lsp_code_actions_refactor_no_disabled_support() {