fix: --frozen flag missing from several subcommands (#29508)

Closes https://github.com/denoland/deno/issues/29498

cli/args/flags.rs

@@ -1748,6 +1748,7 @@ Or multiple dependencies at once:
       )
       .arg(add_dev_arg())
       .arg(allow_scripts_arg())
+      .args(lock_args())
       .args(default_registry_args())
   })
 }
@@ -1781,13 +1782,15 @@ You can remove multiple dependencies at once:
     UnstableArgsConfig::None,
   )
   .defer(|cmd| {
-    cmd.arg(
+    cmd
-      Arg::new("packages")
+      .arg(
-        .help("List of packages to remove")
+        Arg::new("packages")
-        .required_unless_present("help")
+          .help("List of packages to remove")
-        .num_args(1..)
+          .required_unless_present("help")
-        .action(ArgAction::Append),
+          .num_args(1..)
-    )
+          .action(ArgAction::Append),
+      )
+      .args(lock_args())
   })
 }
 
@@ -1882,7 +1885,6 @@ Future runs of this module will trigger no downloads or compilation unless --rel
           .required_unless_present("help")
           .value_hint(ValueHint::FilePath),
       )
-      .arg(frozen_lockfile_arg())
       .arg(allow_scripts_arg())
       .arg(allow_import_arg())
       .arg(env_file_arg())
@@ -1970,7 +1972,6 @@ Unless --reload is specified, this command will not re-download already cached d
             .num_args(1..)
             .value_hint(ValueHint::FilePath),
         )
-        .arg(frozen_lockfile_arg())
         .arg(allow_import_arg())
       }
     )
@@ -2213,8 +2214,7 @@ Show documentation for runtime built-ins:
       cmd
         .arg(import_map_arg())
         .arg(reload_arg())
-        .arg(lock_arg())
+        .args(lock_args())
-        .arg(no_lock_arg())
         .arg(no_npm_arg())
         .arg(no_remote_arg())
         .arg(allow_import_arg())
@@ -2594,8 +2594,7 @@ The following information is shown:
       .arg(no_config_arg())
       .arg(no_remote_arg())
       .arg(no_npm_arg())
-      .arg(lock_arg())
+      .args(lock_args())
-      .arg(no_lock_arg())
       .arg(config_arg())
       .arg(import_map_arg())
       .arg(node_modules_dir_arg())
@@ -2798,8 +2797,7 @@ Specific version requirements to update to can be specified:
                       "If a version requirement is specified, the matching packages will be updated to the given requirement."),
           )
       )
-      .arg(no_lock_arg())
+      .args(lock_args())
-      .arg(lock_arg())
       .arg(
         Arg::new("latest")
           .long("latest")
@@ -2880,6 +2878,7 @@ The installation root is determined, in order of precedence:
           .num_args(1..)
           .action(ArgAction::Append)
       )
+      .args(lock_args())
   })
 }
 
@@ -3175,7 +3174,7 @@ Evaluate a task from string:
       .allow_external_subcommands(true)
       .subcommand_value_name("TASK")
       .arg(config_arg())
-      .arg(frozen_lockfile_arg())
+      .args(lock_args())
       .arg(
         Arg::new("cwd")
           .long("cwd")
@@ -3549,8 +3548,7 @@ fn compile_args_without_check_args(app: Command) -> Command {
     .arg(config_arg())
     .arg(no_config_arg())
     .arg(reload_arg())
-    .arg(lock_arg())
+    .args(lock_args())
-    .arg(no_lock_arg())
     .arg(ca_file_arg())
     .arg(unsafely_ignore_certificate_errors_arg())
 }
@@ -3964,7 +3962,6 @@ fn runtime_args(
 
 fn runtime_misc_args(app: Command) -> Command {
   app
-    .arg(frozen_lockfile_arg())
     .arg(cached_only_arg())
     .arg(location_arg())
     .arg(v8_flags_arg())
@@ -4102,19 +4099,6 @@ fn cached_only_arg() -> Arg {
     .help_heading(DEPENDENCY_MANAGEMENT_HEADING)
 }
 
-fn frozen_lockfile_arg() -> Arg {
-  Arg::new("frozen")
-    .long("frozen")
-    .alias("frozen-lockfile")
-    .value_parser(value_parser!(bool))
-    .value_name("BOOLEAN")
-    .num_args(0..=1)
-    .require_equals(true)
-    .default_missing_value("true")
-    .help("Error out if lockfile is out of date")
-    .help_heading(DEPENDENCY_MANAGEMENT_HEADING)
-}
-
 /// Used for subcommands that operate on executable scripts only.
 /// `deno fmt` has its own `--ext` arg because its possible values differ.
 /// If --ext is not provided and the script doesn't have a file extension,
@@ -4329,25 +4313,34 @@ fn script_arg() -> Arg {
     .value_hint(ValueHint::FilePath)
 }
 
-fn lock_arg() -> Arg {
+fn lock_args() -> [Arg; 3] {
-  Arg::new("lock")
+  [
-    .long("lock")
+    Arg::new("lock")
-    .value_name("FILE")
+      .long("lock")
-    .default_missing_value("./deno.lock")
+      .value_name("FILE")
-    .help("Check the specified lock file. (If value is not provided, defaults to \"./deno.lock\")")
+      .default_missing_value("./deno.lock")
-    .num_args(0..=1)
+      .help("Check the specified lock file. (If value is not provided, defaults to \"./deno.lock\")")
-    .value_parser(value_parser!(String))
+      .num_args(0..=1)
-    .value_hint(ValueHint::FilePath)
+      .value_parser(value_parser!(String))
-    .help_heading(DEPENDENCY_MANAGEMENT_HEADING)
+      .value_hint(ValueHint::FilePath)
-}
+      .help_heading(DEPENDENCY_MANAGEMENT_HEADING),
-
+    Arg::new("no-lock")
-fn no_lock_arg() -> Arg {
+      .long("no-lock")
-  Arg::new("no-lock")
+      .action(ArgAction::SetTrue)
-    .long("no-lock")
+      .help("Disable auto discovery of the lock file")
-    .action(ArgAction::SetTrue)
+      .conflicts_with("lock")
-    .help("Disable auto discovery of the lock file")
+      .help_heading(DEPENDENCY_MANAGEMENT_HEADING),
-    .conflicts_with("lock")
+    Arg::new("frozen")
-    .help_heading(DEPENDENCY_MANAGEMENT_HEADING)
+      .long("frozen")
+      .alias("frozen-lockfile")
+      .value_parser(value_parser!(bool))
+      .value_name("BOOLEAN")
+      .num_args(0..=1)
+      .require_equals(true)
+      .default_missing_value("true")
+      .help("Error out if lockfile is out of date")
+      .help_heading(DEPENDENCY_MANAGEMENT_HEADING)
+  ]
 }
 
 fn config_arg() -> Arg {
@@ -4544,6 +4537,7 @@ fn add_parse(
   matches: &mut ArgMatches,
 ) -> clap::error::Result<()> {
   allow_scripts_arg_parse(flags, matches)?;
+  lock_args_parse(flags, matches);
   flags.subcommand = DenoSubcommand::Add(add_parse_inner(matches, None));
   Ok(())
 }
@@ -4571,6 +4565,7 @@ fn add_parse_inner(
 }
 
 fn remove_parse(flags: &mut Flags, matches: &mut ArgMatches) {
+  lock_args_parse(flags, matches);
   flags.subcommand = DenoSubcommand::Remove(RemoveFlags {
     packages: matches.remove_many::<String>("packages").unwrap().collect(),
   });
@@ -4664,7 +4659,6 @@ fn cache_parse(
 ) -> clap::error::Result<()> {
   compile_args_parse(flags, matches)?;
   unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionOnly);
-  frozen_lockfile_arg_parse(flags, matches);
   allow_scripts_arg_parse(flags, matches)?;
   allow_import_parse(flags, matches)?;
   env_file_arg_parse(flags, matches);
@@ -4680,7 +4674,6 @@ fn check_parse(
   flags.type_check_mode = TypeCheckMode::Local;
   compile_args_without_check_parse(flags, matches)?;
   unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionAndRuntime);
-  frozen_lockfile_arg_parse(flags, matches);
   let files = match matches.remove_many::<String>("file") {
     Some(f) => f.collect(),
     None => vec![".".to_string()], // default
@@ -4838,8 +4831,7 @@ fn doc_parse(
   unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionOnly);
   import_map_arg_parse(flags, matches);
   reload_arg_parse(flags, matches)?;
-  lock_arg_parse(flags, matches);
+  lock_args_parse(flags, matches);
-  no_lock_arg_parse(flags, matches);
   no_npm_arg_parse(flags, matches);
   no_remote_arg_parse(flags, matches);
   allow_import_parse(flags, matches)?;
@@ -5188,6 +5180,7 @@ fn jupyter_parse(flags: &mut Flags, matches: &mut ArgMatches) {
 }
 
 fn uninstall_parse(flags: &mut Flags, matches: &mut ArgMatches) {
+  lock_args_parse(flags, matches);
   let name = matches.remove_one::<String>("name-or-package").unwrap();
 
   let kind = if matches.get_flag("global") {
@@ -5273,7 +5266,6 @@ fn repl_parse(
   unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionAndRuntime);
   compile_args_without_check_parse(flags, matches)?;
   cached_only_arg_parse(flags, matches);
-  frozen_lockfile_arg_parse(flags, matches);
   permission_args_parse(flags, matches)?;
   inspect_arg_parse(flags, matches);
   location_arg_parse(flags, matches);
@@ -5414,7 +5406,7 @@ fn task_parse(
 
   unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionAndRuntime);
   node_modules_arg_parse(flags, matches);
-  frozen_lockfile_arg_parse(flags, matches);
+  lock_args_parse(flags, matches);
 
   let mut recursive = matches.get_flag("recursive");
   let filter = if let Some(filter) = matches.remove_one::<String>("filter") {
@@ -5831,7 +5823,6 @@ fn runtime_args_parse(
   unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionAndRuntime);
   compile_args_parse(flags, matches)?;
   cached_only_arg_parse(flags, matches);
-  frozen_lockfile_arg_parse(flags, matches);
   if include_perms {
     permission_args_parse(flags, matches)?;
   }
@@ -5919,12 +5910,6 @@ fn cached_only_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
   }
 }
 
-fn frozen_lockfile_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
-  if let Some(&v) = matches.get_one::<bool>("frozen") {
-    flags.frozen_lockfile = Some(v);
-  }
-}
-
 fn ext_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
   flags.ext = matches.remove_one::<String>("ext");
 }
@@ -5976,21 +5961,16 @@ fn check_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
 }
 
 fn lock_args_parse(flags: &mut Flags, matches: &mut ArgMatches) {
-  lock_arg_parse(flags, matches);
-  no_lock_arg_parse(flags, matches);
-}
-
-fn lock_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
   if matches.contains_id("lock") {
     let lockfile = matches.remove_one::<String>("lock").unwrap();
     flags.lock = Some(lockfile);
   }
-}
-
-fn no_lock_arg_parse(flags: &mut Flags, matches: &ArgMatches) {
   if matches.get_flag("no-lock") {
     flags.no_lock = true;
   }
+  if let Some(&v) = matches.get_one::<bool>("frozen") {
+    flags.frozen_lockfile = Some(v);
+  }
 }
 
 fn config_args_parse(flags: &mut Flags, matches: &mut ArgMatches) {
@@ -8875,7 +8855,7 @@ mod tests {
     let r = flags_from_vec(svec!["deno", "uninstall"]);
     assert!(r.is_err(),);
 
-    let r = flags_from_vec(svec!["deno", "uninstall", "@std/load"]);
+    let r = flags_from_vec(svec!["deno", "uninstall", "--frozen", "@std/load"]);
     assert_eq!(
       r.unwrap(),
       Flags {
@@ -8884,6 +8864,7 @@ mod tests {
             packages: vec!["@std/load".to_string()],
           }),
         }),
+        frozen_lockfile: Some(true),
         ..Flags::default()
       }
     );
@@ -11520,55 +11501,66 @@ mod tests {
         }
       };
 
-      let r = flags_from_vec(svec!["deno", cmd, "@david/which"]);
+      {
-      assert_eq!(
+        let r = flags_from_vec(svec!["deno", cmd, "@david/which"]);
-        r.unwrap(),
+        assert_eq!(
-        mk_flags(AddFlags {
+          r.unwrap(),
-          packages: svec!["@david/which"],
+          mk_flags(AddFlags {
-          dev: false,
+            packages: svec!["@david/which"],
-          default_registry: None,
+            dev: false, // default is false
-        }) // default is false
+            default_registry: None,
-      );
+          })
-
+        );
-      let r = flags_from_vec(svec!["deno", cmd, "@david/which", "@luca/hello"]);
+      }
-      assert_eq!(
+      {
-        r.unwrap(),
+        let r = flags_from_vec(svec![
-        mk_flags(AddFlags {
+          "deno",
+          cmd,
+          "--frozen",
+          "@david/which",
+          "@luca/hello"
+        ]);
+        let mut expected_flags = mk_flags(AddFlags {
           packages: svec!["@david/which", "@luca/hello"],
           dev: false,
           default_registry: None,
-        })
+        });
-      );
+        expected_flags.frozen_lockfile = Some(true);
-
+        assert_eq!(r.unwrap(), expected_flags);
-      let r = flags_from_vec(svec!["deno", cmd, "--dev", "npm:chalk"]);
+      }
-      assert_eq!(
+      {
-        r.unwrap(),
+        let r = flags_from_vec(svec!["deno", cmd, "--dev", "npm:chalk"]);
-        mk_flags(AddFlags {
+        assert_eq!(
-          packages: svec!["npm:chalk"],
+          r.unwrap(),
-          dev: true,
+          mk_flags(AddFlags {
-          default_registry: None,
+            packages: svec!["npm:chalk"],
-        }),
+            dev: true,
-      );
+            default_registry: None,
-
+          }),
-      let r = flags_from_vec(svec!["deno", cmd, "--npm", "chalk"]);
+        );
-      assert_eq!(
+      }
-        r.unwrap(),
+      {
-        mk_flags(AddFlags {
+        let r = flags_from_vec(svec!["deno", cmd, "--npm", "chalk"]);
-          packages: svec!["chalk"],
+        assert_eq!(
-          dev: false,
+          r.unwrap(),
-          default_registry: Some(DefaultRegistry::Npm),
+          mk_flags(AddFlags {
-        }),
+            packages: svec!["chalk"],
-      );
+            dev: false,
-
+            default_registry: Some(DefaultRegistry::Npm),
-      let r = flags_from_vec(svec!["deno", cmd, "--jsr", "@std/fs"]);
+          }),
-      assert_eq!(
+        );
-        r.unwrap(),
+      }
-        mk_flags(AddFlags {
+      {
-          packages: svec!["@std/fs"],
+        let r = flags_from_vec(svec!["deno", cmd, "--jsr", "@std/fs"]);
-          dev: false,
+        assert_eq!(
-          default_registry: Some(DefaultRegistry::Jsr),
+          r.unwrap(),
-        })
+          mk_flags(AddFlags {
-      )
+            packages: svec!["@std/fs"],
+            dev: false,
+            default_registry: Some(DefaultRegistry::Jsr),
+          }),
+        );
+      }
     }
   }
 
@@ -11588,14 +11580,20 @@ mod tests {
       }
     );
 
-    let r =
+    let r = flags_from_vec(svec![
-      flags_from_vec(svec!["deno", "remove", "@david/which", "@luca/hello"]);
+      "deno",
+      "remove",
+      "--frozen",
+      "@david/which",
+      "@luca/hello"
+    ]);
     assert_eq!(
       r.unwrap(),
       Flags {
         subcommand: DenoSubcommand::Remove(RemoveFlags {
           packages: svec!["@david/which", "@luca/hello"],
         }),
+        frozen_lockfile: Some(true),
         ..Flags::default()
       }
     );