Repository: clap-rs/clap (language: rust)

Issue:
try_update_from returns an Error MissingRequiredArgument for argument with existing values
### Please complete the following tasks

- [X] I have searched the [discussions](https://github.com/clap-rs/clap/discussions)
- [X] I have searched the [open](https://github.com/clap-rs/clap/issues) and [rejected](https://github.com/clap-rs/clap/issues?q=is%3Aissue+label%3AS-wont-fix+is%3Aclosed) issues

### Rust Version

1.66.0

### Clap Version

4.0.32

### Minimal reproducible code

```rust
use clap::Parser;

#[derive(Parser, Debug, Clone)]
#[clap(name = "tool", version = "0.0.1", about="short", long_about="long", term_width=0)]

pub struct Opts {
  #[clap(long, short='f', required=true)]
  pub foo: usize,
  #[clap(long, short='b', required=true)]
  pub bar: usize,
}

pub fn main() {
  let mut opts = Opts {
      foo: 0,
      bar: 1,
  };
  println!("initial opts: {:?}", opts);
  
  let argv = vec!["tool", "--bar", "2"];
  
  match opts.try_update_from(argv.into_iter()) {
      Ok(()) => println!("OK: {:?}", opts),
      Err(err) => println!("ERR: {:?}", err)
  }
}
```


### Steps to reproduce the bug with the above code

Please see: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=43678edf67d7e39b179c141c55d2036d

### Actual Behaviour

`try_update_from` returns an error of kind `MissingRequiredArgument`

### Expected Behaviour

`try_update_from` should succeed as `foo` already set in `opt`

### Additional Context

_No response_

### Debug Output

_No response_


Discussion / hints:
In `update_from`, we skip applying our implicit `required = true` but we are not overriding the users explicit `required = true` which we should

Fix patch:
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
index ad2a6463f0c..ee16eaacf22 100644
--- a/clap_derive/src/derives/args.rs
+++ b/clap_derive/src/derives/args.rs
@@ -187,7 +187,7 @@ pub fn gen_augment(
                     (Ty::Option, Some(sub_type)) => sub_type,
                     _ => &field.ty,
                 };
-                let implicit_methods = if **ty == Ty::Option || override_required {
+                let implicit_methods = if **ty == Ty::Option {
                     quote!()
                 } else {
                     quote_spanned! { kind.span()=>
@@ -196,10 +196,20 @@ pub fn gen_augment(
                     }
                 };
 
+                let override_methods = if override_required {
+                    quote_spanned! { kind.span()=>
+                        .subcommand_required(false)
+                        .arg_required_else_help(false)
+                    }
+                } else {
+                    quote!()
+                };
+
                 Some(quote! {
                     let #app_var = <#subcmd_type as clap::Subcommand>::augment_subcommands( #app_var );
                     let #app_var = #app_var
-                        #implicit_methods;
+                        #implicit_methods
+                        #override_methods;
                 })
             }
             Kind::Flatten(ty) => {
@@ -289,7 +299,7 @@ pub fn gen_augment(
                     }
 
                     Ty::Other => {
-                        let required = item.find_default_method().is_none() && !override_required;
+                        let required = item.find_default_method().is_none();
                         // `ArgAction::takes_values` is assuming `ArgAction::default_value` will be
                         // set though that won't always be true but this should be good enough,
                         // otherwise we'll report an "arg required" error when unwrapping.
@@ -310,6 +320,13 @@ pub fn gen_augment(
                 } else {
                     quote!()
                 };
+                let override_methods = if override_required {
+                    quote_spanned! { kind.span()=>
+                        .required(false)
+                    }
+                } else {
+                    quote!()
+                };
 
                 Some(quote_spanned! { field.span()=>
                     let #app_var = #app_var.arg({
@@ -321,6 +338,10 @@ pub fn gen_augment(
 
                         let arg = arg
                             #explicit_methods;
+
+                        let arg = arg
+                            #override_methods;
+
                         arg
                     });
                 })
diff --git a/clap_derive/src/derives/into_app.rs b/clap_derive/src/derives/into_app.rs
index eabdfa3b6e2..a45667ab296 100644
--- a/clap_derive/src/derives/into_app.rs
+++ b/clap_derive/src/derives/into_app.rs
@@ -85,6 +85,8 @@ pub fn gen_for_enum(item: &Item, item_name: &Ident, generics: &Generics) -> Toke
             fn command_for_update<'b>() -> clap::Command {
                 let #app_var = clap::Command::new(#name);
                 <Self as clap::Subcommand>::augment_subcommands_for_update(#app_var)
+                    .subcommand_required(false)
+                    .arg_required_else_help(false)
             }
         }
     }
diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs
index 009684bfc2c..8a44e0c4219 100644
--- a/clap_derive/src/derives/subcommand.rs
+++ b/clap_derive/src/derives/subcommand.rs
@@ -244,16 +244,24 @@ fn gen_augment(
                     };
                     let initial_app_methods = item.initial_top_level_methods();
                     let final_from_attrs = item.final_top_level_methods();
+                    let override_methods = if override_required {
+                        quote_spanned! { kind.span()=>
+                            .subcommand_required(false)
+                            .arg_required_else_help(false)
+                        }
+                    } else {
+                        quote!()
+                    };
                     let subcommand = quote! {
                         let #app_var = #app_var.subcommand({
                             #deprecations;
                             let #subcommand_var = clap::Command::new(#name);
-                            let #subcommand_var = #subcommand_var #initial_app_methods;
-                            let #subcommand_var = #arg_block;
                             let #subcommand_var = #subcommand_var
                                 .subcommand_required(true)
                                 .arg_required_else_help(true);
-                            #subcommand_var #final_from_attrs
+                            let #subcommand_var = #subcommand_var #initial_app_methods;
+                            let #subcommand_var = #arg_block;
+                            #subcommand_var #final_from_attrs #override_methods
                         });
                     };
                     Some(subcommand)


Test patch:
diff --git a/tests/derive/basic.rs b/tests/derive/basic.rs
index 8cb70dcbdbb..dd088926d75 100644
--- a/tests/derive/basic.rs
+++ b/tests/derive/basic.rs
@@ -31,15 +31,46 @@ fn basic() {
 fn update_basic() {
     #[derive(Parser, PartialEq, Debug)]
     struct Opt {
-        #[arg(short = 'a', long = "arg")]
-        single_value: i32,
+        #[arg(short, long)]
+        first: i32,
+        #[arg(short, long)]
+        second: i32,
+    }
+
+    let mut opt = Opt::try_parse_from(["test", "-f0", "-s1"]).unwrap();
+
+    opt.try_update_from(["test", "-f42"]).unwrap();
+
+    assert_eq!(
+        Opt {
+            first: 42,
+            second: 1
+        },
+        opt
+    );
+}
+
+#[test]
+fn update_explicit_required() {
+    #[derive(Parser, PartialEq, Debug)]
+    struct Opt {
+        #[arg(short, long, required = true)]
+        first: i32,
+        #[arg(short, long, required = true)]
+        second: i32,
     }
 
-    let mut opt = Opt::try_parse_from(["test", "-a0"]).unwrap();
+    let mut opt = Opt::try_parse_from(["test", "-f0", "-s1"]).unwrap();
 
-    opt.update_from(["test", "-a42"]);
+    opt.try_update_from(["test", "-f42"]).unwrap();
 
-    assert_eq!(Opt { single_value: 42 }, opt);
+    assert_eq!(
+        Opt {
+            first: 42,
+            second: 1
+        },
+        opt
+    );
 }
 
 #[test]
diff --git a/tests/derive/boxed.rs b/tests/derive/boxed.rs
index d8b93809580..55b896ea67f 100644
--- a/tests/derive/boxed.rs
+++ b/tests/derive/boxed.rs
@@ -35,7 +35,7 @@ fn boxed_flatten_subcommand() {
 fn update_boxed_flatten_subcommand() {
     let mut opt = Opt::try_parse_from(["test", "flame", "1"]).unwrap();
 
-    opt.update_from(["test", "flame", "42"]);
+    opt.try_update_from(["test", "flame", "42"]).unwrap();
 
     assert_eq!(
         Opt {
diff --git a/tests/derive/skip.rs b/tests/derive/skip.rs
index b6924fb132f..bb39b6f9860 100644
--- a/tests/derive/skip.rs
+++ b/tests/derive/skip.rs
@@ -30,7 +30,7 @@ fn skip_1() {
     );
     opt.s = 42;
 
-    opt.update_from(["test", "-x", "22"]);
+    opt.try_update_from(["test", "-x", "22"]).unwrap();
 
     assert_eq!(opt, Opt { x: 22, s: 42 });
 }
diff --git a/tests/derive/subcommands.rs b/tests/derive/subcommands.rs
index 3b92cdc5fd3..7730a63c87b 100644
--- a/tests/derive/subcommands.rs
+++ b/tests/derive/subcommands.rs
@@ -374,6 +374,33 @@ fn update_subcommands() {
     );
 }
 
+#[test]
+fn update_subcommands_explicit_required() {
+    #[derive(Parser, PartialEq, Eq, Debug)]
+    #[command(subcommand_required = true)]
+    enum Opt {
+        Command1(Command1),
+        Command2(Command2),
+    }
+
+    #[derive(Parser, PartialEq, Eq, Debug)]
+    struct Command1 {
+        arg1: i32,
+
+        arg2: i32,
+    }
+
+    #[derive(Parser, PartialEq, Eq, Debug)]
+    struct Command2 {
+        arg2: i32,
+    }
+
+    // Full subcommand update
+    let mut opt = Opt::Command1(Command1 { arg1: 12, arg2: 14 });
+    opt.try_update_from(["test"]).unwrap();
+    assert_eq!(Opt::Command1(Command1 { arg1: 12, arg2: 14 }), opt);
+}
+
 #[test]
 fn update_sub_subcommands() {
     #[derive(Parser, PartialEq, Eq, Debug)]