fix(lsp): improved npm specifier to import map entry mapping

Cargo.toml

@@ -117,7 +117,7 @@ libz-sys = { version = "1.1", default-features = false }
 log = "=0.4.20"
 lsp-types = "=0.94.1" # used by tower-lsp and "proposed" feature is unstable in patch releases
 memmem = "0.1.1"
-monch = "=0.4.3"
+monch = "=0.5.0"
 notify = "=5.0.0"
 num-bigint = { version = "0.4", features = ["rand"] }
 once_cell = "1.17.1"

cli/Cargo.toml

@@ -64,10 +64,8 @@ deno_lint = { version = "=0.53.0", features = ["docs"] }
 deno_lockfile.workspace = true
 deno_npm = "=0.15.3"
 deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
-# todo(dsherret): investigate https://github.com/denoland/deno_semver/commit/98f9174baef199809295077b3b68c9fa58defb9b causing
-# lsp_completions_auto_import_and_quick_fix_with_import_map to fail when bumping this version
-deno_semver = "=0.5.1"
-deno_task_shell = "=0.14.0"
+deno_semver = "=0.5.4"
+deno_task_shell = "=0.14.3"
 eszip = "=0.57.0"
 napi_sym.workspace = true
 
@@ -99,7 +97,7 @@ flate2.workspace = true
 fs3.workspace = true
 glob = "0.3.1"
 hex.workspace = true
-import_map = { version = "=0.18.1", features = ["ext"] }
+import_map = { version = "=0.18.2", features = ["ext"] }
 indexmap.workspace = true
 jsonc-parser = { version = "=0.23.0", features = ["serde"] }
 lazy-regex.workspace = true

cli/lsp/analysis.rs

@@ -25,12 +25,14 @@ use deno_runtime::deno_node::NodeResolver;
 use deno_runtime::deno_node::NpmResolver;
 use deno_runtime::deno_node::PathClean;
 use deno_runtime::permissions::PermissionsContainer;
+use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageReq;
 use import_map::ImportMap;
 use once_cell::sync::Lazy;
 use regex::Regex;
 use std::cmp::Ordering;
 use std::collections::HashMap;
+use std::collections::HashSet;
 use std::path::Path;
 use tower_lsp::lsp_types as lsp;
 use tower_lsp::lsp_types::Position;
@@ -211,20 +213,31 @@ impl<'a> TsResponseImportMapper<'a> {
           if !pkg_reqs.is_empty() {
             let sub_path = self.resolve_package_path(specifier);
             if let Some(import_map) = self.maybe_import_map {
-              for pkg_req in &pkg_reqs {
-                let paths = vec![
-                  concat_npm_specifier("npm:", pkg_req, sub_path.as_deref()),
-                  concat_npm_specifier("npm:/", pkg_req, sub_path.as_deref()),
-                ];
-                for path in paths {
-                  if let Some(mapped_path) = ModuleSpecifier::parse(&path)
-                    .ok()
-                    .and_then(|s| import_map.lookup(&s, referrer))
+              let pkg_reqs = pkg_reqs.iter().collect::<HashSet<_>>();
+              let mut matches = Vec::new();
+              for entry in import_map.entries_for_referrer(referrer) {
+                if let Some(value) = entry.raw_value {
+                  if let Ok(package_ref) =
+                    NpmPackageReqReference::from_str(value)
                   {
-                    return Some(mapped_path);
+                    if pkg_reqs.contains(package_ref.req()) {
+                      let sub_path = sub_path.as_deref().unwrap_or("");
+                      let value_sub_path = package_ref.sub_path().unwrap_or("");
+                      if let Some(key_sub_path) =
+                        sub_path.strip_prefix(value_sub_path)
+                      {
+                        matches
+                          .push(format!("{}{}", entry.raw_key, key_sub_path));
+                      }
+                    }
                   }
                 }
               }
+              // select the shortest match
+              matches.sort_by_key(|a| a.len());
+              if let Some(matched) = matches.first() {
+                return Some(matched.to_string());
+              }
             }
 
             // if not found in the import map, return the first pkg req
@@ -267,6 +280,22 @@ impl<'a> TsResponseImportMapper<'a> {
     // a search for the .d.ts file instead
     if specifier_path.extension().and_then(|e| e.to_str()) == Some("js") {
       search_paths.insert(0, specifier_path.with_extension("d.ts"));
+    } else if let Some(file_name) =
+      specifier_path.file_name().and_then(|f| f.to_str())
+    {
+      // In some other cases, typescript will provide the .d.ts extension, but the
+      // export might not have a .d.ts defined. In that case, look for the corresponding
+      // JavaScript file after not being able to find the .d.ts file.
+      if let Some(file_stem) = file_name.strip_suffix(".d.ts") {
+        search_paths
+          .push(specifier_path.with_file_name(format!("{}.js", file_stem)));
+      } else if let Some(file_stem) = file_name.strip_suffix(".d.cts") {
+        search_paths
+          .push(specifier_path.with_file_name(format!("{}.cjs", file_stem)));
+      } else if let Some(file_stem) = file_name.strip_suffix(".d.mts") {
+        search_paths
+          .push(specifier_path.with_file_name(format!("{}.mjs", file_stem)));
+      }
     }
 
     for search_path in search_paths {

cli/tests/integration/lsp_tests.rs

@@ -6450,6 +6450,7 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
     "imports": {
       "print_hello": "http://localhost:4545/subdir/print_hello.ts",
       "chalk": "npm:chalk@~5",
+      "nested/": "npm:/@denotest/types-exports-subpaths@1/nested/",
       "types-exports-subpaths/": "npm:/@denotest/types-exports-subpaths@1/"
     }
   }"#;
@@ -6470,6 +6471,7 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
           "import _test1 from 'npm:chalk@^5.0';\n",
           "import chalk from 'npm:chalk@~5';\n",
           "import chalk from 'npm:chalk@~5';\n",
+          "import {entryB} from 'npm:@denotest/types-exports-subpaths@1/nested/entry-b';\n",
           "import {printHello} from 'print_hello';\n",
           "\n",
         ),
@@ -6483,6 +6485,7 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
       "arguments": [
         [
           "npm:@denotest/types-exports-subpaths@1/client",
+          "npm:@denotest/types-exports-subpaths@1/nested/entry-b",
           "npm:chalk@^5.0",
           "npm:chalk@~5",
           "http://localhost:4545/subdir/print_hello.ts",
@@ -6822,6 +6825,54 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
       }
     }])
   );
+
+  // try auto-import with npm package with sub-path on value side of import map
+  client.did_open(json!({
+    "textDocument": {
+      "uri": "file:///a/nested_path.ts",
+      "languageId": "typescript",
+      "version": 1,
+      "text": "entry",
+    }
+  }));
+  let list = client.get_completion_list(
+    "file:///a/nested_path.ts",
+    (0, 5),
+    json!({ "triggerKind": 1 }),
+  );
+  assert!(!list.is_incomplete);
+  let item = list
+    .items
+    .iter()
+    .find(|item| item.label == "entryB")
+    .unwrap();
+
+  let res = client.write_request("completionItem/resolve", item);
+  assert_eq!(
+    res,
+    json!({
+      "label": "entryB",
+      "labelDetails": {
+        "description": "nested/entry-b",
+      },
+      "kind": 3,
+      "detail": "function entryB(): \"b\"",
+      "documentation": {
+        "kind": "markdown",
+        "value": ""
+      },
+      "sortText": "￿16_0",
+      "additionalTextEdits": [
+        {
+          "range": {
+            "start": { "line": 0, "character": 0 },
+            "end": { "line": 0, "character": 0 }
+          },
+          "newText": "import { entryB } from \"nested/entry-b\";\n\n"
+        }
+      ]
+    })
+  );
 }
 
 #[test]

cli/tests/testdata/npm/registry/@denotest/types-exports-subpaths/1.0.0/dist/entry-a.js

@@ -1,3 +1,3 @@
-export function entryC() {
+export function entryA() {
   return 12;
 }

cli/tests/testdata/npm/registry/@denotest/types-exports-subpaths/1.0.0/dist/entry-b.d.ts

@@ -0,0 +1 @@
+export function entryB(): "b";

cli/tests/testdata/npm/registry/@denotest/types-exports-subpaths/1.0.0/dist/entry-b.js

@@ -0,0 +1,3 @@
+export function entryB() {
+  return "b";
+}

cli/tests/testdata/npm/registry/@denotest/types-exports-subpaths/1.0.0/package.json

@@ -18,6 +18,9 @@
     },
     "./entry-a": {
       "import": "./dist/entry-a.js"
+    },
+    "./nested/entry-b": {
+      "import": "./dist/entry-b.js"
     }
   }
 }