From 68fecc6de4b2e6556adeb2730798bf42017c4be6 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 27 Mar 2024 14:25:39 -0400 Subject: [PATCH] fix: less aggressive vendor folder ignoring (#23100) This is slightly breaking as some users want the `vendor` folder excluded and may not have that specified in their deno.json. Closes #22833 --- cli/args/mod.rs | 1 - cli/tools/bench/mod.rs | 21 ++++++++++----- cli/tools/coverage/mod.rs | 11 +++++--- cli/tools/doc.rs | 1 + cli/tools/fmt.rs | 15 ++++++----- cli/tools/lint/mod.rs | 30 ++++++++++++--------- cli/tools/test/mod.rs | 57 ++++++++++++++++++++++++--------------- cli/util/fs.rs | 15 ++++------- 8 files changed, 90 insertions(+), 61 deletions(-) diff --git a/cli/args/mod.rs b/cli/args/mod.rs index de889c6540..330b10d0f4 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -1974,7 +1974,6 @@ mod test { let mut files = FileCollector::new(|_| true) .ignore_git_folder() .ignore_node_modules() - .ignore_vendor_folder() .collect_file_patterns(resolved_files) .unwrap(); diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs index 609dff282f..8663fbbc87 100644 --- a/cli/tools/bench/mod.rs +++ b/cli/tools/bench/mod.rs @@ -432,8 +432,11 @@ pub async fn run_benchmarks( let permissions = Permissions::from_options(&cli_options.permissions_options())?; - let specifiers = - collect_specifiers(bench_options.files, is_supported_bench_path)?; + let specifiers = collect_specifiers( + bench_options.files, + cli_options.vendor_dir_path().map(ToOwned::to_owned), + is_supported_bench_path, + )?; if specifiers.is_empty() { return Err(generic_error("No bench modules found")); @@ -505,6 +508,7 @@ pub async fn run_benchmarks_with_watch( let bench_modules = collect_specifiers( bench_options.files.clone(), + cli_options.vendor_dir_path().map(ToOwned::to_owned), is_supported_bench_path, )?; @@ -543,11 +547,14 @@ pub async fn run_benchmarks_with_watch( // todo(dsherret): why are we collecting specifiers twice in a row? // Seems like a perf bug. - let specifiers = - collect_specifiers(bench_options.files, is_supported_bench_path)? - .into_iter() - .filter(|specifier| bench_modules_to_reload.contains(specifier)) - .collect::>(); + let specifiers = collect_specifiers( + bench_options.files, + cli_options.vendor_dir_path().map(ToOwned::to_owned), + is_supported_bench_path, + )? + .into_iter() + .filter(|specifier| bench_modules_to_reload.contains(specifier)) + .collect::>(); check_specifiers(cli_options, module_load_preparer, specifiers.clone()) .await?; diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index b076895700..67fe979b5a 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use crate::args::CliOptions; use crate::args::CoverageFlags; use crate::args::FileFlags; use crate::args::Flags; @@ -376,6 +377,7 @@ fn range_to_src_line_index( } fn collect_coverages( + cli_options: &CliOptions, files: FileFlags, initial_cwd: &Path, ) -> Result, AnyError> { @@ -405,7 +407,7 @@ fn collect_coverages( }) .ignore_git_folder() .ignore_node_modules() - .ignore_vendor_folder() + .set_vendor_folder(cli_options.vendor_dir_path().map(ToOwned::to_owned)) .collect_file_patterns(file_patterns)?; for file_path in file_paths { @@ -474,8 +476,11 @@ pub async fn cover_files( let coverage_root = cli_options .initial_cwd() .join(&coverage_flags.files.include[0]); - let script_coverages = - collect_coverages(coverage_flags.files, cli_options.initial_cwd())?; + let script_coverages = collect_coverages( + cli_options, + coverage_flags.files, + cli_options.initial_cwd(), + )?; if script_coverages.is_empty() { return Err(generic_error("No coverage files found")); } diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index b3240dd370..0107402df5 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -103,6 +103,7 @@ pub async fn doc(flags: Flags, doc_flags: DocFlags) -> Result<(), AnyError> { ), exclude: Default::default(), }, + cli_options.vendor_dir_path().map(ToOwned::to_owned), |_| true, )?; let graph = module_graph_creator diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index 241648d981..b16639f991 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -71,8 +71,8 @@ pub async fn format(flags: Flags, fmt_flags: FmtFlags) -> Result<(), AnyError> { let factory = CliFactory::from_flags(flags)?; let cli_options = factory.cli_options(); let fmt_options = cli_options.resolve_fmt_options(fmt_flags)?; - let files = - collect_fmt_files(fmt_options.files.clone()).and_then(|files| { + let files = collect_fmt_files(cli_options, fmt_options.files.clone()) + .and_then(|files| { if files.is_empty() { Err(generic_error("No target files found.")) } else { @@ -116,8 +116,8 @@ pub async fn format(flags: Flags, fmt_flags: FmtFlags) -> Result<(), AnyError> { let factory = CliFactory::from_flags(flags)?; let cli_options = factory.cli_options(); let fmt_options = cli_options.resolve_fmt_options(fmt_flags)?; - let files = - collect_fmt_files(fmt_options.files.clone()).and_then(|files| { + let files = collect_fmt_files(cli_options, fmt_options.files.clone()) + .and_then(|files| { if files.is_empty() { Err(generic_error("No target files found.")) } else { @@ -153,11 +153,14 @@ async fn format_files( Ok(()) } -fn collect_fmt_files(files: FilePatterns) -> Result, AnyError> { +fn collect_fmt_files( + cli_options: &CliOptions, + files: FilePatterns, +) -> Result, AnyError> { FileCollector::new(|e| is_supported_ext_fmt(e.path)) .ignore_git_folder() .ignore_node_modules() - .ignore_vendor_folder() + .set_vendor_folder(cli_options.vendor_dir_path().map(ToOwned::to_owned)) .collect_file_patterns(files) } diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index fec6647843..03f5b8676e 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -34,6 +34,7 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use crate::args::CliOptions; use crate::args::Flags; use crate::args::LintFlags; use crate::args::LintOptions; @@ -78,15 +79,15 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { let factory = CliFactory::from_flags(flags)?; let cli_options = factory.cli_options(); let lint_options = cli_options.resolve_lint_options(lint_flags)?; - let files = collect_lint_files(lint_options.files.clone()).and_then( - |files| { - if files.is_empty() { - Err(generic_error("No target files found.")) - } else { - Ok(files) - } - }, - )?; + let files = + collect_lint_files(cli_options, lint_options.files.clone()) + .and_then(|files| { + if files.is_empty() { + Err(generic_error("No target files found.")) + } else { + Ok(files) + } + })?; _ = watcher_communicator.watch_paths(files.clone()); let lint_paths = if let Some(paths) = changed_paths { @@ -133,8 +134,8 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { reporter_lock.lock().close(1); success } else { - let target_files = - collect_lint_files(files.clone()).and_then(|files| { + let target_files = collect_lint_files(cli_options, files.clone()) + .and_then(|files| { if files.is_empty() { Err(generic_error("No target files found.")) } else { @@ -267,11 +268,14 @@ async fn lint_files( Ok(!has_error.is_raised()) } -fn collect_lint_files(files: FilePatterns) -> Result, AnyError> { +fn collect_lint_files( + cli_options: &CliOptions, + files: FilePatterns, +) -> Result, AnyError> { FileCollector::new(|e| is_script_ext(e.path)) .ignore_git_folder() .ignore_node_modules() - .ignore_vendor_folder() + .set_vendor_folder(cli_options.vendor_dir_path().map(ToOwned::to_owned)) .collect_file_patterns(files) } diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 738c8c3048..2c7241de16 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -1567,30 +1567,38 @@ fn is_supported_test_ext(path: &Path) -> bool { /// - Specifiers matching the `is_supported_test_path` are marked as `TestMode::Executable`. /// - Specifiers matching both predicates are marked as `TestMode::Both` fn collect_specifiers_with_test_mode( + cli_options: &CliOptions, files: FilePatterns, include_inline: &bool, ) -> Result, AnyError> { // todo(dsherret): there's no need to collect twice as it's slow - let module_specifiers = - collect_specifiers(files.clone(), is_supported_test_path_predicate)?; + let vendor_folder = cli_options.vendor_dir_path(); + let module_specifiers = collect_specifiers( + files.clone(), + vendor_folder.map(ToOwned::to_owned), + is_supported_test_path_predicate, + )?; if *include_inline { - return collect_specifiers(files, |e| is_supported_test_ext(e.path)).map( - |specifiers| { - specifiers - .into_iter() - .map(|specifier| { - let mode = if module_specifiers.contains(&specifier) { - TestMode::Both - } else { - TestMode::Documentation - }; + return collect_specifiers( + files, + vendor_folder.map(ToOwned::to_owned), + |e| is_supported_test_ext(e.path), + ) + .map(|specifiers| { + specifiers + .into_iter() + .map(|specifier| { + let mode = if module_specifiers.contains(&specifier) { + TestMode::Both + } else { + TestMode::Documentation + }; - (specifier, mode) - }) - .collect() - }, - ); + (specifier, mode) + }) + .collect() + }); } let specifiers_with_mode = module_specifiers @@ -1610,11 +1618,13 @@ fn collect_specifiers_with_test_mode( /// cannot be run, and therefore need to be marked as `TestMode::Documentation` /// as well. async fn fetch_specifiers_with_test_mode( + cli_options: &CliOptions, file_fetcher: &FileFetcher, files: FilePatterns, doc: &bool, ) -> Result, AnyError> { - let mut specifiers_with_mode = collect_specifiers_with_test_mode(files, doc)?; + let mut specifiers_with_mode = + collect_specifiers_with_test_mode(cli_options, files, doc)?; for (specifier, mode) in &mut specifiers_with_mode { let file = file_fetcher @@ -1647,6 +1657,7 @@ pub async fn run_tests( let log_level = cli_options.log_level(); let specifiers_with_mode = fetch_specifiers_with_test_mode( + cli_options, file_fetcher, test_options.files.clone(), &test_options.doc, @@ -1758,12 +1769,15 @@ pub async fn run_tests_with_watch( let module_graph_creator = factory.module_graph_creator().await?; let file_fetcher = factory.file_fetcher()?; let test_modules = if test_options.doc { - collect_specifiers(test_options.files.clone(), |e| { - is_supported_test_ext(e.path) - }) + collect_specifiers( + test_options.files.clone(), + cli_options.vendor_dir_path().map(ToOwned::to_owned), + |e| is_supported_test_ext(e.path), + ) } else { collect_specifiers( test_options.files.clone(), + cli_options.vendor_dir_path().map(ToOwned::to_owned), is_supported_test_path_predicate, ) }?; @@ -1798,6 +1812,7 @@ pub async fn run_tests_with_watch( Arc::new(factory.create_cli_main_worker_factory().await?); let module_load_preparer = factory.module_load_preparer().await?; let specifiers_with_mode = fetch_specifiers_with_test_mode( + &cli_options, file_fetcher, test_options.files.clone(), &test_options.doc, diff --git a/cli/util/fs.rs b/cli/util/fs.rs index 047bf18dc0..ba55eb89dd 100644 --- a/cli/util/fs.rs +++ b/cli/util/fs.rs @@ -260,7 +260,6 @@ pub struct FileCollector bool> { file_filter: TFilter, ignore_git_folder: bool, ignore_node_modules: bool, - ignore_vendor_folder: bool, vendor_folder: Option, use_gitignore: bool, } @@ -271,7 +270,6 @@ impl bool> FileCollector { file_filter, ignore_git_folder: false, ignore_node_modules: false, - ignore_vendor_folder: false, vendor_folder: None, use_gitignore: false, } @@ -282,11 +280,6 @@ impl bool> FileCollector { self } - pub fn ignore_vendor_folder(mut self) -> Self { - self.ignore_vendor_folder = true; - self - } - pub fn set_vendor_folder(mut self, vendor_folder: Option) -> Self { self.vendor_folder = vendor_folder; self @@ -422,7 +415,6 @@ impl bool> FileCollector { let dir_name = dir_name.to_string_lossy().to_lowercase(); let is_ignored_file = match dir_name.as_str() { "node_modules" => self.ignore_node_modules, - "vendor" => self.ignore_vendor_folder, ".git" => self.ignore_git_folder, _ => false, }; @@ -446,6 +438,7 @@ impl bool> FileCollector { /// Note: This ignores all .git and node_modules folders. pub fn collect_specifiers( mut files: FilePatterns, + vendor_folder: Option, predicate: impl Fn(WalkEntry) -> bool, ) -> Result, AnyError> { let mut prepared = vec![]; @@ -484,7 +477,7 @@ pub fn collect_specifiers( let collected_files = FileCollector::new(predicate) .ignore_git_folder() .ignore_node_modules() - .ignore_vendor_folder() + .set_vendor_folder(vendor_folder) .collect_file_patterns(files)?; let mut collected_files_as_urls = collected_files .iter() @@ -958,7 +951,7 @@ mod tests { let file_collector = file_collector .ignore_git_folder() .ignore_node_modules() - .ignore_vendor_folder(); + .set_vendor_folder(Some(child_dir_path.join("vendor").to_path_buf())); let result = file_collector .collect_file_patterns(file_patterns.clone()) .unwrap(); @@ -1074,6 +1067,7 @@ mod tests { ignore_dir_path.to_path_buf(), )]), }, + None, predicate, ) .unwrap(); @@ -1119,6 +1113,7 @@ mod tests { .unwrap()])), exclude: Default::default(), }, + None, predicate, ) .unwrap();