Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option::unwrap on nested flatten/subcommand #388

Closed
ndmitchell opened this issue May 13, 2020 · 5 comments · Fixed by #401
Closed

Option::unwrap on nested flatten/subcommand #388

ndmitchell opened this issue May 13, 2020 · 5 comments · Fixed by #401
Labels
bug This is a BUG. The fix may be released in a patch version even if considered breaking

Comments

@ndmitchell
Copy link

Using structopt-0.3.14 and Rust nightly-2020-04-20 the following program when run with cargo run -- command foo gives a panic Option::unwrap() on a None. RUST_BACKTRACE=1 points to the generated from_clap as containing the error.

use structopt::StructOpt;

#[derive(Debug, StructOpt)]
pub enum Struct1 {
    #[structopt(flatten)]
    Struct1(Struct2),
}

#[derive(Debug, StructOpt)]
pub struct Struct2 {
    #[structopt(subcommand)]
    command_type: Enum3,
}

#[derive(Debug, StructOpt)]
pub enum Enum3 {
    Command { args: Vec<String> },
}

pub fn main() {
    let opt = Struct1::from_args();
    println!("{:?}", opt);
}

Note that changing Enum3 to struct Enum3 {args: Vec<String>} also seems to behave unexpectedly as it always displays the help instead of returning a valid result.

@TeXitoi
Copy link
Owner

TeXitoi commented May 14, 2020

What is the expected behavior? I can't find anything really meaning full from this example.

@ndmitchell
Copy link
Author

I would have thought structopt should never just call unwrap on a None, and that even on unexpected StructOpt derivations should at least give some error message. In this case, I expect the behaviour would be equivalent to Enum3 only, but I'm not familiar enough with the precise semantics of flatten/subcommand to be sure.

This example was boiled down from a larger example which broke when I tried to insert a few additional flatten's in it to avoid nested submodes. As soon as it went to failing with unwrap on None I wasn't sure if I was violating structopt rules (e.g. you aren't allowed a flatten around a subcommand struct) or it was valid input that structopt was failing on.

@CreepySkeleton
Copy link
Collaborator

Hm I would actually expect this to work or, at least, show a meaningful panic message. I'll see what can be done in... a number of days.

Although, I have one concern about flattening structs that contain subcommands.

There must be at most one field marked with #[subcommand] at any given layer of the program. There must be only one enum representing subcommands. Example:

enum Sub {}

// You CAN'T do that
struct Opts {
    #[structopt(subcommand)]
    s1: Sub,

    #[structopt(subcommand)]
    s2: Sub,
}

Structopt can check this case and abort with a compile-time error, that's cool.

But it gets really complicated once flattening is in play:

enum Sub {}

struct Flat {
    #[structopt(subcommand)]
    s1: Sub,
}

// You CAN'T do that
struct Opts {
    #[structopt(flatten)]
    s1: Flat,

    #[structopt(flatten)]
    s1: Flat,
}

Structopt can't hold you hand here because it doesn't know what's at the other end of the flatten at compile time. This kind of invariant is up to user to keep upheld.

It gets only worse with a possibility of transitive flatten...

@CreepySkeleton CreepySkeleton added the bug This is a BUG. The fix may be released in a patch version even if considered breaking label May 14, 2020
@TeXitoi
Copy link
Owner

TeXitoi commented May 14, 2020

So, TLDR, it should fail at compile time because we can handle only one subcommand in the type tree, but that's complicated to implement with flatten.

You validate @CreepySkeleton ?

@CreepySkeleton
Copy link
Collaborator

Yes. The only way to use it correctly is

struct Opt {
    #[flatten]
    flatten: Flat
}

struct Flat {
    #[subcommand]
    sub: Sub,
   
    opt: String
}

Which is no different from:

struct Opt {
    #[flatten]
    flatten: Flat,

    #[subcommand]
    sub: Sub,
}

struct Flat {   
    opt: String
}

I doubt we can detect this at compile time, but I think I'll be able to improve the panic message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a BUG. The fix may be released in a patch version even if considered breaking
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants