1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2025-01-21 21:50:00 -05:00

fix(lsp): handle type deps properly (#9436)

Fixes #9425
This commit is contained in:
Kitson Kelly 2021-02-10 09:46:12 +11:00 committed by GitHub
parent ffe12aa92d
commit 6752be05cd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 128 additions and 101 deletions

View file

@ -16,6 +16,7 @@ use deno_core::error::AnyError;
use deno_core::serde::Deserialize;
use deno_core::serde::Serialize;
use deno_core::serde_json::json;
use deno_core::ModuleResolutionError;
use deno_core::ModuleSpecifier;
use deno_lint::rules;
use lspower::lsp;
@ -23,6 +24,7 @@ use lspower::lsp::Position;
use lspower::lsp::Range;
use std::cmp::Ordering;
use std::collections::HashMap;
use std::fmt;
use std::rc::Rc;
lazy_static! {
@ -142,10 +144,33 @@ pub struct Dependency {
pub maybe_type: Option<ResolvedDependency>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ResolvedDependencyErr {
InvalidDowngrade,
InvalidLocalImport,
InvalidSpecifier(ModuleResolutionError),
Missing,
}
impl fmt::Display for ResolvedDependencyErr {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::InvalidDowngrade => {
write!(f, "HTTPS modules cannot import HTTP modules.")
}
Self::InvalidLocalImport => {
write!(f, "Remote modules cannot import local modules.")
}
Self::InvalidSpecifier(err) => write!(f, "{}", err),
Self::Missing => write!(f, "The module is unexpectedly missing."),
}
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ResolvedDependency {
Resolved(ModuleSpecifier),
Err(String),
Err(ResolvedDependencyErr),
}
pub fn resolve_import(
@ -170,22 +195,23 @@ pub fn resolve_import(
} else {
match ModuleSpecifier::resolve_import(specifier, referrer.as_str()) {
Ok(resolved) => resolved,
Err(err) => return ResolvedDependency::Err(err.to_string()),
Err(err) => {
return ResolvedDependency::Err(
ResolvedDependencyErr::InvalidSpecifier(err),
)
}
}
};
let referrer_scheme = referrer.as_url().scheme();
let specifier_scheme = specifier.as_url().scheme();
if referrer_scheme == "https" && specifier_scheme == "http" {
return ResolvedDependency::Err(
"Modules imported via https are not allowed to import http modules."
.to_string(),
);
return ResolvedDependency::Err(ResolvedDependencyErr::InvalidDowngrade);
}
if (referrer_scheme == "https" || referrer_scheme == "http")
&& !(specifier_scheme == "https" || specifier_scheme == "http")
&& !remapped
{
return ResolvedDependency::Err("Remote modules are not allowed to import local modules. Consider using a dynamic import instead.".to_string());
return ResolvedDependency::Err(ResolvedDependencyErr::InvalidLocalImport);
}
ResolvedDependency::Resolved(specifier)
@ -241,8 +267,8 @@ pub fn analyze_dependencies(
let resolved_import =
resolve_import(&desc.specifier, specifier, maybe_import_map);
// Check for `@deno-types` pragmas that effect the import
let maybe_resolved_type_import =
let maybe_resolved_type_dependency =
// Check for `@deno-types` pragmas that affect the import
if let Some(comment) = desc.leading_comments.last() {
if let Some(deno_types) = parse_deno_types(&comment.text).as_ref() {
Some(resolve_import(deno_types, specifier, maybe_import_map))
@ -276,8 +302,8 @@ pub fn analyze_dependencies(
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;
if maybe_resolved_type_dependency.is_some() && dep.maybe_type.is_none() {
dep.maybe_type = maybe_resolved_type_dependency;
}
}

View file

@ -279,14 +279,14 @@ pub async fn generate_dependency_diagnostics(
&dependency.maybe_code_specifier_range,
) {
match code.clone() {
ResolvedDependency::Err(message) => {
ResolvedDependency::Err(err) => {
diagnostic_list.push(lsp::Diagnostic {
range: *range,
severity: Some(lsp::DiagnosticSeverity::Error),
code: None,
code_description: None,
source: Some("deno".to_string()),
message,
message: format!("{}", err),
related_information: None,
tags: None,
data: None,

View file

@ -3,9 +3,6 @@
use super::analysis;
use super::text::LineIndex;
use crate::import_map::ImportMap;
use crate::media_type::MediaType;
use deno_core::error::custom_error;
use deno_core::error::AnyError;
use deno_core::error::Context;
@ -76,9 +73,10 @@ impl DocumentData {
}
pub fn content(&self) -> Result<Option<String>, AnyError> {
if let Some(bytes) = self.bytes.clone() {
if let Some(bytes) = &self.bytes {
Ok(Some(
String::from_utf8(bytes).context("cannot decode bytes to string")?,
String::from_utf8(bytes.clone())
.context("cannot decode bytes to string")?,
))
} else {
Ok(None)
@ -92,46 +90,12 @@ pub struct DocumentCache {
}
impl DocumentCache {
pub fn analyze_dependencies(
&mut self,
specifier: &ModuleSpecifier,
maybe_import_map: &Option<ImportMap>,
) -> Result<(), AnyError> {
if !self.contains(specifier) {
return Err(custom_error(
"NotFound",
format!(
"The specifier (\"{}\") does not exist in the document cache.",
specifier
),
));
}
let doc = self.docs.get_mut(specifier).unwrap();
if let Some(source) = &doc.content()? {
if let Some((dependencies, _)) = analysis::analyze_dependencies(
specifier,
source,
&MediaType::from(specifier),
maybe_import_map,
) {
doc.dependencies = Some(dependencies);
} else {
doc.dependencies = None;
}
} else {
doc.dependencies = None;
}
Ok(())
}
pub fn change(
&mut self,
specifier: &ModuleSpecifier,
version: i32,
content_changes: Vec<TextDocumentContentChangeEvent>,
) -> Result<(), AnyError> {
) -> Result<Option<String>, AnyError> {
if !self.contains(specifier) {
return Err(custom_error(
"NotFound",
@ -145,7 +109,7 @@ impl DocumentCache {
let doc = self.docs.get_mut(specifier).unwrap();
doc.apply_content_changes(content_changes)?;
doc.version = Some(version);
Ok(())
doc.content()
}
pub fn close(&mut self, specifier: &ModuleSpecifier) {
@ -187,12 +151,7 @@ impl DocumentCache {
doc.line_index.clone()
}
pub fn open(
&mut self,
specifier: ModuleSpecifier,
version: i32,
text: String,
) {
pub fn open(&mut self, specifier: ModuleSpecifier, version: i32, text: &str) {
self.docs.insert(
specifier,
DocumentData {
@ -218,6 +177,25 @@ impl DocumentCache {
.collect()
}
pub fn set_dependencies(
&mut self,
specifier: &ModuleSpecifier,
maybe_dependencies: Option<HashMap<String, analysis::Dependency>>,
) -> Result<(), AnyError> {
if let Some(doc) = self.docs.get_mut(specifier) {
doc.dependencies = maybe_dependencies;
Ok(())
} else {
Err(custom_error(
"NotFound",
format!(
"The specifier (\"{}\") does not exist in the document cache.",
specifier
),
))
}
}
pub fn version(&self, specifier: &ModuleSpecifier) -> Option<i32> {
self.docs.get(specifier).and_then(|doc| doc.version)
}
@ -234,11 +212,7 @@ mod tests {
let specifier = ModuleSpecifier::resolve_url("file:///a/b.ts").unwrap();
let missing_specifier =
ModuleSpecifier::resolve_url("file:///a/c.ts").unwrap();
document_cache.open(
specifier.clone(),
1,
"console.log(\"Hello Deno\");\n".to_owned(),
);
document_cache.open(specifier.clone(), 1, "console.log(\"Hello Deno\");\n");
assert!(document_cache.contains(&specifier));
assert!(!document_cache.contains(&missing_specifier));
}
@ -247,11 +221,7 @@ mod tests {
fn test_document_cache_change() {
let mut document_cache = DocumentCache::default();
let specifier = ModuleSpecifier::resolve_url("file:///a/b.ts").unwrap();
document_cache.open(
specifier.clone(),
1,
"console.log(\"Hello deno\");\n".to_owned(),
);
document_cache.open(specifier.clone(), 1, "console.log(\"Hello deno\");\n");
document_cache
.change(
&specifier,
@ -282,11 +252,7 @@ mod tests {
fn test_document_cache_change_utf16() {
let mut document_cache = DocumentCache::default();
let specifier = ModuleSpecifier::resolve_url("file:///a/b.ts").unwrap();
document_cache.open(
specifier.clone(),
1,
"console.log(\"Hello 🦕\");\n".to_owned(),
);
document_cache.open(specifier.clone(), 1, "console.log(\"Hello 🦕\");\n");
document_cache
.change(
&specifier,

View file

@ -26,14 +26,17 @@ use tokio::fs;
use crate::deno_dir;
use crate::import_map::ImportMap;
use crate::media_type::MediaType;
use crate::tsc_config::parse_config;
use crate::tsc_config::TsConfig;
use super::analysis;
use super::analysis::ts_changes_to_edit;
use super::analysis::CodeActionCollection;
use super::analysis::CodeActionData;
use super::analysis::CodeLensData;
use super::analysis::CodeLensSource;
use super::analysis::ResolvedDependency;
use super::capabilities;
use super::config::Config;
use super::diagnostics;
@ -82,7 +85,7 @@ pub(crate) struct Inner {
/// file which will be used by the Deno LSP.
maybe_config_uri: Option<Url>,
/// An optional import map which is used to resolve modules.
maybe_import_map: Option<ImportMap>,
pub maybe_import_map: Option<ImportMap>,
/// The URL for the import map which is used to determine relative imports.
maybe_import_map_uri: Option<Url>,
/// A map of all the cached navigation trees.
@ -128,6 +131,33 @@ impl Inner {
}
}
/// Analyzes dependencies of a document that has been opened in the editor and
/// sets the dependencies property on the document.
fn analyze_dependencies(
&mut self,
specifier: &ModuleSpecifier,
source: &str,
) {
if let Some((mut deps, _)) = analysis::analyze_dependencies(
specifier,
source,
&MediaType::from(specifier),
&self.maybe_import_map,
) {
for (_, dep) in deps.iter_mut() {
if dep.maybe_type.is_none() {
if let Some(ResolvedDependency::Resolved(resolved)) = &dep.maybe_code
{
dep.maybe_type = self.sources.get_maybe_types(resolved);
}
}
}
if let Err(err) = self.documents.set_dependencies(specifier, Some(deps)) {
error!("{}", err);
}
}
}
fn enabled(&self) -> bool {
self.config.settings.enable
}
@ -633,19 +663,11 @@ impl Inner {
self.documents.open(
specifier.clone(),
params.text_document.version,
params.text_document.text,
&params.text_document.text,
);
if let Err(err) = self
.documents
.analyze_dependencies(&specifier, &self.maybe_import_map)
{
error!("{}", err);
}
// there are scenarios where local documents with a nav tree are opened in
// the editor
self.navigation_trees.remove(&specifier);
self.analyze_dependencies(&specifier, &params.text_document.text);
self.performance.measure(mark);
// TODO(@kitsonk): how to better lazily do this?
if let Err(err) = self.prepare_diagnostics().await {
error!("{}", err);
@ -655,22 +677,17 @@ impl Inner {
async fn did_change(&mut self, params: DidChangeTextDocumentParams) {
let mark = self.performance.mark("did_change");
let specifier = utils::normalize_url(params.text_document.uri);
if let Err(err) = self.documents.change(
match self.documents.change(
&specifier,
params.text_document.version,
params.content_changes,
) {
error!("{}", err);
Ok(Some(source)) => self.analyze_dependencies(&specifier, &source),
Ok(_) => error!("No content returned from change."),
Err(err) => error!("{}", err),
}
if let Err(err) = self
.documents
.analyze_dependencies(&specifier, &self.maybe_import_map)
{
error!("{}", err);
}
self.navigation_trees.remove(&specifier);
self.performance.measure(mark);
// TODO(@kitsonk): how to better lazily do this?
if let Err(err) = self.prepare_diagnostics().await {
error!("{}", err);
@ -1700,6 +1717,11 @@ impl lspower::LanguageServer for LanguageServer {
self.0.lock().await.did_change(params).await
}
async fn did_save(&self, _params: DidSaveTextDocumentParams) {
// We don't need to do anything on save at the moment, but if this isn't
// implemented, lspower complains about it not being implemented.
}
async fn did_close(&self, params: DidCloseTextDocumentParams) {
self.0.lock().await.did_close(params).await
}

View file

@ -98,6 +98,14 @@ impl Sources {
Some(metadata.line_index)
}
pub fn get_maybe_types(
&mut self,
specifier: &ModuleSpecifier,
) -> Option<analysis::ResolvedDependency> {
let metadata = self.get_metadata(specifier)?;
metadata.maybe_types
}
pub fn get_media_type(
&mut self,
specifier: &ModuleSpecifier,
@ -115,6 +123,8 @@ impl Sources {
}
}
}
// TODO(@kitsonk) this needs to be refactored, lots of duplicate logic and
// is really difficult to follow.
let version = self.get_script_version(specifier)?;
let path = self.get_path(specifier)?;
if let Ok(bytes) = fs::read(path) {
@ -123,12 +133,13 @@ impl Sources {
if let Ok(source) = get_source_from_bytes(bytes, Some(charset)) {
let media_type = MediaType::from(specifier);
let mut maybe_types = None;
let maybe_import_map = self.maybe_import_map.clone();
let dependencies = if let Some((dependencies, mt)) =
analysis::analyze_dependencies(
&specifier,
&source,
&media_type,
&None,
&maybe_import_map,
) {
maybe_types = mt;
Some(dependencies)
@ -165,12 +176,13 @@ impl Sources {
} else {
None
};
let maybe_import_map = self.maybe_import_map.clone();
let dependencies = if let Some((dependencies, mt)) =
analysis::analyze_dependencies(
&specifier,
&source,
&media_type,
&None,
&maybe_import_map,
) {
if maybe_types.is_none() {
maybe_types = mt;

View file

@ -2,6 +2,7 @@
use super::analysis::CodeLensSource;
use super::analysis::ResolvedDependency;
use super::analysis::ResolvedDependencyErr;
use super::language_server;
use super::language_server::StateSnapshot;
use super::text;
@ -1096,7 +1097,7 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
} else if let Some(resolved_import) = &dependency.maybe_code {
resolved_import.clone()
} else {
ResolvedDependency::Err("missing dependency".to_string())
ResolvedDependency::Err(ResolvedDependencyErr::Missing)
};
if let ResolvedDependency::Resolved(resolved_specifier) =
resolved_import
@ -1476,7 +1477,7 @@ mod tests {
for (specifier, content, version) in sources {
let specifier = ModuleSpecifier::resolve_url(specifier)
.expect("failed to create specifier");
documents.open(specifier, version, content.to_string());
documents.open(specifier, version, content);
}
StateSnapshot {
assets: Default::default(),