Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(lsp): remove boolean parameters on documents.documents(...) #18493

Merged
merged 2 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cli/lsp/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use super::client::Client;
use super::config::ConfigSnapshot;
use super::documents::Documents;
use super::documents::DocumentsFilter;
use super::lsp_custom;
use super::registries::ModuleRegistry;
use super::tsc;
Expand Down Expand Up @@ -278,7 +279,7 @@ fn get_import_map_completions(
if let Ok(resolved) = import_map.resolve(&key, specifier) {
let resolved = resolved.to_string();
let workspace_items: Vec<lsp::CompletionItem> = documents
.documents(false, true)
.documents(DocumentsFilter::AllDiagnosable)
.into_iter()
.filter_map(|d| {
let specifier_str = d.specifier().to_string();
Expand Down Expand Up @@ -460,7 +461,7 @@ fn get_workspace_completions(
documents: &Documents,
) -> Vec<lsp::CompletionItem> {
let workspace_specifiers = documents
.documents(false, true)
.documents(DocumentsFilter::AllDiagnosable)
.into_iter()
.map(|d| d.specifier().clone())
.collect();
Expand Down
12 changes: 9 additions & 3 deletions cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use super::client::Client;
use super::config::ConfigSnapshot;
use super::documents;
use super::documents::Document;
use super::documents::DocumentsFilter;
use super::language_server;
use super::language_server::StateSnapshot;
use super::performance::Performance;
Expand Down Expand Up @@ -454,7 +455,9 @@ async fn generate_lint_diagnostics(
lint_options: &LintOptions,
token: CancellationToken,
) -> DiagnosticVec {
let documents = snapshot.documents.documents(true, true);
let documents = snapshot
.documents
.documents(DocumentsFilter::OpenDiagnosable);
let workspace_settings = config.settings.workspace.clone();
let lint_rules = get_configured_rules(lint_options.rules.clone());
let mut diagnostics_vec = Vec::new();
Expand Down Expand Up @@ -530,7 +533,7 @@ async fn generate_ts_diagnostics(
let mut diagnostics_vec = Vec::new();
let specifiers = snapshot
.documents
.documents(true, true)
.documents(DocumentsFilter::OpenDiagnosable)
.into_iter()
.map(|d| d.specifier().clone());
let (enabled_specifiers, disabled_specifiers) = specifiers
Expand Down Expand Up @@ -1025,7 +1028,10 @@ async fn generate_deno_diagnostics(
) -> DiagnosticVec {
let mut diagnostics_vec = Vec::new();

for document in snapshot.documents.documents(true, true) {
for document in snapshot
.documents
.documents(DocumentsFilter::OpenDiagnosable)
{
if token.is_cancelled() {
break;
}
Expand Down
76 changes: 42 additions & 34 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,17 @@ fn get_document_path(
}
}

/// Specify the documents to include on a `documents.documents(...)` call.
#[derive(Debug, Clone, Copy)]
pub enum DocumentsFilter {
/// Includes all the documents (diagnosable & non-diagnosable, open & file system).
All,
/// Includes all the diagnosable documents (open & file system).
AllDiagnosable,
/// Includes only the diagnosable documents that are open.
OpenDiagnosable,
}

#[derive(Debug, Clone, Default)]
pub struct Documents {
/// The DENO_DIR that the documents looks for non-file based modules.
Expand Down Expand Up @@ -1011,47 +1022,44 @@ impl Documents {
}
}

/// Return a vector of documents that are contained in the document store,
/// where `open_only` flag would provide only those documents currently open
/// in the editor and `diagnosable_only` would provide only those documents
/// that the language server can provide diagnostics for.
pub fn documents(
&self,
open_only: bool,
diagnosable_only: bool,
) -> Vec<Document> {
if open_only {
self
/// Return a collection of documents that are contained in the document store
/// based on the provided filter.
pub fn documents(&self, filter: DocumentsFilter) -> Vec<Document> {
match filter {
DocumentsFilter::OpenDiagnosable => self
.open_docs
.values()
.filter_map(|doc| {
if !diagnosable_only || doc.is_diagnosable() {
if doc.is_diagnosable() {
Some(doc.clone())
} else {
None
}
})
.collect()
} else {
// it is technically possible for a Document to end up in both the open
// and closed documents so we need to ensure we don't return duplicates
let mut seen_documents = HashSet::new();
let file_system_docs = self.file_system_docs.lock();
self
.open_docs
.values()
.chain(file_system_docs.docs.values())
.filter_map(|doc| {
// this prefers the open documents
if seen_documents.insert(doc.specifier().clone())
&& (!diagnosable_only || doc.is_diagnosable())
{
Some(doc.clone())
} else {
None
}
})
.collect()
.collect(),
DocumentsFilter::AllDiagnosable | DocumentsFilter::All => {
let diagnosable_only =
matches!(filter, DocumentsFilter::AllDiagnosable);
// it is technically possible for a Document to end up in both the open
// and closed documents so we need to ensure we don't return duplicates
let mut seen_documents = HashSet::new();
let file_system_docs = self.file_system_docs.lock();
self
.open_docs
.values()
.chain(file_system_docs.docs.values())
.filter_map(|doc| {
// this prefers the open documents
if seen_documents.insert(doc.specifier().clone())
&& (!diagnosable_only || doc.is_diagnosable())
{
Some(doc.clone())
} else {
None
}
})
.collect()
}
}
}

Expand Down Expand Up @@ -1592,7 +1600,7 @@ console.log(b, "hello deno");

// At this point the document will be in both documents and the shared file system documents.
// Now make sure that the original documents doesn't return both copies
assert_eq!(documents.documents(false, false).len(), 1);
assert_eq!(documents.documents(DocumentsFilter::All).len(), 1);
}

#[test]
Expand Down
5 changes: 3 additions & 2 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ use super::documents::to_lsp_range;
use super::documents::AssetOrDocument;
use super::documents::Document;
use super::documents::Documents;
use super::documents::DocumentsFilter;
use super::documents::LanguageId;
use super::logging::lsp_log;
use super::lsp_custom;
Expand Down Expand Up @@ -3223,7 +3224,7 @@ impl Inner {
)?;
cli_options.set_import_map_specifier(self.maybe_import_map_uri.clone());

let open_docs = self.documents.documents(true, true);
let open_docs = self.documents.documents(DocumentsFilter::OpenDiagnosable);
Ok(Some(PrepareCacheResult {
cli_options,
open_docs,
Expand Down Expand Up @@ -3341,7 +3342,7 @@ impl Inner {
let mut contents = String::new();
let mut documents_specifiers = self
.documents
.documents(false, false)
.documents(DocumentsFilter::All)
.into_iter()
.map(|d| d.specifier().clone())
.collect::<Vec<_>>();
Expand Down
6 changes: 5 additions & 1 deletion cli/lsp/testing/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use super::lsp_custom;
use crate::lsp::client::Client;
use crate::lsp::client::TestingNotification;
use crate::lsp::config;
use crate::lsp::documents::DocumentsFilter;
use crate::lsp::language_server::StateSnapshot;
use crate::lsp::performance::Performance;

Expand Down Expand Up @@ -92,7 +93,10 @@ impl TestServer {
// eliminating any we go over when iterating over the document
let mut keys: HashSet<ModuleSpecifier> =
tests.keys().cloned().collect();
for document in snapshot.documents.documents(false, true) {
for document in snapshot
.documents
.documents(DocumentsFilter::AllDiagnosable)
{
let specifier = document.specifier();
keys.remove(specifier);
let script_version = document.script_version();
Expand Down
3 changes: 2 additions & 1 deletion cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use super::code_lens;
use super::config;
use super::documents::AssetOrDocument;
use super::documents::DocumentsFilter;
use super::language_server;
use super::language_server::StateSnapshot;
use super::performance::Performance;
Expand Down Expand Up @@ -2760,7 +2761,7 @@ fn op_respond(state: &mut OpState, args: Response) -> bool {
fn op_script_names(state: &mut OpState) -> Vec<String> {
let state = state.borrow_mut::<State>();
let documents = &state.state_snapshot.documents;
let open_docs = documents.documents(true, true);
let open_docs = documents.documents(DocumentsFilter::OpenDiagnosable);
let mut result = Vec::new();
let mut seen = HashSet::new();

Expand Down