Shell completions include --version on subcommands despite no version set

[epage]

Please complete the following tasks

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

Rust Version

rustc 1.55.0 (c8dfcfe04 2021-09-06)

Clap Version

master

Minimal reproducible code

This snippet is using the API the generators used, so it can demonstrate the problem

fn main() {
    use clap::*;
    let mut app = App::new("fig")
        .about("Tests completions")
        .arg(
            Arg::new("file")
                .value_hint(ValueHint::FilePath)
                .about("some input file"),
        )
        .subcommand(
            App::new("test").about("tests things").arg(
                Arg::new("case")
                    .long("case")
                    .takes_value(true)
                    .about("the case to test"),
            ),
        );

    app.get_matches_mut();
    for flag in clap_generate::utils::flags(&app) {
        if flag.get_name() == "version" {
            dbg!(flag);
        }
    }

    for subcommand in app.get_subcommands() {
        for flag in clap_generate::utils::flags(&subcommand) {
            if flag.get_name() == "version" {
                dbg!(subcommand.get_name());
                dbg!(flag);
            }
        }
    }
}

Steps to reproduce the bug with the above code

Run it

Actual Behaviour

See a version flag included for each subcommand

Expected Behaviour

No output should happen

Additional Context

This was found as part of #2858.

If I'm reading this correctly, #2831 relied on fn _build to remove --version in the call to _check_help_and_version but we don't recursively call _build on subcommands, leaving the auto-generated flag behind.

if this is the case, then probably a lot of subcommand behavior is broken because we'd never call _build on their args.

Debug Output

[            clap::build::app]  App::_do_parse
[            clap::build::app]  App::_build
[            clap::build::app]  App::_propagate:fig
[            clap::build::app]  App::_check_help_and_version
[            clap::build::app]  App::_check_help_and_version: Removing generated version
[            clap::build::app]  App::_check_help_and_version: Building help subcommand
[            clap::build::app]  App::_propagate_global_args:fig
[            clap::build::app]  App::_derive_display_order:fig
[            clap::build::app]  App::_derive_display_order:test
[            clap::build::app]  App::_derive_display_order:help
[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:file
[         clap::parse::parser]  Parser::get_matches_with
[         clap::parse::parser]  Parser::_build
[         clap::parse::parser]  Parser::_verify_positionals
[         clap::parse::parser]  Parser::remove_overrides
[      clap::parse::validator]  Validator::validate
[         clap::parse::parser]  Parser::add_env: Checking arg `--help`
[         clap::parse::parser]  Parser::add_env: Checking arg `<file>`
[         clap::parse::parser]  Parser::add_defaults
[         clap::parse::parser]  Parser::add_defaults:iter:file:
[         clap::parse::parser]  Parser::add_value: doesn't have conditional defaults
[         clap::parse::parser]  Parser::add_value:iter:file: doesn't have default vals
[         clap::parse::parser]  Parser::add_value:iter:file: doesn't have default missing vals
[      clap::parse::validator]  Validator::validate_conflicts
[      clap::parse::validator]  Validator::validate_exclusive
[      clap::parse::validator]  Validator::gather_conflicts
[      clap::parse::validator]  Validator::validate_required: required=ChildGraph([])
[      clap::parse::validator]  Validator::gather_requirements
[      clap::parse::validator]  Validator::validate_required_unless
[      clap::parse::validator]  Validator::validate_matched_args
[    clap::parse::arg_matcher]  ArgMatcher::get_global_values: global_arg_vec=[help]

Note the single logged App::_build, rather than one-per-subcommand.

[pksunkara]

Oof, I forgot about tracking this issue. The context is:

We do lazy building of the subcommand to improve performance in our parser once we match a subcommand. But generators need all the subcommands to be built. I have a TODO marked here about this.

[epage]

Thanks for the pointer!