From 84e12386480d76e97ad85d9c5314168e7ab03e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Nov 2024 11:18:17 +0000 Subject: [PATCH] feat(task): support object notation, remove support for JSDocs (#26886) This commit changes three aspects of `deno task`: 1. Tasks can now be written using object notation like so: ```jsonc { "tasks": { "foo": "deno run foo.js", "bar": { "command": "deno run bar.js" } } ``` 2. Support for comments for tasks is now removed. Comments above tasks will no longer be printed when running `deno task`. 3. Tasks written using object notation can have "description" field that replaces support for comments above tasks: ```jsonc { "tasks": { "bar": { "description": "This is a bar task" "command": "deno run bar.js" } } ``` ``` $ deno task Available tasks: - bar // This is a bar task deno run bar.js ``` Pulled most of the changes from https://github.com/denoland/deno/pull/26467 to support "dependencies" in tasks. Additionally some cleanup was performed to make code easier to read. --------- Co-authored-by: David Sherret --- Cargo.lock | 4 +- Cargo.toml | 2 +- cli/args/mod.rs | 8 +- cli/lsp/language_server.rs | 5 +- cli/schemas/config-file.v1.json | 23 +- cli/tools/task.rs | 426 ++++++++++-------- tests/specs/task/description/__test__.jsonc | 4 + tests/specs/task/description/deno.json | 8 + tests/specs/task/description/main.out | 4 + .../doc_comments_incorrect/__test__.jsonc | 5 - .../task/doc_comments_incorrect/deno.jsonc | 7 - .../task/doc_comments_incorrect/task.out | 6 - .../task/doc_comments_no_args/__test__.jsonc | 5 - .../task/doc_comments_no_args/deno.jsonc | 12 - .../specs/task/doc_comments_no_args/task.out | 9 - 15 files changed, 279 insertions(+), 249 deletions(-) create mode 100644 tests/specs/task/description/__test__.jsonc create mode 100644 tests/specs/task/description/deno.json create mode 100644 tests/specs/task/description/main.out delete mode 100644 tests/specs/task/doc_comments_incorrect/__test__.jsonc delete mode 100644 tests/specs/task/doc_comments_incorrect/deno.jsonc delete mode 100644 tests/specs/task/doc_comments_incorrect/task.out delete mode 100644 tests/specs/task/doc_comments_no_args/__test__.jsonc delete mode 100644 tests/specs/task/doc_comments_no_args/deno.jsonc delete mode 100644 tests/specs/task/doc_comments_no_args/task.out diff --git a/Cargo.lock b/Cargo.lock index eccd52fffe..56d2417419 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1421,9 +1421,9 @@ dependencies = [ [[package]] name = "deno_config" -version = "0.38.2" +version = "0.39.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "966825073480a6ac7e01977a3879d13edc8d6ea2d65ea164b37156a5fb206e9a" +checksum = "0a91aa99751ebe305a7edad12a3ad751f3b3b9f5ecddbfe4a0459e3cdc8493b6" dependencies = [ "anyhow", "deno_package_json", diff --git a/Cargo.toml b/Cargo.toml index ad63355d21..9d625f082a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ deno_ast = { version = "=0.43.3", features = ["transpiling"] } deno_core = { version = "0.319.0" } deno_bench_util = { version = "0.171.0", path = "./bench_util" } -deno_config = { version = "=0.38.2", features = ["workspace", "sync"] } +deno_config = { version = "=0.39.1", features = ["workspace", "sync"] } deno_lockfile = "=0.23.1" deno_media_type = { version = "0.2.0", features = ["module_specifier"] } deno_npm = "=0.25.4" diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 50a37b3346..318a6ca76b 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -868,12 +868,8 @@ impl CliOptions { } else { &[] }; - let config_parse_options = deno_config::deno_json::ConfigParseOptions { - include_task_comments: matches!( - flags.subcommand, - DenoSubcommand::Task(..) - ), - }; + let config_parse_options = + deno_config::deno_json::ConfigParseOptions::default(); let discover_pkg_json = flags.config_flag != ConfigFlag::Disabled && !flags.no_npm && !has_flag_env_var("DENO_NO_PACKAGE_JSON"); diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index e56adafef4..c93628555b 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -3632,9 +3632,8 @@ impl Inner { deno_json_cache: None, pkg_json_cache: None, workspace_cache: None, - config_parse_options: deno_config::deno_json::ConfigParseOptions { - include_task_comments: false, - }, + config_parse_options: + deno_config::deno_json::ConfigParseOptions::default(), additional_config_file_names: &[], discover_pkg_json: !has_flag_env_var("DENO_NO_PACKAGE_JSON"), maybe_vendor_override: if force_global_cache { diff --git a/cli/schemas/config-file.v1.json b/cli/schemas/config-file.v1.json index 9f4737fa0a..56a8090f9b 100644 --- a/cli/schemas/config-file.v1.json +++ b/cli/schemas/config-file.v1.json @@ -431,8 +431,27 @@ "type": "object", "patternProperties": { "^[A-Za-z][A-Za-z0-9_\\-:]*$": { - "type": "string", - "description": "Command to execute for this task name." + "oneOf": [ + { + "type": "string", + "description": "Command to execute for this task name." + }, + { + "type": "object", + "description": "A definition of a task to execute", + "properties": { + "description": { + "type": "string", + "description": "Description of a task that will be shown when running `deno task` without a task name" + }, + "command": { + "type": "string", + "required": true, + "description": "The task to execute" + } + } + } + ] } }, "additionalProperties": false diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 502b09d2c2..a13efbaf45 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -1,6 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use std::borrow::Cow; use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; @@ -8,7 +7,7 @@ use std::path::PathBuf; use std::rc::Rc; use std::sync::Arc; -use deno_config::deno_json::Task; +use deno_config::workspace::TaskDefinition; use deno_config::workspace::TaskOrScript; use deno_config::workspace::WorkspaceDirectory; use deno_config::workspace::WorkspaceTasksConfig; @@ -16,8 +15,11 @@ use deno_core::anyhow::anyhow; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; +use deno_core::url::Url; use deno_path_util::normalize_path; +use deno_runtime::deno_node::NodeResolver; use deno_task_shell::ShellCommand; +use indexmap::IndexMap; use crate::args::CliOptions; use crate::args::Flags; @@ -48,155 +50,188 @@ pub async fn execute_script( v == "1" }) .unwrap_or(false); - let tasks_config = start_dir.to_tasks_config()?; - let tasks_config = if force_use_pkg_json { - tasks_config.with_only_pkg_json() - } else { - tasks_config - }; + let mut tasks_config = start_dir.to_tasks_config()?; + if force_use_pkg_json { + tasks_config = tasks_config.with_only_pkg_json() + } - let task_name = match &task_flags.task { - Some(task) => task, - None => { - print_available_tasks( - &mut std::io::stdout(), - &cli_options.start_dir, - &tasks_config, - )?; - return Ok(0); - } + let Some(task_name) = &task_flags.task else { + print_available_tasks( + &mut std::io::stdout(), + &cli_options.start_dir, + &tasks_config, + )?; + return Ok(0); }; let npm_resolver = factory.npm_resolver().await?; let node_resolver = factory.node_resolver().await?; let env_vars = task_runner::real_env_vars(); - match tasks_config.task(task_name) { - Some((dir_url, task_or_script)) => match task_or_script { - TaskOrScript::Task(_tasks, script) => { - let cwd = match task_flags.cwd { - Some(path) => canonicalize_path(&PathBuf::from(path)) - .context("failed canonicalizing --cwd")?, - None => normalize_path(dir_url.to_file_path().unwrap()), - }; + let task_runner = TaskRunner { + tasks_config, + task_flags: &task_flags, + npm_resolver: npm_resolver.as_ref(), + node_resolver: node_resolver.as_ref(), + env_vars, + cli_options, + }; + task_runner.run_task(task_name).await +} - let custom_commands = task_runner::resolve_custom_commands( - npm_resolver.as_ref(), - node_resolver, - )?; - run_task(RunTaskOptions { - task_name, - script, - cwd: &cwd, - env_vars, - custom_commands, - npm_resolver: npm_resolver.as_ref(), - cli_options, - }) - .await - } - TaskOrScript::Script(scripts, _script) => { - // ensure the npm packages are installed if using a managed resolver - if let Some(npm_resolver) = npm_resolver.as_managed() { - npm_resolver.ensure_top_level_package_json_install().await?; - } +struct RunSingleOptions<'a> { + task_name: &'a str, + script: &'a str, + cwd: &'a Path, + custom_commands: HashMap>, +} - let cwd = match task_flags.cwd { - Some(path) => canonicalize_path(&PathBuf::from(path))?, - None => normalize_path(dir_url.to_file_path().unwrap()), - }; +struct TaskRunner<'a> { + tasks_config: WorkspaceTasksConfig, + task_flags: &'a TaskFlags, + npm_resolver: &'a dyn CliNpmResolver, + node_resolver: &'a NodeResolver, + env_vars: HashMap, + cli_options: &'a CliOptions, +} - // At this point we already checked if the task name exists in package.json. - // We can therefore check for "pre" and "post" scripts too, since we're only - // dealing with package.json here and not deno.json - let task_names = vec![ - format!("pre{}", task_name), - task_name.clone(), - format!("post{}", task_name), - ]; - let custom_commands = task_runner::resolve_custom_commands( - npm_resolver.as_ref(), - node_resolver, - )?; - for task_name in &task_names { - if let Some(script) = scripts.get(task_name) { - let exit_code = run_task(RunTaskOptions { - task_name, - script, - cwd: &cwd, - env_vars: env_vars.clone(), - custom_commands: custom_commands.clone(), - npm_resolver: npm_resolver.as_ref(), - cli_options, - }) - .await?; - if exit_code > 0 { - return Ok(exit_code); - } - } - } - - Ok(0) - } - }, - None => { - if task_flags.is_run { +impl<'a> TaskRunner<'a> { + async fn run_task( + &self, + task_name: &String, + ) -> Result { + let Some((dir_url, task_or_script)) = self.tasks_config.task(task_name) + else { + if self.task_flags.is_run { return Err(anyhow!("Task not found: {}", task_name)); } + log::error!("Task not found: {}", task_name); if log::log_enabled!(log::Level::Error) { print_available_tasks( &mut std::io::stderr(), - &cli_options.start_dir, - &tasks_config, + &self.cli_options.start_dir, + &self.tasks_config, )?; } - Ok(1) + return Ok(1); + }; + + match task_or_script { + TaskOrScript::Task(_tasks, definition) => { + self.run_deno_task(dir_url, task_name, definition).await + } + TaskOrScript::Script(scripts, _script) => { + self.run_npm_script(dir_url, task_name, scripts).await + } } } -} -struct RunTaskOptions<'a> { - task_name: &'a str, - script: &'a str, - cwd: &'a Path, - env_vars: HashMap, - custom_commands: HashMap>, - npm_resolver: &'a dyn CliNpmResolver, - cli_options: &'a CliOptions, -} + async fn run_deno_task( + &self, + dir_url: &Url, + task_name: &String, + definition: &TaskDefinition, + ) -> Result { + let cwd = match &self.task_flags.cwd { + Some(path) => canonicalize_path(&PathBuf::from(path)) + .context("failed canonicalizing --cwd")?, + None => normalize_path(dir_url.to_file_path().unwrap()), + }; -async fn run_task(opts: RunTaskOptions<'_>) -> Result { - let RunTaskOptions { - task_name, - script, - cwd, - env_vars, - custom_commands, - npm_resolver, - cli_options, - } = opts; + let custom_commands = task_runner::resolve_custom_commands( + self.npm_resolver, + self.node_resolver, + )?; + self + .run_single(RunSingleOptions { + task_name, + script: &definition.command, + cwd: &cwd, + custom_commands, + }) + .await + } - output_task( - opts.task_name, - &task_runner::get_script_with_args(script, cli_options.argv()), - ); + async fn run_npm_script( + &self, + dir_url: &Url, + task_name: &String, + scripts: &IndexMap, + ) -> Result { + // ensure the npm packages are installed if using a managed resolver + if let Some(npm_resolver) = self.npm_resolver.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + } - Ok( - task_runner::run_task(task_runner::RunTaskOptions { + let cwd = match &self.task_flags.cwd { + Some(path) => canonicalize_path(&PathBuf::from(path))?, + None => normalize_path(dir_url.to_file_path().unwrap()), + }; + + // At this point we already checked if the task name exists in package.json. + // We can therefore check for "pre" and "post" scripts too, since we're only + // dealing with package.json here and not deno.json + let task_names = vec![ + format!("pre{}", task_name), + task_name.clone(), + format!("post{}", task_name), + ]; + let custom_commands = task_runner::resolve_custom_commands( + self.npm_resolver, + self.node_resolver, + )?; + for task_name in &task_names { + if let Some(script) = scripts.get(task_name) { + let exit_code = self + .run_single(RunSingleOptions { + task_name, + script, + cwd: &cwd, + custom_commands: custom_commands.clone(), + }) + .await?; + if exit_code > 0 { + return Ok(exit_code); + } + } + } + + Ok(0) + } + + async fn run_single( + &self, + opts: RunSingleOptions<'_>, + ) -> Result { + let RunSingleOptions { task_name, script, cwd, - env_vars, custom_commands, - init_cwd: opts.cli_options.initial_cwd(), - argv: cli_options.argv(), - root_node_modules_dir: npm_resolver.root_node_modules_path(), - stdio: None, - }) - .await? - .exit_code, - ) + } = opts; + + output_task( + opts.task_name, + &task_runner::get_script_with_args(script, self.cli_options.argv()), + ); + + Ok( + task_runner::run_task(task_runner::RunTaskOptions { + task_name, + script, + cwd, + env_vars: self.env_vars.clone(), + custom_commands, + init_cwd: self.cli_options.initial_cwd(), + argv: self.cli_options.argv(), + root_node_modules_dir: self.npm_resolver.root_node_modules_path(), + stdio: None, + }) + .await? + .exit_code, + ) + } } fn output_task(task_name: &str, script: &str) { @@ -222,80 +257,89 @@ fn print_available_tasks( " {}", colors::red("No tasks found in configuration file") )?; - } else { - let mut seen_task_names = - HashSet::with_capacity(tasks_config.tasks_count()); - for maybe_config in [&tasks_config.member, &tasks_config.root] { - let Some(config) = maybe_config else { - continue; - }; - for (is_root, is_deno, (key, task)) in config - .deno_json - .as_ref() - .map(|config| { - let is_root = !is_cwd_root_dir - && config.folder_url - == *workspace_dir.workspace.root_dir().as_ref(); - config - .tasks - .iter() - .map(move |(k, t)| (is_root, true, (k, Cow::Borrowed(t)))) - }) - .into_iter() - .flatten() - .chain( - config - .package_json - .as_ref() - .map(|config| { - let is_root = !is_cwd_root_dir - && config.folder_url - == *workspace_dir.workspace.root_dir().as_ref(); - config.tasks.iter().map(move |(k, v)| { - (is_root, false, (k, Cow::Owned(Task::Definition(v.clone())))) - }) - }) - .into_iter() - .flatten(), - ) - { - if !seen_task_names.insert(key) { + return Ok(()); + } + + struct AvailableTaskDescription { + is_root: bool, + is_deno: bool, + name: String, + task: TaskDefinition, + } + let mut seen_task_names = HashSet::with_capacity(tasks_config.tasks_count()); + let mut task_descriptions = Vec::with_capacity(tasks_config.tasks_count()); + + for maybe_config in [&tasks_config.member, &tasks_config.root] { + let Some(config) = maybe_config else { + continue; + }; + + if let Some(config) = config.deno_json.as_ref() { + let is_root = !is_cwd_root_dir + && config.folder_url == *workspace_dir.workspace.root_dir().as_ref(); + + for (name, definition) in &config.tasks { + if !seen_task_names.insert(name) { continue; // already seen } - writeln!( - writer, - "- {}{}", - colors::cyan(key), - if is_root { - if is_deno { - format!(" {}", colors::italic_gray("(workspace)")) - } else { - format!(" {}", colors::italic_gray("(workspace package.json)")) - } - } else if is_deno { - "".to_string() - } else { - format!(" {}", colors::italic_gray("(package.json)")) - } - )?; - let definition = match task.as_ref() { - Task::Definition(definition) => definition, - Task::Commented { definition, .. } => definition, - }; - if let Task::Commented { comments, .. } = task.as_ref() { - let slash_slash = colors::italic_gray("//"); - for comment in comments { - writeln!( - writer, - " {slash_slash} {}", - colors::italic_gray(comment) - )?; - } + task_descriptions.push(AvailableTaskDescription { + is_root, + is_deno: true, + name: name.to_string(), + task: definition.clone(), + }); + } + } + + if let Some(config) = config.package_json.as_ref() { + let is_root = !is_cwd_root_dir + && config.folder_url == *workspace_dir.workspace.root_dir().as_ref(); + for (name, script) in &config.tasks { + if !seen_task_names.insert(name) { + continue; // already seen } - writeln!(writer, " {definition}")?; + + task_descriptions.push(AvailableTaskDescription { + is_root, + is_deno: false, + name: name.to_string(), + task: deno_config::deno_json::TaskDefinition { + command: script.to_string(), + dependencies: vec![], + description: None, + }, + }); } } } + for desc in task_descriptions { + writeln!( + writer, + "- {}{}", + colors::cyan(desc.name), + if desc.is_root { + if desc.is_deno { + format!(" {}", colors::italic_gray("(workspace)")) + } else { + format!(" {}", colors::italic_gray("(workspace package.json)")) + } + } else if desc.is_deno { + "".to_string() + } else { + format!(" {}", colors::italic_gray("(package.json)")) + } + )?; + if let Some(description) = &desc.task.description { + let slash_slash = colors::italic_gray("//"); + writeln!( + writer, + " {slash_slash} {}", + colors::italic_gray(description) + )?; + } + writeln!(writer, " {}", desc.task.command)?; + } + Ok(()) } diff --git a/tests/specs/task/description/__test__.jsonc b/tests/specs/task/description/__test__.jsonc new file mode 100644 index 0000000000..100550de0d --- /dev/null +++ b/tests/specs/task/description/__test__.jsonc @@ -0,0 +1,4 @@ +{ + "args": "task", + "output": "main.out" +} diff --git a/tests/specs/task/description/deno.json b/tests/specs/task/description/deno.json new file mode 100644 index 0000000000..a86b7a5dcb --- /dev/null +++ b/tests/specs/task/description/deno.json @@ -0,0 +1,8 @@ +{ + "tasks": { + "echo_emoji": { + "description": "This is some task", + "command": "echo 1" + } + } +} diff --git a/tests/specs/task/description/main.out b/tests/specs/task/description/main.out new file mode 100644 index 0000000000..ed28506567 --- /dev/null +++ b/tests/specs/task/description/main.out @@ -0,0 +1,4 @@ +Available tasks: +- echo_emoji + // This is some task + echo 1 diff --git a/tests/specs/task/doc_comments_incorrect/__test__.jsonc b/tests/specs/task/doc_comments_incorrect/__test__.jsonc deleted file mode 100644 index 7346290292..0000000000 --- a/tests/specs/task/doc_comments_incorrect/__test__.jsonc +++ /dev/null @@ -1,5 +0,0 @@ -{ - "args": "task doesntexist", - "output": "task.out", - "exitCode": 1 -} diff --git a/tests/specs/task/doc_comments_incorrect/deno.jsonc b/tests/specs/task/doc_comments_incorrect/deno.jsonc deleted file mode 100644 index 6b27f28506..0000000000 --- a/tests/specs/task/doc_comments_incorrect/deno.jsonc +++ /dev/null @@ -1,7 +0,0 @@ -{ - "tasks": { - // some docs - // on what this does - "lint": "deno lint" - } -} diff --git a/tests/specs/task/doc_comments_incorrect/task.out b/tests/specs/task/doc_comments_incorrect/task.out deleted file mode 100644 index 9d81c1768f..0000000000 --- a/tests/specs/task/doc_comments_incorrect/task.out +++ /dev/null @@ -1,6 +0,0 @@ -Task not found: doesntexist -Available tasks: -- lint - // some docs - // on what this does - deno lint diff --git a/tests/specs/task/doc_comments_no_args/__test__.jsonc b/tests/specs/task/doc_comments_no_args/__test__.jsonc deleted file mode 100644 index f3e76cdaa7..0000000000 --- a/tests/specs/task/doc_comments_no_args/__test__.jsonc +++ /dev/null @@ -1,5 +0,0 @@ -{ - "args": "task", - "output": "task.out", - "exitCode": 0 -} diff --git a/tests/specs/task/doc_comments_no_args/deno.jsonc b/tests/specs/task/doc_comments_no_args/deno.jsonc deleted file mode 100644 index 4b6d690c8d..0000000000 --- a/tests/specs/task/doc_comments_no_args/deno.jsonc +++ /dev/null @@ -1,12 +0,0 @@ -{ - "tasks": { - // this task has documentation - // - // in the form of comments - "lint": "deno lint", - /* - * block comments are fine too - */ - "fmt": "deno fmt" - } -} diff --git a/tests/specs/task/doc_comments_no_args/task.out b/tests/specs/task/doc_comments_no_args/task.out deleted file mode 100644 index 635e360907..0000000000 --- a/tests/specs/task/doc_comments_no_args/task.out +++ /dev/null @@ -1,9 +0,0 @@ -Available tasks: -- lint - // this task has documentation - // - // in the form of comments - deno lint -- fmt - // block comments are fine too - deno fmt