Exit cleanly on unrecognized arguments (#990)

Refactor set_flags to return a Result

src/flags.rs

@@ -68,7 +68,9 @@ DENO_DIR           Set deno's base directory."
 }
 
 // Parses flags for deno. This does not do v8_set_flags() - call that separately.
-pub fn set_flags(args: Vec<String>) -> (DenoFlags, Vec<String>) {
+pub fn set_flags(
+  args: Vec<String>,
+) -> Result<(DenoFlags, Vec<String>), String> {
   let args = v8_set_flags(args);
 
   let mut flags = DenoFlags::default();
@@ -89,7 +91,7 @@ pub fn set_flags(args: Vec<String>) -> (DenoFlags, Vec<String>) {
         "--deps" => flags.deps_flag = true,
         "--types" => flags.types_flag = true,
         "--" => break,
-        _ => unimplemented!(),
+        other => return Err(format!("bad option {}", other)),
       }
     } else if a.len() > 1 && &a[0..1] == "-" {
       let mut iter = a.chars().skip(1); // skip the "-"
@@ -99,7 +101,7 @@ pub fn set_flags(args: Vec<String>) -> (DenoFlags, Vec<String>) {
           'D' => flags.log_debug = true,
           'v' => flags.version = true,
           'r' => flags.reload = true,
-          _ => unimplemented!(),
+          other => return Err(format!("bad option -{}", other)),
         }
       }
     } else {
@@ -109,12 +111,12 @@ pub fn set_flags(args: Vec<String>) -> (DenoFlags, Vec<String>) {
 
   // add any remaining arguments to `rest`
   rest.extend(arg_iter.map(|s| s.clone()));
-  return (flags, rest);
+  return Ok((flags, rest));
 }
 
 #[test]
 fn test_set_flags_1() {
-  let (flags, rest) = set_flags(svec!["deno", "--version"]);
+  let (flags, rest) = set_flags(svec!["deno", "--version"]).unwrap();
   assert_eq!(rest, svec!["deno"]);
   assert_eq!(
     flags,
@@ -127,7 +129,8 @@ fn test_set_flags_1() {
 
 #[test]
 fn test_set_flags_2() {
-  let (flags, rest) = set_flags(svec!["deno", "-r", "-D", "script.ts"]);
+  let (flags, rest) =
+    set_flags(svec!["deno", "-r", "-D", "script.ts"]).unwrap();
   assert_eq!(rest, svec!["deno", "script.ts"]);
   assert_eq!(
     flags,
@@ -142,7 +145,8 @@ fn test_set_flags_2() {
 #[test]
 fn test_set_flags_3() {
   let (flags, rest) =
-    set_flags(svec!["deno", "-r", "--deps", "script.ts", "--allow-write"]);
+    set_flags(svec!["deno", "-r", "--deps", "script.ts", "--allow-write"])
+      .unwrap();
   assert_eq!(rest, svec!["deno", "script.ts"]);
   assert_eq!(
     flags,
@@ -158,7 +162,7 @@ fn test_set_flags_3() {
 #[test]
 fn test_set_flags_4() {
   let (flags, rest) =
-    set_flags(svec!["deno", "-Dr", "script.ts", "--allow-write"]);
+    set_flags(svec!["deno", "-Dr", "script.ts", "--allow-write"]).unwrap();
   assert_eq!(rest, svec!["deno", "script.ts"]);
   assert_eq!(
     flags,
@@ -173,7 +177,7 @@ fn test_set_flags_4() {
 
 #[test]
 fn test_set_flags_5() {
-  let (flags, rest) = set_flags(svec!["deno", "--types"]);
+  let (flags, rest) = set_flags(svec!["deno", "--types"]).unwrap();
   assert_eq!(rest, svec!["deno"]);
   assert_eq!(
     flags,
@@ -184,6 +188,19 @@ fn test_set_flags_5() {
   )
 }
 
+#[test]
+fn test_set_bad_flags_1() {
+  let err = set_flags(svec!["deno", "--unknown-flag"]).unwrap_err();
+  assert_eq!(err, "bad option --unknown-flag");
+}
+
+#[test]
+fn test_set_bad_flags_2() {
+  // This needs to be changed if -z is added as a flag
+  let err = set_flags(svec!["deno", "-z"]).unwrap_err();
+  assert_eq!(err, "bad option -z");
+}
+
 // Returns args passed to V8, followed by args passed to JS
 fn v8_set_flags_preprocess(args: Vec<String>) -> (Vec<String>, Vec<String>) {
   let mut rest = vec![];

src/isolate.rs

@@ -100,12 +100,15 @@ pub struct Metrics {
 static DENO_INIT: std::sync::Once = std::sync::ONCE_INIT;
 
 impl Isolate {
-  pub fn new(argv: Vec<String>, dispatch: Dispatch) -> Isolate {
+  pub fn new(
+    flags: flags::DenoFlags,
+    argv_rest: Vec<String>,
+    dispatch: Dispatch,
+  ) -> Isolate {
     DENO_INIT.call_once(|| {
       unsafe { libdeno::deno_init() };
     });
 
-    let (flags, argv_rest) = flags::set_flags(argv);
     let libdeno_isolate = unsafe { libdeno::deno_new(pre_dispatch) };
     // This channel handles sending async messages back to the runtime.
     let (tx, rx) = mpsc::channel::<(i32, Buf)>();
@@ -353,7 +356,8 @@ mod tests {
   #[test]
   fn test_dispatch_sync() {
     let argv = vec![String::from("./deno"), String::from("hello.js")];
-    let mut isolate = Isolate::new(argv, dispatch_sync);
+    let (flags, rest_argv) = flags::set_flags(argv).unwrap();
+    let mut isolate = Isolate::new(flags, rest_argv, dispatch_sync);
     tokio_util::init(|| {
       isolate
         .execute(
@@ -393,7 +397,8 @@ mod tests {
   #[test]
   fn test_metrics_sync() {
     let argv = vec![String::from("./deno"), String::from("hello.js")];
-    let mut isolate = Isolate::new(argv, metrics_dispatch_sync);
+    let (flags, rest_argv) = flags::set_flags(argv).unwrap();
+    let mut isolate = Isolate::new(flags, rest_argv, metrics_dispatch_sync);
     tokio_util::init(|| {
       // Verify that metrics have been properly initialized.
       {
@@ -428,7 +433,8 @@ mod tests {
   #[test]
   fn test_metrics_async() {
     let argv = vec![String::from("./deno"), String::from("hello.js")];
-    let mut isolate = Isolate::new(argv, metrics_dispatch_async);
+    let (flags, rest_argv) = flags::set_flags(argv).unwrap();
+    let mut isolate = Isolate::new(flags, rest_argv, metrics_dispatch_async);
     tokio_util::init(|| {
       // Verify that metrics have been properly initialized.
       {

src/main.rs

@@ -59,16 +59,20 @@ fn main() {
   // Therefore this hack.
   std::panic::set_hook(Box::new(|panic_info| {
     if let Some(location) = panic_info.location() {
-      println!("PANIC file '{}' line {}", location.file(), location.line());
+      eprintln!("PANIC file '{}' line {}", location.file(), location.line());
     } else {
-      println!("PANIC occurred but can't get location information...");
+      eprintln!("PANIC occurred but can't get location information...");
     }
     std::process::abort();
   }));
 
   log::set_logger(&LOGGER).unwrap();
   let args = env::args().collect();
-  let mut isolate = isolate::Isolate::new(args, ops::dispatch);
+  let (flags, rest_argv) = flags::set_flags(args).unwrap_or_else(|err| {
+    eprintln!("{}", err);
+    std::process::exit(1)
+  });
+  let mut isolate = isolate::Isolate::new(flags, rest_argv, ops::dispatch);
   flags::process(&isolate.state.flags);
   tokio_util::init(|| {
     isolate