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

I think this makes things clearer at the call sites.

cli/lsp/completions.rs

@@ -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;
@@ -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();
@@ -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();

cli/lsp/diagnostics.rs

@@ -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;
@@ -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();
@@ -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
@@ -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;
     }

cli/lsp/documents.rs

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

cli/lsp/language_server.rs

@@ -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;
@@ -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,
@@ -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<_>>();

cli/lsp/testing/server.rs

@@ -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;
 
@@ -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();

cli/lsp/tsc.rs

@@ -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;
@@ -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();