feat(lsp): auto-import completions from byonm dependencies (#26680)

cli/lsp/analysis.rs

@@ -12,7 +12,9 @@ use super::urls::url_to_uri;
 use crate::args::jsr_url;
 use crate::lsp::search::PackageSearchApi;
 use crate::tools::lint::CliLinter;
+use crate::util::path::relative_specifier;
 use deno_config::workspace::MappedResolution;
+use deno_graph::source::ResolutionMode;
 use deno_lint::diagnostic::LintDiagnosticRange;
 
 use deno_ast::SourceRange;
@@ -228,6 +230,7 @@ pub struct TsResponseImportMapper<'a> {
   documents: &'a Documents,
   maybe_import_map: Option<&'a ImportMap>,
   resolver: &'a LspResolver,
+  tsc_specifier_map: &'a tsc::TscSpecifierMap,
   file_referrer: ModuleSpecifier,
 }
 
@@ -236,12 +239,14 @@ impl<'a> TsResponseImportMapper<'a> {
     documents: &'a Documents,
     maybe_import_map: Option<&'a ImportMap>,
     resolver: &'a LspResolver,
+    tsc_specifier_map: &'a tsc::TscSpecifierMap,
     file_referrer: &ModuleSpecifier,
   ) -> Self {
     Self {
       documents,
       maybe_import_map,
       resolver,
+      tsc_specifier_map,
       file_referrer: file_referrer.clone(),
     }
   }
@@ -387,6 +392,11 @@ impl<'a> TsResponseImportMapper<'a> {
           }
         }
       }
+    } else if let Some(dep_name) = self
+      .resolver
+      .file_url_to_package_json_dep(specifier, Some(&self.file_referrer))
+    {
+      return Some(dep_name);
     }
 
     // check if the import map has this specifier
@@ -457,19 +467,36 @@ impl<'a> TsResponseImportMapper<'a> {
     specifier: &str,
     referrer: &ModuleSpecifier,
   ) -> Option<String> {
-    if let Ok(specifier) = referrer.join(specifier) {
-      if let Some(specifier) = self.check_specifier(&specifier, referrer) {
-        return Some(specifier);
-      }
-    }
-    let specifier = specifier.strip_suffix(".js").unwrap_or(specifier);
-    for ext in SUPPORTED_EXTENSIONS {
-      let specifier_with_ext = format!("{specifier}{ext}");
-      if self
-        .documents
-        .contains_import(&specifier_with_ext, referrer)
+    let specifier_stem = specifier.strip_suffix(".js").unwrap_or(specifier);
+    let specifiers = std::iter::once(Cow::Borrowed(specifier)).chain(
+      SUPPORTED_EXTENSIONS
+        .iter()
+        .map(|ext| Cow::Owned(format!("{specifier_stem}{ext}"))),
+    );
+    for specifier in specifiers {
+      if let Some(specifier) = self
+        .resolver
+        .as_graph_resolver(Some(&self.file_referrer))
+        .resolve(
+          &specifier,
+          &deno_graph::Range {
+            specifier: referrer.clone(),
+            start: deno_graph::Position::zeroed(),
+            end: deno_graph::Position::zeroed(),
+          },
+          ResolutionMode::Types,
+        )
+        .ok()
+        .and_then(|s| self.tsc_specifier_map.normalize(s.as_str()).ok())
+        .filter(|s| self.documents.exists(s, Some(&self.file_referrer)))
       {
-        return Some(specifier_with_ext);
+        if let Some(specifier) = self
+          .check_specifier(&specifier, referrer)
+          .or_else(|| relative_specifier(referrer, &specifier))
+          .filter(|s| !s.contains("/node_modules/"))
+        {
+          return Some(specifier);
+        }
       }
     }
     None
@@ -559,8 +586,9 @@ fn try_reverse_map_package_json_exports(
 pub fn fix_ts_import_changes(
   referrer: &ModuleSpecifier,
   changes: &[tsc::FileTextChanges],
-  import_mapper: &TsResponseImportMapper,
+  language_server: &language_server::Inner,
 ) -> Result<Vec<tsc::FileTextChanges>, AnyError> {
+  let import_mapper = language_server.get_ts_response_import_mapper(referrer);
   let mut r = Vec::new();
   for change in changes {
     let mut text_changes = Vec::new();
@@ -605,7 +633,7 @@ pub fn fix_ts_import_changes(
 fn fix_ts_import_action<'a>(
   referrer: &ModuleSpecifier,
   action: &'a tsc::CodeFixAction,
-  import_mapper: &TsResponseImportMapper,
+  language_server: &language_server::Inner,
 ) -> Option<Cow<'a, tsc::CodeFixAction>> {
   if !matches!(
     action.fix_name.as_str(),
@@ -621,6 +649,7 @@ fn fix_ts_import_action<'a>(
   let Some(specifier) = specifier else {
     return Some(Cow::Borrowed(action));
   };
+  let import_mapper = language_server.get_ts_response_import_mapper(referrer);
   if let Some(new_specifier) =
     import_mapper.check_unresolved_specifier(specifier, referrer)
   {
@@ -728,7 +757,7 @@ pub fn ts_changes_to_edit(
   }))
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Debug, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct CodeActionData {
   pub specifier: ModuleSpecifier,
@@ -998,11 +1027,8 @@ impl CodeActionCollection {
         "The action returned from TypeScript is unsupported.",
       ));
     }
-    let Some(action) = fix_ts_import_action(
-      specifier,
-      action,
-      &language_server.get_ts_response_import_mapper(specifier),
-    ) else {
+    let Some(action) = fix_ts_import_action(specifier, action, language_server)
+    else {
       return Ok(());
     };
     let edit = ts_changes_to_edit(&action.changes, language_server)?;
@@ -1051,10 +1077,12 @@ impl CodeActionCollection {
     specifier: &ModuleSpecifier,
     diagnostic: &lsp::Diagnostic,
   ) {
-    let data = Some(json!({
-      "specifier": specifier,
-      "fixId": action.fix_id,
-    }));
+    let data = action.fix_id.as_ref().map(|fix_id| {
+      json!(CodeActionData {
+        specifier: specifier.clone(),
+        fix_id: fix_id.clone(),
+      })
+    });
     let title = if let Some(description) = &action.fix_all_description {
       description.clone()
     } else {

cli/lsp/documents.rs

@@ -1059,34 +1059,6 @@ impl Documents {
     self.cache.is_valid_file_referrer(specifier)
   }
 
-  /// Return `true` if the provided specifier can be resolved to a document,
-  /// otherwise `false`.
-  pub fn contains_import(
-    &self,
-    specifier: &str,
-    referrer: &ModuleSpecifier,
-  ) -> bool {
-    let file_referrer = self.get_file_referrer(referrer);
-    let maybe_specifier = self
-      .resolver
-      .as_graph_resolver(file_referrer.as_deref())
-      .resolve(
-        specifier,
-        &deno_graph::Range {
-          specifier: referrer.clone(),
-          start: deno_graph::Position::zeroed(),
-          end: deno_graph::Position::zeroed(),
-        },
-        ResolutionMode::Types,
-      )
-      .ok();
-    if let Some(import_specifier) = maybe_specifier {
-      self.exists(&import_specifier, file_referrer.as_deref())
-    } else {
-      false
-    }
-  }
-
   pub fn resolve_document_specifier(
     &self,
     specifier: &ModuleSpecifier,

cli/lsp/language_server.rs

@@ -1837,7 +1837,7 @@ impl Inner {
         fix_ts_import_changes(
           &code_action_data.specifier,
           &combined_code_actions.changes,
-          &self.get_ts_response_import_mapper(&code_action_data.specifier),
+          self,
         )
         .map_err(|err| {
           error!("Unable to remap changes: {:#}", err);
@@ -1890,7 +1890,7 @@ impl Inner {
         refactor_edit_info.edits = fix_ts_import_changes(
           &action_data.specifier,
           &refactor_edit_info.edits,
-          &self.get_ts_response_import_mapper(&action_data.specifier),
+          self,
         )
         .map_err(|err| {
           error!("Unable to remap changes: {:#}", err);
@@ -1921,7 +1921,8 @@ impl Inner {
         // todo(dsherret): this should probably just take the resolver itself
         // as the import map is an implementation detail
         .and_then(|d| d.resolver.maybe_import_map()),
-      self.resolver.as_ref(),
+      &self.resolver,
+      &self.ts_server.specifier_map,
       file_referrer,
     )
   }
@@ -2284,7 +2285,11 @@ impl Inner {
             .into(),
           scope.cloned(),
         )
-        .await;
+        .await
+        .unwrap_or_else(|err| {
+          error!("Unable to get completion info from TypeScript: {:#}", err);
+          None
+        });
 
       if let Some(completions) = maybe_completion_info {
         response = Some(

cli/lsp/resolver.rs

@@ -74,6 +74,7 @@ struct LspScopeResolver {
   pkg_json_resolver: Option<Arc<PackageJsonResolver>>,
   redirect_resolver: Option<Arc<RedirectResolver>>,
   graph_imports: Arc<IndexMap<ModuleSpecifier, GraphImport>>,
+  package_json_deps_by_resolution: Arc<IndexMap<ModuleSpecifier, String>>,
   config_data: Option<Arc<ConfigData>>,
 }
 
@@ -88,6 +89,7 @@ impl Default for LspScopeResolver {
       pkg_json_resolver: None,
       redirect_resolver: None,
       graph_imports: Default::default(),
+      package_json_deps_by_resolution: Default::default(),
       config_data: None,
     }
   }
@@ -165,6 +167,33 @@ impl LspScopeResolver {
         )
       })
       .unwrap_or_default();
+    let package_json_deps_by_resolution = (|| {
+      let node_resolver = node_resolver.as_ref()?;
+      let package_json = config_data?.maybe_pkg_json()?;
+      let referrer = package_json.specifier();
+      let dependencies = package_json.dependencies.as_ref()?;
+      let result = dependencies
+        .iter()
+        .flat_map(|(name, _)| {
+          let req_ref =
+            NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()?;
+          let specifier = into_specifier_and_media_type(Some(
+            node_resolver
+              .resolve_req_reference(
+                &req_ref,
+                &referrer,
+                NodeResolutionMode::Types,
+              )
+              .ok()?,
+          ))
+          .0;
+          Some((specifier, name.clone()))
+        })
+        .collect();
+      Some(result)
+    })();
+    let package_json_deps_by_resolution =
+      Arc::new(package_json_deps_by_resolution.unwrap_or_default());
     Self {
       cjs_tracker: lsp_cjs_tracker,
       graph_resolver,
@@ -174,6 +203,7 @@ impl LspScopeResolver {
       pkg_json_resolver: Some(pkg_json_resolver),
       redirect_resolver,
       graph_imports,
+      package_json_deps_by_resolution,
       config_data: config_data.cloned(),
     }
   }
@@ -216,6 +246,9 @@ impl LspScopeResolver {
       redirect_resolver: self.redirect_resolver.clone(),
       pkg_json_resolver: Some(pkg_json_resolver),
       graph_imports: self.graph_imports.clone(),
+      package_json_deps_by_resolution: self
+        .package_json_deps_by_resolution
+        .clone(),
       config_data: self.config_data.clone(),
     })
   }
@@ -407,6 +440,18 @@ impl LspResolver {
     )))
   }
 
+  pub fn file_url_to_package_json_dep(
+    &self,
+    specifier: &ModuleSpecifier,
+    file_referrer: Option<&ModuleSpecifier>,
+  ) -> Option<String> {
+    let resolver = self.get_scope_resolver(file_referrer);
+    resolver
+      .package_json_deps_by_resolution
+      .get(specifier)
+      .cloned()
+  }
+
   pub fn in_node_modules(&self, specifier: &ModuleSpecifier) -> bool {
     fn has_node_modules_dir(specifier: &ModuleSpecifier) -> bool {
       // consider any /node_modules/ directory as being in the node_modules