Skip to content

Commit

Permalink
feat(lsp): auto-imports with @deno-types directives (#26821)
Browse files Browse the repository at this point in the history
Co-authored-by: David Sherret <dsherret@gmail.com>
  • Loading branch information
nayeemrmn and dsherret authored Nov 15, 2024
1 parent 032ae7f commit 3f26310
Show file tree
Hide file tree
Showing 5 changed files with 338 additions and 78 deletions.
120 changes: 86 additions & 34 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::cache::LspCache;
use super::config::Config;
use super::resolver::LspIsCjsResolver;
use super::resolver::LspResolver;
use super::resolver::ScopeDepInfo;
use super::resolver::SingleReferrerGraphResolver;
use super::testing::TestCollector;
use super::testing::TestModule;
Expand Down Expand Up @@ -38,7 +39,6 @@ use indexmap::IndexSet;
use node_resolver::NodeModuleKind;
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::collections::HashMap;
use std::collections::HashSet;
use std::fs;
Expand Down Expand Up @@ -989,12 +989,7 @@ pub struct Documents {
open_docs: HashMap<ModuleSpecifier, Arc<Document>>,
/// Documents stored on the file system.
file_system_docs: Arc<FileSystemDocuments>,
/// The npm package requirements found in npm specifiers.
npm_reqs_by_scope:
Arc<BTreeMap<Option<ModuleSpecifier>, BTreeSet<PackageReq>>>,
/// Config scopes that contain a node: specifier such that a @types/node
/// package should be injected.
scopes_with_node_specifier: Arc<HashSet<Option<ModuleSpecifier>>>,
dep_info_by_scope: Arc<BTreeMap<Option<ModuleSpecifier>, Arc<ScopeDepInfo>>>,
}

impl Documents {
Expand Down Expand Up @@ -1157,17 +1152,20 @@ impl Documents {
false
}

pub fn npm_reqs_by_scope(
pub fn dep_info_by_scope(
&mut self,
) -> Arc<BTreeMap<Option<ModuleSpecifier>, BTreeSet<PackageReq>>> {
self.calculate_npm_reqs_if_dirty();
self.npm_reqs_by_scope.clone()
) -> Arc<BTreeMap<Option<ModuleSpecifier>, Arc<ScopeDepInfo>>> {
self.calculate_dep_info_if_dirty();
self.dep_info_by_scope.clone()
}

pub fn scopes_with_node_specifier(
&self,
) -> &Arc<HashSet<Option<ModuleSpecifier>>> {
&self.scopes_with_node_specifier
pub fn scopes_with_node_specifier(&self) -> HashSet<Option<ModuleSpecifier>> {
self
.dep_info_by_scope
.iter()
.filter(|(_, i)| i.has_node_specifier)
.map(|(s, _)| s.clone())
.collect::<HashSet<_>>()
}

/// Return a document for the specifier.
Expand Down Expand Up @@ -1410,34 +1408,46 @@ impl Documents {
/// Iterate through the documents, building a map where the key is a unique
/// document and the value is a set of specifiers that depend on that
/// document.
fn calculate_npm_reqs_if_dirty(&mut self) {
let mut npm_reqs_by_scope: BTreeMap<_, BTreeSet<_>> = Default::default();
let mut scopes_with_specifier = HashSet::new();
fn calculate_dep_info_if_dirty(&mut self) {
let mut dep_info_by_scope: BTreeMap<_, ScopeDepInfo> = Default::default();
let is_fs_docs_dirty = self.file_system_docs.set_dirty(false);
if !is_fs_docs_dirty && !self.dirty {
return;
}
let mut visit_doc = |doc: &Arc<Document>| {
let scope = doc.scope();
let reqs = npm_reqs_by_scope.entry(scope.cloned()).or_default();
let dep_info = dep_info_by_scope.entry(scope.cloned()).or_default();
for dependency in doc.dependencies().values() {
if let Some(dep) = dependency.get_code() {
let code_specifier = dependency.get_code();
let type_specifier = dependency.get_type();
if let Some(dep) = code_specifier {
if dep.scheme() == "node" {
scopes_with_specifier.insert(scope.cloned());
dep_info.has_node_specifier = true;
}
if let Ok(reference) = NpmPackageReqReference::from_specifier(dep) {
reqs.insert(reference.into_inner().req);
dep_info.npm_reqs.insert(reference.into_inner().req);
}
}
if let Some(dep) = dependency.get_type() {
if let Some(dep) = type_specifier {
if let Ok(reference) = NpmPackageReqReference::from_specifier(dep) {
reqs.insert(reference.into_inner().req);
dep_info.npm_reqs.insert(reference.into_inner().req);
}
}
if dependency.maybe_deno_types_specifier.is_some() {
if let (Some(code_specifier), Some(type_specifier)) =
(code_specifier, type_specifier)
{
if MediaType::from_specifier(type_specifier).is_declaration() {
dep_info
.deno_types_to_code_resolutions
.insert(type_specifier.clone(), code_specifier.clone());
}
}
}
}
if let Some(dep) = doc.maybe_types_dependency().maybe_specifier() {
if let Ok(reference) = NpmPackageReqReference::from_specifier(dep) {
reqs.insert(reference.into_inner().req);
dep_info.npm_reqs.insert(reference.into_inner().req);
}
}
};
Expand All @@ -1448,14 +1458,49 @@ impl Documents {
visit_doc(doc);
}

// fill the reqs from the lockfile
for (scope, config_data) in self.config.tree.data_by_scope().as_ref() {
let dep_info = dep_info_by_scope.entry(Some(scope.clone())).or_default();
(|| {
let config_file = config_data.maybe_deno_json()?;
let jsx_config =
config_file.to_maybe_jsx_import_source_config().ok()??;
let type_specifier = jsx_config.default_types_specifier.as_ref()?;
let code_specifier = jsx_config.default_specifier.as_ref()?;
let cli_resolver = self.resolver.as_cli_resolver(Some(scope));
let range = deno_graph::Range {
specifier: jsx_config.base_url.clone(),
start: deno_graph::Position::zeroed(),
end: deno_graph::Position::zeroed(),
};
let type_specifier = cli_resolver
.resolve(
type_specifier,
&range,
// todo(dsherret): this is wrong because it doesn't consider CJS referrers
deno_package_json::NodeModuleKind::Esm,
ResolutionMode::Types,
)
.ok()?;
let code_specifier = cli_resolver
.resolve(
code_specifier,
&range,
// todo(dsherret): this is wrong because it doesn't consider CJS referrers
deno_package_json::NodeModuleKind::Esm,
ResolutionMode::Execution,
)
.ok()?;
dep_info
.deno_types_to_code_resolutions
.insert(type_specifier, code_specifier);
Some(())
})();
// fill the reqs from the lockfile
if let Some(lockfile) = config_data.lockfile.as_ref() {
let reqs = npm_reqs_by_scope.entry(Some(scope.clone())).or_default();
let lockfile = lockfile.lock();
for dep_req in lockfile.content.packages.specifiers.keys() {
if dep_req.kind == deno_semver::package::PackageKind::Npm {
reqs.insert(dep_req.req.clone());
dep_info.npm_reqs.insert(dep_req.req.clone());
}
}
}
Expand All @@ -1464,15 +1509,22 @@ impl Documents {
// Ensure a @types/node package exists when any module uses a node: specifier.
// Unlike on the command line, here we just add @types/node to the npm package
// requirements since this won't end up in the lockfile.
for scope in &scopes_with_specifier {
let reqs = npm_reqs_by_scope.entry(scope.clone()).or_default();
if !reqs.iter().any(|r| r.name == "@types/node") {
reqs.insert(PackageReq::from_str("@types/node").unwrap());
for dep_info in dep_info_by_scope.values_mut() {
if dep_info.has_node_specifier
&& !dep_info.npm_reqs.iter().any(|r| r.name == "@types/node")
{
dep_info
.npm_reqs
.insert(PackageReq::from_str("@types/node").unwrap());
}
}

self.npm_reqs_by_scope = Arc::new(npm_reqs_by_scope);
self.scopes_with_node_specifier = Arc::new(scopes_with_specifier);
self.dep_info_by_scope = Arc::new(
dep_info_by_scope
.into_iter()
.map(|(s, i)| (s, Arc::new(i)))
.collect(),
);
self.dirty = false;
}

Expand Down
31 changes: 17 additions & 14 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ impl Inner {

// refresh the npm specifiers because it might have discovered
// a @types/node package and now's a good time to do that anyway
self.refresh_npm_specifiers().await;
self.refresh_dep_info().await;

self.project_changed([], true);
}
Expand Down Expand Up @@ -1082,7 +1082,7 @@ impl Inner {
);
if document.is_diagnosable() {
self.project_changed([(document.specifier(), ChangeKind::Opened)], false);
self.refresh_npm_specifiers().await;
self.refresh_dep_info().await;
self.diagnostics_server.invalidate(&[specifier]);
self.send_diagnostics_update();
self.send_testing_update();
Expand All @@ -1103,8 +1103,8 @@ impl Inner {
Ok(document) => {
if document.is_diagnosable() {
let old_scopes_with_node_specifier =
self.documents.scopes_with_node_specifier().clone();
self.refresh_npm_specifiers().await;
self.documents.scopes_with_node_specifier();
self.refresh_dep_info().await;
let mut config_changed = false;
if !self
.documents
Expand Down Expand Up @@ -1155,13 +1155,15 @@ impl Inner {
}));
}

async fn refresh_npm_specifiers(&mut self) {
let package_reqs = self.documents.npm_reqs_by_scope();
async fn refresh_dep_info(&mut self) {
let dep_info_by_scope = self.documents.dep_info_by_scope();
let resolver = self.resolver.clone();
// spawn due to the lsp's `Send` requirement
spawn(async move { resolver.set_npm_reqs(&package_reqs).await })
.await
.ok();
spawn(
async move { resolver.set_dep_info_by_scope(&dep_info_by_scope).await },
)
.await
.ok();
}

async fn did_close(&mut self, params: DidCloseTextDocumentParams) {
Expand All @@ -1180,7 +1182,7 @@ impl Inner {
.uri_to_specifier(&params.text_document.uri, LspUrlKind::File);
self.diagnostics_state.clear(&specifier);
if self.is_diagnosable(&specifier) {
self.refresh_npm_specifiers().await;
self.refresh_dep_info().await;
self.diagnostics_server.invalidate(&[specifier.clone()]);
self.send_diagnostics_update();
self.send_testing_update();
Expand Down Expand Up @@ -3600,15 +3602,16 @@ impl Inner {

if byonm {
roots.retain(|s| s.scheme() != "npm");
} else if let Some(npm_reqs) = self
} else if let Some(dep_info) = self
.documents
.npm_reqs_by_scope()
.dep_info_by_scope()
.get(&config_data.map(|d| d.scope.as_ref().clone()))
{
// always include the npm packages since resolution of one npm package
// might affect the resolution of other npm packages
roots.extend(
npm_reqs
dep_info
.npm_reqs
.iter()
.map(|req| ModuleSpecifier::parse(&format!("npm:{}", req)).unwrap()),
);
Expand Down Expand Up @@ -3686,7 +3689,7 @@ impl Inner {

async fn post_cache(&mut self) {
self.resolver.did_cache();
self.refresh_npm_specifiers().await;
self.refresh_dep_info().await;
self.diagnostics_server.invalidate_all();
self.project_changed([], true);
self.ts_server.cleanup_semantic_cache(self.snapshot()).await;
Expand Down
Loading

0 comments on commit 3f26310

Please sign in to comment.