summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--cli/lsp/diagnostics.rs24
-rw-r--r--cli/lsp/documents.rs89
-rw-r--r--cli/lsp/language_server.rs38
-rw-r--r--cli/lsp/tsc.rs32
-rw-r--r--cli/tests/integration/lsp_tests.rs6
5 files changed, 109 insertions, 80 deletions
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index f84d22b45..ddc18f18f 100644
--- a/cli/lsp/diagnostics.rs
+++ b/cli/lsp/diagnostics.rs
@@ -492,19 +492,17 @@ async fn generate_deps_diagnostics(
.get_version(document.specifier(), &DiagnosticSource::Deno);
if version != current_version {
let mut diagnostics = Vec::new();
- if let Some(dependencies) = document.dependencies() {
- for (_, dependency) in dependencies {
- diagnose_dependency(
- &mut diagnostics,
- &documents,
- &dependency.maybe_code,
- );
- diagnose_dependency(
- &mut diagnostics,
- &documents,
- &dependency.maybe_type,
- );
- }
+ for (_, dependency) in document.dependencies() {
+ diagnose_dependency(
+ &mut diagnostics,
+ &documents,
+ &dependency.maybe_code,
+ );
+ diagnose_dependency(
+ &mut diagnostics,
+ &documents,
+ &dependency.maybe_type,
+ );
}
diagnostics_vec.push((
document.specifier().clone(),
diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index 9892eab1f..51a63eb22 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -245,6 +245,8 @@ impl SyntheticModule {
}
#[derive(Debug, Clone)]
struct DocumentInner {
+ /// contains the last-known-good set of dependencies from parsing the module
+ dependencies: Arc<BTreeMap<String, deno_graph::Dependency>>,
fs_version: String,
line_index: Arc<LineIndex>,
maybe_language_id: Option<LanguageId>,
@@ -282,9 +284,15 @@ impl Document {
maybe_resolver,
Some(&parser),
));
+ let dependencies = if let Some(Ok(module)) = &maybe_module {
+ Arc::new(module.dependencies.clone())
+ } else {
+ Arc::new(BTreeMap::new())
+ };
let text_info = SourceTextInfo::new(content);
let line_index = Arc::new(LineIndex::new(text_info.text_str()));
Self(Arc::new(DocumentInner {
+ dependencies,
fs_version,
line_index,
maybe_language_id: None,
@@ -317,9 +325,15 @@ impl Document {
} else {
None
};
+ let dependencies = if let Some(Ok(module)) = &maybe_module {
+ Arc::new(module.dependencies.clone())
+ } else {
+ Arc::new(BTreeMap::new())
+ };
let source = SourceTextInfo::new(content);
let line_index = Arc::new(LineIndex::new(source.text_str()));
Self(Arc::new(DocumentInner {
+ dependencies,
fs_version: "1".to_string(),
line_index,
maybe_language_id: Some(language_id),
@@ -379,14 +393,20 @@ impl Document {
} else {
None
};
- let source = SourceTextInfo::new(content);
+ let dependencies = if let Some(Ok(module)) = &maybe_module {
+ Arc::new(module.dependencies.clone())
+ } else {
+ self.0.dependencies.clone()
+ };
+ let text_info = SourceTextInfo::new(content);
let line_index = if index_valid == IndexValid::All {
line_index
} else {
- Arc::new(LineIndex::new(source.text_str()))
+ Arc::new(LineIndex::new(text_info.text_str()))
};
Ok(Document(Arc::new(DocumentInner {
- text_info: source,
+ dependencies,
+ text_info,
line_index,
maybe_module,
maybe_lsp_version: Some(version),
@@ -499,15 +519,13 @@ impl Document {
self.0.maybe_warning.clone()
}
- pub fn dependencies(&self) -> Option<Vec<(String, deno_graph::Dependency)>> {
- let module = self.maybe_module()?.as_ref().ok()?;
- Some(
- module
- .dependencies
- .iter()
- .map(|(s, d)| (s.clone(), d.clone()))
- .collect(),
- )
+ pub fn dependencies(&self) -> Vec<(String, deno_graph::Dependency)> {
+ self
+ .0
+ .dependencies
+ .iter()
+ .map(|(s, d)| (s.clone(), d.clone()))
+ .collect()
}
/// If the supplied position is within a dependency range, return the resolved
@@ -911,35 +929,32 @@ impl DocumentsInner {
specifiers: Vec<String>,
referrer: &ModuleSpecifier,
) -> Option<Vec<Option<(ModuleSpecifier, MediaType)>>> {
- let doc = self.get(referrer)?;
+ let dependencies = self.get(referrer)?.0.dependencies.clone();
let mut results = Vec::new();
- if let Some(Ok(module)) = doc.maybe_module() {
- let dependencies = module.dependencies.clone();
- for specifier in specifiers {
- if specifier.starts_with("asset:") {
- if let Ok(specifier) = ModuleSpecifier::parse(&specifier) {
- let media_type = MediaType::from(&specifier);
- results.push(Some((specifier, media_type)));
- } else {
- results.push(None);
- }
- } else if let Some(dep) = dependencies.get(&specifier) {
- if let Some(Ok((specifier, _))) = &dep.maybe_type {
- results.push(self.resolve_dependency(specifier));
- } else if let Some(Ok((specifier, _))) = &dep.maybe_code {
- results.push(self.resolve_dependency(specifier));
- } else {
- results.push(None);
- }
- } else if let Some(Some(Ok((specifier, _)))) =
- self.resolve_imports_dependency(&specifier)
- {
- // clone here to avoid double borrow of self
- let specifier = specifier.clone();
- results.push(self.resolve_dependency(&specifier));
+ for specifier in specifiers {
+ if specifier.starts_with("asset:") {
+ if let Ok(specifier) = ModuleSpecifier::parse(&specifier) {
+ let media_type = MediaType::from(&specifier);
+ results.push(Some((specifier, media_type)));
+ } else {
+ results.push(None);
+ }
+ } else if let Some(dep) = dependencies.get(&specifier) {
+ if let Some(Ok((specifier, _))) = &dep.maybe_type {
+ results.push(self.resolve_dependency(specifier));
+ } else if let Some(Ok((specifier, _))) = &dep.maybe_code {
+ results.push(self.resolve_dependency(specifier));
} else {
results.push(None);
}
+ } else if let Some(Some(Ok((specifier, _)))) =
+ self.resolve_imports_dependency(&specifier)
+ {
+ // clone here to avoid double borrow of self
+ let specifier = specifier.clone();
+ results.push(self.resolve_dependency(&specifier));
+ } else {
+ results.push(None);
}
}
Some(results)
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index dbe00861e..e82afd9d0 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -987,17 +987,16 @@ impl Inner {
let asset_or_document = self.get_cached_asset_or_document(&specifier)?;
let line_index = asset_or_document.line_index();
- let req = tsc::RequestMethod::GetNavigationTree(specifier);
- let navigation_tree: tsc::NavigationTree = self
- .ts_server
- .request(self.snapshot()?, req)
- .await
- .map_err(|err| {
- error!("Failed to request to tsserver {}", err);
- LspError::invalid_request()
+ let navigation_tree =
+ self.get_navigation_tree(&specifier).await.map_err(|err| {
+ error!(
+ "Error getting document symbols for \"{}\": {}",
+ specifier, err
+ );
+ LspError::internal_error()
})?;
- let response = if let Some(child_items) = navigation_tree.child_items {
+ let response = if let Some(child_items) = &navigation_tree.child_items {
let mut document_symbols = Vec::<DocumentSymbol>::new();
for item in child_items {
item
@@ -1543,7 +1542,7 @@ impl Inner {
let asset_or_doc = self.get_cached_asset_or_document(&specifier)?;
let line_index = asset_or_doc.line_index();
let req = tsc::RequestMethod::GetReferences((
- specifier,
+ specifier.clone(),
line_index.offset_tsc(params.text_document_position.position)?,
));
let maybe_references: Option<Vec<tsc::ReferenceEntry>> = self
@@ -1563,9 +1562,14 @@ impl Inner {
}
let reference_specifier =
resolve_url(&reference.document_span.file_name).unwrap();
- let asset_or_doc =
- self.get_asset_or_document(&reference_specifier).await?;
- results.push(reference.to_location(asset_or_doc.line_index(), self));
+ let reference_line_index = if reference_specifier == specifier {
+ line_index.clone()
+ } else {
+ let asset_or_doc =
+ self.get_asset_or_document(&reference_specifier).await?;
+ asset_or_doc.line_index()
+ };
+ results.push(reference.to_location(reference_line_index, self));
}
self.performance.measure(mark);
@@ -2157,8 +2161,8 @@ impl Inner {
LspError::invalid_request()
})?;
- let semantic_tokens: SemanticTokens =
- semantic_classification.to_semantic_tokens(line_index);
+ let semantic_tokens =
+ semantic_classification.to_semantic_tokens(line_index)?;
let response = if !semantic_tokens.data.is_empty() {
Some(SemanticTokensResult::Tokens(semantic_tokens))
} else {
@@ -2200,8 +2204,8 @@ impl Inner {
LspError::invalid_request()
})?;
- let semantic_tokens: SemanticTokens =
- semantic_classification.to_semantic_tokens(line_index);
+ let semantic_tokens =
+ semantic_classification.to_semantic_tokens(line_index)?;
let response = if !semantic_tokens.data.is_empty() {
Some(SemanticTokensRangeResult::Tokens(semantic_tokens))
} else {
diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs
index b5ce48fab..4a61d7403 100644
--- a/cli/lsp/tsc.rs
+++ b/cli/lsp/tsc.rs
@@ -38,6 +38,8 @@ use deno_core::OpFn;
use deno_core::RuntimeOptions;
use deno_runtime::tokio_util::create_basic_runtime;
use log::warn;
+use lspower::jsonrpc::Error as LspError;
+use lspower::jsonrpc::Result as LspResult;
use lspower::lsp;
use regex::Captures;
use regex::Regex;
@@ -1122,7 +1124,7 @@ impl Classifications {
pub fn to_semantic_tokens(
&self,
line_index: Arc<LineIndex>,
- ) -> lsp::SemanticTokens {
+ ) -> LspResult<lsp::SemanticTokens> {
let token_count = self.spans.len() / 3;
let mut builder = SemanticTokensBuilder::new();
for i in 0..token_count {
@@ -1141,16 +1143,26 @@ impl Classifications {
let start_pos = line_index.position_tsc(offset.into());
let end_pos = line_index.position_tsc(TextSize::from(offset + length));
- // start_pos.line == end_pos.line is always true as there are no multiline tokens
- builder.push(
- start_pos.line,
- start_pos.character,
- end_pos.character - start_pos.character,
- token_type,
- token_modifiers,
- );
+ if start_pos.line == end_pos.line
+ && start_pos.character <= end_pos.character
+ {
+ builder.push(
+ start_pos.line,
+ start_pos.character,
+ end_pos.character - start_pos.character,
+ token_type,
+ token_modifiers,
+ );
+ } else {
+ log::error!(
+ "unexpected positions\nstart_pos: {:?}\nend_pos: {:?}",
+ start_pos,
+ end_pos
+ );
+ return Err(LspError::internal_error());
+ }
}
- builder.build(None)
+ Ok(builder.build(None))
}
fn get_token_type_from_classification(ts_classification: u32) -> u32 {
diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs
index 96689f8a3..bf9258925 100644
--- a/cli/tests/integration/lsp_tests.rs
+++ b/cli/tests/integration/lsp_tests.rs
@@ -1066,10 +1066,10 @@ fn lsp_hover_dependency() {
);
}
-// Regression test for #12753
+// This tests for a regression covered by denoland/deno#12753 where the lsp was
+// unable to resolve dependencies when there was an invalid syntax in the module
#[test]
-#[ignore]
-fn lsp_hover_keep_type_info_after_invalid_syntax_change() {
+fn lsp_hover_deps_preserved_when_invalid_parse() {
let mut client = init("initialize_params.json");
did_open(
&mut client,