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
This commit is contained in:
David Sherret 2024-03-27 14:25:39 -04:00 committed by GitHub
parent 0e4d1cb5f9
commit 68fecc6de4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 90 additions and 61 deletions

View file

@ -1974,7 +1974,6 @@ mod test {
let mut files = FileCollector::new(|_| true) let mut files = FileCollector::new(|_| true)
.ignore_git_folder() .ignore_git_folder()
.ignore_node_modules() .ignore_node_modules()
.ignore_vendor_folder()
.collect_file_patterns(resolved_files) .collect_file_patterns(resolved_files)
.unwrap(); .unwrap();

View file

@ -432,8 +432,11 @@ pub async fn run_benchmarks(
let permissions = let permissions =
Permissions::from_options(&cli_options.permissions_options())?; Permissions::from_options(&cli_options.permissions_options())?;
let specifiers = let specifiers = collect_specifiers(
collect_specifiers(bench_options.files, is_supported_bench_path)?; bench_options.files,
cli_options.vendor_dir_path().map(ToOwned::to_owned),
is_supported_bench_path,
)?;
if specifiers.is_empty() { if specifiers.is_empty() {
return Err(generic_error("No bench modules found")); return Err(generic_error("No bench modules found"));
@ -505,6 +508,7 @@ pub async fn run_benchmarks_with_watch(
let bench_modules = collect_specifiers( let bench_modules = collect_specifiers(
bench_options.files.clone(), bench_options.files.clone(),
cli_options.vendor_dir_path().map(ToOwned::to_owned),
is_supported_bench_path, is_supported_bench_path,
)?; )?;
@ -543,8 +547,11 @@ pub async fn run_benchmarks_with_watch(
// todo(dsherret): why are we collecting specifiers twice in a row? // todo(dsherret): why are we collecting specifiers twice in a row?
// Seems like a perf bug. // Seems like a perf bug.
let specifiers = let specifiers = collect_specifiers(
collect_specifiers(bench_options.files, is_supported_bench_path)? bench_options.files,
cli_options.vendor_dir_path().map(ToOwned::to_owned),
is_supported_bench_path,
)?
.into_iter() .into_iter()
.filter(|specifier| bench_modules_to_reload.contains(specifier)) .filter(|specifier| bench_modules_to_reload.contains(specifier))
.collect::<Vec<ModuleSpecifier>>(); .collect::<Vec<ModuleSpecifier>>();

View file

@ -1,5 +1,6 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
use crate::args::CliOptions;
use crate::args::CoverageFlags; use crate::args::CoverageFlags;
use crate::args::FileFlags; use crate::args::FileFlags;
use crate::args::Flags; use crate::args::Flags;
@ -376,6 +377,7 @@ fn range_to_src_line_index(
} }
fn collect_coverages( fn collect_coverages(
cli_options: &CliOptions,
files: FileFlags, files: FileFlags,
initial_cwd: &Path, initial_cwd: &Path,
) -> Result<Vec<cdp::ScriptCoverage>, AnyError> { ) -> Result<Vec<cdp::ScriptCoverage>, AnyError> {
@ -405,7 +407,7 @@ fn collect_coverages(
}) })
.ignore_git_folder() .ignore_git_folder()
.ignore_node_modules() .ignore_node_modules()
.ignore_vendor_folder() .set_vendor_folder(cli_options.vendor_dir_path().map(ToOwned::to_owned))
.collect_file_patterns(file_patterns)?; .collect_file_patterns(file_patterns)?;
for file_path in file_paths { for file_path in file_paths {
@ -474,8 +476,11 @@ pub async fn cover_files(
let coverage_root = cli_options let coverage_root = cli_options
.initial_cwd() .initial_cwd()
.join(&coverage_flags.files.include[0]); .join(&coverage_flags.files.include[0]);
let script_coverages = let script_coverages = collect_coverages(
collect_coverages(coverage_flags.files, cli_options.initial_cwd())?; cli_options,
coverage_flags.files,
cli_options.initial_cwd(),
)?;
if script_coverages.is_empty() { if script_coverages.is_empty() {
return Err(generic_error("No coverage files found")); return Err(generic_error("No coverage files found"));
} }

View file

@ -103,6 +103,7 @@ pub async fn doc(flags: Flags, doc_flags: DocFlags) -> Result<(), AnyError> {
), ),
exclude: Default::default(), exclude: Default::default(),
}, },
cli_options.vendor_dir_path().map(ToOwned::to_owned),
|_| true, |_| true,
)?; )?;
let graph = module_graph_creator let graph = module_graph_creator

View file

@ -71,8 +71,8 @@ pub async fn format(flags: Flags, fmt_flags: FmtFlags) -> Result<(), AnyError> {
let factory = CliFactory::from_flags(flags)?; let factory = CliFactory::from_flags(flags)?;
let cli_options = factory.cli_options(); let cli_options = factory.cli_options();
let fmt_options = cli_options.resolve_fmt_options(fmt_flags)?; let fmt_options = cli_options.resolve_fmt_options(fmt_flags)?;
let files = let files = collect_fmt_files(cli_options, fmt_options.files.clone())
collect_fmt_files(fmt_options.files.clone()).and_then(|files| { .and_then(|files| {
if files.is_empty() { if files.is_empty() {
Err(generic_error("No target files found.")) Err(generic_error("No target files found."))
} else { } else {
@ -116,8 +116,8 @@ pub async fn format(flags: Flags, fmt_flags: FmtFlags) -> Result<(), AnyError> {
let factory = CliFactory::from_flags(flags)?; let factory = CliFactory::from_flags(flags)?;
let cli_options = factory.cli_options(); let cli_options = factory.cli_options();
let fmt_options = cli_options.resolve_fmt_options(fmt_flags)?; let fmt_options = cli_options.resolve_fmt_options(fmt_flags)?;
let files = let files = collect_fmt_files(cli_options, fmt_options.files.clone())
collect_fmt_files(fmt_options.files.clone()).and_then(|files| { .and_then(|files| {
if files.is_empty() { if files.is_empty() {
Err(generic_error("No target files found.")) Err(generic_error("No target files found."))
} else { } else {
@ -153,11 +153,14 @@ async fn format_files(
Ok(()) Ok(())
} }
fn collect_fmt_files(files: FilePatterns) -> Result<Vec<PathBuf>, AnyError> { fn collect_fmt_files(
cli_options: &CliOptions,
files: FilePatterns,
) -> Result<Vec<PathBuf>, AnyError> {
FileCollector::new(|e| is_supported_ext_fmt(e.path)) FileCollector::new(|e| is_supported_ext_fmt(e.path))
.ignore_git_folder() .ignore_git_folder()
.ignore_node_modules() .ignore_node_modules()
.ignore_vendor_folder() .set_vendor_folder(cli_options.vendor_dir_path().map(ToOwned::to_owned))
.collect_file_patterns(files) .collect_file_patterns(files)
} }

View file

@ -34,6 +34,7 @@ use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::Arc; use std::sync::Arc;
use crate::args::CliOptions;
use crate::args::Flags; use crate::args::Flags;
use crate::args::LintFlags; use crate::args::LintFlags;
use crate::args::LintOptions; 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 factory = CliFactory::from_flags(flags)?;
let cli_options = factory.cli_options(); let cli_options = factory.cli_options();
let lint_options = cli_options.resolve_lint_options(lint_flags)?; let lint_options = cli_options.resolve_lint_options(lint_flags)?;
let files = collect_lint_files(lint_options.files.clone()).and_then( let files =
|files| { collect_lint_files(cli_options, lint_options.files.clone())
.and_then(|files| {
if files.is_empty() { if files.is_empty() {
Err(generic_error("No target files found.")) Err(generic_error("No target files found."))
} else { } else {
Ok(files) Ok(files)
} }
}, })?;
)?;
_ = watcher_communicator.watch_paths(files.clone()); _ = watcher_communicator.watch_paths(files.clone());
let lint_paths = if let Some(paths) = changed_paths { 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); reporter_lock.lock().close(1);
success success
} else { } else {
let target_files = let target_files = collect_lint_files(cli_options, files.clone())
collect_lint_files(files.clone()).and_then(|files| { .and_then(|files| {
if files.is_empty() { if files.is_empty() {
Err(generic_error("No target files found.")) Err(generic_error("No target files found."))
} else { } else {
@ -267,11 +268,14 @@ async fn lint_files(
Ok(!has_error.is_raised()) Ok(!has_error.is_raised())
} }
fn collect_lint_files(files: FilePatterns) -> Result<Vec<PathBuf>, AnyError> { fn collect_lint_files(
cli_options: &CliOptions,
files: FilePatterns,
) -> Result<Vec<PathBuf>, AnyError> {
FileCollector::new(|e| is_script_ext(e.path)) FileCollector::new(|e| is_script_ext(e.path))
.ignore_git_folder() .ignore_git_folder()
.ignore_node_modules() .ignore_node_modules()
.ignore_vendor_folder() .set_vendor_folder(cli_options.vendor_dir_path().map(ToOwned::to_owned))
.collect_file_patterns(files) .collect_file_patterns(files)
} }

View file

@ -1567,16 +1567,25 @@ fn is_supported_test_ext(path: &Path) -> bool {
/// - Specifiers matching the `is_supported_test_path` are marked as `TestMode::Executable`. /// - Specifiers matching the `is_supported_test_path` are marked as `TestMode::Executable`.
/// - Specifiers matching both predicates are marked as `TestMode::Both` /// - Specifiers matching both predicates are marked as `TestMode::Both`
fn collect_specifiers_with_test_mode( fn collect_specifiers_with_test_mode(
cli_options: &CliOptions,
files: FilePatterns, files: FilePatterns,
include_inline: &bool, include_inline: &bool,
) -> Result<Vec<(ModuleSpecifier, TestMode)>, AnyError> { ) -> Result<Vec<(ModuleSpecifier, TestMode)>, AnyError> {
// todo(dsherret): there's no need to collect twice as it's slow // todo(dsherret): there's no need to collect twice as it's slow
let module_specifiers = let vendor_folder = cli_options.vendor_dir_path();
collect_specifiers(files.clone(), is_supported_test_path_predicate)?; let module_specifiers = collect_specifiers(
files.clone(),
vendor_folder.map(ToOwned::to_owned),
is_supported_test_path_predicate,
)?;
if *include_inline { if *include_inline {
return collect_specifiers(files, |e| is_supported_test_ext(e.path)).map( return collect_specifiers(
|specifiers| { files,
vendor_folder.map(ToOwned::to_owned),
|e| is_supported_test_ext(e.path),
)
.map(|specifiers| {
specifiers specifiers
.into_iter() .into_iter()
.map(|specifier| { .map(|specifier| {
@ -1589,8 +1598,7 @@ fn collect_specifiers_with_test_mode(
(specifier, mode) (specifier, mode)
}) })
.collect() .collect()
}, });
);
} }
let specifiers_with_mode = module_specifiers 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` /// cannot be run, and therefore need to be marked as `TestMode::Documentation`
/// as well. /// as well.
async fn fetch_specifiers_with_test_mode( async fn fetch_specifiers_with_test_mode(
cli_options: &CliOptions,
file_fetcher: &FileFetcher, file_fetcher: &FileFetcher,
files: FilePatterns, files: FilePatterns,
doc: &bool, doc: &bool,
) -> Result<Vec<(ModuleSpecifier, TestMode)>, AnyError> { ) -> Result<Vec<(ModuleSpecifier, TestMode)>, 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 { for (specifier, mode) in &mut specifiers_with_mode {
let file = file_fetcher let file = file_fetcher
@ -1647,6 +1657,7 @@ pub async fn run_tests(
let log_level = cli_options.log_level(); let log_level = cli_options.log_level();
let specifiers_with_mode = fetch_specifiers_with_test_mode( let specifiers_with_mode = fetch_specifiers_with_test_mode(
cli_options,
file_fetcher, file_fetcher,
test_options.files.clone(), test_options.files.clone(),
&test_options.doc, &test_options.doc,
@ -1758,12 +1769,15 @@ pub async fn run_tests_with_watch(
let module_graph_creator = factory.module_graph_creator().await?; let module_graph_creator = factory.module_graph_creator().await?;
let file_fetcher = factory.file_fetcher()?; let file_fetcher = factory.file_fetcher()?;
let test_modules = if test_options.doc { let test_modules = if test_options.doc {
collect_specifiers(test_options.files.clone(), |e| { collect_specifiers(
is_supported_test_ext(e.path) test_options.files.clone(),
}) cli_options.vendor_dir_path().map(ToOwned::to_owned),
|e| is_supported_test_ext(e.path),
)
} else { } else {
collect_specifiers( collect_specifiers(
test_options.files.clone(), test_options.files.clone(),
cli_options.vendor_dir_path().map(ToOwned::to_owned),
is_supported_test_path_predicate, 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?); Arc::new(factory.create_cli_main_worker_factory().await?);
let module_load_preparer = factory.module_load_preparer().await?; let module_load_preparer = factory.module_load_preparer().await?;
let specifiers_with_mode = fetch_specifiers_with_test_mode( let specifiers_with_mode = fetch_specifiers_with_test_mode(
&cli_options,
file_fetcher, file_fetcher,
test_options.files.clone(), test_options.files.clone(),
&test_options.doc, &test_options.doc,

View file

@ -260,7 +260,6 @@ pub struct FileCollector<TFilter: Fn(WalkEntry) -> bool> {
file_filter: TFilter, file_filter: TFilter,
ignore_git_folder: bool, ignore_git_folder: bool,
ignore_node_modules: bool, ignore_node_modules: bool,
ignore_vendor_folder: bool,
vendor_folder: Option<PathBuf>, vendor_folder: Option<PathBuf>,
use_gitignore: bool, use_gitignore: bool,
} }
@ -271,7 +270,6 @@ impl<TFilter: Fn(WalkEntry) -> bool> FileCollector<TFilter> {
file_filter, file_filter,
ignore_git_folder: false, ignore_git_folder: false,
ignore_node_modules: false, ignore_node_modules: false,
ignore_vendor_folder: false,
vendor_folder: None, vendor_folder: None,
use_gitignore: false, use_gitignore: false,
} }
@ -282,11 +280,6 @@ impl<TFilter: Fn(WalkEntry) -> bool> FileCollector<TFilter> {
self 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<PathBuf>) -> Self { pub fn set_vendor_folder(mut self, vendor_folder: Option<PathBuf>) -> Self {
self.vendor_folder = vendor_folder; self.vendor_folder = vendor_folder;
self self
@ -422,7 +415,6 @@ impl<TFilter: Fn(WalkEntry) -> bool> FileCollector<TFilter> {
let dir_name = dir_name.to_string_lossy().to_lowercase(); let dir_name = dir_name.to_string_lossy().to_lowercase();
let is_ignored_file = match dir_name.as_str() { let is_ignored_file = match dir_name.as_str() {
"node_modules" => self.ignore_node_modules, "node_modules" => self.ignore_node_modules,
"vendor" => self.ignore_vendor_folder,
".git" => self.ignore_git_folder, ".git" => self.ignore_git_folder,
_ => false, _ => false,
}; };
@ -446,6 +438,7 @@ impl<TFilter: Fn(WalkEntry) -> bool> FileCollector<TFilter> {
/// Note: This ignores all .git and node_modules folders. /// Note: This ignores all .git and node_modules folders.
pub fn collect_specifiers( pub fn collect_specifiers(
mut files: FilePatterns, mut files: FilePatterns,
vendor_folder: Option<PathBuf>,
predicate: impl Fn(WalkEntry) -> bool, predicate: impl Fn(WalkEntry) -> bool,
) -> Result<Vec<ModuleSpecifier>, AnyError> { ) -> Result<Vec<ModuleSpecifier>, AnyError> {
let mut prepared = vec![]; let mut prepared = vec![];
@ -484,7 +477,7 @@ pub fn collect_specifiers(
let collected_files = FileCollector::new(predicate) let collected_files = FileCollector::new(predicate)
.ignore_git_folder() .ignore_git_folder()
.ignore_node_modules() .ignore_node_modules()
.ignore_vendor_folder() .set_vendor_folder(vendor_folder)
.collect_file_patterns(files)?; .collect_file_patterns(files)?;
let mut collected_files_as_urls = collected_files let mut collected_files_as_urls = collected_files
.iter() .iter()
@ -958,7 +951,7 @@ mod tests {
let file_collector = file_collector let file_collector = file_collector
.ignore_git_folder() .ignore_git_folder()
.ignore_node_modules() .ignore_node_modules()
.ignore_vendor_folder(); .set_vendor_folder(Some(child_dir_path.join("vendor").to_path_buf()));
let result = file_collector let result = file_collector
.collect_file_patterns(file_patterns.clone()) .collect_file_patterns(file_patterns.clone())
.unwrap(); .unwrap();
@ -1074,6 +1067,7 @@ mod tests {
ignore_dir_path.to_path_buf(), ignore_dir_path.to_path_buf(),
)]), )]),
}, },
None,
predicate, predicate,
) )
.unwrap(); .unwrap();
@ -1119,6 +1113,7 @@ mod tests {
.unwrap()])), .unwrap()])),
exclude: Default::default(), exclude: Default::default(),
}, },
None,
predicate, predicate,
) )
.unwrap(); .unwrap();