Macro expansion order appears to cause failure with order-dependent configurations

[chriskrycho]

Using the AppSettings::VersionlessSubcommand setting requires that it be set prior to any subcommands being defined. Using raw(setting = "structopt::clap::AppSettings::VersionlessSubcommands"), on a top-level struct does not work, however, which implies that the expansion order is probably generating the Subcommand invocations prior to the app settings being applied.

[TeXitoi]

If no other method in clap need to be at the end, then we can change it to be at the begining. But I suspect that's not the case.

If that's not the case, it would need some special attribute, as prior(...) or some other name.

[kw217]

Using raw(group = r#"ArgGroup::with_name("...").args(&[...]).required(true)"#) requires that the arg names already be generated. Instead I get

thread 'main' panicked at 'The group '...' contains the arg '...' that doesn't actually exist.', /opt/rust/registry/src/gitlab.datcon.co.uk-27bb2a0041c1489d/clap-2.33.0/src/app/parser.rs:156:9

which suggests that we also need an after(...) or similar.

[kw217]

Actually I can get the behaviour I need in this specific case using the techniques from #151, marking the group on each argument rather than in an args parameter on the App. So this may not be strong support for this feature.

[CreepySkeleton]

@chriskrycho @kw217 @TeXitoi I don't believe macro expansion order can affect this.

Let's consider this code (repo:

use structopt::{clap::AppSettings, StructOpt};

#[derive(StructOpt)]
#[structopt(raw(setting = "AppSettings::VersionlessSubcommands"))]
struct Foo {
    #[structopt(subcommand)]
    bar: Bar,

    #[structopt(short)]
    a: u64
}

#[derive(StructOpt)]
enum Bar {
    One {
        i: i32
    },
    Two {
        i: i32
    }
}

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

structopt generates something like this (the snipped is quite long, explanation is on the way)

impl ::structopt::StructOpt for Foo {
    fn from_args() -> Self
    where
        Self: Sized,
    {
        Self::from_clap(&Self::clap().get_matches())
    }
    fn clap<'a, 'b>() -> ::structopt::clap::App<'a, 'b> {
        let app = ::structopt::clap::App::new("probe")
            .version("0.2.0")
            .author("noname")
            .setting(AppSettings::VersionlessSubcommands);
        Self::augment_clap(app)
    }
    fn from_clap(matches: &::structopt::clap::ArgMatches) -> Self {
        Foo {
            bar: <Bar>::from_subcommand(matches.subcommand()).unwrap(),
            a: matches
                .value_of("a")
                .map(|s| ::std::str::FromStr::from_str(s).unwrap())
                .unwrap(),
        }
    }
}

impl Foo {
    pub fn augment_clap<'a, 'b>(
        app: ::structopt::clap::App<'a, 'b>,
    ) -> ::structopt::clap::App<'a, 'b> {
        {
            let app = app.arg(
                ::structopt::clap::Arg::with_name("a")
                    .takes_value(true)
                    .multiple(false)
                    .required(true)
                    .validator(|s| {
                        ::std::str::FromStr::from_str(&s)
                            .map(|_: u64| ())
                            .map_err(|e| (&e).to_string())
                    })
                    .short("a"),
            );
            let app = <Bar>::augment_clap(app);
            let app = app.setting(::structopt::clap::AppSettings::SubcommandRequiredElseHelp);
            app
        }
    }
    pub fn is_subcommand() -> bool {
        false
    }
}

impl ::structopt::StructOpt for Bar {
    fn clap<'a, 'b>() -> ::structopt::clap::App<'a, 'b> {
        let app = ::structopt::clap::App::new("probe")
            .version("0.2.0")
            .author("noname")
            .setting(::structopt::clap::AppSettings::SubcommandRequiredElseHelp);
        Self::augment_clap(app)
    }
    fn from_clap(matches: &::structopt::clap::ArgMatches) -> Self {
        <Bar>::from_subcommand(matches.subcommand()).unwrap()
    }
}

impl Bar {
    pub fn augment_clap<'a, 'b>(
        app: ::structopt::clap::App<'a, 'b>,
    ) -> ::structopt::clap::App<'a, 'b> {
        app.subcommand({
            let subcommand = ::structopt::clap::SubCommand::with_name("One");
            let subcommand = {
                let subcommand = subcommand.arg(
                    ::structopt::clap::Arg::with_name("i")
                        .takes_value(true)
                        .multiple(false)
                        .required(true)
                        .validator(|s| {
                            ::std::str::FromStr::from_str(&s)
                                .map(|_: i32| ())
                                .map_err(|e| (&e).to_string())
                        }),
                );
                subcommand
            };
            subcommand.version("0.2.0").author("noname")
        })
        .subcommand({
            let subcommand = ::structopt::clap::SubCommand::with_name("Two");
            let subcommand = {
                let subcommand = subcommand.arg(
                    ::structopt::clap::Arg::with_name("i")
                        .takes_value(true)
                        .multiple(false)
                        .required(true)
                        .validator(|s| {
                            ::std::str::FromStr::from_str(&s)
                                .map(|_: i32| ())
                                .map_err(|e| (&e).to_string())
                        }),
                );
                subcommand
            };
            subcommand.version("0.2.0").author("noname")
        })
    }
    pub fn from_subcommand<'a, 'b>(
        sub: (&'b str, Option<&'b ::structopt::clap::ArgMatches<'a>>),
    ) -> Option<Self> {
        match sub {
            ("One", Some(matches)) => Some(Bar::One {
                i: matches
                    .value_of("i")
                    .map(|s| ::std::str::FromStr::from_str(s).unwrap())
                    .unwrap(),
            }),
            ("Two", Some(matches)) => Some(Bar::Two {
                i: matches
                    .value_of("i")
                    .map(|s| ::std::str::FromStr::from_str(s).unwrap())
                    .unwrap(),
            }),
            _ => None,
        }
    }
    pub fn is_subcommand() -> bool {
        true
    }
}

when we use Structopts::from_args method the call order is following

Structopts::from_args
  Self::clap
     App::new
     App::version
     App::Author
     App::setting(VersionlessSubcommands)
     Self::augment_clap
       \<"a"\> Arg building...
      ...
  App::get_matches

What would have change if macro expansion order is reversed? Only impls relative order, they're independent from each other. Method call inside will stay the same.

This is why I believe this bug is caused by some code in methods generating.

Can anyone provide a specific code snipped that would reproduce the issue? Bonus points if it works on current master.

[TeXitoi]

I think the problem is not about macro expantion, it's that attribute are stacked before or after adding args, and some app commands need to be called on the other side.

[CreepySkeleton]

generating the Subcommand invocations prior to the app settings being applied

As far as I get it from the code, this never happens. That's why I'm asking for a reproducer, I almost believe this is already solved

[CreepySkeleton]

@TeXitoi I suggest to close this unless someone can show a reproducer. I'm sure this was solved somewhere half-way through the v0.3