fix(lsp): provide diagnostics for unresolved modules (#8872)

This commit is contained in:
Kitson Kelly 2020-12-24 21:53:03 +11:00 committed by GitHub
parent 06fa5eb7f3
commit a4d557126e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 166 additions and 21 deletions

View File

@ -86,7 +86,6 @@ pub fn references_to_diagnostics(
severity: Some(lsp_types::DiagnosticSeverity::Warning),
code: Some(lsp_types::NumberOrString::String(code)),
code_description: None,
// TODO(@kitsonk) this won't make sense for every diagnostic
source: Some("deno-lint".to_string()),
message,
related_information: None,
@ -100,12 +99,13 @@ pub fn references_to_diagnostics(
#[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct Dependency {
pub is_dynamic: bool,
pub maybe_code: Option<ResolvedImport>,
pub maybe_type: Option<ResolvedImport>,
pub maybe_code: Option<ResolvedDependency>,
pub maybe_code_specifier_range: Option<Range>,
pub maybe_type: Option<ResolvedDependency>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ResolvedImport {
pub enum ResolvedDependency {
Resolved(ModuleSpecifier),
Err(String),
}
@ -114,7 +114,7 @@ pub fn resolve_import(
specifier: &str,
referrer: &ModuleSpecifier,
maybe_import_map: &Option<ImportMap>,
) -> ResolvedImport {
) -> ResolvedDependency {
let maybe_mapped = if let Some(import_map) = maybe_import_map {
if let Ok(maybe_specifier) =
import_map.resolve(specifier, referrer.as_str())
@ -132,13 +132,13 @@ pub fn resolve_import(
} else {
match ModuleSpecifier::resolve_import(specifier, referrer.as_str()) {
Ok(resolved) => resolved,
Err(err) => return ResolvedImport::Err(err.to_string()),
Err(err) => return ResolvedDependency::Err(err.to_string()),
}
};
let referrer_scheme = referrer.as_url().scheme();
let specifier_scheme = specifier.as_url().scheme();
if referrer_scheme == "https" && specifier_scheme == "http" {
return ResolvedImport::Err(
return ResolvedDependency::Err(
"Modules imported via https are not allowed to import http modules."
.to_string(),
);
@ -147,10 +147,10 @@ pub fn resolve_import(
&& !(specifier_scheme == "https" || specifier_scheme == "http")
&& !remapped
{
return ResolvedImport::Err("Remote modules are not allowed to import local modules. Consider using a dynamic import instead.".to_string());
return ResolvedDependency::Err("Remote modules are not allowed to import local modules. Consider using a dynamic import instead.".to_string());
}
ResolvedImport::Resolved(specifier)
ResolvedDependency::Resolved(specifier)
}
// TODO(@kitsonk) a lot of this logic is duplicated in module_graph.rs in
@ -160,7 +160,7 @@ pub fn analyze_dependencies(
source: &str,
media_type: &MediaType,
maybe_import_map: &Option<ImportMap>,
) -> Option<(HashMap<String, Dependency>, Option<ResolvedImport>)> {
) -> Option<(HashMap<String, Dependency>, Option<ResolvedDependency>)> {
let specifier_str = specifier.to_string();
let source_map = Rc::new(swc_common::SourceMap::default());
let mut maybe_type = None;
@ -222,7 +222,21 @@ pub fn analyze_dependencies(
| swc_ecmascript::dep_graph::DependencyKind::ImportType => {
dep.maybe_type = Some(resolved_import)
}
_ => dep.maybe_code = Some(resolved_import),
_ => {
dep.maybe_code_specifier_range = Some(Range {
start: Position {
line: (desc.specifier_line - 1) as u32,
character: desc.specifier_col as u32,
},
end: Position {
line: (desc.specifier_line - 1) as u32,
character: (desc.specifier_col
+ desc.specifier.chars().count()
+ 2) as u32,
},
});
dep.maybe_code = Some(resolved_import);
}
}
if maybe_resolved_type_import.is_some() && dep.maybe_type.is_none() {
dep.maybe_type = maybe_resolved_type_import;
@ -293,27 +307,47 @@ mod tests {
actual.get("https://cdn.skypack.dev/react").cloned(),
Some(Dependency {
is_dynamic: false,
maybe_code: Some(ResolvedImport::Resolved(
maybe_code: Some(ResolvedDependency::Resolved(
ModuleSpecifier::resolve_url("https://cdn.skypack.dev/react")
.unwrap()
)),
maybe_type: Some(ResolvedImport::Resolved(
maybe_type: Some(ResolvedDependency::Resolved(
ModuleSpecifier::resolve_url(
"https://deno.land/x/types/react/index.d.ts"
)
.unwrap()
)),
maybe_code_specifier_range: Some(Range {
start: Position {
line: 8,
character: 27,
},
end: Position {
line: 8,
character: 58,
}
}),
})
);
assert_eq!(
actual.get("https://deno.land/x/oak@v6.3.2/mod.ts").cloned(),
Some(Dependency {
is_dynamic: false,
maybe_code: Some(ResolvedImport::Resolved(
maybe_code: Some(ResolvedDependency::Resolved(
ModuleSpecifier::resolve_url("https://deno.land/x/oak@v6.3.2/mod.ts")
.unwrap()
)),
maybe_type: None,
maybe_code_specifier_range: Some(Range {
start: Position {
line: 5,
character: 11,
},
end: Position {
line: 5,
character: 50,
}
}),
})
);
}

View File

@ -2,6 +2,7 @@
use super::analysis::get_lint_references;
use super::analysis::references_to_diagnostics;
use super::analysis::ResolvedDependency;
use super::language_server::StateSnapshot;
use super::memory_cache::FileId;
use super::tsc;
@ -9,6 +10,7 @@ use super::tsc;
use crate::diagnostics;
use crate::media_type::MediaType;
use deno_core::error::custom_error;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_core::serde_json::Value;
@ -19,6 +21,7 @@ use std::mem;
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub enum DiagnosticSource {
Deno,
Lint,
TypeScript,
}
@ -261,3 +264,76 @@ pub async fn generate_ts_diagnostics(
Ok(diagnostics)
}
pub async fn generate_dependency_diagnostics(
state_snapshot: StateSnapshot,
diagnostic_collection: DiagnosticCollection,
) -> Result<DiagnosticVec, AnyError> {
tokio::task::spawn_blocking(move || {
let mut diagnostics = Vec::new();
let file_cache = state_snapshot.file_cache.read().unwrap();
let mut sources = if let Ok(sources) = state_snapshot.sources.write() {
sources
} else {
return Err(custom_error("Deadlock", "deadlock locking sources"));
};
for (specifier, doc_data) in state_snapshot.doc_data.iter() {
let file_id = file_cache.lookup(specifier).unwrap();
let version = doc_data.version;
let current_version = diagnostic_collection.get_version(&file_id);
if version != current_version {
let mut diagnostic_list = Vec::new();
if let Some(dependencies) = &doc_data.dependencies {
for (_, dependency) in dependencies.iter() {
if let (Some(code), Some(range)) = (
&dependency.maybe_code,
&dependency.maybe_code_specifier_range,
) {
match code.clone() {
ResolvedDependency::Err(message) => {
diagnostic_list.push(lsp_types::Diagnostic {
range: *range,
severity: Some(lsp_types::DiagnosticSeverity::Error),
code: None,
code_description: None,
source: Some("deno".to_string()),
message,
related_information: None,
tags: None,
data: None,
})
}
ResolvedDependency::Resolved(specifier) => {
if !(state_snapshot.doc_data.contains_key(&specifier) || sources.contains(&specifier)) {
let is_local = specifier.as_url().scheme() == "file";
diagnostic_list.push(lsp_types::Diagnostic {
range: *range,
severity: Some(lsp_types::DiagnosticSeverity::Error),
code: None,
code_description: None,
source: Some("deno".to_string()),
message: if is_local {
format!("Unable to load a local module: \"{}\".\n Please check the file path.", specifier)
} else {
format!("Unable to load the module: \"{}\".\n If the module exists, running `deno cache {}` should resolve this error.", specifier, specifier)
},
related_information: None,
tags: None,
data: None,
})
}
},
}
}
}
}
diagnostics.push((file_id, version, diagnostic_list))
}
}
Ok(diagnostics)
})
.await
.unwrap()
}

View File

@ -178,9 +178,35 @@ impl LanguageServer {
Ok::<(), AnyError>(())
};
let (lint_res, ts_res) = tokio::join!(lint, ts);
let deps = async {
if enabled {
let diagnostics_collection = self.diagnostics.read().unwrap().clone();
let diagnostics = diagnostics::generate_dependency_diagnostics(
self.snapshot(),
diagnostics_collection,
)
.await?;
{
let mut diagnostics_collection = self.diagnostics.write().unwrap();
for (file_id, version, diagnostics) in diagnostics {
diagnostics_collection.set(
file_id,
DiagnosticSource::Deno,
version,
diagnostics,
);
}
}
self.publish_diagnostics().await?
};
Ok::<(), AnyError>(())
};
let (lint_res, ts_res, deps_res) = tokio::join!(lint, ts, deps);
lint_res?;
ts_res?;
deps_res?;
Ok(())
}
@ -213,6 +239,11 @@ impl LanguageServer {
.diagnostics_for(file_id, DiagnosticSource::TypeScript)
.cloned(),
);
diagnostics.extend(
diagnostics_collection
.diagnostics_for(file_id, DiagnosticSource::Deno)
.cloned(),
);
}
let specifier = {
let file_cache = self.file_cache.read().unwrap();

View File

@ -23,7 +23,7 @@ use std::time::SystemTime;
#[derive(Debug, Clone, Default)]
struct Metadata {
dependencies: Option<HashMap<String, analysis::Dependency>>,
maybe_types: Option<analysis::ResolvedImport>,
maybe_types: Option<analysis::ResolvedDependency>,
media_type: MediaType,
source: String,
version: String,
@ -255,7 +255,7 @@ impl Sources {
let dependencies = &metadata.dependencies?;
let dependency = dependencies.get(specifier)?;
if let Some(type_dependency) = &dependency.maybe_type {
if let analysis::ResolvedImport::Resolved(resolved_specifier) =
if let analysis::ResolvedDependency::Resolved(resolved_specifier) =
type_dependency
{
self.resolution_result(resolved_specifier)
@ -264,7 +264,7 @@ impl Sources {
}
} else {
let code_dependency = &dependency.maybe_code.clone()?;
if let analysis::ResolvedImport::Resolved(resolved_specifier) =
if let analysis::ResolvedDependency::Resolved(resolved_specifier) =
code_dependency
{
self.resolution_result(resolved_specifier)

View File

@ -1,6 +1,6 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
use super::analysis::ResolvedImport;
use super::analysis::ResolvedDependency;
use super::language_server::StateSnapshot;
use super::text;
use super::utils;
@ -839,9 +839,10 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
} else if let Some(resolved_import) = &dependency.maybe_code {
resolved_import.clone()
} else {
ResolvedImport::Err("missing dependency".to_string())
ResolvedDependency::Err("missing dependency".to_string())
};
if let ResolvedImport::Resolved(resolved_specifier) = resolved_import
if let ResolvedDependency::Resolved(resolved_specifier) =
resolved_import
{
if state
.state_snapshot

View File

@ -128,6 +128,9 @@ delete Object.prototype.__proto__;
// TS2691: An import path cannot end with a '.ts' extension. Consider
// importing 'bad-module' instead.
2691,
// TS2792: Cannot find module. Did you mean to set the 'moduleResolution'
// option to 'node', or to add aliases to the 'paths' option?
2792,
// TS5009: Cannot find the common subdirectory path for the input files.
5009,
// TS5055: Cannot write file