Generic derive support

[kuviman]

Closes #2769

Unresolved questions:

• Should trait bounds be inferred instead of copy pasting?

[kuviman]

At the moment no additional trait bounds are added to the impls, so one should put necessary trait bounds on the type definition itself:

#[derive(Parser)]
struct Opt<T: Args> {
    #[clap(flatten)]
    inner: T,
}

[pksunkara]

Looks good as it is for a phased approach. But this doesn't completely close the original issue. I think the trait bounds can be inferred with some extra logic in our derive implementation.

[kuviman]

Trait bounds can be inferred, sure, but:

1. As I said in Deriving for generic structs #2769 (comment), my opinion is that errors should be caught as early as possible.
   My usecase is making a library with a struct Opt<T: Args>: Parser.
   When creating a generic function to handle this struct I have to put generic trait bounds there too. It is easier to look at the type's definition that generating docs to see what trait bounds do I need. If I do not put correct bounds I will see weird errors, but if the bounds are on the type initially the errors are much better.
   The type does not make much sense without generic arguments implementing those trait bounds anyway.

   Let's say we have

   #[derive(Parser)]
   struct Opt {
       arg: (),
   }

   This will currently error saying the trait `FromStr` is not implemented for `()` .
   Imagine if there instead would be an impl Parser for Opt where (): FromStr.
   Do you think this is good to have? Why is generic arguments any different.

2. I am now using this implementation for myself with success. Inferring bounds can be done but requires more work that already done here. If inferring is really needed, maybe it can be split into a different PR?

[epage]

Looks good as it is for a phased approach. But this doesn't completely close the original issue. I think the trait bounds can be inferred with some extra logic in our derive implementation.

Our issue and the linked structopt PR both have explicit trait bounds. If you are referring to my comment, I can't remember what else I was referring to. As long as we can cleanly port over structopt's test cases, it seems like this is sufficient for merging and closing out our issue which will help us be feature parity with structopt when we release so people can port to clap3 more easily.

[kuviman]

@epage I have ported tests from structopt. Helped me notice I forgot Parser implementation for enums.

One of the changes needed compared to structopt was for generic fields that are not flattened.

While structopt requires <T as FromStr>::Err: fmt::Debug + fmt::Display, clap requires <T as FromStr>::Err: std::error::Error + Sync + Send + 'static.

[epage]

@pksunkara the tests are showing parity with structopt. Any further concerns?

[pksunkara]

bors r+

I didn't realize that #2769 is purely a parity issue. In that case, yeah this is good enough to close that.

But, should we create a new issue to discuss further improvements to these generics about inferring trait bounds?

[epage]

But, should we create a new issue to discuss further improvements to these generics about inferring trait bounds?

Created #3032

[bors]

Build succeeded:

• CI