Derive: adding visible_alias attribute to complex subcommands results in duplicated aliases

[tranzystorekk]

StructOpt Issue: TeXitoi/structopt#418
StructOpt PR: TeXitoi/structopt#504

Please complete the following tasks

• I have searched the discussions
• I have searched the existing issues

Rust Version

1.55.0

Clap Version

3.0.0-beta.4

Minimal reproducible code

use clap::Clap;

#[derive(Clap)]
enum Cmd {
    #[clap(visible_alias = "alias")]
    Test { args: Vec<String> },
}

#[derive(Clap)]
struct Cli {
    #[clap(subcommand)]
    cmd: Cmd,
}

fn main() {
    let cli = Cli::parse();
}

Steps to reproduce the bug with the above code

cargo run -- --help

Actual Behaviour

FLAGS:
    -h, --help       Print help information
    -V, --version    Print version information

SUBCOMMANDS:
    help    Print this message or the help of the given subcommand(s)
    test     [aliases: alias, alias]

Expected Behaviour

FLAGS:
    -h, --help       Print help information
    -V, --version    Print version information

SUBCOMMANDS:
    help    Print this message or the help of the given subcommand(s)
    test     [aliases: alias]

Additional Context

Output of cargo expand with duplicated calls to visible_alias:

impl clap::Subcommand for Cmd {
    fn augment_subcommands<'b>(app: clap::App<'b>) -> clap::App<'b> {
        let app = app;
        let app = app.subcommand({
            let subcommand = clap::App::new("test");
            let subcommand = subcommand;
            let subcommand = {
                let subcommand = subcommand;
                let subcommand = subcommand.arg(
                    clap::Arg::new("args")
                        .takes_value(true)
                        .value_name("ARGS")
                        .multiple_values(true)
                        .validator(|s| ::std::str::FromStr::from_str(s).map(|_: String| ())),
                );
                subcommand.visible_alias("alias")
            };
            subcommand.visible_alias("alias")
        });
        app
    }

Debug Output

No response

[pksunkara]

@epage Might be fixed by your recent changes.

[epage]

It isn't. This was also recently reported for structopt. You can see the proposed fix TeXitoi/structopt#504

[epage]

My proposed fix for #2894 should also fix this without having to duplicate logic like in the structopt fix (due to our divergence, the level of duplication is different, I don't remember if its worse or better). So the question is whether we get to #2894 or go ahead and port structopt's fix over in the mean time.

[pksunkara]

I am of the opinion that we should focus on #2894 after the release and fix it correctly. Structopt's fixes are temporary because it is basically in maintenance mode.

[epage]

#2894 involves a breaking change (just added the tag). We'll have to change the trait definitions.

This is one of those things I classify as a non-disruptive breaking change. Most users will be using the derive and will not notice the breaking change, only the bug fixes (and where you draw the line on behavior changes being breaking is another question)