From 7f7d65aa3a3b443bc85f55e2e3754cbc14fde82d Mon Sep 17 00:00:00 2001
From: David Sherret <dsherret@users.noreply.github.com>
Date: Tue, 25 Jan 2022 09:21:59 -0500
Subject: [PATCH] refactor(lsp): Documents - combine duplicate exists methods
 (#13479)

---
 cli/lsp/completions.rs |  2 +-
 cli/lsp/documents.rs   | 58 +++++++++++++++++-------------------------
 2 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs
index beab247b73..c3026697ae 100644
--- a/cli/lsp/completions.rs
+++ b/cli/lsp/completions.rs
@@ -146,7 +146,7 @@ pub(crate) async fn get_import_completions(
     };
     let maybe_list = module_registries
       .get_completions(&text, offset, &range, |specifier| {
-        documents.contains_specifier(specifier)
+        documents.exists(specifier)
       })
       .await;
     let list = maybe_list.unwrap_or_else(|| lsp::CompletionList {
diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index e200538971..5d583330d5 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -719,7 +719,8 @@ struct FileSystemDocuments {
 }
 
 impl FileSystemDocuments {
-  /// Adds or updates a document by reading the document from the file system.
+  /// Adds or updates a document by reading the document from the file system
+  /// returning the document.
   fn refresh_document(
     &mut self,
     cache: &HttpCache,
@@ -756,7 +757,8 @@ impl FileSystemDocuments {
       )
     };
     self.dirty = true;
-    self.docs.insert(specifier.clone(), doc)
+    self.docs.insert(specifier.clone(), doc.clone());
+    Some(doc)
   }
 }
 
@@ -780,12 +782,7 @@ fn get_document_path(
   if specifier.scheme() == "file" {
     specifier_to_file_path(specifier).ok()
   } else {
-    let path = cache.get_cache_filename(specifier)?;
-    if path.is_file() {
-      Some(path)
-    } else {
-      None
-    }
+    cache.get_cache_filename(specifier)
   }
 }
 
@@ -921,15 +918,25 @@ impl Documents {
       deno_core::resolve_import(specifier, referrer.as_str()).ok()
     };
     if let Some(import_specifier) = maybe_specifier {
-      self.contains_specifier(&import_specifier)
+      self.exists(&import_specifier)
     } else {
       false
     }
   }
 
   /// Return `true` if the specifier can be resolved to a document.
-  pub fn contains_specifier(&self, specifier: &ModuleSpecifier) -> bool {
-    self.get(specifier).is_some()
+  pub fn exists(&self, specifier: &ModuleSpecifier) -> bool {
+    // keep this fast because it's used by op_exists, which is a hot path in tsc
+    let specifier = self.specifier_resolver.resolve(specifier);
+    if let Some(specifier) = specifier {
+      if self.open_docs.contains_key(&specifier) {
+        return true;
+      }
+      if let Some(path) = get_document_path(&self.cache, &specifier) {
+        return path.is_file();
+      }
+    }
+    false
   }
 
   /// Return an array of specifiers, if any, that are dependent upon the
@@ -949,22 +956,6 @@ impl Documents {
     }
   }
 
-  /// Used by the tsc op_exists to shortcut trying to load a document to provide
-  /// information to CLI without allocating a document.
-  pub(crate) fn exists(&self, specifier: &ModuleSpecifier) -> bool {
-    let specifier = self.specifier_resolver.resolve(specifier);
-    if let Some(specifier) = specifier {
-      if self.open_docs.contains_key(&specifier) {
-        return true;
-      }
-      if let Some(path) = get_document_path(&self.cache, &specifier) {
-        return path.is_file();
-      }
-    }
-
-    false
-  }
-
   /// Return a document for the specifier.
   pub fn get(&self, specifier: &ModuleSpecifier) -> Option<Document> {
     let specifier = self.specifier_resolver.resolve(specifier)?;
@@ -975,20 +966,17 @@ impl Documents {
       let fs_version = get_document_path(&self.cache, &specifier)
         .map(|path| calculate_fs_version(&path))
         .flatten();
-      if file_system_docs
-        .docs
-        .get(&specifier)
-        .map(|d| d.fs_version().to_string())
-        != fs_version
-      {
+      let file_system_doc = file_system_docs.docs.get(&specifier);
+      if file_system_doc.map(|d| d.fs_version().to_string()) != fs_version {
         // attempt to update the file on the file system
         file_system_docs.refresh_document(
           &self.cache,
           self.get_maybe_resolver(),
           &specifier,
-        );
+        )
+      } else {
+        file_system_doc.cloned()
       }
-      file_system_docs.docs.get(&specifier).cloned()
     }
   }