fix(#10765): lsp import fixes include extensions (#10778)

Fixes #10765

cli/lsp/analysis.rs

@@ -11,6 +11,7 @@ use crate::module_graph::parse_ts_reference;
 use crate::module_graph::TypeScriptReference;
 use crate::tools::lint::create_linter;
 
+use deno_core::error::anyhow;
 use deno_core::error::custom_error;
 use deno_core::error::AnyError;
 use deno_core::serde::Deserialize;
@@ -23,6 +24,7 @@ use deno_lint::rules;
 use lspower::lsp;
 use lspower::lsp::Position;
 use lspower::lsp::Range;
+use regex::Regex;
 use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::fmt;
@@ -56,8 +58,12 @@ lazy_static::lazy_static! {
   .iter()
   .cloned()
   .collect();
+
+  static ref IMPORT_SPECIFIER_RE: Regex = Regex::new(r#"\sfrom\s+["']([^"']*)["']"#).unwrap();
 }
 
+const SUPPORTED_EXTENSIONS: &[&str] = &[".ts", ".tsx", ".js", ".jsx", ".mjs"];
+
 /// Category of self-generated diagnostic messages (those not coming from)
 /// TypeScript.
 #[derive(Debug, PartialEq, Eq)]
@@ -417,6 +423,143 @@ fn code_as_string(code: &Option<lsp::NumberOrString>) -> String {
   }
 }
 
+/// Iterate over the supported extensions, concatenating the extension on the
+/// specifier, returning the first specifier that is resolve-able, otherwise
+/// None if none match.
+fn check_specifier(
+  specifier: &str,
+  referrer: &ModuleSpecifier,
+  snapshot: &language_server::StateSnapshot,
+  maybe_import_map: &Option<ImportMap>,
+) -> Option<String> {
+  for ext in SUPPORTED_EXTENSIONS {
+    let specifier_with_ext = format!("{}{}", specifier, ext);
+    if let ResolvedDependency::Resolved(resolved_specifier) =
+      resolve_import(&specifier_with_ext, referrer, maybe_import_map)
+    {
+      if snapshot.documents.contains_key(&resolved_specifier)
+        || snapshot.sources.contains_key(&resolved_specifier)
+      {
+        return Some(specifier_with_ext);
+      }
+    }
+  }
+
+  None
+}
+
+/// For a set of tsc changes, can them for any that contain something that looks
+/// like an import and rewrite the import specifier to include the extension
+pub(crate) fn fix_ts_import_changes(
+  referrer: &ModuleSpecifier,
+  changes: &[tsc::FileTextChanges],
+  language_server: &language_server::Inner,
+) -> Result<Vec<tsc::FileTextChanges>, AnyError> {
+  let mut r = Vec::new();
+  let snapshot = language_server.snapshot()?;
+  for change in changes {
+    let mut text_changes = Vec::new();
+    for text_change in &change.text_changes {
+      if let Some(captures) =
+        IMPORT_SPECIFIER_RE.captures(&text_change.new_text)
+      {
+        let specifier = captures
+          .get(1)
+          .ok_or_else(|| anyhow!("Missing capture."))?
+          .as_str();
+        if let Some(new_specifier) = check_specifier(
+          specifier,
+          referrer,
+          &snapshot,
+          &language_server.maybe_import_map,
+        ) {
+          let new_text =
+            text_change.new_text.replace(specifier, &new_specifier);
+          text_changes.push(tsc::TextChange {
+            span: text_change.span.clone(),
+            new_text,
+          });
+        } else {
+          text_changes.push(text_change.clone());
+        }
+      } else {
+        text_changes.push(text_change.clone());
+      }
+    }
+    r.push(tsc::FileTextChanges {
+      file_name: change.file_name.clone(),
+      text_changes,
+      is_new_file: change.is_new_file,
+    });
+  }
+  Ok(r)
+}
+
+/// Fix tsc import code actions so that the module specifier is correct for
+/// resolution by Deno (includes the extension).
+fn fix_ts_import_action(
+  referrer: &ModuleSpecifier,
+  action: &tsc::CodeFixAction,
+  language_server: &language_server::Inner,
+) -> Result<tsc::CodeFixAction, AnyError> {
+  if action.fix_name == "import" {
+    let change = action
+      .changes
+      .get(0)
+      .ok_or_else(|| anyhow!("Unexpected action changes."))?;
+    let text_change = change
+      .text_changes
+      .get(0)
+      .ok_or_else(|| anyhow!("Missing text change."))?;
+    if let Some(captures) = IMPORT_SPECIFIER_RE.captures(&text_change.new_text)
+    {
+      let specifier = captures
+        .get(1)
+        .ok_or_else(|| anyhow!("Missing capture."))?
+        .as_str();
+      let snapshot = language_server.snapshot()?;
+      if let Some(new_specifier) = check_specifier(
+        specifier,
+        referrer,
+        &snapshot,
+        &language_server.maybe_import_map,
+      ) {
+        let description = action.description.replace(specifier, &new_specifier);
+        let changes = action
+          .changes
+          .iter()
+          .map(|c| {
+            let text_changes = c
+              .text_changes
+              .iter()
+              .map(|tc| tsc::TextChange {
+                span: tc.span.clone(),
+                new_text: tc.new_text.replace(specifier, &new_specifier),
+              })
+              .collect();
+            tsc::FileTextChanges {
+              file_name: c.file_name.clone(),
+              text_changes,
+              is_new_file: c.is_new_file,
+            }
+          })
+          .collect();
+
+        return Ok(tsc::CodeFixAction {
+          description,
+          changes,
+          commands: None,
+          fix_name: action.fix_name.clone(),
+          fix_id: None,
+          fix_all_description: None,
+        });
+      }
+    }
+  }
+
+  Ok(action.clone())
+}
+
 /// Determines if two TypeScript diagnostic codes are effectively equivalent.
 fn is_equivalent_code(
   a: &Option<lsp::NumberOrString>,
@@ -547,6 +690,7 @@ impl CodeActionCollection {
   /// Add a TypeScript code fix action to the code actions collection.
   pub(crate) async fn add_ts_fix_action(
     &mut self,
+    specifier: &ModuleSpecifier,
     action: &tsc::CodeFixAction,
     diagnostic: &lsp::Diagnostic,
     language_server: &mut language_server::Inner,
@@ -564,6 +708,7 @@ impl CodeActionCollection {
         "The action returned from TypeScript is unsupported.",
       ));
     }
+    let action = fix_ts_import_action(specifier, action, language_server)?;
     let edit = ts_changes_to_edit(&action.changes, language_server).await?;
     let code_action = lsp::CodeAction {
       title: action.description.clone(),

cli/lsp/language_server.rs

@@ -29,6 +29,7 @@ use std::sync::Arc;
 use tokio::fs;
 
 use super::analysis;
+use super::analysis::fix_ts_import_changes;
 use super::analysis::ts_changes_to_edit;
 use super::analysis::CodeActionCollection;
 use super::analysis::CodeActionData;
@@ -964,7 +965,7 @@ impl Inner {
             };
           for action in actions {
             code_actions
-              .add_ts_fix_action(&action, diagnostic, self)
+              .add_ts_fix_action(&specifier, &action, diagnostic, self)
               .await
               .map_err(|err| {
                 error!("Unable to convert fix: {}", err);
@@ -1009,7 +1010,7 @@ impl Inner {
           LspError::invalid_params("The CodeAction's data is invalid.")
         })?;
       let req = tsc::RequestMethod::GetCombinedCodeFix((
-        code_action_data.specifier,
+        code_action_data.specifier.clone(),
         json!(code_action_data.fix_id.clone()),
       ));
       let combined_code_actions: tsc::CombinedCodeActions = self
@@ -1024,14 +1025,25 @@ impl Inner {
         error!("Deno does not support code actions with commands.");
         Err(LspError::invalid_request())
       } else {
+        let changes = if code_action_data.fix_id == "fixMissingImport" {
+          fix_ts_import_changes(
+            &code_action_data.specifier,
+            &combined_code_actions.changes,
+            self,
+          )
+          .map_err(|err| {
+            error!("Unable to remap changes: {}", err);
+            LspError::internal_error()
+          })?
+        } else {
+          combined_code_actions.changes.clone()
+        };
         let mut code_action = params.clone();
         code_action.edit =
-          ts_changes_to_edit(&combined_code_actions.changes, self)
+          ts_changes_to_edit(&changes, self).await.map_err(|err| {
-            .await
+            error!("Unable to convert changes to edits: {}", err);
-            .map_err(|err| {
+            LspError::internal_error()
-              error!("Unable to convert changes to edits: {}", err);
+          })?;
-              LspError::internal_error()
-            })?;
         Ok(code_action)
       }
     } else {

cli/lsp/tsc.rs

@@ -925,8 +925,8 @@ impl DocumentHighlights {
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
 #[serde(rename_all = "camelCase")]
 pub struct TextChange {
-  span: TextSpan,
+  pub span: TextSpan,
-  new_text: String,
+  pub new_text: String,
 }
 
 impl TextChange {
@@ -944,10 +944,10 @@ impl TextChange {
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
 #[serde(rename_all = "camelCase")]
 pub struct FileTextChanges {
-  file_name: String,
+  pub file_name: String,
-  text_changes: Vec<TextChange>,
+  pub text_changes: Vec<TextChange>,
   #[serde(skip_serializing_if = "Option::is_none")]
-  is_new_file: Option<bool>,
+  pub is_new_file: Option<bool>,
 }
 
 impl FileTextChanges {

cli/tests/integration_tests_lsp.rs

@@ -1425,6 +1425,57 @@ fn lsp_code_actions_deno_cache() {
   shutdown(&mut client);
 }
 
+#[test]
+fn lsp_code_actions_imports() {
+  let mut client = init("initialize_params.json");
+  did_open(
+    &mut client,
+    json!({
+      "textDocument": {
+        "uri": "file:///a/file00.ts",
+        "languageId": "typescript",
+        "version": 1,
+        "text": "export const abc = \"abc\";\nexport const def = \"def\";\n"
+      }
+    }),
+  );
+  did_open(
+    &mut client,
+    json!({
+      "textDocument": {
+        "uri": "file:///a/file01.ts",
+        "languageId": "typescript",
+        "version": 1,
+        "text": "\nconsole.log(abc);\nconsole.log(def)\n"
+      }
+    }),
+  );
+
+  let (maybe_res, maybe_err) = client
+    .write_request(
+      "textDocument/codeAction",
+      load_fixture("code_action_params_imports.json"),
+    )
+    .unwrap();
+  assert!(maybe_err.is_none());
+  assert_eq!(
+    maybe_res,
+    Some(load_fixture("code_action_response_imports.json"))
+  );
+  let (maybe_res, maybe_err) = client
+    .write_request(
+      "codeAction/resolve",
+      load_fixture("code_action_resolve_params_imports.json"),
+    )
+    .unwrap();
+  assert!(maybe_err.is_none());
+  assert_eq!(
+    maybe_res,
+    Some(load_fixture("code_action_resolve_response_imports.json"))
+  );
+  shutdown(&mut client);
+}
+
 #[test]
 fn lsp_code_actions_deadlock() {
   let mut client = init("initialize_params.json");

cli/tests/lsp/code_action_params_imports.json

@@ -0,0 +1,54 @@
+{
+  "textDocument": {
+    "uri": "file:///a/file01.ts"
+  },
+  "range": {
+    "start": {
+      "line": 1,
+      "character": 12
+    },
+    "end": {
+      "line": 1,
+      "character": 15
+    }
+  },
+  "context": {
+    "diagnostics": [
+      {
+        "range": {
+          "start": {
+            "line": 1,
+            "character": 12
+          },
+          "end": {
+            "line": 1,
+            "character": 15
+          }
+        },
+        "severity": 1,
+        "code": 2304,
+        "source": "deno-ts",
+        "message": "Cannot find name 'abc'."
+      },
+      {
+        "range": {
+          "start": {
+            "line": 2,
+            "character": 12
+          },
+          "end": {
+            "line": 2,
+            "character": 15
+          }
+        },
+        "severity": 1,
+        "code": 2304,
+        "source": "deno-ts",
+        "message": "Cannot find name 'def'."
+      }
+    ],
+    "only": [
+      "quickfix"
+    ]
+  }
+}

cli/tests/lsp/code_action_resolve_params_imports.json

@@ -0,0 +1,26 @@
+{
+  "title": "Add all missing imports",
+  "kind": "quickfix",
+  "diagnostics": [
+    {
+      "range": {
+        "start": {
+          "line": 1,
+          "character": 12
+        },
+        "end": {
+          "line": 1,
+          "character": 15
+        }
+      },
+      "severity": 1,
+      "code": 2304,
+      "source": "deno-ts",
+      "message": "Cannot find name 'abc'."
+    }
+  ],
+  "data": {
+    "specifier": "file:///a/file01.ts",
+    "fixId": "fixMissingImport"
+  }
+}

cli/tests/lsp/code_action_resolve_response_imports.json

@@ -0,0 +1,51 @@
+{
+  "title": "Add all missing imports",
+  "kind": "quickfix",
+  "diagnostics": [
+    {
+      "range": {
+        "start": {
+          "line": 1,
+          "character": 12
+        },
+        "end": {
+          "line": 1,
+          "character": 15
+        }
+      },
+      "severity": 1,
+      "code": 2304,
+      "source": "deno-ts",
+      "message": "Cannot find name 'abc'."
+    }
+  ],
+  "edit": {
+    "documentChanges": [
+      {
+        "textDocument": {
+          "uri": "file:///a/file01.ts",
+          "version": 1
+        },
+        "edits": [
+          {
+            "range": {
+              "start": {
+                "line": 0,
+                "character": 0
+              },
+              "end": {
+                "line": 0,
+                "character": 0
+              }
+            },
+            "newText": "import { abc,def } from \"./file00.ts\";\n"
+          }
+        ]
+      }
+    ]
+  },
+  "data": {
+    "specifier": "file:///a/file01.ts",
+    "fixId": "fixMissingImport"
+  }
+}

cli/tests/lsp/code_action_response_imports.json

@@ -0,0 +1,242 @@
+[
+  {
+    "title": "Import 'abc' from module \"./file00.ts\"",
+    "kind": "quickfix",
+    "diagnostics": [
+      {
+        "range": {
+          "start": {
+            "line": 1,
+            "character": 12
+          },
+          "end": {
+            "line": 1,
+            "character": 15
+          }
+        },
+        "severity": 1,
+        "code": 2304,
+        "source": "deno-ts",
+        "message": "Cannot find name 'abc'."
+      }
+    ],
+    "edit": {
+      "documentChanges": [
+        {
+          "textDocument": {
+            "uri": "file:///a/file01.ts",
+            "version": 1
+          },
+          "edits": [
+            {
+              "range": {
+                "start": {
+                  "line": 0,
+                  "character": 0
+                },
+                "end": {
+                  "line": 0,
+                  "character": 0
+                }
+              },
+              "newText": "import { abc } from \"./file00.ts\";\n"
+            }
+          ]
+        }
+      ]
+    }
+  },
+  {
+    "title": "Add all missing imports",
+    "kind": "quickfix",
+    "diagnostics": [
+      {
+        "range": {
+          "start": {
+            "line": 1,
+            "character": 12
+          },
+          "end": {
+            "line": 1,
+            "character": 15
+          }
+        },
+        "severity": 1,
+        "code": 2304,
+        "source": "deno-ts",
+        "message": "Cannot find name 'abc'."
+      }
+    ],
+    "data": {
+      "specifier": "file:///a/file01.ts",
+      "fixId": "fixMissingImport"
+    }
+  },
+  {
+    "title": "Add missing function declaration 'abc'",
+    "kind": "quickfix",
+    "diagnostics": [
+      {
+        "range": {
+          "start": {
+            "line": 1,
+            "character": 12
+          },
+          "end": {
+            "line": 1,
+            "character": 15
+          }
+        },
+        "severity": 1,
+        "code": 2304,
+        "source": "deno-ts",
+        "message": "Cannot find name 'abc'."
+      }
+    ],
+    "edit": {
+      "documentChanges": [
+        {
+          "textDocument": {
+            "uri": "file:///a/file01.ts",
+            "version": 1
+          },
+          "edits": [
+            {
+              "range": {
+                "start": {
+                  "line": 3,
+                  "character": 0
+                },
+                "end": {
+                  "line": 3,
+                  "character": 0
+                }
+              },
+              "newText": "\nfunction abc(abc: any) {\nthrow new Error(\"Function not implemented.\");\n}\n"
+            }
+          ]
+        }
+      ]
+    }
+  },
+  {
+    "title": "Import 'def' from module \"./file00.ts\"",
+    "kind": "quickfix",
+    "diagnostics": [
+      {
+        "range": {
+          "start": {
+            "line": 2,
+            "character": 12
+          },
+          "end": {
+            "line": 2,
+            "character": 15
+          }
+        },
+        "severity": 1,
+        "code": 2304,
+        "source": "deno-ts",
+        "message": "Cannot find name 'def'."
+      }
+    ],
+    "edit": {
+      "documentChanges": [
+        {
+          "textDocument": {
+            "uri": "file:///a/file01.ts",
+            "version": 1
+          },
+          "edits": [
+            {
+              "range": {
+                "start": {
+                  "line": 0,
+                  "character": 0
+                },
+                "end": {
+                  "line": 0,
+                  "character": 0
+                }
+              },
+              "newText": "import { def } from \"./file00.ts\";\n"
+            }
+          ]
+        }
+      ]
+    }
+  },
+  {
+    "title": "Add missing function declaration 'def'",
+    "kind": "quickfix",
+    "diagnostics": [
+      {
+        "range": {
+          "start": {
+            "line": 2,
+            "character": 12
+          },
+          "end": {
+            "line": 2,
+            "character": 15
+          }
+        },
+        "severity": 1,
+        "code": 2304,
+        "source": "deno-ts",
+        "message": "Cannot find name 'def'."
+      }
+    ],
+    "edit": {
+      "documentChanges": [
+        {
+          "textDocument": {
+            "uri": "file:///a/file01.ts",
+            "version": 1
+          },
+          "edits": [
+            {
+              "range": {
+                "start": {
+                  "line": 3,
+                  "character": 0
+                },
+                "end": {
+                  "line": 3,
+                  "character": 0
+                }
+              },
+              "newText": "\nfunction def(def: any) {\nthrow new Error(\"Function not implemented.\");\n}\n"
+            }
+          ]
+        }
+      ]
+    }
+  },
+  {
+    "title": "Add all missing function declarations",
+    "kind": "quickfix",
+    "diagnostics": [
+      {
+        "range": {
+          "start": {
+            "line": 1,
+            "character": 12
+          },
+          "end": {
+            "line": 1,
+            "character": 15
+          }
+        },
+        "severity": 1,
+        "code": 2304,
+        "source": "deno-ts",
+        "message": "Cannot find name 'abc'."
+      }
+    ],
+    "data": {
+      "specifier": "file:///a/file01.ts",
+      "fixId": "fixMissingFunctionDeclaration"
+    }
+  }
+]