summaryrefslogtreecommitdiff
path: root/cli
diff options
context:
space:
mode:
authorDavid Sherret <dsherret@users.noreply.github.com>2023-06-13 15:48:53 -0400
committerGitHub <noreply@github.com>2023-06-13 15:48:53 -0400
commit015ea60d25a3c773fc9d9bf17fc904de236814f9 (patch)
tree7ad367b85a48fc5c0980bbfd91e98139d9f9da19 /cli
parent92e7287f4a744cad1fbe46ba1ce84c2b479ce6ac (diff)
fix(lsp): don't pre-load documents matched in the config file's "exclude" (#19431)
This prevents documents specified in a deno.json's "exclude" from being pre-loaded by the lsp. For example, someone may have something like: ```jsonc // deno.json { "exclude": [ "dist" // build directory ] } ```
Diffstat (limited to 'cli')
-rw-r--r--cli/args/mod.rs39
-rw-r--r--cli/lsp/documents.rs179
-rw-r--r--cli/tests/integration/lsp_tests.rs56
-rw-r--r--cli/util/glob.rs108
-rw-r--r--cli/util/mod.rs1
-rw-r--r--cli/util/path.rs4
6 files changed, 299 insertions, 88 deletions
diff --git a/cli/args/mod.rs b/cli/args/mod.rs
index 98e8f3564..ef5d3119e 100644
--- a/cli/args/mod.rs
+++ b/cli/args/mod.rs
@@ -71,6 +71,7 @@ use crate::file_fetcher::FileFetcher;
use crate::npm::CliNpmRegistryApi;
use crate::npm::NpmProcessState;
use crate::util::fs::canonicalize_path_maybe_not_exists;
+use crate::util::glob::expand_globs;
use crate::version;
use self::config_file::FmtConfig;
@@ -1312,40 +1313,6 @@ impl StorageKeyResolver {
}
}
-fn expand_globs(paths: &[PathBuf]) -> Result<Vec<PathBuf>, AnyError> {
- let mut new_paths = vec![];
- for path in paths {
- let path_str = path.to_string_lossy();
- if path_str.chars().any(|c| matches!(c, '*' | '?')) {
- // Escape brackets - we currently don't support them, because with introduction
- // of glob expansion paths like "pages/[id].ts" would suddenly start giving
- // wrong results. We might want to revisit that in the future.
- let escaped_path_str = path_str.replace('[', "[[]").replace(']', "[]]");
- let globbed_paths = glob::glob_with(
- &escaped_path_str,
- // Matches what `deno_task_shell` does
- glob::MatchOptions {
- // false because it should work the same way on case insensitive file systems
- case_sensitive: false,
- // true because it copies what sh does
- require_literal_separator: true,
- // true because it copies with sh does—these files are considered "hidden"
- require_literal_leading_dot: true,
- },
- )
- .with_context(|| format!("Failed to expand glob: \"{}\"", path_str))?;
-
- for globbed_path_result in globbed_paths {
- new_paths.push(globbed_path_result?);
- }
- } else {
- new_paths.push(path.clone());
- }
- }
-
- Ok(new_paths)
-}
-
/// Collect included and ignored files. CLI flags take precedence
/// over config file, i.e. if there's `files.ignore` in config file
/// and `--ignore` CLI flag, only the flag value is taken into account.
@@ -1364,11 +1331,11 @@ fn resolve_files(
}
// Now expand globs if there are any
if !result.include.is_empty() {
- result.include = expand_globs(&result.include)?;
+ result.include = expand_globs(result.include)?;
}
if !result.exclude.is_empty() {
- result.exclude = expand_globs(&result.exclude)?;
+ result.exclude = expand_globs(result.exclude)?;
}
Ok(result)
diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index 1282f8a18..1f0487296 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -20,6 +20,7 @@ use crate::npm::CliNpmRegistryApi;
use crate::npm::NpmResolution;
use crate::npm::PackageJsonDepsInstaller;
use crate::resolver::CliGraphResolver;
+use crate::util::glob;
use crate::util::path::specifier_to_file_path;
use crate::util::text_encoding;
@@ -1259,7 +1260,20 @@ impl Documents {
// only refresh the dependencies if the underlying configuration has changed
if self.resolver_config_hash != new_resolver_config_hash {
self.refresh_dependencies(
- options.enabled_urls,
+ options
+ .enabled_urls
+ .iter()
+ .filter_map(|url| specifier_to_file_path(url).ok())
+ .collect(),
+ options
+ .maybe_config_file
+ .and_then(|cf| {
+ cf.to_files_config()
+ .ok()
+ .flatten()
+ .map(|files| files.exclude)
+ })
+ .unwrap_or_default(),
options.document_preload_limit,
);
self.resolver_config_hash = new_resolver_config_hash;
@@ -1270,7 +1284,8 @@ impl Documents {
fn refresh_dependencies(
&mut self,
- enabled_urls: Vec<Url>,
+ enabled_paths: Vec<PathBuf>,
+ disabled_paths: Vec<PathBuf>,
document_preload_limit: usize,
) {
let resolver = self.resolver.as_graph_resolver();
@@ -1288,10 +1303,12 @@ impl Documents {
let open_docs = &mut self.open_docs;
log::debug!("Preloading documents from enabled urls...");
- let mut finder = PreloadDocumentFinder::from_enabled_urls_with_limit(
- &enabled_urls,
- document_preload_limit,
- );
+ let mut finder =
+ PreloadDocumentFinder::new(PreloadDocumentFinderOptions {
+ enabled_paths,
+ disabled_paths,
+ limit: document_preload_limit,
+ });
for specifier in finder.by_ref() {
// mark this document as having been found
not_found_docs.remove(&specifier);
@@ -1585,19 +1602,41 @@ enum PendingEntry {
ReadDir(Box<ReadDir>),
}
+struct PreloadDocumentFinderOptions {
+ enabled_paths: Vec<PathBuf>,
+ disabled_paths: Vec<PathBuf>,
+ limit: usize,
+}
+
/// Iterator that finds documents that can be preloaded into
/// the LSP on startup.
struct PreloadDocumentFinder {
limit: usize,
entry_count: usize,
pending_entries: VecDeque<PendingEntry>,
+ disabled_globs: glob::GlobSet,
+ disabled_paths: HashSet<PathBuf>,
}
impl PreloadDocumentFinder {
- pub fn from_enabled_urls_with_limit(
- enabled_urls: &Vec<Url>,
- limit: usize,
- ) -> Self {
+ pub fn new(options: PreloadDocumentFinderOptions) -> Self {
+ fn paths_into_globs_and_paths(
+ input_paths: Vec<PathBuf>,
+ ) -> (glob::GlobSet, HashSet<PathBuf>) {
+ let mut globs = Vec::with_capacity(input_paths.len());
+ let mut paths = HashSet::with_capacity(input_paths.len());
+ for path in input_paths {
+ if let Ok(Some(glob)) =
+ glob::GlobPattern::new_if_pattern(&path.to_string_lossy())
+ {
+ globs.push(glob);
+ } else {
+ paths.insert(path);
+ }
+ }
+ (glob::GlobSet::new(globs), paths)
+ }
+
fn is_allowed_root_dir(dir_path: &Path) -> bool {
if dir_path.parent().is_none() {
// never search the root directory of a drive
@@ -1606,23 +1645,27 @@ impl PreloadDocumentFinder {
true
}
+ let (disabled_globs, disabled_paths) =
+ paths_into_globs_and_paths(options.disabled_paths);
let mut finder = PreloadDocumentFinder {
- limit,
+ limit: options.limit,
entry_count: 0,
pending_entries: Default::default(),
+ disabled_globs,
+ disabled_paths,
};
- let mut dirs = Vec::with_capacity(enabled_urls.len());
- for enabled_url in enabled_urls {
- if let Ok(path) = enabled_url.to_file_path() {
- if path.is_dir() {
- if is_allowed_root_dir(&path) {
- dirs.push(path);
- }
- } else {
- finder
- .pending_entries
- .push_back(PendingEntry::SpecifiedRootFile(path));
+
+ // initialize the finder with the initial paths
+ let mut dirs = Vec::with_capacity(options.enabled_paths.len());
+ for path in options.enabled_paths {
+ if path.is_dir() {
+ if is_allowed_root_dir(&path) {
+ dirs.push(path);
}
+ } else {
+ finder
+ .pending_entries
+ .push_back(PendingEntry::SpecifiedRootFile(path));
}
}
for dir in sort_and_remove_non_leaf_dirs(dirs) {
@@ -1737,17 +1780,21 @@ impl Iterator for PreloadDocumentFinder {
if let Ok(entry) = entry {
let path = entry.path();
if let Ok(file_type) = entry.file_type() {
- if file_type.is_dir() && is_discoverable_dir(&path) {
- self
- .pending_entries
- .push_back(PendingEntry::Dir(path.to_path_buf()));
- } else if file_type.is_file() && is_discoverable_file(&path) {
- if let Some(specifier) = Self::get_valid_specifier(&path) {
- // restore the next entries for next time
+ if !self.disabled_paths.contains(&path)
+ && !self.disabled_globs.matches_path(&path)
+ {
+ if file_type.is_dir() && is_discoverable_dir(&path) {
self
.pending_entries
- .push_front(PendingEntry::ReadDir(entries));
- return Some(specifier);
+ .push_back(PendingEntry::Dir(path.to_path_buf()));
+ } else if file_type.is_file() && is_discoverable_file(&path) {
+ if let Some(specifier) = Self::get_valid_specifier(&path) {
+ // restore the next entries for next time
+ self
+ .pending_entries
+ .push_front(PendingEntry::ReadDir(entries));
+ return Some(specifier);
+ }
}
}
}
@@ -2018,23 +2065,28 @@ console.log(b, "hello deno");
temp_dir.write("root1/target/main.ts", ""); // no, because there is a Cargo.toml in the root directory
temp_dir.create_dir_all("root2/folder");
+ temp_dir.create_dir_all("root2/sub_folder");
temp_dir.write("root2/file1.ts", ""); // yes, provided
temp_dir.write("root2/file2.ts", ""); // no, not provided
temp_dir.write("root2/main.min.ts", ""); // yes, provided
temp_dir.write("root2/folder/main.ts", ""); // yes, provided
+ temp_dir.write("root2/sub_folder/a.js", ""); // no, not provided
+ temp_dir.write("root2/sub_folder/b.ts", ""); // no, not provided
+ temp_dir.write("root2/sub_folder/c.js", ""); // no, not provided
temp_dir.create_dir_all("root3/");
temp_dir.write("root3/mod.ts", ""); // no, not provided
- let mut urls = PreloadDocumentFinder::from_enabled_urls_with_limit(
- &vec![
- temp_dir.uri().join("root1/").unwrap(),
- temp_dir.uri().join("root2/file1.ts").unwrap(),
- temp_dir.uri().join("root2/main.min.ts").unwrap(),
- temp_dir.uri().join("root2/folder/").unwrap(),
+ let mut urls = PreloadDocumentFinder::new(PreloadDocumentFinderOptions {
+ enabled_paths: vec![
+ temp_dir.path().join("root1"),
+ temp_dir.path().join("root2").join("file1.ts"),
+ temp_dir.path().join("root2").join("main.min.ts"),
+ temp_dir.path().join("root2").join("folder"),
],
- 1_000,
- )
+ disabled_paths: Vec::new(),
+ limit: 1_000,
+ })
.collect::<Vec<_>>();
// Ideally we would test for order here, which should be BFS, but
@@ -2061,32 +2113,57 @@ console.log(b, "hello deno");
);
// now try iterating with a low limit
- let urls = PreloadDocumentFinder::from_enabled_urls_with_limit(
- &vec![temp_dir.uri()],
- 10, // entries and not results
- )
+ let urls = PreloadDocumentFinder::new(PreloadDocumentFinderOptions {
+ enabled_paths: vec![temp_dir.path().to_path_buf()],
+ disabled_paths: Vec::new(),
+ limit: 10, // entries and not results
+ })
.collect::<Vec<_>>();
// since different file system have different iteration
// order, the number here may vary, so just assert it's below
// a certain amount
assert!(urls.len() < 5, "Actual length: {}", urls.len());
+
+ // now try with certain directories and files disabled
+ let mut urls = PreloadDocumentFinder::new(PreloadDocumentFinderOptions {
+ enabled_paths: vec![temp_dir.path().to_path_buf()],
+ disabled_paths: vec![
+ temp_dir.path().to_path_buf().join("root1"),
+ temp_dir.path().to_path_buf().join("root2").join("file1.ts"),
+ temp_dir.path().to_path_buf().join("**/*.js"), // ignore js files
+ ],
+ limit: 1_000,
+ })
+ .collect::<Vec<_>>();
+ urls.sort();
+ assert_eq!(
+ urls,
+ vec![
+ temp_dir.uri().join("root2/file2.ts").unwrap(),
+ temp_dir.uri().join("root2/folder/main.ts").unwrap(),
+ temp_dir.uri().join("root2/sub_folder/b.ts").unwrap(), // won't have the javascript files
+ temp_dir.uri().join("root3/mod.ts").unwrap(),
+ ]
+ );
}
#[test]
pub fn test_pre_load_document_finder_disallowed_dirs() {
if cfg!(windows) {
- let paths = PreloadDocumentFinder::from_enabled_urls_with_limit(
- &vec![Url::parse("file:///c:/").unwrap()],
- 1_000,
- )
+ let paths = PreloadDocumentFinder::new(PreloadDocumentFinderOptions {
+ enabled_paths: vec![PathBuf::from("C:\\")],
+ disabled_paths: Vec::new(),
+ limit: 1_000,
+ })
.collect::<Vec<_>>();
assert_eq!(paths, vec![]);
} else {
- let paths = PreloadDocumentFinder::from_enabled_urls_with_limit(
- &vec![Url::parse("file:///").unwrap()],
- 1_000,
- )
+ let paths = PreloadDocumentFinder::new(PreloadDocumentFinderOptions {
+ enabled_paths: vec![PathBuf::from("/")],
+ disabled_paths: Vec::new(),
+ limit: 1_000,
+ })
.collect::<Vec<_>>();
assert_eq!(paths, vec![]);
}
diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs
index b6ed08e30..71a3d541f 100644
--- a/cli/tests/integration/lsp_tests.rs
+++ b/cli/tests/integration/lsp_tests.rs
@@ -7531,6 +7531,62 @@ fn lsp_closed_file_find_references_low_document_pre_load() {
}
#[test]
+fn lsp_closed_file_find_references_excluded_path() {
+ // we exclude any files or folders in the "exclude" part of
+ // the config file from being pre-loaded
+ let context = TestContextBuilder::new().use_temp_cwd().build();
+ let temp_dir = context.temp_dir();
+ temp_dir.create_dir_all("sub_dir");
+ temp_dir.create_dir_all("other_dir/sub_dir");
+ temp_dir.write("./sub_dir/mod.ts", "export const a = 5;");
+ temp_dir.write(
+ "./sub_dir/mod.test.ts",
+ "import { a } from './mod.ts'; console.log(a);",
+ );
+ temp_dir.write(
+ "./other_dir/sub_dir/mod.test.ts",
+ "import { a } from '../../sub_dir/mod.ts'; console.log(a);",
+ );
+ temp_dir.write(
+ "deno.json",
+ r#"{
+ "exclude": [
+ "./sub_dir/mod.test.ts",
+ "./other_dir/sub_dir",
+ ]
+}"#,
+ );
+ let temp_dir_url = temp_dir.uri();
+ let mut client = context.new_lsp_command().build();
+ client.initialize_default();
+ client.did_open(json!({
+ "textDocument": {
+ "uri": temp_dir_url.join("sub_dir/mod.ts").unwrap(),
+ "languageId": "typescript",
+ "version": 1,
+ "text": r#"export const a = 5;"#
+ }
+ }));
+ let res = client.write_request(
+ "textDocument/references",
+ json!({
+ "textDocument": {
+ "uri": temp_dir_url.join("sub_dir/mod.ts").unwrap(),
+ },
+ "position": { "line": 0, "character": 13 },
+ "context": {
+ "includeDeclaration": false
+ }
+ }),
+ );
+
+ // won't have results because the documents won't be pre-loaded
+ assert_eq!(res, json!([]));
+
+ client.shutdown();
+}
+
+#[test]
fn lsp_data_urls_with_jsx_compiler_option() {
let context = TestContextBuilder::new().use_temp_cwd().build();
let temp_dir = context.temp_dir();
diff --git a/cli/util/glob.rs b/cli/util/glob.rs
new file mode 100644
index 000000000..55c9a516e
--- /dev/null
+++ b/cli/util/glob.rs
@@ -0,0 +1,108 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+use std::path::Path;
+use std::path::PathBuf;
+
+use deno_core::anyhow::Context;
+use deno_core::error::AnyError;
+
+pub fn expand_globs(paths: Vec<PathBuf>) -> Result<Vec<PathBuf>, AnyError> {
+ let mut new_paths = vec![];
+ for path in paths {
+ let path_str = path.to_string_lossy();
+ if is_glob_pattern(&path_str) {
+ let globbed_paths = glob(&path_str)?;
+
+ for globbed_path_result in globbed_paths {
+ new_paths.push(globbed_path_result?);
+ }
+ } else {
+ new_paths.push(path);
+ }
+ }
+
+ Ok(new_paths)
+}
+
+pub fn glob(pattern: &str) -> Result<glob::Paths, AnyError> {
+ glob::glob_with(&escape_brackets(pattern), match_options())
+ .with_context(|| format!("Failed to expand glob: \"{}\"", pattern))
+}
+
+pub struct GlobPattern(glob::Pattern);
+
+impl GlobPattern {
+ pub fn new_if_pattern(pattern: &str) -> Result<Option<Self>, AnyError> {
+ if !is_glob_pattern(pattern) {
+ return Ok(None);
+ }
+ Self::new(pattern).map(Some)
+ }
+
+ pub fn new(pattern: &str) -> Result<Self, AnyError> {
+ let pattern = glob::Pattern::new(pattern)
+ .with_context(|| format!("Failed to expand glob: \"{}\"", pattern))?;
+ Ok(Self(pattern))
+ }
+
+ pub fn matches_path(&self, path: &Path) -> bool {
+ self.0.matches_path(path)
+ }
+}
+
+pub struct GlobSet(Vec<GlobPattern>);
+
+impl GlobSet {
+ pub fn new(matchers: Vec<GlobPattern>) -> Self {
+ Self(matchers)
+ }
+
+ pub fn matches_path(&self, path: &Path) -> bool {
+ for pattern in &self.0 {
+ if pattern.matches_path(path) {
+ return true;
+ }
+ }
+ false
+ }
+}
+
+pub fn is_glob_pattern(path: &str) -> bool {
+ path.chars().any(|c| matches!(c, '*' | '?'))
+}
+
+fn escape_brackets(pattern: &str) -> String {
+ // Escape brackets - we currently don't support them, because with introduction
+ // of glob expansion paths like "pages/[id].ts" would suddenly start giving
+ // wrong results. We might want to revisit that in the future.
+ pattern.replace('[', "[[]").replace(']', "[]]")
+}
+
+fn match_options() -> glob::MatchOptions {
+ // Matches what `deno_task_shell` does
+ glob::MatchOptions {
+ // false because it should work the same way on case insensitive file systems
+ case_sensitive: false,
+ // true because it copies what sh does
+ require_literal_separator: true,
+ // true because it copies with sh does—these files are considered "hidden"
+ require_literal_leading_dot: true,
+ }
+}
+
+#[cfg(test)]
+mod test {
+ use super::*;
+
+ #[test]
+ pub fn glob_set_matches_path() {
+ let glob_set = GlobSet::new(vec![
+ GlobPattern::new("foo/bar").unwrap(),
+ GlobPattern::new("foo/baz").unwrap(),
+ ]);
+
+ assert!(glob_set.matches_path(Path::new("foo/bar")));
+ assert!(glob_set.matches_path(Path::new("foo/baz")));
+ assert!(!glob_set.matches_path(Path::new("foo/qux")));
+ }
+}
diff --git a/cli/util/mod.rs b/cli/util/mod.rs
index 61511679f..0c160dbc8 100644
--- a/cli/util/mod.rs
+++ b/cli/util/mod.rs
@@ -8,6 +8,7 @@ pub mod display;
pub mod draw_thread;
pub mod file_watcher;
pub mod fs;
+pub mod glob;
pub mod logger;
pub mod path;
pub mod progress_bar;
diff --git a/cli/util/path.rs b/cli/util/path.rs
index 39ba96d6d..ba1d0d926 100644
--- a/cli/util/path.rs
+++ b/cli/util/path.rs
@@ -73,7 +73,9 @@ pub fn mapped_specifier_for_tsc(
pub fn specifier_to_file_path(
specifier: &ModuleSpecifier,
) -> Result<PathBuf, AnyError> {
- let result = if cfg!(windows) {
+ let result = if specifier.scheme() != "file" {
+ Err(())
+ } else if cfg!(windows) {
match specifier.to_file_path() {
Ok(path) => Ok(path),
Err(()) => {