1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2025-01-21 21:50:00 -05:00

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
This commit is contained in:
David Sherret 2023-04-01 12:02:44 -04:00 committed by GitHub
parent ae1ba2af3c
commit bac8e4f6f2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 110 additions and 26 deletions

View file

@ -26,6 +26,13 @@ pub enum TestingNotification {
Progress(testing_lsp_custom::TestRunProgressParams), Progress(testing_lsp_custom::TestRunProgressParams),
} }
#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)]
pub enum LspClientKind {
#[default]
CodeEditor,
Repl,
}
#[derive(Clone)] #[derive(Clone)]
pub struct Client(Arc<dyn ClientTrait>); pub struct Client(Arc<dyn ClientTrait>);
@ -44,6 +51,10 @@ impl Client {
Self(Arc::new(ReplClient)) Self(Arc::new(ReplClient))
} }
pub fn kind(&self) -> LspClientKind {
self.0.kind()
}
/// Gets additional methods that should only be called outside /// Gets additional methods that should only be called outside
/// the LSP's lock to prevent deadlocking scenarios. /// the LSP's lock to prevent deadlocking scenarios.
pub fn when_outside_lsp_lock(&self) -> OutsideLockClient { pub fn when_outside_lsp_lock(&self) -> OutsideLockClient {
@ -149,6 +160,7 @@ impl OutsideLockClient {
#[async_trait] #[async_trait]
trait ClientTrait: Send + Sync { trait ClientTrait: Send + Sync {
fn kind(&self) -> LspClientKind;
async fn publish_diagnostics( async fn publish_diagnostics(
&self, &self,
uri: lsp::Url, uri: lsp::Url,
@ -177,6 +189,10 @@ struct TowerClient(tower_lsp::Client);
#[async_trait] #[async_trait]
impl ClientTrait for TowerClient { impl ClientTrait for TowerClient {
fn kind(&self) -> LspClientKind {
LspClientKind::CodeEditor
}
async fn publish_diagnostics( async fn publish_diagnostics(
&self, &self,
uri: lsp::Url, uri: lsp::Url,
@ -296,6 +312,10 @@ struct ReplClient;
#[async_trait] #[async_trait]
impl ClientTrait for ReplClient { impl ClientTrait for ReplClient {
fn kind(&self) -> LspClientKind {
LspClientKind::Repl
}
async fn publish_diagnostics( async fn publish_diagnostics(
&self, &self,
_uri: lsp::Url, _uri: lsp::Url,

View file

@ -519,7 +519,7 @@ mod tests {
source_fixtures: &[(&str, &str)], source_fixtures: &[(&str, &str)],
location: &Path, location: &Path,
) -> Documents { ) -> Documents {
let mut documents = Documents::new(location); let mut documents = Documents::new(location, Default::default());
for (specifier, source, version, language_id) in fixtures { for (specifier, source, version, language_id) in fixtures {
let specifier = let specifier =
resolve_url(specifier).expect("failed to create specifier"); resolve_url(specifier).expect("failed to create specifier");

View file

@ -1079,7 +1079,7 @@ mod tests {
location: &Path, location: &Path,
maybe_import_map: Option<(&str, &str)>, maybe_import_map: Option<(&str, &str)>,
) -> StateSnapshot { ) -> StateSnapshot {
let mut documents = Documents::new(location); let mut documents = Documents::new(location, Default::default());
for (specifier, source, version, language_id) in fixtures { for (specifier, source, version, language_id) in fixtures {
let specifier = let specifier =
resolve_url(specifier).expect("failed to create specifier"); resolve_url(specifier).expect("failed to create specifier");

View file

@ -1,6 +1,7 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
use super::cache::calculate_fs_version; use super::cache::calculate_fs_version;
use super::client::LspClientKind;
use super::text::LineIndex; use super::text::LineIndex;
use super::tsc; use super::tsc;
use super::tsc::AssetDocument; use super::tsc::AssetDocument;
@ -815,6 +816,8 @@ pub struct Documents {
open_docs: HashMap<ModuleSpecifier, Document>, open_docs: HashMap<ModuleSpecifier, Document>,
/// Documents stored on the file system. /// Documents stored on the file system.
file_system_docs: Arc<Mutex<FileSystemDocuments>>, 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 /// Hash of the config used for resolution. When the hash changes we update
/// dependencies. /// dependencies.
resolver_config_hash: u64, resolver_config_hash: u64,
@ -834,13 +837,14 @@ pub struct Documents {
} }
impl Documents { impl Documents {
pub fn new(location: &Path) -> Self { pub fn new(location: &Path, lsp_client_kind: LspClientKind) -> Self {
Self { Self {
cache: HttpCache::new(location), cache: HttpCache::new(location),
dirty: true, dirty: true,
dependents_map: Default::default(), dependents_map: Default::default(),
open_docs: HashMap::default(), open_docs: HashMap::default(),
file_system_docs: Default::default(), file_system_docs: Default::default(),
lsp_client_kind,
resolver_config_hash: 0, resolver_config_hash: 0,
imports: Default::default(), imports: Default::default(),
resolver: CliGraphResolver::default(), resolver: CliGraphResolver::default(),
@ -1248,33 +1252,50 @@ impl Documents {
// update the file system documents // update the file system documents
let mut fs_docs = self.file_system_docs.lock(); let mut fs_docs = self.file_system_docs.lock();
let mut not_found_docs = match self.lsp_client_kind {
fs_docs.docs.keys().cloned().collect::<HashSet<_>>(); LspClientKind::CodeEditor => {
let open_docs = &mut self.open_docs; 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) { log::debug!("Preloading documents from root urls...");
// mark this document as having been found for specifier in PreloadDocumentFinder::from_root_urls(&root_urls) {
not_found_docs.remove(&specifier); // mark this document as having been found
not_found_docs.remove(&specifier);
if !open_docs.contains_key(&specifier) if !open_docs.contains_key(&specifier)
&& !fs_docs.docs.contains_key(&specifier) && !fs_docs.docs.contains_key(&specifier)
{ {
fs_docs.refresh_document(&self.cache, resolver, &specifier); fs_docs.refresh_document(&self.cache, resolver, &specifier);
} else { } else {
// update the existing entry to have the new resolver // update the existing entry to have the new resolver
if let Some(doc) = fs_docs.docs.get_mut(&specifier) { if let Some(doc) = fs_docs.docs.get_mut(&specifier) {
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);
}
}
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) { if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) {
*doc = new_doc; *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; fs_docs.dirty = true;
} }
@ -1516,6 +1537,14 @@ struct PreloadDocumentFinder {
} }
impl 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 { pub fn from_root_urls(root_urls: &Vec<Url>) -> Self {
let mut finder = PreloadDocumentFinder { let mut finder = PreloadDocumentFinder {
pending_dirs: Default::default(), pending_dirs: Default::default(),
@ -1525,7 +1554,9 @@ impl PreloadDocumentFinder {
for root_url in root_urls { for root_url in root_urls {
if let Ok(path) = root_url.to_file_path() { if let Ok(path) = root_url.to_file_path() {
if path.is_dir() { if path.is_dir() {
finder.pending_dirs.push(path); if Self::is_allowed_root_dir(&path) {
finder.pending_dirs.push(path);
}
} else { } else {
finder.pending_files.push(path); finder.pending_files.push(path);
} }
@ -1655,7 +1686,7 @@ mod tests {
fn setup(temp_dir: &TempDir) -> (Documents, PathBuf) { fn setup(temp_dir: &TempDir) -> (Documents, PathBuf) {
let location = temp_dir.path().join("deps"); let location = temp_dir.path().join("deps");
let documents = Documents::new(&location); let documents = Documents::new(&location, Default::default());
(documents, location) (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![]);
}
}
} }

View file

@ -463,7 +463,7 @@ impl Inner {
ModuleRegistry::new(&module_registries_location, http_client.clone()) ModuleRegistry::new(&module_registries_location, http_client.clone())
.unwrap(); .unwrap();
let location = dir.deps_folder_path(); 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 deps_http_cache = HttpCache::new(&location);
let cache_metadata = cache::CacheMetadata::new(deps_http_cache.clone()); let cache_metadata = cache::CacheMetadata::new(deps_http_cache.clone());
let performance = Arc::new(Performance::default()); let performance = Arc::new(Performance::default());

View file

@ -3530,7 +3530,7 @@ mod tests {
fixtures: &[(&str, &str, i32, LanguageId)], fixtures: &[(&str, &str, i32, LanguageId)],
location: &Path, location: &Path,
) -> StateSnapshot { ) -> StateSnapshot {
let mut documents = Documents::new(location); let mut documents = Documents::new(location, Default::default());
for (specifier, source, version, language_id) in fixtures { for (specifier, source, version, language_id) in fixtures {
let specifier = let specifier =
resolve_url(specifier).expect("failed to create specifier"); resolve_url(specifier).expect("failed to create specifier");

View file

@ -5,6 +5,7 @@ use test_util::assert_contains;
use test_util::assert_ends_with; use test_util::assert_ends_with;
use test_util::assert_not_contains; use test_util::assert_not_contains;
use util::TempDir; use util::TempDir;
use util::TestContext;
use util::TestContextBuilder; use util::TestContextBuilder;
#[test] #[test]
@ -957,3 +958,16 @@ fn package_json_uncached_no_error() {
console.expect("42") 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.",
);
});
}