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 <dsherret@gmail.com>

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",

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"

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");

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 {

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

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<String, Rc<dyn ShellCommand>>,
+}
 
-        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<String, String>,
+  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<i32, deno_core::anyhow::Error> {
+    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<String, String>,
-  custom_commands: HashMap<String, Rc<dyn ShellCommand>>,
-  npm_resolver: &'a dyn CliNpmResolver,
-  cli_options: &'a CliOptions,
-}
+  async fn run_deno_task(
+    &self,
+    dir_url: &Url,
+    task_name: &String,
+    definition: &TaskDefinition,
+  ) -> Result<i32, deno_core::anyhow::Error> {
+    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<i32, AnyError> {
-  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<String, String>,
+  ) -> Result<i32, deno_core::anyhow::Error> {
+    // 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<i32, AnyError> {
+    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(())
 }

tests/specs/task/description/__test__.jsonc

@@ -0,0 +1,4 @@
+{
+  "args": "task",
+  "output": "main.out"
+}

tests/specs/task/description/deno.json

@@ -0,0 +1,8 @@
+{
+  "tasks": {
+    "echo_emoji": {
+      "description": "This is some task",
+      "command": "echo 1"
+    }
+  }
+}

tests/specs/task/description/main.out

@@ -0,0 +1,4 @@
+Available tasks:
+- echo_emoji
+    // This is some task
+    echo 1

tests/specs/task/doc_comments_incorrect/__test__.jsonc

@@ -1,5 +0,0 @@
-{
-  "args": "task doesntexist",
-  "output": "task.out",
-  "exitCode": 1
-}

tests/specs/task/doc_comments_incorrect/deno.jsonc

@@ -1,7 +0,0 @@
-{
-  "tasks": {
-    // some docs
-    // on what this does
-    "lint": "deno lint"
-  }
-}

tests/specs/task/doc_comments_incorrect/task.out

@@ -1,6 +0,0 @@
-Task not found: doesntexist
-Available tasks:
-- lint
-    // some docs
-    // on what this does
-    deno lint

tests/specs/task/doc_comments_no_args/__test__.jsonc

@@ -1,5 +0,0 @@
-{
-  "args": "task",
-  "output": "task.out",
-  "exitCode": 0
-}

tests/specs/task/doc_comments_no_args/deno.jsonc

@@ -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"
-  }
-}

tests/specs/task/doc_comments_no_args/task.out

@@ -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