Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lsp): auto-imports with @deno-types directives #26821

Merged
merged 24 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
Copy link
Member

@dsherret dsherret Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nayeemrmn note that this isn't always correct for CJS referrers, but it's probably fine to merge.

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