summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Sherret <dsherret@users.noreply.github.com>2023-04-01 12:02:44 -0400
committerGitHub <noreply@github.com>2023-04-01 12:02:44 -0400
commitbac8e4f6f25367cf5b6c2095249cf144035a4fbd (patch)
tree75f4c3f2174d1481d95942fd8200bc4fb8475b18
parentae1ba2af3c0dc45de074285f0e0a8440cf2895ec (diff)
fix(repl): disable language server document preloading in the repl (#18543)
This was an oversight because the repl uses the language server under the hood. Also, never preloads from a root directory. Part of #18538
-rw-r--r--cli/lsp/client.rs20
-rw-r--r--cli/lsp/completions.rs2
-rw-r--r--cli/lsp/diagnostics.rs2
-rw-r--r--cli/lsp/documents.rs96
-rw-r--r--cli/lsp/language_server.rs2
-rw-r--r--cli/lsp/tsc.rs2
-rw-r--r--cli/tests/integration/repl_tests.rs14
7 files changed, 111 insertions, 27 deletions
diff --git a/cli/lsp/client.rs b/cli/lsp/client.rs
index d24d4c2a9..e684dc09f 100644
--- a/cli/lsp/client.rs
+++ b/cli/lsp/client.rs
@@ -26,6 +26,13 @@ pub enum TestingNotification {
Progress(testing_lsp_custom::TestRunProgressParams),
}
+#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)]
+pub enum LspClientKind {
+ #[default]
+ CodeEditor,
+ Repl,
+}
+
#[derive(Clone)]
pub struct Client(Arc<dyn ClientTrait>);
@@ -44,6 +51,10 @@ impl Client {
Self(Arc::new(ReplClient))
}
+ pub fn kind(&self) -> LspClientKind {
+ self.0.kind()
+ }
+
/// Gets additional methods that should only be called outside
/// the LSP's lock to prevent deadlocking scenarios.
pub fn when_outside_lsp_lock(&self) -> OutsideLockClient {
@@ -149,6 +160,7 @@ impl OutsideLockClient {
#[async_trait]
trait ClientTrait: Send + Sync {
+ fn kind(&self) -> LspClientKind;
async fn publish_diagnostics(
&self,
uri: lsp::Url,
@@ -177,6 +189,10 @@ struct TowerClient(tower_lsp::Client);
#[async_trait]
impl ClientTrait for TowerClient {
+ fn kind(&self) -> LspClientKind {
+ LspClientKind::CodeEditor
+ }
+
async fn publish_diagnostics(
&self,
uri: lsp::Url,
@@ -296,6 +312,10 @@ struct ReplClient;
#[async_trait]
impl ClientTrait for ReplClient {
+ fn kind(&self) -> LspClientKind {
+ LspClientKind::Repl
+ }
+
async fn publish_diagnostics(
&self,
_uri: lsp::Url,
diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs
index cb8bd446d..c40d6f238 100644
--- a/cli/lsp/completions.rs
+++ b/cli/lsp/completions.rs
@@ -519,7 +519,7 @@ mod tests {
source_fixtures: &[(&str, &str)],
location: &Path,
) -> Documents {
- let mut documents = Documents::new(location);
+ let mut documents = Documents::new(location, Default::default());
for (specifier, source, version, language_id) in fixtures {
let specifier =
resolve_url(specifier).expect("failed to create specifier");
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index 8ba8ce074..539868eca 100644
--- a/cli/lsp/diagnostics.rs
+++ b/cli/lsp/diagnostics.rs
@@ -1079,7 +1079,7 @@ mod tests {
location: &Path,
maybe_import_map: Option<(&str, &str)>,
) -> StateSnapshot {
- let mut documents = Documents::new(location);
+ let mut documents = Documents::new(location, Default::default());
for (specifier, source, version, language_id) in fixtures {
let specifier =
resolve_url(specifier).expect("failed to create specifier");
diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index 0c27893a7..ca7bd2c82 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -1,6 +1,7 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
use super::cache::calculate_fs_version;
+use super::client::LspClientKind;
use super::text::LineIndex;
use super::tsc;
use super::tsc::AssetDocument;
@@ -815,6 +816,8 @@ pub struct Documents {
open_docs: HashMap<ModuleSpecifier, Document>,
/// Documents stored on the file system.
file_system_docs: Arc<Mutex<FileSystemDocuments>>,
+ /// Kind of the client that is using the documents.
+ lsp_client_kind: LspClientKind,
/// Hash of the config used for resolution. When the hash changes we update
/// dependencies.
resolver_config_hash: u64,
@@ -834,13 +837,14 @@ pub struct Documents {
}
impl Documents {
- pub fn new(location: &Path) -> Self {
+ pub fn new(location: &Path, lsp_client_kind: LspClientKind) -> Self {
Self {
cache: HttpCache::new(location),
dirty: true,
dependents_map: Default::default(),
open_docs: HashMap::default(),
file_system_docs: Default::default(),
+ lsp_client_kind,
resolver_config_hash: 0,
imports: Default::default(),
resolver: CliGraphResolver::default(),
@@ -1248,33 +1252,50 @@ impl Documents {
// update the file system documents
let mut fs_docs = self.file_system_docs.lock();
- let mut not_found_docs =
- fs_docs.docs.keys().cloned().collect::<HashSet<_>>();
- let open_docs = &mut self.open_docs;
-
- for specifier in PreloadDocumentFinder::from_root_urls(&root_urls) {
- // mark this document as having been found
- not_found_docs.remove(&specifier);
+ match self.lsp_client_kind {
+ LspClientKind::CodeEditor => {
+ let mut not_found_docs =
+ fs_docs.docs.keys().cloned().collect::<HashSet<_>>();
+ let open_docs = &mut self.open_docs;
+
+ log::debug!("Preloading documents from root urls...");
+ for specifier in PreloadDocumentFinder::from_root_urls(&root_urls) {
+ // mark this document as having been found
+ not_found_docs.remove(&specifier);
+
+ if !open_docs.contains_key(&specifier)
+ && !fs_docs.docs.contains_key(&specifier)
+ {
+ fs_docs.refresh_document(&self.cache, resolver, &specifier);
+ } else {
+ // update the existing entry to have the new resolver
+ if let Some(doc) = fs_docs.docs.get_mut(&specifier) {
+ if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) {
+ *doc = new_doc;
+ }
+ }
+ }
+ }
- if !open_docs.contains_key(&specifier)
- && !fs_docs.docs.contains_key(&specifier)
- {
- fs_docs.refresh_document(&self.cache, resolver, &specifier);
- } else {
- // update the existing entry to have the new resolver
- if let Some(doc) = fs_docs.docs.get_mut(&specifier) {
+ // clean up and remove any documents that weren't found
+ for uri in not_found_docs {
+ fs_docs.docs.remove(&uri);
+ }
+ }
+ LspClientKind::Repl => {
+ // This log statement is used in the tests to ensure preloading doesn't
+ // happen, which is not useful in the repl and could be very expensive
+ // if the repl is launched from a directory with a lot of descendants.
+ log::debug!("Skipping document preload for repl.");
+
+ // for the repl, just update to use the new resolver
+ for doc in fs_docs.docs.values_mut() {
if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) {
*doc = new_doc;
}
}
}
}
-
- // clean up and remove any documents that weren't found
- for uri in not_found_docs {
- fs_docs.docs.remove(&uri);
- }
-
fs_docs.dirty = true;
}
@@ -1516,6 +1537,14 @@ struct PreloadDocumentFinder {
}
impl PreloadDocumentFinder {
+ fn is_allowed_root_dir(dir_path: &Path) -> bool {
+ if dir_path.parent().is_none() {
+ // never search the root directory of a drive
+ return false;
+ }
+ true
+ }
+
pub fn from_root_urls(root_urls: &Vec<Url>) -> Self {
let mut finder = PreloadDocumentFinder {
pending_dirs: Default::default(),
@@ -1525,7 +1554,9 @@ impl PreloadDocumentFinder {
for root_url in root_urls {
if let Ok(path) = root_url.to_file_path() {
if path.is_dir() {
- finder.pending_dirs.push(path);
+ if Self::is_allowed_root_dir(&path) {
+ finder.pending_dirs.push(path);
+ }
} else {
finder.pending_files.push(path);
}
@@ -1655,7 +1686,7 @@ mod tests {
fn setup(temp_dir: &TempDir) -> (Documents, PathBuf) {
let location = temp_dir.path().join("deps");
- let documents = Documents::new(&location);
+ let documents = Documents::new(&location, Default::default());
(documents, location)
}
@@ -1905,4 +1936,23 @@ console.log(b, "hello deno");
]
);
}
+
+ #[test]
+ pub fn test_pre_load_document_finder_disallowed_dirs() {
+ if cfg!(windows) {
+ let paths =
+ PreloadDocumentFinder::from_root_urls(&vec![
+ Url::parse("file:///c:/").unwrap()
+ ])
+ .collect::<Vec<_>>();
+ assert_eq!(paths, vec![]);
+ } else {
+ let paths =
+ PreloadDocumentFinder::from_root_urls(&vec![
+ Url::parse("file:///").unwrap()
+ ])
+ .collect::<Vec<_>>();
+ assert_eq!(paths, vec![]);
+ }
+ }
}
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 754b5c95c..15a7d907b 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -463,7 +463,7 @@ impl Inner {
ModuleRegistry::new(&module_registries_location, http_client.clone())
.unwrap();
let location = dir.deps_folder_path();
- let documents = Documents::new(&location);
+ let documents = Documents::new(&location, client.kind());
let deps_http_cache = HttpCache::new(&location);
let cache_metadata = cache::CacheMetadata::new(deps_http_cache.clone());
let performance = Arc::new(Performance::default());
diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs
index 2980e546b..ef5d0e645 100644
--- a/cli/lsp/tsc.rs
+++ b/cli/lsp/tsc.rs
@@ -3530,7 +3530,7 @@ mod tests {
fixtures: &[(&str, &str, i32, LanguageId)],
location: &Path,
) -> StateSnapshot {
- let mut documents = Documents::new(location);
+ let mut documents = Documents::new(location, Default::default());
for (specifier, source, version, language_id) in fixtures {
let specifier =
resolve_url(specifier).expect("failed to create specifier");
diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs
index c3e7c42d6..a473dc200 100644
--- a/cli/tests/integration/repl_tests.rs
+++ b/cli/tests/integration/repl_tests.rs
@@ -5,6 +5,7 @@ use test_util::assert_contains;
use test_util::assert_ends_with;
use test_util::assert_not_contains;
use util::TempDir;
+use util::TestContext;
use util::TestContextBuilder;
#[test]
@@ -957,3 +958,16 @@ fn package_json_uncached_no_error() {
console.expect("42")
});
}
+
+#[test]
+fn closed_file_pre_load_does_not_occur() {
+ TestContext::default()
+ .new_command()
+ .args_vec(["repl", "-A", "--log-level=debug"])
+ .with_pty(|console| {
+ assert_contains!(
+ console.all_output(),
+ "Skipping document preload for repl.",
+ );
+ });
+}