Skip to content

Commit

Permalink
refactor(lsp): prefer using document instead of documents collection (d…
Browse files Browse the repository at this point in the history 
…enoland#12720)
  • Loading branch information
@dsherret
dsherret authored Nov 12, 2021
1 parent fdf890a commit 28dbb4a
Show file tree
Hide file tree
Showing 7 changed files with 691 additions and 860 deletions.
21 changes: 12 additions & 9 deletions cli/lsp/code_lens.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,9 @@ async fn resolve_implementation_code_lens(
data: CodeLensData,
language_server: &mut language_server::Inner,
) -> Result<lsp::CodeLens, AnyError> {
let line_index = language_server
.get_line_index_sync(&data.specifier)
.unwrap();
let asset_or_doc =
language_server.get_cached_asset_or_document(&data.specifier)?;
let line_index = asset_or_doc.line_index();
let req = tsc::RequestMethod::GetImplementation((
data.specifier.clone(),
line_index.offset_tsc(code_lens.range.start)?,
Expand Down Expand Up @@ -289,9 +289,9 @@ async fn resolve_references_code_lens(
data: CodeLensData,
language_server: &mut language_server::Inner,
) -> Result<lsp::CodeLens, AnyError> {
let line_index = language_server
.get_line_index_sync(&data.specifier)
.unwrap();
let asset_or_document =
language_server.get_cached_asset_or_document(&data.specifier)?;
let line_index = asset_or_document.line_index();
let req = tsc::RequestMethod::GetReferences((
data.specifier.clone(),
line_index.offset_tsc(code_lens.range.start)?,
Expand All @@ -307,9 +307,12 @@ async fn resolve_references_code_lens(
}
let reference_specifier =
resolve_url(&reference.document_span.file_name)?;
let line_index =
language_server.get_line_index(reference_specifier).await?;
locations.push(reference.to_location(line_index, language_server));
let asset_or_doc = language_server
.get_asset_or_document(&reference_specifier)
.await?;
locations.push(
reference.to_location(asset_or_doc.line_index(), language_server),
);
}
let command = if !locations.is_empty() {
let title = if locations.len() > 1 {
Expand Down
18 changes: 12 additions & 6 deletions cli/lsp/completions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ pub(crate) async fn get_import_completions(
state_snapshot: &language_server::StateSnapshot,
client: lspower::Client,
) -> Option<lsp::CompletionResponse> {
let (text, _, range) = state_snapshot
.documents
.get_maybe_dependency(specifier, position)?;
let document = state_snapshot.documents.get(specifier)?;
let (text, _, range) = document.get_maybe_dependency(position)?;
let range = to_narrow_lsp_range(&range);
// completions for local relative modules
if text.starts_with("./") || text.starts_with("../") {
Expand All @@ -134,7 +133,9 @@ pub(crate) async fn get_import_completions(
};
let maybe_items = state_snapshot
.module_registries
.get_completions(&text, offset, &range, state_snapshot)
.get_completions(&text, offset, &range, |specifier| {
state_snapshot.documents.contains_specifier(specifier)
})
.await;
let items = maybe_items.unwrap_or_else(|| {
get_workspace_completions(specifier, &text, &range, state_snapshot)
Expand Down Expand Up @@ -276,7 +277,12 @@ fn get_workspace_completions(
range: &lsp::Range,
state_snapshot: &language_server::StateSnapshot,
) -> Vec<lsp::CompletionItem> {
let workspace_specifiers = state_snapshot.documents.specifiers(false, true);
let workspace_specifiers = state_snapshot
.documents
.documents(false, true)
.into_iter()
.map(|d| d.specifier().clone())
.collect();
let specifier_strings =
get_relative_specifiers(specifier, workspace_specifiers);
specifier_strings
Expand Down Expand Up @@ -449,7 +455,7 @@ mod tests {
.set(&specifier, HashMap::default(), source.as_bytes())
.expect("could not cache file");
assert!(
documents.content(&specifier).is_some(),
documents.get(&specifier).is_some(),
"source could not be setup"
);
}
Expand Down
85 changes: 52 additions & 33 deletions cli/lsp/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,20 +303,20 @@ async fn generate_lint_diagnostics(
snapshot: &language_server::StateSnapshot,
collection: Arc<Mutex<DiagnosticCollection>>,
) -> Result<DiagnosticVec, AnyError> {
let documents = snapshot.documents.clone();
let documents = snapshot.documents.documents(true, true);
let workspace_settings = snapshot.config.settings.workspace.clone();
let maybe_lint_config = snapshot.maybe_lint_config.clone();
tokio::task::spawn(async move {
let mut diagnostics_vec = Vec::new();
if workspace_settings.lint {
for specifier in documents.specifiers(true, true) {
let version = documents.lsp_version(&specifier);
for document in documents {
let version = document.maybe_lsp_version();
let current_version = collection
.lock()
.await
.get_version(&specifier, &DiagnosticSource::DenoLint);
.get_version(document.specifier(), &DiagnosticSource::DenoLint);
if version != current_version {
let diagnostics = match documents.parsed_source(&specifier) {
let diagnostics = match document.maybe_parsed_source() {
Some(Ok(parsed_source)) => {
if let Ok(references) = analysis::get_lint_references(
&parsed_source,
Expand All @@ -332,11 +332,15 @@ async fn generate_lint_diagnostics(
}
Some(Err(_)) => Vec::new(),
None => {
error!("Missing file contents for: {}", specifier);
error!("Missing file contents for: {}", document.specifier());
Vec::new()
}
};
diagnostics_vec.push((specifier.clone(), version, diagnostics));
diagnostics_vec.push((
document.specifier().clone(),
version,
diagnostics,
));
}
}
}
Expand All @@ -356,14 +360,14 @@ async fn generate_ts_diagnostics(
let collection = collection.lock().await;
snapshot
.documents
.specifiers(true, true)
.documents(true, true)
.iter()
.filter_map(|s| {
let version = snapshot.documents.lsp_version(s);
.filter_map(|d| {
let version = d.maybe_lsp_version();
let current_version =
collection.get_version(s, &DiagnosticSource::TypeScript);
collection.get_version(d.specifier(), &DiagnosticSource::TypeScript);
if version != current_version {
Some(s.clone())
Some(d.specifier().clone())
} else {
None
}
Expand All @@ -376,7 +380,11 @@ async fn generate_ts_diagnostics(
ts_server.request(snapshot.clone(), req).await?;
for (specifier_str, ts_diagnostics) in ts_diagnostics_map {
let specifier = resolve_url(&specifier_str)?;
let version = snapshot.documents.lsp_version(&specifier);
let version = snapshot
.documents
.get(&specifier)
.map(|d| d.maybe_lsp_version())
.flatten();
diagnostics_vec.push((
specifier,
version,
Expand Down Expand Up @@ -421,7 +429,18 @@ fn diagnose_dependency(
) {
match resolved {
Some(Ok((specifier, range))) => {
if !documents.contains_specifier(specifier) {
if let Some(doc) = documents.get(specifier) {
if let Some(message) = doc.maybe_warning() {
diagnostics.push(lsp::Diagnostic {
range: documents::to_lsp_range(range),
severity: Some(lsp::DiagnosticSeverity::Warning),
code: Some(lsp::NumberOrString::String("deno-warn".to_string())),
source: Some("deno".to_string()),
message,
..Default::default()
})
}
} else {
let (code, message) = match specifier.scheme() {
"file" => (Some(lsp::NumberOrString::String("no-local".to_string())), format!("Unable to load a local module: \"{}\".\n Please check the file path.", specifier)),
"data" => (Some(lsp::NumberOrString::String("no-cache-data".to_string())), "Uncached data URL.".to_string()),
Expand All @@ -437,15 +456,6 @@ fn diagnose_dependency(
data: Some(json!({ "specifier": specifier })),
..Default::default()
});
} else if let Some(message) = documents.maybe_warning(specifier) {
diagnostics.push(lsp::Diagnostic {
range: documents::to_lsp_range(range),
severity: Some(lsp::DiagnosticSeverity::Warning),
code: Some(lsp::NumberOrString::String("deno-warn".to_string())),
source: Some("deno".to_string()),
message,
..Default::default()
})
}
}
Some(Err(err)) => diagnostics.push(lsp::Diagnostic {
Expand All @@ -471,18 +481,18 @@ async fn generate_deps_diagnostics(
tokio::task::spawn(async move {
let mut diagnostics_vec = Vec::new();

for specifier in documents.specifiers(true, true) {
if !config.specifier_enabled(&specifier) {
for document in documents.documents(true, true) {
if !config.specifier_enabled(document.specifier()) {
continue;
}
let version = documents.lsp_version(&specifier);
let version = document.maybe_lsp_version();
let current_version = collection
.lock()
.await
.get_version(&specifier, &DiagnosticSource::Deno);
.get_version(document.specifier(), &DiagnosticSource::Deno);
if version != current_version {
let mut diagnostics = Vec::new();
if let Some(dependencies) = documents.dependencies(&specifier) {
if let Some(dependencies) = document.dependencies() {
for (_, dependency) in dependencies {
diagnose_dependency(
&mut diagnostics,
Expand All @@ -496,7 +506,11 @@ async fn generate_deps_diagnostics(
);
}
}
diagnostics_vec.push((specifier.clone(), version, diagnostics));
diagnostics_vec.push((
document.specifier().clone(),
version,
diagnostics,
));
}
}

Expand Down Expand Up @@ -533,9 +547,14 @@ async fn publish_diagnostics(
diagnostics
.extend(collection.get(&specifier, DiagnosticSource::Deno).cloned());
}
let uri = specifier.clone();
let version = snapshot.documents.lsp_version(&specifier);
client.publish_diagnostics(uri, diagnostics, version).await;
let version = snapshot
.documents
.get(&specifier)
.map(|d| d.maybe_lsp_version())
.flatten();
client
.publish_diagnostics(specifier.clone(), diagnostics, version)
.await;
}
}
}
Expand Down Expand Up @@ -678,7 +697,7 @@ mod tests {
let (snapshot, collection, _) = setup(&[(
"file:///a.ts",
r#"import * as b from "./b.ts";
let a = "a";
console.log(a);
"#,
Expand Down
Loading

0 comments on commit 28dbb4a

Please sign in to comment.