BREAKING: remove CLI 'deno script.ts' hack (#5026)

This PR removes the hack in CLI that allows to run scripts with shorthand: deno script.ts.

Removing this functionality because it hacks around short-comings of clap our CLI parser. We agree that this shorthand syntax is desirable, but it needs to be rethinked and reimplemented. For 1.0 we should go with conservative approach that is correct.

3 files changed, 39 insertions, 160 deletions

diff --git a/cli/flags.rs b/cli/flags.rs
index 866b36820..239382e03 100644
--- a/cli/flags.rs
+++ b/cli/flags.rs
@@ -6,7 +6,6 @@ use clap::Arg;
 use clap::ArgMatches;
 use clap::SubCommand;
 use log::Level;
-use std::collections::HashSet;
 use std::net::SocketAddr;
 use std::path::{Path, PathBuf};
 
@@ -14,14 +13,6 @@ use std::path::{Path, PathBuf};
 macro_rules! svec {
     ($($x:expr),*) => (vec![$($x.to_string()),*]);
 }
-/// Creates HashSet<String> from string literals
-macro_rules! sset {
-  ($($x:expr),*) => {{
-    let _v = svec![$($x.to_string()),*];
-    let hash_set: HashSet<String> = _v.iter().cloned().collect();
-    hash_set
-  }}
-}
 
 #[derive(Clone, Debug, PartialEq)]
 pub enum DenoSubcommand {
@@ -196,17 +187,15 @@ Docs: https://deno.land/std/manual.md
 Modules: https://deno.land/std/ https://deno.land/x/
 Bugs: https://github.com/denoland/deno/issues
 
-To start the REPL, supply no arguments:
+To start the REPL:
   deno
 
 To execute a script:
   deno run https://deno.land/std/examples/welcome.ts
-  deno https://deno.land/std/examples/welcome.ts
 
 To evaluate code in the shell:
   deno eval \"console.log(30933 + 404)\"
-
-Run 'deno help run' for 'run'-specific flags.";
+";
 
 lazy_static! {
   static ref LONG_VERSION: String = format!(
@@ -228,7 +217,6 @@ pub fn flags_from_vec(args: Vec<String>) -> Flags {
 
 /// Same as flags_from_vec but does not exit on error.
 pub fn flags_from_vec_safe(args: Vec<String>) -> clap::Result<Flags> {
-  let args = arg_hacks(args);
   let app = clap_root();
   let matches = app.get_matches_from_safe(args)?;
 
@@ -272,7 +260,7 @@ pub fn flags_from_vec_safe(args: Vec<String>) -> clap::Result<Flags> {
   } else if let Some(m) = matches.subcommand_matches("doc") {
     doc_parse(&mut flags, m);
   } else {
-    unimplemented!();
+    repl_parse(&mut flags, &matches);
   }
 
   Ok(flags)
@@ -1342,80 +1330,12 @@ fn resolve_hosts(paths: Vec<String>) -> Vec<String> {
   out
 }
 
-fn arg_hacks(mut args: Vec<String>) -> Vec<String> {
-  // Hack #1 We want to default the subcommand to "run"
-  // Clap does not let us have a default sub-command. But we want to allow users
-  // to do "deno script.js" instead of "deno run script.js".
-  // This function insert the "run" into the second position of the args.
-  assert!(!args.is_empty());
-  // Rational:
-  // deno -> deno repl
-  if args.len() == 1 {
-    args.insert(1, "repl".to_string());
-    return args;
-  }
-  let subcommands = sset![
-    "bundle",
-    "completions",
-    "doc",
-    "eval",
-    "cache",
-    "fmt",
-    "test",
-    "info",
-    "repl",
-    "run",
-    "types",
-    "install",
-    "help",
-    "version",
-    "upgrade"
-  ];
-  let modifier_flags = sset!["-h", "--help", "-V", "--version"];
-  // deno [subcommand|behavior modifier flags] -> do nothing
-  if subcommands.contains(&args[1]) || modifier_flags.contains(&args[1]) {
-    return args;
-  }
-  // This is not perfect either, since originally we should also
-  // support e.g. `-L debug` which `debug` would be treated as main module.
-  // Instead `-L=debug` must be used
-  let mut has_main_module = false;
-  for arg in args.iter().skip(1) {
-    if !arg.starts_with('-') {
-      has_main_module = true;
-      break;
-    }
-  }
-  if has_main_module {
-    // deno ...-[flags] NAME ... -> deno run ...-[flags] NAME ...
-    args.insert(1, "run".to_string());
-  } else {
-    // deno ...-[flags] -> deno repl ...-[flags]
-    args.insert(1, "repl".to_string());
-  }
-  args
-}
-
 #[cfg(test)]
 mod tests {
   use super::*;
   use std::env::current_dir;
 
   #[test]
-  fn arg_hacks_test() {
-    let args0 = arg_hacks(svec!["deno", "--version"]);
-    assert_eq!(args0, ["deno", "--version"]);
-    let args1 = arg_hacks(svec!["deno"]);
-    assert_eq!(args1, ["deno", "repl"]);
-    let args2 = arg_hacks(svec!["deno", "-L=debug", "-h"]);
-    assert_eq!(args2, ["deno", "repl", "-L=debug", "-h"]);
-    let args3 = arg_hacks(svec!["deno", "script.js"]);
-    assert_eq!(args3, ["deno", "run", "script.js"]);
-    let args4 = arg_hacks(svec!["deno", "-A", "script.js", "-L=info"]);
-    assert_eq!(args4, ["deno", "run", "-A", "script.js", "-L=info"]);
-  }
-
-  #[test]
   fn upgrade() {
     let r =
       flags_from_vec_safe(svec!["deno", "upgrade", "--dry-run", "--force"]);
@@ -1968,41 +1888,6 @@ mod tests {
   }
 
   #[test]
-  fn default_to_run() {
-    let r = flags_from_vec_safe(svec!["deno", "script.ts"]);
-    assert_eq!(
-      r.unwrap(),
-      Flags {
-        subcommand: DenoSubcommand::Run {
-          script: "script.ts".to_string(),
-        },
-        ..Flags::default()
-      }
-    );
-  }
-
-  #[test]
-  fn default_to_run_with_permissions() {
-    let r = flags_from_vec_safe(svec![
-      "deno",
-      "--allow-net",
-      "--allow-read",
-      "script.ts"
-    ]);
-    assert_eq!(
-      r.unwrap(),
-      Flags {
-        subcommand: DenoSubcommand::Run {
-          script: "script.ts".to_string(),
-        },
-        allow_net: true,
-        allow_read: true,
-        ..Flags::default()
-      }
-    );
-  }
-
-  #[test]
   fn bundle() {
     let r = flags_from_vec_safe(svec!["deno", "bundle", "source.ts"]);
     assert_eq!(
@@ -2072,25 +1957,6 @@ mod tests {
   }
 
   #[test]
-  fn default_to_run_importmap() {
-    let r = flags_from_vec_safe(svec![
-      "deno",
-      "--importmap=importmap.json",
-      "script.ts"
-    ]);
-    assert_eq!(
-      r.unwrap(),
-      Flags {
-        subcommand: DenoSubcommand::Run {
-          script: "script.ts".to_string(),
-        },
-        import_map_path: Some("importmap.json".to_owned()),
-        ..Flags::default()
-      }
-    );
-  }
-
-  #[test]
   fn cache_importmap() {
     let r = flags_from_vec_safe(svec![
       "deno",
@@ -2250,8 +2116,12 @@ mod tests {
 
   #[test]
   fn log_level() {
-    let r =
-      flags_from_vec_safe(svec!["deno", "--log-level=debug", "script.ts"]);
+    let r = flags_from_vec_safe(svec![
+      "deno",
+      "run",
+      "--log-level=debug",
+      "script.ts"
+    ]);
     assert_eq!(
       r.unwrap(),
       Flags {
@@ -2266,7 +2136,7 @@ mod tests {
 
   #[test]
   fn quiet() {
-    let r = flags_from_vec_safe(svec!["deno", "-q", "script.ts"]);
+    let r = flags_from_vec_safe(svec!["deno", "run", "-q", "script.ts"]);
     assert_eq!(
       r.unwrap(),
       Flags {
@@ -2350,7 +2220,8 @@ mod tests {
 
   #[test]
   fn no_remote() {
-    let r = flags_from_vec_safe(svec!["deno", "--no-remote", "script.ts"]);
+    let r =
+      flags_from_vec_safe(svec!["deno", "run", "--no-remote", "script.ts"]);
     assert_eq!(
       r.unwrap(),
       Flags {
@@ -2365,7 +2236,8 @@ mod tests {
 
   #[test]
   fn cached_only() {
-    let r = flags_from_vec_safe(svec!["deno", "--cached-only", "script.ts"]);
+    let r =
+      flags_from_vec_safe(svec!["deno", "run", "--cached-only", "script.ts"]);
     assert_eq!(
       r.unwrap(),
       Flags {
@@ -2382,6 +2254,7 @@ mod tests {
   fn allow_net_whitelist_with_ports() {
     let r = flags_from_vec_safe(svec![
       "deno",
+      "run",
       "--allow-net=deno.land,:8000,:4545",
       "script.ts"
     ]);
@@ -2409,6 +2282,7 @@ mod tests {
   fn lock_write() {
     let r = flags_from_vec_safe(svec![
       "deno",
+      "run",
       "--lock-write",
       "--lock=lock.json",
       "script.ts"
diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs
index 7827d386a..1d02226b4 100644
--- a/cli/tests/integration_tests.rs
+++ b/cli/tests/integration_tests.rs
@@ -1322,7 +1322,7 @@ itest!(error_014_catch_dynamic_import_error {
 });
 
 itest!(error_015_dynamic_import_permissions {
-  args: "--reload error_015_dynamic_import_permissions.js",
+  args: "run --reload error_015_dynamic_import_permissions.js",
   output: "error_015_dynamic_import_permissions.out",
   check_stderr: true,
   exit_code: 1,
@@ -1331,7 +1331,7 @@ itest!(error_015_dynamic_import_permissions {
 
 // We have an allow-net flag but not allow-read, it should still result in error.
 itest!(error_016_dynamic_import_permissions2 {
-  args: "--reload --allow-net error_016_dynamic_import_permissions2.js",
+  args: "run --reload --allow-net error_016_dynamic_import_permissions2.js",
   output: "error_016_dynamic_import_permissions2.out",
   check_stderr: true,
   exit_code: 1,
@@ -1339,63 +1339,63 @@ itest!(error_016_dynamic_import_permissions2 {
 });
 
 itest!(error_017_hide_long_source_ts {
-  args: "--reload error_017_hide_long_source_ts.ts",
+  args: "run --reload error_017_hide_long_source_ts.ts",
   output: "error_017_hide_long_source_ts.ts.out",
   check_stderr: true,
   exit_code: 1,
 });
 
 itest!(error_018_hide_long_source_js {
-  args: "error_018_hide_long_source_js.js",
+  args: "run error_018_hide_long_source_js.js",
   output: "error_018_hide_long_source_js.js.out",
   check_stderr: true,
   exit_code: 1,
 });
 
 itest!(error_019_stack_function {
-  args: "error_019_stack_function.ts",
+  args: "run error_019_stack_function.ts",
   output: "error_019_stack_function.ts.out",
   check_stderr: true,
   exit_code: 1,
 });
 
 itest!(error_020_stack_constructor {
-  args: "error_020_stack_constructor.ts",
+  args: "run error_020_stack_constructor.ts",
   output: "error_020_stack_constructor.ts.out",
   check_stderr: true,
   exit_code: 1,
 });
 
 itest!(error_021_stack_method {
-  args: "error_021_stack_method.ts",
+  args: "run error_021_stack_method.ts",
   output: "error_021_stack_method.ts.out",
   check_stderr: true,
   exit_code: 1,
 });
 
 itest!(error_022_stack_custom_error {
-  args: "error_022_stack_custom_error.ts",
+  args: "run error_022_stack_custom_error.ts",
   output: "error_022_stack_custom_error.ts.out",
   check_stderr: true,
   exit_code: 1,
 });
 
 itest!(error_023_stack_async {
-  args: "error_023_stack_async.ts",
+  args: "run error_023_stack_async.ts",
   output: "error_023_stack_async.ts.out",
   check_stderr: true,
   exit_code: 1,
 });
 
 itest!(error_024_stack_promise_all {
-  args: "error_024_stack_promise_all.ts",
+  args: "run error_024_stack_promise_all.ts",
   output: "error_024_stack_promise_all.ts.out",
   check_stderr: true,
   exit_code: 1,
 });
 
 itest!(error_025_tab_indent {
-  args: "error_025_tab_indent",
+  args: "run error_025_tab_indent",
   output: "error_025_tab_indent.out",
   check_stderr: true,
   exit_code: 1,
@@ -1534,7 +1534,7 @@ itest!(run_v8_flags {
 });
 
 itest!(run_v8_help {
-  args: "--v8-flags=--help",
+  args: "repl --v8-flags=--help",
   output: "v8_help.out",
 });
 
@@ -1544,27 +1544,27 @@ itest!(wasm {
 });
 
 itest!(wasm_async {
-  args: "wasm_async.js",
+  args: "run wasm_async.js",
   output: "wasm_async.out",
 });
 
 itest!(top_level_await {
-  args: "--allow-read top_level_await.js",
+  args: "run --allow-read top_level_await.js",
   output: "top_level_await.out",
 });
 
 itest!(top_level_await_ts {
-  args: "--allow-read top_level_await.ts",
+  args: "run --allow-read top_level_await.ts",
   output: "top_level_await.out",
 });
 
 itest!(top_level_for_await {
-  args: "top_level_for_await.js",
+  args: "run top_level_for_await.js",
   output: "top_level_for_await.out",
 });
 
 itest!(top_level_for_await_ts {
-  args: "top_level_for_await.ts",
+  args: "run top_level_for_await.ts",
   output: "top_level_for_await.out",
 });
 
diff --git a/cli/tests/lock_write_fetch.ts b/cli/tests/lock_write_fetch.ts
index 2286d7a0c..9e7190596 100644
--- a/cli/tests/lock_write_fetch.ts
+++ b/cli/tests/lock_write_fetch.ts
@@ -35,7 +35,12 @@ console.log(`fetch check code: ${fetchCheckProcCode}`);
 const runProc = Deno.run({
   stdout: "null",
   stderr: "null",
-  cmd: [Deno.execPath(), "--lock=lock_write_fetch.json", "https_import.ts"],
+  cmd: [
+    Deno.execPath(),
+    "run",
+    "--lock=lock_write_fetch.json",
+    "https_import.ts",
+  ],
 });
 
 const runCode = (await runProc.status()).code;