From 120b3811eb1ea8796daadd7b8da1bd705252bbde Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Wed, 4 Dec 2024 13:06:09 +0100 Subject: [PATCH] fix(task): don't panic with filter on missing task argument (#27180) We were panicing when running `deno task --filter foo` without a task argument. Fixes https://github.com/denoland/deno/issues/27177 --- cli/tools/task.rs | 128 +++++++++++++----- tests/specs/task/filter/__test__.jsonc | 20 +++ .../filter/deno_filter_no_match_no_task.out | 1 + .../specs/task/filter/deno_filter_no_task.out | 6 + .../filter/npm_filter_no_match_no_task.out | 1 + .../specs/task/filter/npm_filter_no_task.out | 6 + 6 files changed, 131 insertions(+), 31 deletions(-) create mode 100644 tests/specs/task/filter/deno_filter_no_match_no_task.out create mode 100644 tests/specs/task/filter/deno_filter_no_task.out create mode 100644 tests/specs/task/filter/npm_filter_no_match_no_task.out create mode 100644 tests/specs/task/filter/npm_filter_no_task.out diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 21919be3c3..01d6ac16ee 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -78,41 +78,24 @@ pub async fn execute_script( let packages_task_configs: Vec = if let Some(filter) = &task_flags.filter { - let task_name = task_flags.task.as_ref().unwrap(); - // Filter based on package name let package_regex = arg_to_regex(filter)?; - let task_regex = arg_to_regex(task_name)?; + let workspace = cli_options.workspace(); + let Some(task_name) = &task_flags.task else { + print_available_tasks_workspace( + cli_options, + &package_regex, + filter, + force_use_pkg_json, + )?; + + return Ok(0); + }; + + let task_regex = arg_to_regex(task_name)?; let mut packages_task_info: Vec = vec![]; - fn matches_package( - config: &FolderConfigs, - force_use_pkg_json: bool, - regex: &Regex, - ) -> bool { - if !force_use_pkg_json { - if let Some(deno_json) = &config.deno_json { - if let Some(name) = &deno_json.json.name { - if regex.is_match(name) { - return true; - } - } - } - } - - if let Some(package_json) = &config.pkg_json { - if let Some(name) = &package_json.name { - if regex.is_match(name) { - return true; - } - } - } - - false - } - - let workspace = cli_options.workspace(); for folder in workspace.config_folders() { if !matches_package(folder.1, force_use_pkg_json, &package_regex) { continue; @@ -198,6 +181,7 @@ pub async fn execute_script( &mut std::io::stdout(), &cli_options.start_dir, &tasks_config, + None, )?; return Ok(0); }; @@ -315,6 +299,7 @@ impl<'a> TaskRunner<'a> { &mut std::io::stderr(), &self.cli_options.start_dir, tasks_config, + None, ) } @@ -675,6 +660,32 @@ fn sort_tasks_topo<'a>( Ok(sorted) } +fn matches_package( + config: &FolderConfigs, + force_use_pkg_json: bool, + regex: &Regex, +) -> bool { + if !force_use_pkg_json { + if let Some(deno_json) = &config.deno_json { + if let Some(name) = &deno_json.json.name { + if regex.is_match(name) { + return true; + } + } + } + } + + if let Some(package_json) = &config.pkg_json { + if let Some(name) = &package_json.name { + if regex.is_match(name) { + return true; + } + } + } + + false +} + fn output_task(task_name: &str, script: &str) { log::info!( "{} {} {}", @@ -684,12 +695,67 @@ fn output_task(task_name: &str, script: &str) { ); } +fn print_available_tasks_workspace( + cli_options: &Arc, + package_regex: &Regex, + filter: &str, + force_use_pkg_json: bool, +) -> Result<(), AnyError> { + let workspace = cli_options.workspace(); + + let mut matched = false; + for folder in workspace.config_folders() { + if !matches_package(folder.1, force_use_pkg_json, package_regex) { + continue; + } + matched = true; + + let member_dir = workspace.resolve_member_dir(folder.0); + let mut tasks_config = member_dir.to_tasks_config()?; + + let mut pkg_name = folder + .1 + .deno_json + .as_ref() + .and_then(|deno| deno.json.name.clone()) + .or(folder.1.pkg_json.as_ref().and_then(|pkg| pkg.name.clone())); + + if force_use_pkg_json { + tasks_config = tasks_config.with_only_pkg_json(); + pkg_name = folder.1.pkg_json.as_ref().and_then(|pkg| pkg.name.clone()); + } + + print_available_tasks( + &mut std::io::stdout(), + &cli_options.start_dir, + &tasks_config, + pkg_name, + )?; + } + + if !matched { + log::warn!( + "{}", + colors::red(format!("No package name matched the filter '{}' in available 'deno.json' or 'package.json' files.", filter)) + ); + } + + Ok(()) +} + fn print_available_tasks( writer: &mut dyn std::io::Write, workspace_dir: &Arc, tasks_config: &WorkspaceTasksConfig, + pkg_name: Option, ) -> Result<(), std::io::Error> { - writeln!(writer, "{}", colors::green("Available tasks:"))?; + let heading = if let Some(s) = pkg_name { + format!("Available tasks ({}):", colors::cyan(s)) + } else { + "Available tasks:".to_string() + }; + + writeln!(writer, "{}", colors::green(heading))?; let is_cwd_root_dir = tasks_config.root.is_none(); if tasks_config.is_empty() { diff --git a/tests/specs/task/filter/__test__.jsonc b/tests/specs/task/filter/__test__.jsonc index 8baaf04dd4..45f4e7d7d8 100644 --- a/tests/specs/task/filter/__test__.jsonc +++ b/tests/specs/task/filter/__test__.jsonc @@ -30,6 +30,16 @@ "args": "task --filter @foo/* dev", "output": "npm_workspace_order.out" }, + "npm_filter_no_match_no_task": { + "cwd": "./npm", + "args": "task --filter @asdf", + "output": "npm_filter_no_match_no_task.out" + }, + "npm_filter_no_task": { + "cwd": "./npm", + "args": "task --filter *", + "output": "npm_filter_no_task.out" + }, "deno_all": { "cwd": "./deno", "args": "task --filter * dev", @@ -54,6 +64,16 @@ "cwd": "./deno_workspace_order", "args": "task --filter @foo/* dev", "output": "deno_workspace_order.out" + }, + "deno_filter_no_match_no_task": { + "cwd": "./deno", + "args": "task --filter @asdf", + "output": "deno_filter_no_match_no_task.out" + }, + "deno_filter_no_task": { + "cwd": "./deno", + "args": "task --filter *", + "output": "deno_filter_no_task.out" } } } diff --git a/tests/specs/task/filter/deno_filter_no_match_no_task.out b/tests/specs/task/filter/deno_filter_no_match_no_task.out new file mode 100644 index 0000000000..b2cf6371b0 --- /dev/null +++ b/tests/specs/task/filter/deno_filter_no_match_no_task.out @@ -0,0 +1 @@ +No package name matched the filter '@asdf' in available 'deno.json' or 'package.json' files. diff --git a/tests/specs/task/filter/deno_filter_no_task.out b/tests/specs/task/filter/deno_filter_no_task.out new file mode 100644 index 0000000000..b24b6641e9 --- /dev/null +++ b/tests/specs/task/filter/deno_filter_no_task.out @@ -0,0 +1,6 @@ +Available tasks (@deno/bar): +- dev + echo '@deno/bar' +Available tasks (@deno/foo): +- dev + echo '@deno/foo' diff --git a/tests/specs/task/filter/npm_filter_no_match_no_task.out b/tests/specs/task/filter/npm_filter_no_match_no_task.out new file mode 100644 index 0000000000..b2cf6371b0 --- /dev/null +++ b/tests/specs/task/filter/npm_filter_no_match_no_task.out @@ -0,0 +1 @@ +No package name matched the filter '@asdf' in available 'deno.json' or 'package.json' files. diff --git a/tests/specs/task/filter/npm_filter_no_task.out b/tests/specs/task/filter/npm_filter_no_task.out new file mode 100644 index 0000000000..7863c0f9d6 --- /dev/null +++ b/tests/specs/task/filter/npm_filter_no_task.out @@ -0,0 +1,6 @@ +Available tasks (bar): +- dev (package.json) + echo 'bar' +Available tasks (foo): +- dev (package.json) + echo 'foo'