Repository: clap-rs/clap (rust)

Issue:
'Argument 'x' is required and can't have a default value' when using default_value_os with #[derive(Clap)]
### Please complete the following tasks

- [X] I have searched the [discussions](https://github.com/clap-rs/clap/discussions)
- [X] I have searched the existing issues

### Rust Version

rustc 1.56.1 (59eed8a2a 2021-11-01)

### Clap Version

3.0.0-beta.5

### Minimal reproducible code

```rust
#[derive(clap::Parser)]
pub struct Options2 {
    #[clap(default_value_os = ("123".as_ref()))]
    x: String,
}

fn main() {
    let args = Options2::parse();
}
```


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

`cargo run`

### Actual Behaviour

Panics with 
```
thread 'main' panicked at 'Argument 'x' is required and can't have a default value', C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\arg\debug_asserts.rs:39:9
stack backtrace:
   0:     0x7ff798963f7e - std::backtrace_rs::backtrace::dbghelp::trace
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\..\..\backtrace\src\backtrace\dbghelp.rs:98
   1:     0x7ff798963f7e - std::backtrace_rs::backtrace::trace_unsynchronized
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\..\..\backtrace\src\backtrace\mod.rs:66
   2:     0x7ff798963f7e - std::sys_common::backtrace::_print_fmt
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\sys_common\backtrace.rs:67
   3:     0x7ff798963f7e - std::sys_common::backtrace::_print::impl$0::fmt
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\sys_common\backtrace.rs:46
   4:     0x7ff798975d9a - core::fmt::write
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\core\src\fmt\mod.rs:1150
   5:     0x7ff798961738 - std::io::Write::write_fmt<std::sys::windows::stdio::Stderr>
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\io\mod.rs:1667
   6:     0x7ff798966466 - std::sys_common::backtrace::_print
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\sys_common\backtrace.rs:49
   7:     0x7ff798966466 - std::sys_common::backtrace::print
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\sys_common\backtrace.rs:36
   8:     0x7ff798966466 - std::panicking::default_hook::closure$1
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:210
   9:     0x7ff798965f54 - std::panicking::default_hook
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:227
  10:     0x7ff798966ac5 - std::panicking::rust_panic_with_hook
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:624
  11:     0x7ff7989666ab - std::panicking::begin_panic_handler::closure$0
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:521
  12:     0x7ff7989648c7 - std::sys_common::backtrace::__rust_end_short_backtrace<std::panicking::begin_panic_handler::closure$0,never$>
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\sys_common\backtrace.rs:141
  13:     0x7ff798966609 - std::panicking::begin_panic_handler
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:517
  14:     0x7ff79897b57c - std::panicking::begin_panic_fmt
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:460
  15:     0x7ff79883b332 - clap::build::arg::debug_asserts::assert_arg
                               at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\arg\debug_asserts.rs:39
  16:     0x7ff7988169bd - clap::build::app::debug_asserts::assert_app
                               at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\app\debug_asserts.rs:90
  17:     0x7ff7987d7504 - clap::build::app::App::_build
                               at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\app\mod.rs:2460
  18:     0x7ff7987d7060 - clap::build::app::App::_do_parse
                               at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\app\mod.rs:2389
  19:     0x7ff7987d6a99 - clap::build::app::App::try_get_matches_from_mut<ref_mut$<std::env::ArgsOs>,std::ffi::os_str::OsString>
                               at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\app\mod.rs:2278
  20:     0x7ff798791a25 - clap::build::app::App::get_matches_from<ref_mut$<std::env::ArgsOs>,std::ffi::os_str::OsString>
                               at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\app\mod.rs:2112
  21:     0x7ff7987935a4 - clap::build::app::App::get_matches
                               at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\app\mod.rs:2012
  22:     0x7ff7987918c1 - clap::derive::Parser::parse<edge::Options2>
                               at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\derive.rs:76
  23:     0x7ff79879110e - edge::main
                               at D:\git\spectrum\edge-cli\src\main.rs:120
  24:     0x7ff798792f9b - core::ops::function::FnOnce::call_once<void (*)(),tuple$<> >
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\library\core\src\ops\function.rs:227
  25:     0x7ff79879236b - std::sys_common::backtrace::__rust_begin_short_backtrace<void (*)(),tuple$<> >
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\library\std\src\sys_common\backtrace.rs:125
  26:     0x7ff7987925c1 - std::rt::lang_start::closure$0<tuple$<> >
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\library\std\src\rt.rs:63
  27:     0x7ff798966f16 - core::ops::function::impls::impl$2::call_once
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\library\core\src\ops\function.rs:259
  28:     0x7ff798966f16 - std::panicking::try::do_call
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:403
  29:     0x7ff798966f16 - std::panicking::try
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:367
  30:     0x7ff798966f16 - std::panic::catch_unwind
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panic.rs:129
  31:     0x7ff798966f16 - std::rt::lang_start_internal::closure$2
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\rt.rs:45
  32:     0x7ff798966f16 - std::panicking::try::do_call
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:403
  33:     0x7ff798966f16 - std::panicking::try
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panicking.rs:367
  34:     0x7ff798966f16 - std::panic::catch_unwind
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\panic.rs:129
  35:     0x7ff798966f16 - std::rt::lang_start_internal
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\/library\std\src\rt.rs:45
  36:     0x7ff79879258f - std::rt::lang_start<tuple$<> >
                               at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35\library\std\src\rt.rs:62
  37:     0x7ff798791476 - main
  38:     0x7ff79897a170 - invoke_main
                               at d:\a01\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
  39:     0x7ff79897a170 - __scrt_common_main_seh
                               at d:\a01\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
  40:     0x7ff9916137e4 - BaseThreadInitThunk
  41:     0x7ff993dbcb61 - RtlUserThreadStart
```

### Expected Behaviour

The behaviour should be the same as if I used `default_value = "123"`, i.e it exits successfully.

### Additional Context

_No response_

### Debug Output

```
[            clap::build::app]  App::_do_parse
[            clap::build::app]  App::_build
[            clap::build::app]  App::_propagate:edge-cli
[            clap::build::app]  App::_check_help_and_version
[            clap::build::app]  App::_check_help_and_version: Removing generated version
[            clap::build::app]  App::_propagate_global_args:edge-cli
[            clap::build::app]  App::_derive_display_order:edge-cli
[clap::build::app::debug_asserts]       App::_debug_asserts
[clap::build::arg::debug_asserts]       Arg::_debug_asserts:help
[clap::build::arg::debug_asserts]       Arg::_debug_asserts:x
thread 'main' panicked at 'Argument 'x' is required and can't have a default value', C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\clap-3.0.0-beta.5\src\build\arg\debug_asserts.rs:39:9
```

Pull request:
fix(derive): Treat `default_value_os` like `default_value`
The test went from panicing to not-panicing

Fixes #3031

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9f04bf9b..d81502e8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -283,6 +283,7 @@ On top of the clap 2 changes
 - Support `SubcommandsNegateReqs` by allowing required `Option<_>`s ([clap-rs/clap#2255](https://github.com/clap-rs/clap/issues/2255))
 - Infer `AllowInvalidUtf8` based on parser ([clap-rs/clap#751](https://github.com/clap-rs/clap/issues/2255))
 - Gracefully handle empty `authors` field in `Cargo.toml`
+- Don't panic with `default_value_os` but treat it like `default_value` ([clap-rs/clap#3031](https://github.com/clap-rs/clap/issues/3031))
 
 On top of the clap 2 changes
 
diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs
index 262fb754..648be95b 100644
--- a/clap_derive/src/attrs.rs
+++ b/clap_derive/src/attrs.rs
@@ -323,7 +323,7 @@ impl Attrs {
                         if res.is_enum {
                             abort!(field.ty, "`arg_enum` is meaningless for bool")
                         }
-                        if let Some(m) = res.find_method("default_value") {
+                        if let Some(m) = res.find_default_method() {
                             abort!(m.name, "default_value is meaningless for bool")
                         }
                         if let Some(m) = res.find_method("required") {
@@ -331,7 +331,7 @@ impl Attrs {
                         }
                     }
                     Ty::Option => {
-                        if let Some(m) = res.find_method("default_value") {
+                        if let Some(m) = res.find_default_method() {
                             abort!(m.name, "default_value is meaningless for Option")
                         }
                     }
@@ -557,14 +557,16 @@ impl Attrs {
         }
     }
 
-    pub fn has_method(&self, name: &str) -> bool {
-        self.find_method(name).is_some()
-    }
-
     pub fn find_method(&self, name: &str) -> Option<&Method> {
         self.methods.iter().find(|m| m.name == name)
     }
 
+    pub fn find_default_method(&self) -> Option<&Method> {
+        self.methods
+            .iter()
+            .find(|m| m.name == "default_value" || m.name == "default_value_os")
+    }
+
     /// generate methods from attributes on top of struct or enum
     pub fn initial_top_level_methods(&self) -> TokenStream {
         let help_heading = self.help_heading.as_ref().into_iter();
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
index 2842b9ba..461892a1 100644
--- a/clap_derive/src/derives/args.rs
+++ b/clap_derive/src/derives/args.rs
@@ -328,7 +328,7 @@ pub fn gen_augment(
                     },
 
                     Ty::Other => {
-                        let required = !attrs.has_method("default_value") && !override_required;
+                        let required = attrs.find_default_method().is_none() && !override_required;
                         quote_spanned! { ty.span()=>
                             .takes_value(true)
                             .value_name(#value_name)


Test patch:
diff --git a/tests/derive/default_value.rs b/tests/derive/default_value.rs
index b3a47099..94dbdaf1 100644
--- a/tests/derive/default_value.rs
+++ b/tests/derive/default_value.rs
@@ -1,4 +1,4 @@
-use clap::Parser;
+use clap::{IntoApp, Parser};
 
 use crate::utils;
 
@@ -43,3 +43,15 @@ fn auto_default_value_t() {
     let help = utils::get_long_help::<Opt>();
     assert!(help.contains("[default: 0]"));
 }
+
+#[test]
+fn detect_os_variant() {
+    #![allow(unused_parens)] // needed for `as_ref` call
+
+    #[derive(clap::Parser)]
+    pub struct Options {
+        #[clap(default_value_os = ("123".as_ref()))]
+        x: String,
+    }
+    Options::into_app().debug_assert();
+}