feat(unstable/npm): initial type checking of npm specifiers (#16332)

This commit is contained in:
David Sherret 2022-10-21 11:20:18 -04:00 committed by GitHub
parent 0e1a71fec6
commit bcfe279fba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
64 changed files with 2135 additions and 280 deletions

View file

@ -458,6 +458,13 @@ async fn generate_lint_diagnostics(
break;
}
// ignore any npm package files
if let Some(npm_resolver) = &snapshot.maybe_npm_resolver {
if npm_resolver.in_npm_package(document.specifier()) {
continue;
}
}
let version = document.maybe_lsp_version();
diagnostics_vec.push((
document.specifier().clone(),
@ -597,6 +604,8 @@ pub enum DenoDiagnostic {
NoCacheBlob,
/// A data module was not found in the cache.
NoCacheData(ModuleSpecifier),
/// A remote npm package reference was not found in the cache.
NoCacheNpm(NpmPackageReference, ModuleSpecifier),
/// A local module was not found on the local file system.
NoLocal(ModuleSpecifier),
/// The specifier resolved to a remote specifier that was redirected to
@ -622,6 +631,7 @@ impl DenoDiagnostic {
Self::NoCache(_) => "no-cache",
Self::NoCacheBlob => "no-cache-blob",
Self::NoCacheData(_) => "no-cache-data",
Self::NoCacheNpm(_, _) => "no-cache-npm",
Self::NoLocal(_) => "no-local",
Self::Redirect { .. } => "redirect",
Self::ResolutionError(err) => match err {
@ -690,16 +700,17 @@ impl DenoDiagnostic {
}),
..Default::default()
},
"no-cache" | "no-cache-data" => {
"no-cache" | "no-cache-data" | "no-cache-npm" => {
let data = diagnostic
.data
.clone()
.ok_or_else(|| anyhow!("Diagnostic is missing data"))?;
let data: DiagnosticDataSpecifier = serde_json::from_value(data)?;
let title = if code == "no-cache" {
format!("Cache \"{}\" and its dependencies.", data.specifier)
} else {
"Cache the data URL and its dependencies.".to_string()
let title = match code.as_str() {
"no-cache" | "no-cache-npm" => {
format!("Cache \"{}\" and its dependencies.", data.specifier)
}
_ => "Cache the data URL and its dependencies.".to_string(),
};
lsp::CodeAction {
title,
@ -757,6 +768,7 @@ impl DenoDiagnostic {
code.as_str(),
"import-map-remap"
| "no-cache"
| "no-cache-npm"
| "no-cache-data"
| "no-assert-type"
| "redirect"
@ -777,6 +789,7 @@ impl DenoDiagnostic {
Self::NoCache(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing remote URL: \"{}\".", specifier), Some(json!({ "specifier": specifier }))),
Self::NoCacheBlob => (lsp::DiagnosticSeverity::ERROR, "Uncached blob URL.".to_string(), None),
Self::NoCacheData(specifier) => (lsp::DiagnosticSeverity::ERROR, "Uncached data URL.".to_string(), Some(json!({ "specifier": specifier }))),
Self::NoCacheNpm(pkg_ref, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing npm package: \"{}\".", pkg_ref.req), Some(json!({ "specifier": specifier }))),
Self::NoLocal(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Unable to load a local module: \"{}\".\n Please check the file path.", specifier), None),
Self::Redirect { from, to} => (lsp::DiagnosticSeverity::INFORMATION, format!("The import of \"{}\" was redirected to \"{}\".", from, to), Some(json!({ "specifier": from, "redirect": to }))),
Self::ResolutionError(err) => (lsp::DiagnosticSeverity::ERROR, err.to_string(), None),
@ -847,8 +860,20 @@ fn diagnose_resolved(
.push(DenoDiagnostic::NoAssertType.to_lsp_diagnostic(&range)),
}
}
} else if NpmPackageReference::from_specifier(specifier).is_ok() {
// ignore npm specifiers for now
} else if let Ok(pkg_ref) = NpmPackageReference::from_specifier(specifier)
{
if let Some(npm_resolver) = &snapshot.maybe_npm_resolver {
// show diagnostics for npm package references that aren't cached
if npm_resolver
.resolve_package_folder_from_deno_module(&pkg_ref.req)
.is_err()
{
diagnostics.push(
DenoDiagnostic::NoCacheNpm(pkg_ref, specifier.clone())
.to_lsp_diagnostic(&range),
);
}
}
} else {
// When the document is not available, it means that it cannot be found
// in the cache or locally on the disk, so we want to issue a diagnostic
@ -882,6 +907,12 @@ fn diagnose_dependency(
dependency_key: &str,
dependency: &deno_graph::Dependency,
) {
if let Some(npm_resolver) = &snapshot.maybe_npm_resolver {
if npm_resolver.in_npm_package(referrer) {
return; // ignore, surface typescript errors instead
}
}
if let Some(import_map) = &snapshot.maybe_import_map {
if let Resolved::Ok {
specifier, range, ..
@ -938,8 +969,8 @@ async fn generate_deno_diagnostics(
&mut diagnostics,
snapshot,
specifier,
&dependency_key,
&dependency,
dependency_key,
dependency,
);
}
}

View file

@ -12,6 +12,13 @@ use crate::file_fetcher::SUPPORTED_SCHEMES;
use crate::fs_util::specifier_to_file_path;
use crate::http_cache;
use crate::http_cache::HttpCache;
use crate::node;
use crate::node::node_resolve_npm_reference;
use crate::node::NodeResolution;
use crate::node::NodeResolutionMode;
use crate::npm::NpmPackageReference;
use crate::npm::NpmPackageReq;
use crate::npm::NpmPackageResolver;
use crate::resolver::ImportMapResolver;
use crate::resolver::JsxResolver;
use crate::text_encoding;
@ -209,6 +216,29 @@ impl AssetOrDocument {
}
}
#[derive(Debug, Default)]
struct DocumentDependencies {
deps: BTreeMap<String, deno_graph::Dependency>,
maybe_types_dependency: Option<(String, Resolved)>,
}
impl DocumentDependencies {
pub fn from_maybe_module(maybe_module: &MaybeModuleResult) -> Self {
if let Some(Ok(module)) = &maybe_module {
Self::from_module(module)
} else {
Self::default()
}
}
pub fn from_module(module: &deno_graph::Module) -> Self {
Self {
deps: module.dependencies.clone(),
maybe_types_dependency: module.maybe_types_dependency.clone(),
}
}
}
type MaybeModuleResult =
Option<Result<deno_graph::Module, deno_graph::ModuleGraphError>>;
type MaybeParsedSourceResult =
@ -217,7 +247,7 @@ type MaybeParsedSourceResult =
#[derive(Debug, Clone)]
struct DocumentInner {
/// contains the last-known-good set of dependencies from parsing the module
dependencies: Arc<BTreeMap<String, deno_graph::Dependency>>,
dependencies: Arc<DocumentDependencies>,
fs_version: String,
line_index: Arc<LineIndex>,
maybe_language_id: Option<LanguageId>,
@ -249,12 +279,9 @@ impl Document {
maybe_headers,
maybe_resolver,
);
let dependencies = if let Some(Ok(module)) = &maybe_module {
Arc::new(module.dependencies.clone())
} else {
Arc::new(BTreeMap::new())
};
// todo(dsherret): retrieve this from the parsed source if it
let dependencies =
Arc::new(DocumentDependencies::from_maybe_module(&maybe_module));
// todo(dsherret): retrieve this from the parsed source if it exists
let text_info = SourceTextInfo::new(content);
let line_index = Arc::new(LineIndex::new(text_info.text_str()));
Self(Arc::new(DocumentInner {
@ -289,11 +316,8 @@ impl Document {
} else {
(None, None)
};
let dependencies = if let Some(Ok(module)) = &maybe_module {
Arc::new(module.dependencies.clone())
} else {
Arc::new(BTreeMap::new())
};
let dependencies =
Arc::new(DocumentDependencies::from_maybe_module(&maybe_module));
let source = SourceTextInfo::new(content);
let line_index = Arc::new(LineIndex::new(source.text_str()));
Self(Arc::new(DocumentInner {
@ -355,9 +379,9 @@ impl Document {
(None, None)
};
let dependencies = if let Some(Ok(module)) = &maybe_module {
Arc::new(module.dependencies.clone())
Arc::new(DocumentDependencies::from_module(module))
} else {
self.0.dependencies.clone()
self.0.dependencies.clone() // use the last known good
};
let text_info = SourceTextInfo::new(content);
let line_index = if index_valid == IndexValid::All {
@ -435,15 +459,9 @@ impl Document {
}
pub fn maybe_types_dependency(&self) -> deno_graph::Resolved {
let module_result = match self.0.maybe_module.as_ref() {
Some(module_result) => module_result,
_ => return deno_graph::Resolved::None,
};
let module = match module_result.as_ref() {
Ok(module) => module,
Err(_) => return deno_graph::Resolved::None,
};
if let Some((_, maybe_dep)) = module.maybe_types_dependency.as_ref() {
if let Some((_, maybe_dep)) =
self.0.dependencies.maybe_types_dependency.as_ref()
{
maybe_dep.clone()
} else {
deno_graph::Resolved::None
@ -479,13 +497,8 @@ impl Document {
self.0.maybe_navigation_tree.clone()
}
pub fn dependencies(&self) -> Vec<(String, deno_graph::Dependency)> {
self
.0
.dependencies
.iter()
.map(|(s, d)| (s.clone(), d.clone()))
.collect()
pub fn dependencies(&self) -> &BTreeMap<String, deno_graph::Dependency> {
&self.0.dependencies.deps
}
/// If the supplied position is within a dependency range, return the resolved
@ -698,6 +711,8 @@ pub struct Documents {
maybe_import_map: Option<ImportMapResolver>,
/// The optional JSX resolver, which is used when JSX imports are configured.
maybe_jsx_resolver: Option<JsxResolver>,
/// The npm package requirements.
npm_reqs: HashSet<NpmPackageReq>,
/// Resolves a specifier to its final redirected to specifier.
specifier_resolver: Arc<SpecifierResolver>,
}
@ -713,6 +728,7 @@ impl Documents {
imports: Default::default(),
maybe_import_map: None,
maybe_jsx_resolver: None,
npm_reqs: HashSet::new(),
specifier_resolver: Arc::new(SpecifierResolver::new(location)),
}
}
@ -847,6 +863,12 @@ impl Documents {
}
}
/// Returns a collection of npm package requirements.
pub fn npm_package_reqs(&mut self) -> HashSet<NpmPackageReq> {
self.calculate_dependents_if_dirty();
self.npm_reqs.clone()
}
/// Return a document for the specifier.
pub fn get(&self, original_specifier: &ModuleSpecifier) -> Option<Document> {
let specifier = self.specifier_resolver.resolve(original_specifier)?;
@ -921,10 +943,28 @@ impl Documents {
&self,
specifiers: Vec<String>,
referrer: &ModuleSpecifier,
maybe_npm_resolver: Option<&NpmPackageResolver>,
) -> Option<Vec<Option<(ModuleSpecifier, MediaType)>>> {
let dependencies = self.get(referrer)?.0.dependencies.clone();
let mut results = Vec::new();
for specifier in specifiers {
if let Some(npm_resolver) = maybe_npm_resolver {
if npm_resolver.in_npm_package(referrer) {
// we're in an npm package, so use node resolution
results.push(Some(NodeResolution::into_specifier_and_media_type(
node::node_resolve(
&specifier,
referrer,
node::NodeResolutionMode::Types,
npm_resolver,
)
.ok()
.flatten(),
)));
continue;
}
}
// handle npm:<package> urls
if specifier.starts_with("asset:") {
if let Ok(specifier) = ModuleSpecifier::parse(&specifier) {
let media_type = MediaType::from(&specifier);
@ -932,11 +972,11 @@ impl Documents {
} else {
results.push(None);
}
} else if let Some(dep) = dependencies.get(&specifier) {
} else if let Some(dep) = dependencies.deps.get(&specifier) {
if let Resolved::Ok { specifier, .. } = &dep.maybe_type {
results.push(self.resolve_dependency(specifier));
results.push(self.resolve_dependency(specifier, maybe_npm_resolver));
} else if let Resolved::Ok { specifier, .. } = &dep.maybe_code {
results.push(self.resolve_dependency(specifier));
results.push(self.resolve_dependency(specifier, maybe_npm_resolver));
} else {
results.push(None);
}
@ -945,7 +985,19 @@ impl Documents {
{
// clone here to avoid double borrow of self
let specifier = specifier.clone();
results.push(self.resolve_dependency(&specifier));
results.push(self.resolve_dependency(&specifier, maybe_npm_resolver));
} else if let Ok(npm_ref) = NpmPackageReference::from_str(&specifier) {
results.push(maybe_npm_resolver.map(|npm_resolver| {
NodeResolution::into_specifier_and_media_type(
node_resolve_npm_reference(
&npm_ref,
NodeResolutionMode::Types,
npm_resolver,
)
.ok()
.flatten(),
)
}));
} else {
results.push(None);
}
@ -1038,32 +1090,36 @@ impl Documents {
// favour documents that are open in case a document exists in both collections
let documents = file_system_docs.docs.iter().chain(self.open_docs.iter());
for (specifier, doc) in documents {
if let Some(Ok(module)) = doc.maybe_module() {
for dependency in module.dependencies.values() {
if let Some(dep) = dependency.get_code() {
dependents_map
.entry(dep.clone())
.or_default()
.insert(specifier.clone());
}
if let Some(dep) = dependency.get_type() {
dependents_map
.entry(dep.clone())
.or_default()
.insert(specifier.clone());
}
for dependency in doc.dependencies().values() {
if let Some(dep) = dependency.get_code() {
dependents_map
.entry(dep.clone())
.or_default()
.insert(specifier.clone());
}
if let Some((_, Resolved::Ok { specifier: dep, .. })) =
&module.maybe_types_dependency
{
if let Some(dep) = dependency.get_type() {
dependents_map
.entry(dep.clone())
.or_default()
.insert(specifier.clone());
}
}
if let Resolved::Ok { specifier: dep, .. } = doc.maybe_types_dependency()
{
dependents_map
.entry(dep.clone())
.or_default()
.insert(specifier.clone());
}
}
let mut npm_reqs = HashSet::new();
for specifier in dependents_map.keys() {
if let Ok(reference) = NpmPackageReference::from_specifier(specifier) {
npm_reqs.insert(reference.req);
}
}
self.dependents_map = Arc::new(dependents_map);
self.npm_reqs = npm_reqs;
self.dirty = false;
file_system_docs.dirty = false;
}
@ -1079,7 +1135,21 @@ impl Documents {
fn resolve_dependency(
&self,
specifier: &ModuleSpecifier,
maybe_npm_resolver: Option<&NpmPackageResolver>,
) -> Option<(ModuleSpecifier, MediaType)> {
if let Ok(npm_ref) = NpmPackageReference::from_specifier(specifier) {
return maybe_npm_resolver.map(|npm_resolver| {
NodeResolution::into_specifier_and_media_type(
node_resolve_npm_reference(
&npm_ref,
NodeResolutionMode::Types,
npm_resolver,
)
.ok()
.flatten(),
)
});
}
let doc = self.get(specifier)?;
let maybe_module = doc.maybe_module().and_then(|r| r.as_ref().ok());
let maybe_types_dependency = maybe_module.and_then(|m| {
@ -1088,7 +1158,7 @@ impl Documents {
.map(|(_, resolved)| resolved.clone())
});
if let Some(Resolved::Ok { specifier, .. }) = maybe_types_dependency {
self.resolve_dependency(&specifier)
self.resolve_dependency(&specifier, maybe_npm_resolver)
} else {
let media_type = doc.media_type();
Some((specifier.clone(), media_type))
@ -1113,12 +1183,12 @@ impl Documents {
}
/// Loader that will look at the open documents.
pub struct DocumentsDenoGraphLoader<'a> {
pub struct OpenDocumentsGraphLoader<'a> {
pub inner_loader: &'a mut dyn deno_graph::source::Loader,
pub open_docs: &'a HashMap<ModuleSpecifier, Document>,
}
impl<'a> deno_graph::source::Loader for DocumentsDenoGraphLoader<'a> {
impl<'a> deno_graph::source::Loader for OpenDocumentsGraphLoader<'a> {
fn load(
&mut self,
specifier: &ModuleSpecifier,

View file

@ -66,10 +66,15 @@ use crate::args::LintConfig;
use crate::args::TsConfig;
use crate::deno_dir;
use crate::file_fetcher::get_source_from_data_url;
use crate::file_fetcher::CacheSetting;
use crate::fs_util;
use crate::graph_util::graph_valid;
use crate::npm::NpmCache;
use crate::npm::NpmPackageResolver;
use crate::npm::NpmRegistryApi;
use crate::proc_state::import_map_from_text;
use crate::proc_state::ProcState;
use crate::progress_bar::ProgressBar;
use crate::tools::fmt::format_file;
use crate::tools::fmt::format_parsed_source;
@ -87,6 +92,7 @@ pub struct StateSnapshot {
pub documents: Documents,
pub maybe_import_map: Option<Arc<ImportMap>>,
pub root_uri: Option<Url>,
pub maybe_npm_resolver: Option<NpmPackageResolver>,
}
#[derive(Debug)]
@ -125,6 +131,8 @@ pub struct Inner {
pub maybe_lint_config: Option<LintConfig>,
/// A lazily create "server" for handling test run requests.
maybe_testing_server: Option<testing::TestServer>,
/// Resolver for npm packages.
npm_resolver: NpmPackageResolver,
/// A collection of measurements which instrument that performance of the LSP.
performance: Arc<Performance>,
/// A memoized version of fixable diagnostic codes retrieved from TypeScript.
@ -250,6 +258,26 @@ impl Inner {
ts_server.clone(),
);
let assets = Assets::new(ts_server.clone());
let registry_url = NpmRegistryApi::default_url();
// Use an "only" cache setting in order to make the
// user do an explicit "cache" command and prevent
// the cache from being filled with lots of packages while
// the user is typing.
let cache_setting = CacheSetting::Only;
let progress_bar = ProgressBar::default();
let npm_cache = NpmCache::from_deno_dir(
&dir,
cache_setting.clone(),
progress_bar.clone(),
);
let api = NpmRegistryApi::new(
registry_url,
npm_cache.clone(),
cache_setting,
progress_bar,
);
let npm_resolver =
NpmPackageResolver::new(npm_cache, api, true, false, None);
Self {
assets,
@ -267,6 +295,7 @@ impl Inner {
maybe_testing_server: None,
module_registries,
module_registries_location,
npm_resolver,
performance,
ts_fixable_diagnostics: Default::default(),
ts_server,
@ -435,6 +464,7 @@ impl Inner {
cache_metadata: self.cache_metadata.clone(),
documents: self.documents.clone(),
maybe_import_map: self.maybe_import_map.clone(),
maybe_npm_resolver: Some(self.npm_resolver.snapshotted()),
root_uri: self.config.root_uri.clone(),
})
}
@ -828,7 +858,7 @@ impl Inner {
if let Err(err) =
self.client.register_capability(vec![registration]).await
{
warn!("Client errored on capabilities.\n{}", err);
warn!("Client errored on capabilities.\n{:#}", err);
}
}
self.config.update_enabled_paths(self.client.clone()).await;
@ -891,6 +921,7 @@ impl Inner {
) {
Ok(document) => {
if document.is_diagnosable() {
self.refresh_npm_specifiers().await;
self
.diagnostics_server
.invalidate(&self.documents.dependents(&specifier));
@ -903,6 +934,13 @@ impl Inner {
self.performance.measure(mark);
}
async fn refresh_npm_specifiers(&mut self) {
let package_reqs = self.documents.npm_package_reqs();
if let Err(err) = self.npm_resolver.set_package_reqs(package_reqs).await {
warn!("Could not set npm package requirements. {:#}", err);
}
}
async fn did_close(&mut self, params: DidCloseTextDocumentParams) {
let mark = self.performance.mark("did_close", Some(&params));
if params.text_document.uri.scheme() == "deno" {
@ -917,6 +955,7 @@ impl Inner {
error!("{}", err);
}
if self.is_diagnosable(&specifier) {
self.refresh_npm_specifiers().await;
let mut specifiers = self.documents.dependents(&specifier);
specifiers.push(specifier.clone());
self.diagnostics_server.invalidate(&specifiers);
@ -1135,7 +1174,7 @@ impl Inner {
Ok(None) => Some(Vec::new()),
Err(err) => {
// TODO(lucacasonato): handle error properly
warn!("Format error: {}", err);
warn!("Format error: {:#}", err);
None
}
}
@ -2476,6 +2515,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
let has_specifier_settings =
inner.config.has_specifier_settings(&specifier);
if document.is_diagnosable() {
inner.refresh_npm_specifiers().await;
let specifiers = inner.documents.dependents(&specifier);
inner.diagnostics_server.invalidate(&specifiers);
// don't send diagnostics yet if we don't have the specifier settings
@ -2834,7 +2874,7 @@ impl Inner {
.collect::<HashMap<_, _>>();
let ps = ProcState::from_options(Arc::new(cli_options)).await?;
let mut inner_loader = ps.create_graph_loader();
let mut loader = crate::lsp::documents::DocumentsDenoGraphLoader {
let mut loader = crate::lsp::documents::OpenDocumentsGraphLoader {
inner_loader: &mut inner_loader,
open_docs: &open_docs,
};
@ -2870,6 +2910,9 @@ impl Inner {
ca_stores: None,
ca_file: None,
unsafely_ignore_certificate_errors: None,
// this is to allow loading npm specifiers, so we can remove this
// once stabilizing them
unstable: true,
..Default::default()
},
self.maybe_config_file.clone(),
@ -2892,6 +2935,7 @@ impl Inner {
// For that we're invalidating all the existing diagnostics and restarting
// the language server for TypeScript (as it might hold to some stale
// documents).
self.refresh_npm_specifiers().await;
self.diagnostics_server.invalidate_all();
let _: bool = self
.ts_server

View file

@ -2678,6 +2678,20 @@ fn op_is_cancelled(state: &mut OpState) -> bool {
state.token.is_cancelled()
}
#[op]
fn op_is_node_file(state: &mut OpState, path: String) -> bool {
let state = state.borrow::<State>();
match ModuleSpecifier::parse(&path) {
Ok(specifier) => state
.state_snapshot
.maybe_npm_resolver
.as_ref()
.map(|r| r.in_npm_package(&specifier))
.unwrap_or(false),
Err(_) => false,
}
}
#[op]
fn op_load(
state: &mut OpState,
@ -2692,7 +2706,7 @@ fn op_load(
Some(doc) => {
json!({
"data": doc.text(),
"scriptKind": crate::tsc::as_ts_script_kind(&doc.media_type()),
"scriptKind": crate::tsc::as_ts_script_kind(doc.media_type()),
"version": state.script_version(&specifier),
})
}
@ -2709,11 +2723,11 @@ fn op_resolve(
let mark = state.performance.mark("op_resolve", Some(&args));
let referrer = state.normalize_specifier(&args.base)?;
let result = if let Some(resolved) = state
.state_snapshot
.documents
.resolve(args.specifiers, &referrer)
{
let result = if let Some(resolved) = state.state_snapshot.documents.resolve(
args.specifiers,
&referrer,
state.state_snapshot.maybe_npm_resolver.as_ref(),
) {
Ok(
resolved
.into_iter()
@ -2789,6 +2803,7 @@ fn init_extension(performance: Arc<Performance>) -> Extension {
.ops(vec![
op_exists::decl(),
op_is_cancelled::decl(),
op_is_node_file::decl(),
op_load::decl(),
op_resolve::decl(),
op_respond::decl(),