Repository: denoland/deno
Pull request: fix(lsp): prefer relative auto-import specifier against import map

Fixes #28662.

Changed files:
filename: cli/lsp/analysis.rs
patch: @@ -17,6 +17,7 @@ use deno_core::serde::Deserialize;
 use deno_core::serde::Serialize;
 use deno_core::serde_json;
 use deno_core::serde_json::json;
+use deno_core::url::Url;
 use deno_core::ModuleSpecifier;
 use deno_error::JsErrorBox;
 use deno_lint::diagnostic::LintDiagnosticRange;
@@ -233,6 +234,38 @@ fn code_as_string(code: &Option<lsp::NumberOrString>) -> String {
   }
 }
 
+/// Given a specifier and a referring specifier, determine if a value in the
+/// import map could be used as an import specifier that resolves using the
+/// import map.
+///
+/// This was inlined from the import_map crate in order to ignore more
+/// entries.
+pub fn import_map_lookup(
+  import_map: &ImportMap,
+  specifier: &Url,
+  referrer: &Url,
+) -> Option<String> {
+  let specifier_str = specifier.as_str();
+  for entry in import_map.entries_for_referrer(referrer) {
+    if let Some(address) = entry.value {
+      let address_str = address.as_str();
+      if referrer.as_str().starts_with(address_str) {
+        // ignore when the referrer has a common base with the
+        // import map entry (ex. `./src/a.ts` importing `./src/b.ts`
+        // and there's a `"$src/": "./src/"` import map entry)
+        continue;
+      }
+      if address_str == specifier_str {
+        return Some(entry.raw_key.to_string());
+      }
+      if address_str.ends_with('/') && specifier_str.starts_with(address_str) {
+        return Some(specifier_str.replace(address_str, entry.raw_key));
+      }
+    }
+  }
+  None
+}
+
 /// Rewrites imports in quick fixes and code changes to be Deno specific.
 pub struct TsResponseImportMapper<'a> {
   document_modules: &'a DocumentModules,
@@ -335,14 +368,18 @@ impl<'a> TsResponseImportMapper<'a> {
       };
       let specifier = ModuleSpecifier::parse(&spec_str).ok()?;
       if let Some(import_map) = self.maybe_import_map {
-        if let Some(result) = import_map.lookup(&specifier, referrer) {
+        if let Some(result) =
+          import_map_lookup(import_map, &specifier, referrer)
+        {
           return Some(result);
         }
         if let Some(req_ref_str) = specifier.as_str().strip_prefix("jsr:") {
           if !req_ref_str.starts_with('/') {
             let specifier_str = format!("jsr:/{req_ref_str}");
             if let Ok(specifier) = ModuleSpecifier::parse(&specifier_str) {
-              if let Some(result) = import_map.lookup(&specifier, referrer) {
+              if let Some(result) =
+                import_map_lookup(import_map, &specifier, referrer)
+              {
                 return Some(result);
               }
             }
@@ -432,7 +469,7 @@ impl<'a> TsResponseImportMapper<'a> {
 
     // check if the import map has this specifier
     if let Some(import_map) = self.maybe_import_map {
-      if let Some(result) = import_map.lookup(specifier, referrer) {
+      if let Some(result) = import_map_lookup(import_map, specifier, referrer) {
         return Some(result);
       }
     }

filename: cli/lsp/diagnostics.rs
patch: @@ -47,6 +47,7 @@ use tokio_util::sync::CancellationToken;
 use tower_lsp::lsp_types as lsp;
 
 use super::analysis;
+use super::analysis::import_map_lookup;
 use super::client::Client;
 use super::config::Config;
 use super::documents::Document;
@@ -1721,39 +1722,6 @@ fn diagnose_dependency(
   dependency_key: &str,
   dependency: &deno_graph::Dependency,
 ) {
-  /// Given a specifier and a referring specifier, determine if a value in the
-  /// import map could be used as an import specifier that resolves using the
-  /// import map.
-  ///
-  /// This was inlined from the import_map crate in order to ignore more
-  /// entries.
-  fn import_map_lookup(
-    import_map: &ImportMap,
-    specifier: &Url,
-    referrer: &Url,
-  ) -> Option<String> {
-    let specifier_str = specifier.as_str();
-    for entry in import_map.entries_for_referrer(referrer) {
-      if let Some(address) = entry.value {
-        let address_str = address.as_str();
-        if referrer.as_str().starts_with(address_str) {
-          // ignore when the referrer has a common base with the
-          // import map entry (ex. `./src/a.ts` importing `./src/b.ts`
-          // and there's a `"$src/": "./src/"` import map entry)
-          continue;
-        }
-        if address_str == specifier_str {
-          return Some(entry.raw_key.to_string());
-        }
-        if address_str.ends_with('/') && specifier_str.starts_with(address_str)
-        {
-          return Some(specifier_str.replace(address_str, entry.raw_key));
-        }
-      }
-    }
-    None
-  }
-
   if snapshot
     .resolver
     .in_node_modules(&referrer_module.specifier)

filename: tests/integration/lsp_tests.rs
patch: @@ -9558,6 +9558,63 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
   client.shutdown();
 }
 
+#[test]
+#[timeout(300_000)]
+fn lsp_auto_import_import_map_prefer_relative() {
+  let context = TestContextBuilder::new()
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write(
+    "deno.json",
+    json!({
+      "imports": {
+        "src/": "./src/",
+      },
+    })
+    .to_string(),
+  );
+  temp_dir.write("src/other.ts", "export const someValue = 1;\n");
+  let file = temp_dir.source_file("src/file.ts", "someValue;\n");
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  let list = client.get_completion_list(
+    file.uri().as_str(),
+    (0, 9),
+    json!({ "triggerKind": 1 }),
+  );
+  let items = list
+    .items
+    .iter()
+    .filter(|i| i.label == "someValue")
+    .map(|i| client.write_request("completionItem/resolve", json!(i)))
+    .collect::<Vec<_>>();
+  assert_eq!(
+    json!(items),
+    json!([
+      {
+        "label": "someValue",
+        "labelDetails": { "description": "./other.ts" },
+        "kind": 6,
+        "detail": "Add import from \"./other.ts\"\n\nconst someValue: 1",
+        "documentation": { "kind": "markdown", "value": "" },
+        "sortText": "\u{ffff}16_0",
+        "additionalTextEdits": [
+          {
+            "range": {
+              "start": { "line": 0, "character": 0 },
+              "end": { "line": 0, "character": 0 },
+            },
+            "newText": "import { someValue } from \"./other.ts\";\n\n",
+          },
+        ],
+      },
+    ]),
+  );
+  client.shutdown();
+}
+
 // Regression test for https://github.com/denoland/deno/issues/25775.
 #[test]
 #[timeout(300_000)]