Repository: denoland/deno
Pull request: fix(lsp): auto-import dep entries from workspace member import maps

Fixes #30666.

Changed files:
filename: cli/lsp/analysis.rs
patch: @@ -322,7 +322,7 @@ impl<'a> TsResponseImportMapper<'a> {
       self.resolver.get_scoped_resolver(self.scope.as_deref());
 
     if let Some(dep_name) =
-      scoped_resolver.resource_url_to_configured_dep_key(specifier)
+      scoped_resolver.resource_url_to_configured_dep_key(specifier, referrer)
     {
       return Some(dep_name);
     }

filename: cli/lsp/resolver.rs
patch: @@ -49,7 +49,6 @@ 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::DenoIsBuiltInNodeModuleChecker;
 use node_resolver::NodeResolutionKind;
@@ -163,7 +162,7 @@ impl LspScopedResolver {
     let configured_dep_resolutions = (|| {
       let npm_pkg_req_resolver = npm_pkg_req_resolver.as_ref()?;
       Some(Arc::new(ConfiguredDepResolutions::new(
-        workspace_resolver.maybe_import_map(),
+        workspace_resolver.clone(),
         config_data.and_then(|d| d.maybe_pkg_json().map(|p| p.as_ref())),
         npm_pkg_req_resolver,
         &pkg_json_resolver,
@@ -348,11 +347,12 @@ impl LspScopedResolver {
 
   pub fn resource_url_to_configured_dep_key(
     &self,
-    specifier: &ModuleSpecifier,
+    specifier: &Url,
+    referrer: &Url,
   ) -> Option<String> {
     self
       .configured_dep_resolutions
-      .dep_key_from_resolution(specifier)
+      .dep_key_from_resolution(specifier, referrer)
   }
 
   pub fn npm_reqs(&self) -> BTreeSet<PackageReq> {
@@ -614,14 +614,21 @@ pub struct ScopeDepInfo {
   pub has_node_specifier: bool,
 }
 
+#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+enum ConfiguredDepKind {
+  ImportMap { key: String, value: Url },
+  PackageJson,
+}
+
 #[derive(Debug, Default)]
 struct ConfiguredDepResolutions {
-  deps_by_resolution: IndexMap<ModuleSpecifier, String>,
+  workspace_resolver: Option<Arc<WorkspaceResolver<CliSys>>>,
+  deps_by_resolution: IndexMap<ModuleSpecifier, (String, ConfiguredDepKind)>,
 }
 
 impl ConfiguredDepResolutions {
   fn new(
-    import_map: Option<&ImportMap>,
+    workspace_resolver: Arc<WorkspaceResolver<CliSys>>,
     package_json: Option<&PackageJson>,
     npm_pkg_req_resolver: &CliNpmReqResolver,
     pkg_json_resolver: &CliPackageJsonResolver,
@@ -632,6 +639,7 @@ impl ConfiguredDepResolutions {
        dep_req_str: &str,
        dep_package_json: &PackageJson,
        referrer,
+       dep_kind: &ConfiguredDepKind,
        result: &mut Self| {
         let export_keys = dep_package_json
           .exports
@@ -669,13 +677,14 @@ impl ConfiguredDepResolutions {
             let Some(file_url) = url_or_path.into_url().ok() else {
               continue;
             };
-            result
-              .deps_by_resolution
-              .insert(file_url, format!("{key_prefix}/{export_name}"));
+            result.deps_by_resolution.insert(
+              file_url,
+              (format!("{key_prefix}/{export_name}"), dep_kind.clone()),
+            );
           }
         }
       };
-    if let Some(import_map) = import_map {
+    if let Some(import_map) = workspace_resolver.maybe_import_map() {
       let referrer = import_map.base_url();
       for entry in import_map.imports().entries().chain(
         import_map
@@ -688,12 +697,10 @@ impl ConfiguredDepResolutions {
         let Ok(req_ref) = NpmPackageReqReference::from_specifier(value) else {
           continue;
         };
-        if !import_map
-          .resolve(entry.key, referrer)
-          .is_ok_and(|s| &s == value)
-        {
-          continue;
-        }
+        let dep_kind = ConfiguredDepKind::ImportMap {
+          key: entry.key.to_string(),
+          value: value.clone(),
+        };
         let mut dep_package_json = None;
         for kind in [NodeResolutionKind::Types, NodeResolutionKind::Execution] {
           let Some(file_url) = npm_pkg_req_resolver
@@ -716,9 +723,16 @@ impl ConfiguredDepResolutions {
             })();
           }
           if !entry.key.ends_with('/') {
-            result
-              .deps_by_resolution
-              .insert(file_url, entry.key.to_string());
+            result.deps_by_resolution.insert(
+              file_url,
+              (
+                entry.key.to_string(),
+                ConfiguredDepKind::ImportMap {
+                  key: entry.key.to_string(),
+                  value: value.clone(),
+                },
+              ),
+            );
           }
         }
         if let Some(key_prefix) = entry.key.strip_suffix('/')
@@ -730,6 +744,7 @@ impl ConfiguredDepResolutions {
             &req_ref.req().to_string(),
             dep_package_json,
             referrer,
+            &dep_kind,
             &mut result,
           );
         }
@@ -766,24 +781,45 @@ impl ConfiguredDepResolutions {
               pkg_json_resolver.get_closest_package_json(&path).ok()?
             })();
           }
-          result.deps_by_resolution.insert(file_url, name.clone());
+          result
+            .deps_by_resolution
+            .insert(file_url, (name.clone(), ConfiguredDepKind::PackageJson));
         }
         if let Some(dep_package_json) = &dep_package_json {
           insert_export_resolutions(
             name,
             name,
             dep_package_json,
             &referrer,
+            &ConfiguredDepKind::PackageJson,
             &mut result,
           );
         }
       }
     }
+    result.workspace_resolver = Some(workspace_resolver);
     result
   }
 
-  fn dep_key_from_resolution(&self, resolution: &Url) -> Option<String> {
-    self.deps_by_resolution.get(resolution).cloned()
+  fn dep_key_from_resolution(
+    &self,
+    resolution: &Url,
+    referrer: &Url,
+  ) -> Option<String> {
+    self
+      .deps_by_resolution
+      .get(resolution)
+      .and_then(|(dep_key, kind)| match kind {
+        // Ensure the mapping this entry came from is valid for this referrer.
+        ConfiguredDepKind::ImportMap { key, value } => self
+          .workspace_resolver
+          .as_ref()?
+          .maybe_import_map()?
+          .resolve(key, referrer)
+          .is_ok_and(|s| &s == value)
+          .then(|| dep_key.clone()),
+        ConfiguredDepKind::PackageJson => Some(dep_key.clone()),
+      })
   }
 }
 

filename: tests/integration/lsp_tests.rs
patch: @@ -10390,6 +10390,68 @@ fn lsp_auto_import_npm_export_node_modules_dir_no_package_json() {
   client.shutdown();
 }
 
+// Regression test for https://github.com/denoland/deno/issues/30666.
+#[test]
+#[timeout(300_000)]
+fn lsp_auto_import_npm_export_import_map_workspace_member() {
+  let context = TestContextBuilder::for_npm().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write(
+    "deno.json",
+    json!({
+      "workspace": ["member"],
+      "nodeModulesDir": "manual",
+    })
+    .to_string(),
+  );
+  temp_dir.write(
+    "member/deno.json",
+    json!({
+      "imports": {
+        "preact": "npm:preact",
+      },
+    })
+    .to_string(),
+  );
+  temp_dir.write("member/other.ts", "import \"preact/hooks\";\n");
+  let file = temp_dir.source_file("member/mod.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)]