Repository: denoland/deno
Pull request: fix(lsp): lookup mapped npm specifier resolutions for auto-import

Fixes #29387.

Changed files:
filename: cli/lsp/analysis.rs
patch: @@ -462,7 +462,7 @@ impl<'a> TsResponseImportMapper<'a> {
         return Some(result);
       }
     } else if let Some(dep_name) =
-      scoped_resolver.file_url_to_package_json_dep(specifier)
+      scoped_resolver.file_url_to_npm_dep_key(specifier)
     {
       return Some(dep_name);
     }

filename: cli/lsp/resolver.rs
patch: @@ -46,6 +46,7 @@ use deno_semver::jsr::JsrPackageReqReference;
 use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageNv;
 use deno_semver::package::PackageReq;
+use import_map::ImportMap;
 use indexmap::IndexMap;
 use node_resolver::cache::NodeResolutionSys;
 use node_resolver::cache::NodeResolutionThreadLocalCache;
@@ -102,7 +103,7 @@ pub struct LspScopedResolver {
   redirect_resolver: Option<Arc<RedirectResolver>>,
   graph_imports: Arc<IndexMap<ModuleSpecifier, GraphImport>>,
   dep_info: Arc<Mutex<Arc<ScopeDepInfo>>>,
-  package_json_dep_resolutions: Arc<PackageJsonDepResolutions>,
+  configured_npm_resolutions: Arc<ConfiguredNpmResolutions>,
   config_data: Option<Arc<ConfigData>>,
 }
 
@@ -124,7 +125,7 @@ impl Default for LspScopedResolver {
       redirect_resolver: None,
       graph_imports: Default::default(),
       dep_info: Default::default(),
-      package_json_dep_resolutions: Default::default(),
+      configured_npm_resolutions: Default::default(),
       config_data: None,
     }
   }
@@ -183,11 +184,11 @@ impl LspScopedResolver {
         )
       })
       .unwrap_or_default();
-    let package_json_dep_resolutions = (|| {
+    let configured_npm_resolutions = (|| {
       let npm_pkg_req_resolver = npm_pkg_req_resolver.as_ref()?;
-      let package_json = config_data?.maybe_pkg_json()?;
-      Some(Arc::new(PackageJsonDepResolutions::new(
-        package_json,
+      Some(Arc::new(ConfiguredNpmResolutions::new(
+        config_data.and_then(|d| d.resolver.maybe_import_map()),
+        config_data.and_then(|d| d.maybe_pkg_json().map(|p| p.as_ref())),
         npm_pkg_req_resolver,
         &pkg_json_resolver,
       )))
@@ -208,7 +209,7 @@ impl LspScopedResolver {
       redirect_resolver,
       graph_imports,
       dep_info: Default::default(),
-      package_json_dep_resolutions,
+      configured_npm_resolutions,
       config_data: config_data.cloned(),
     }
   }
@@ -282,7 +283,7 @@ impl LspScopedResolver {
       pkg_json_resolver: factory.pkg_json_resolver().clone(),
       graph_imports: self.graph_imports.clone(),
       dep_info: self.dep_info.clone(),
-      package_json_dep_resolutions: self.package_json_dep_resolutions.clone(),
+      configured_npm_resolutions: self.configured_npm_resolutions.clone(),
       config_data: self.config_data.clone(),
     })
   }
@@ -383,13 +384,13 @@ impl LspScopedResolver {
     )))
   }
 
-  pub fn file_url_to_package_json_dep(
+  pub fn file_url_to_npm_dep_key(
     &self,
     specifier: &ModuleSpecifier,
   ) -> Option<String> {
     self
-      .package_json_dep_resolutions
-      .package_ref_from_resolution(specifier)
+      .configured_npm_resolutions
+      .dep_key_from_resolution(specifier)
       .or_else(|| {
         self
           .node_resolver
@@ -645,100 +646,176 @@ pub struct ScopeDepInfo {
 }
 
 #[derive(Debug, Default)]
-struct PackageJsonDepResolutions {
+struct ConfiguredNpmResolutions {
   deps_by_resolution: IndexMap<ModuleSpecifier, String>,
 }
 
-impl PackageJsonDepResolutions {
+impl ConfiguredNpmResolutions {
   fn new(
-    package_json: &PackageJson,
+    import_map: Option<&ImportMap>,
+    package_json: Option<&PackageJson>,
     npm_pkg_req_resolver: &CliNpmReqResolver,
     pkg_json_resolver: &CliPackageJsonResolver,
   ) -> Self {
-    let referrer = package_json.specifier();
-    let Some(dependencies) = package_json.dependencies.as_ref() else {
-      return Self::default();
-    };
     let mut result = Self::default();
-    for name in dependencies.keys() {
-      let Some(req_ref) =
-        NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()
-      else {
-        continue;
+    let insert_export_resolutions =
+      |key_prefix: &str,
+       dep_req_str: &str,
+       dep_package_json: &PackageJson,
+       referrer,
+       result: &mut Self| {
+        let export_keys = dep_package_json
+          .exports
+          .as_ref()
+          .into_iter()
+          .flat_map(|e| e.keys());
+        for export_key in export_keys {
+          let Some(export_name) = export_key.strip_prefix("./") else {
+            continue;
+          };
+          // Wildcards are not supported here.
+          if export_name.chars().filter(|c| *c == '*').count() == 1 {
+            continue;
+          }
+          let Some(req_ref) = NpmPackageReqReference::from_str(&format!(
+            "npm:{dep_req_str}/{export_name}"
+          ))
+          .ok() else {
+            continue;
+          };
+          for kind in [NodeResolutionKind::Types, NodeResolutionKind::Execution]
+          {
+            let Some(url_or_path) = npm_pkg_req_resolver
+              .resolve_req_reference(
+                &req_ref,
+                referrer,
+                // todo(dsherret): this is wrong because it doesn't consider CJS referrers
+                ResolutionMode::Import,
+                kind,
+              )
+              .ok()
+            else {
+              continue;
+            };
+            let Some(file_url) = url_or_path.into_url().ok() else {
+              continue;
+            };
+            result
+              .deps_by_resolution
+              .insert(file_url, format!("{key_prefix}/{export_name}"));
+          }
+        }
       };
-      let mut dep_package_json = None;
-      for kind in [NodeResolutionKind::Types, NodeResolutionKind::Execution] {
-        let Some(req) = npm_pkg_req_resolver
-          .resolve_req_reference(
-            &req_ref,
-            &referrer,
-            // todo(dsherret): this is wrong because it doesn't consider CJS referrers
-            ResolutionMode::Import,
-            kind,
-          )
-          .ok()
-        else {
+    if let Some(import_map) = import_map {
+      let referrer = import_map.base_url();
+      for entry in import_map.imports().entries().chain(
+        import_map
+          .scopes()
+          .flat_map(|scope| scope.imports.entries()),
+      ) {
+        let Some(value) = entry.value else {
           continue;
         };
-        let Some(url) = req.into_url().ok() else {
+        let Ok(req_ref) = NpmPackageReqReference::from_specifier(value) else {
           continue;
         };
-        let specifier = into_specifier_and_media_type(Some(url)).0;
-        if dep_package_json.is_none() {
-          dep_package_json = (|| {
-            let path = url_to_file_path(&specifier).ok()?;
-            pkg_json_resolver.get_closest_package_json(&path).ok()?
-          })();
-        }
-        result.deps_by_resolution.insert(specifier, name.clone());
-      }
-      let export_keys = dep_package_json
-        .as_ref()
-        .and_then(|p| p.exports.as_ref())
-        .into_iter()
-        .flat_map(|e| e.keys());
-      for export_key in export_keys {
-        let Some(export_name) = export_key.strip_prefix("./") else {
-          continue;
-        };
-        // Wildcards are not supported here.
-        if export_name.chars().filter(|c| *c == '*').count() == 1 {
+        if !import_map
+          .resolve(entry.key, referrer)
+          .is_ok_and(|s| &s == value)
+        {
           continue;
         }
-        let package_ref = format!("{name}/{export_name}");
-        let Some(req_ref) =
-          NpmPackageReqReference::from_str(&format!("npm:{}", &package_ref))
-            .ok()
-        else {
-          continue;
-        };
+        let mut dep_package_json = None;
         for kind in [NodeResolutionKind::Types, NodeResolutionKind::Execution] {
-          let Some(req) = npm_pkg_req_resolver
+          let Some(file_url) = npm_pkg_req_resolver
             .resolve_req_reference(
               &req_ref,
-              &referrer,
+              referrer,
               // todo(dsherret): this is wrong because it doesn't consider CJS referrers
               ResolutionMode::Import,
               kind,
             )
             .ok()
+            .and_then(|u| u.into_url().ok())
           else {
             continue;
           };
+          if dep_package_json.is_none() {
+            dep_package_json = (|| {
+              let path = url_to_file_path(&file_url).ok()?;
+              pkg_json_resolver.get_closest_package_json(&path).ok()?
+            })();
+          }
+          if !entry.key.ends_with('/') {
+            result
+              .deps_by_resolution
+              .insert(file_url, entry.key.to_string());
+          }
+        }
+        if let Some(key_prefix) = entry.key.strip_suffix('/') {
+          if req_ref.sub_path().is_none() {
+            if let Some(dep_package_json) = &dep_package_json {
+              insert_export_resolutions(
+                key_prefix,
+                &req_ref.req().to_string(),
+                dep_package_json,
+                referrer,
+                &mut result,
+              );
+            }
+          }
+        }
+      }
+    }
+    if let Some(package_json) = package_json {
+      let referrer = package_json.specifier();
+      let Some(dependencies) = package_json.dependencies.as_ref() else {
+        return Self::default();
+      };
+      for name in dependencies.keys() {
+        let Some(req_ref) =
+          NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()
+        else {
+          continue;
+        };
+        let mut dep_package_json = None;
+        for kind in [NodeResolutionKind::Types, NodeResolutionKind::Execution] {
+          let Ok(req) = npm_pkg_req_resolver.resolve_req_reference(
+            &req_ref,
+            &referrer,
+            // todo(dsherret): this is wrong because it doesn't consider CJS referrers
+            ResolutionMode::Import,
+            kind,
+          ) else {
+            continue;
+          };
           let Some(url) = req.into_url().ok() else {
             continue;
           };
           let specifier = into_specifier_and_media_type(Some(url)).0;
-          result
-            .deps_by_resolution
-            .insert(specifier, package_ref.clone());
+          if dep_package_json.is_none() {
+            dep_package_json = (|| {
+              let path = url_to_file_path(&specifier).ok()?;
+              pkg_json_resolver.get_closest_package_json(&path).ok()?
+            })();
+          }
+          result.deps_by_resolution.insert(specifier, name.clone());
+        }
+        if let Some(dep_package_json) = &dep_package_json {
+          insert_export_resolutions(
+            name,
+            name,
+            dep_package_json,
+            &referrer,
+            &mut result,
+          );
         }
       }
     }
     result
   }
 
-  fn package_ref_from_resolution(&self, resolution: &Url) -> Option<String> {
+  fn dep_key_from_resolution(&self, resolution: &Url) -> Option<String> {
     self.deps_by_resolution.get(resolution).cloned()
   }
 }

filename: tests/integration/lsp_tests.rs
patch: @@ -10327,6 +10327,69 @@ fn lsp_auto_import_npm_export_node_modules_dir_manual() {
   client.shutdown();
 }
 
+#[test]
+#[timeout(300_000)]
+fn lsp_auto_import_npm_export_node_modules_dir_no_package_json() {
+  let context = TestContextBuilder::for_npm().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write(
+    "deno.json",
+    json!({
+      "nodeModulesDir": "manual",
+    })
+    .to_string(),
+  );
+  temp_dir.write(
+    "deno.json",
+    json!({
+      "imports": {
+        "preact": "npm:preact",
+      },
+    })
+    .to_string(),
+  );
+  temp_dir.write(
+    "mod.ts",
+    "import { useEffect } from 'preact/hooks'; console.log(useEffect);",
+  );
+  let file = temp_dir.source_file("file.ts", "useEffect;\n");
+  context.run_deno("install");
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  let diagnostics = client.did_open_file(&file).all();
+  assert_eq!(diagnostics.len(), 1);
+  let diagnostic = diagnostics.first().unwrap();
+  let res = client.write_request(
+    "textDocument/codeAction",
+    json!({
+      "textDocument": { "uri": file.uri() },
+      "range": {
+        "start": { "line": 0, "character": 0 },
+        "end": { "line": 0, "character": 9 },
+      },
+      "context": {
+        "diagnostics": [diagnostic],
+        "only": ["quickfix"],
+      },
+    }),
+  );
+  assert_eq!(
+    res
+      .as_array()
+      .unwrap()
+      .first()
+      .unwrap()
+      .as_object()
+      .unwrap()
+      .get("title")
+      .unwrap()
+      .as_str()
+      .unwrap(),
+    "Add import from \"preact/hooks\"",
+  );
+  client.shutdown();
+}
+
 // Regression test for https://github.com/denoland/deno/issues/23869.
 #[test]
 #[timeout(300_000)]