From a61b60816cee1cad6ec1474c4f5376766c1af9e6 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Tue, 12 Oct 2021 13:59:56 -0500
Subject: [PATCH] fix(gen): Ensure subcommands are post-processed

`App::get_matches` lazily post-processes `App`s and `Arg`s so we don't
do it to subcommands that are never run (downside being people have to
exercise their full app to get debug_asserts).

`clap_generate` was only post-processing the top-level `App` and `Arg`s,
ignoring the sub-commands.  In #2858, we noticed that `--version` was
being left in the completions instead of being removed during the
`_build` step.  We would also have an incorrect `num_vals` and a host of
other problems.

This change adds a `App::_build_all` function for `clap_generate` to use
to eagerly build everything.  By having it there, we make sure
everywhere that needs eager building, gets it (like some tests).

In `clap_generate::utils`, we add a unit test to ensure the subcommand's
`--version` was removed.

For some other tests specifying `.version()`, I added
`AppSettings::PropagateVersion` to make it behave more consistently.
The places I didn't were generally where the version was conditionally
set.

For `clap_generate/tests/generate_completions.rs`, I had to adjust the
`conflicts_with` because the subcommand was inheriting the argument with
it defined *but* the subcommand did not have the argument, tripping up a
debug assert.

Fixes #2860
---
 clap_generate/src/lib.rs                      |  4 +-
 clap_generate/src/utils.rs                    | 58 +++++++++++++------
 clap_generate/tests/completions/bash.rs       |  5 +-
 clap_generate/tests/completions/elvish.rs     |  5 +-
 clap_generate/tests/completions/fish.rs       | 17 +++---
 clap_generate/tests/completions/mod.rs        |  2 +-
 clap_generate/tests/completions/powershell.rs |  5 +-
 clap_generate/tests/completions/zsh.rs        | 25 +++++---
 clap_generate/tests/generate_completions.rs   |  9 +--
 clap_generate_fig/tests/completions/fig.rs    | 31 +++++-----
 clap_generate_fig/tests/completions/mod.rs    |  2 +-
 .../tests/generate_completions.rs             |  9 +--
 src/build/app/mod.rs                          | 10 ++++
 13 files changed, 104 insertions(+), 78 deletions(-)

diff --git a/clap_generate/src/lib.rs b/clap_generate/src/lib.rs
index ba234dd1..c97804b9 100644
--- a/clap_generate/src/lib.rs
+++ b/clap_generate/src/lib.rs
@@ -250,9 +250,7 @@ where
     G: Generator,
     S: Into<String>,
 {
-    // TODO: All the subcommands need to be built instead of just the top one
-    app._build();
-    app._build_bin_names();
+    app._build_all();
 
     gen.generate(app, buf)
 }
diff --git a/clap_generate/src/utils.rs b/clap_generate/src/utils.rs
index c877e609..1ae8281e 100644
--- a/clap_generate/src/utils.rs
+++ b/clap_generate/src/utils.rs
@@ -127,9 +127,8 @@ mod tests {
     use clap::Arg;
     use pretty_assertions::assert_eq;
 
-    fn common() -> App<'static> {
-        let mut app = App::new("myapp")
-            .version("3.0")
+    fn common_app() -> App<'static> {
+        App::new("myapp")
             .subcommand(
                 App::new("test").subcommand(App::new("config")).arg(
                     Arg::new("file")
@@ -141,16 +140,26 @@ mod tests {
                 ),
             )
             .subcommand(App::new("hello"))
-            .bin_name("my-app");
+            .bin_name("my-app")
+    }
 
-        app._build();
-        app._build_bin_names();
+    fn built() -> App<'static> {
+        let mut app = common_app();
+
+        app._build_all();
+        app
+    }
+
+    fn built_with_version() -> App<'static> {
+        let mut app = common_app().version("3.0");
+
+        app._build_all();
         app
     }
 
     #[test]
     fn test_subcommands() {
-        let app = common();
+        let app = built_with_version();
 
         assert_eq!(
             subcommands(&app),
@@ -164,7 +173,7 @@ mod tests {
 
     #[test]
     fn test_all_subcommands() {
-        let app = common();
+        let app = built_with_version();
 
         assert_eq!(
             all_subcommands(&app),
@@ -173,13 +182,14 @@ mod tests {
                 ("hello".to_string(), "my-app hello".to_string()),
                 ("help".to_string(), "my-app help".to_string()),
                 ("config".to_string(), "my-app test config".to_string()),
+                ("help".to_string(), "my-app test help".to_string()),
             ]
         );
     }
 
     #[test]
     fn test_find_subcommand_with_path() {
-        let app = common();
+        let app = built_with_version();
         let sc_app = find_subcommand_with_path(&app, "test config".split(' ').collect());
 
         assert_eq!(sc_app.get_name(), "config");
@@ -187,7 +197,7 @@ mod tests {
 
     #[test]
     fn test_flags() {
-        let app = common();
+        let app = built_with_version();
         let actual_flags = flags(&app);
 
         assert_eq!(actual_flags.len(), 2);
@@ -196,15 +206,29 @@ mod tests {
 
         let sc_flags = flags(find_subcommand_with_path(&app, vec!["test"]));
 
-        assert_eq!(sc_flags.len(), 3);
+        assert_eq!(sc_flags.len(), 2);
+        assert_eq!(sc_flags[0].get_long(), Some("file"));
+        assert_eq!(sc_flags[1].get_long(), Some("help"));
+    }
+
+    #[test]
+    fn test_flag_subcommand() {
+        let app = built();
+        let actual_flags = flags(&app);
+
+        assert_eq!(actual_flags.len(), 1);
+        assert_eq!(actual_flags[0].get_long(), Some("help"));
+
+        let sc_flags = flags(find_subcommand_with_path(&app, vec!["test"]));
+
+        assert_eq!(sc_flags.len(), 2);
         assert_eq!(sc_flags[0].get_long(), Some("file"));
         assert_eq!(sc_flags[1].get_long(), Some("help"));
-        assert_eq!(sc_flags[2].get_long(), Some("version"));
     }
 
     #[test]
     fn test_shorts() {
-        let app = common();
+        let app = built_with_version();
         let shorts = shorts_and_visible_aliases(&app);
 
         assert_eq!(shorts.len(), 2);
@@ -213,16 +237,15 @@ mod tests {
 
         let sc_shorts = shorts_and_visible_aliases(find_subcommand_with_path(&app, vec!["test"]));
 
-        assert_eq!(sc_shorts.len(), 4);
+        assert_eq!(sc_shorts.len(), 3);
         assert_eq!(sc_shorts[0], 'p');
         assert_eq!(sc_shorts[1], 'f');
         assert_eq!(sc_shorts[2], 'h');
-        assert_eq!(sc_shorts[3], 'V');
     }
 
     #[test]
     fn test_longs() {
-        let app = common();
+        let app = built_with_version();
         let longs = longs_and_visible_aliases(&app);
 
         assert_eq!(longs.len(), 2);
@@ -231,10 +254,9 @@ mod tests {
 
         let sc_longs = longs_and_visible_aliases(find_subcommand_with_path(&app, vec!["test"]));
 
-        assert_eq!(sc_longs.len(), 4);
+        assert_eq!(sc_longs.len(), 3);
         assert_eq!(sc_longs[0], "path");
         assert_eq!(sc_longs[1], "file");
         assert_eq!(sc_longs[2], "help");
-        assert_eq!(sc_longs[3], "version");
     }
 }
diff --git a/clap_generate/tests/completions/bash.rs b/clap_generate/tests/completions/bash.rs
index 8f388a80..5a1773b1 100644
--- a/clap_generate/tests/completions/bash.rs
+++ b/clap_generate/tests/completions/bash.rs
@@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
 fn build_app_with_name(s: &'static str) -> App<'static> {
     App::new(s)
         .version("3.0")
+        .setting(AppSettings::PropagateVersion)
         .about("Tests completions")
         .arg(
             Arg::new("file")
@@ -70,7 +71,7 @@ static BASH: &str = r#"_myapp() {
             return 0
             ;;
         myapp__help)
-            opts=" -h -V  --help --version  "
+            opts=" -h  --help  "
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0
@@ -173,7 +174,7 @@ static BASH_SPECIAL_CMDS: &str = r#"_my_app() {
             return 0
             ;;
         my_app__help)
-            opts=" -h -V  --help --version  "
+            opts=" -h  --help  "
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0
diff --git a/clap_generate/tests/completions/elvish.rs b/clap_generate/tests/completions/elvish.rs
index 6ab8d411..07b19f5f 100644
--- a/clap_generate/tests/completions/elvish.rs
+++ b/clap_generate/tests/completions/elvish.rs
@@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
 fn build_app_with_name(s: &'static str) -> App<'static> {
     App::new(s)
         .version("3.0")
+        .setting(AppSettings::PropagateVersion)
         .about("Tests completions")
         .arg(
             Arg::new("file")
@@ -66,8 +67,6 @@ set edit:completion:arg-completer[my_app] = [@words]{
         &'my_app;help'= {
             cand -h 'Print help information'
             cand --help 'Print help information'
-            cand -V 'Print version information'
-            cand --version 'Print version information'
         }
     ]
     $completions[$command]
@@ -145,8 +144,6 @@ set edit:completion:arg-completer[my_app] = [@words]{
         &'my_app;help'= {
             cand -h 'Print help information'
             cand --help 'Print help information'
-            cand -V 'Print version information'
-            cand --version 'Print version information'
         }
     ]
     $completions[$command]
diff --git a/clap_generate/tests/completions/fish.rs b/clap_generate/tests/completions/fish.rs
index 89fb05d6..8456027b 100644
--- a/clap_generate/tests/completions/fish.rs
+++ b/clap_generate/tests/completions/fish.rs
@@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
 fn build_app_with_name(s: &'static str) -> App<'static> {
     App::new(s)
         .version("3.0")
+        .setting(AppSettings::PropagateVersion)
         .about("Tests completions")
         .arg(
             Arg::new("file")
@@ -37,7 +38,6 @@ complete -c myapp -n "__fish_seen_subcommand_from test" -l case -d 'the case to
 complete -c myapp -n "__fish_seen_subcommand_from test" -s h -l help -d 'Print help information'
 complete -c myapp -n "__fish_seen_subcommand_from test" -s V -l version -d 'Print version information'
 complete -c myapp -n "__fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
-complete -c myapp -n "__fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
 "#;
 
 #[test]
@@ -74,7 +74,6 @@ complete -c my_app -n "__fish_seen_subcommand_from some_cmd" -s V -l version -d
 complete -c my_app -n "__fish_seen_subcommand_from some-cmd-with-hypens" -s h -l help -d 'Print help information'
 complete -c my_app -n "__fish_seen_subcommand_from some-cmd-with-hypens" -s V -l version -d 'Print version information'
 complete -c my_app -n "__fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
-complete -c my_app -n "__fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
 "#;
 
 #[test]
@@ -189,12 +188,14 @@ complete -c my_app -n "__fish_use_subcommand" -f -a "help" -d 'Print this messag
 complete -c my_app -n "__fish_seen_subcommand_from test" -l case -d 'the case to test' -r
 complete -c my_app -n "__fish_seen_subcommand_from test" -s h -l help -d 'Print help information'
 complete -c my_app -n "__fish_seen_subcommand_from test" -s V -l version -d 'Print version information'
-complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd" -s h -l help -d 'Print help information'
-complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd" -s V -l version -d 'Print version information'
-complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd" -f -a "sub_cmd" -d 'sub-subcommand'
+complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd; and not __fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
+complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd; and not __fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
+complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd; and not __fish_seen_subcommand_from help" -f -a "sub_cmd" -d 'sub-subcommand'
+complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd; and not __fish_seen_subcommand_from help" -f -a "help" -d 'Print this message or the help of the given subcommand(s)'
 complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -l config -d 'the other case to test' -r
-complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -l help -d 'Print help information'
-complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -l version -d 'Print version information'
+complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -s h -l help -d 'Print help information'
+complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -s V -l version -d 'Print version information'
+complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
+complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
 complete -c my_app -n "__fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
-complete -c my_app -n "__fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
 "#;
diff --git a/clap_generate/tests/completions/mod.rs b/clap_generate/tests/completions/mod.rs
index 5a5dd050..b0cf8e2a 100644
--- a/clap_generate/tests/completions/mod.rs
+++ b/clap_generate/tests/completions/mod.rs
@@ -1,4 +1,4 @@
-use clap::{App, Arg, ValueHint};
+use clap::{App, AppSettings, Arg, ValueHint};
 use clap_generate::{generate, generators::*};
 use std::fmt;
 
diff --git a/clap_generate/tests/completions/powershell.rs b/clap_generate/tests/completions/powershell.rs
index ca063acd..de8ffa1e 100644
--- a/clap_generate/tests/completions/powershell.rs
+++ b/clap_generate/tests/completions/powershell.rs
@@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
 fn build_app_with_name(s: &'static str) -> App<'static> {
     App::new(s)
         .version("3.0")
+        .setting(AppSettings::PropagateVersion)
         .about("Tests completions")
         .arg(
             Arg::new("file")
@@ -82,8 +83,6 @@ Register-ArgumentCompleter -Native -CommandName 'my_app' -ScriptBlock {
         'my_app;help' {
             [CompletionResult]::new('-h', 'h', [CompletionResultType]::ParameterName, 'Print help information')
             [CompletionResult]::new('--help', 'help', [CompletionResultType]::ParameterName, 'Print help information')
-            [CompletionResult]::new('-V', 'V', [CompletionResultType]::ParameterName, 'Print version information')
-            [CompletionResult]::new('--version', 'version', [CompletionResultType]::ParameterName, 'Print version information')
             break
         }
     })
@@ -174,8 +173,6 @@ Register-ArgumentCompleter -Native -CommandName 'my_app' -ScriptBlock {
         'my_app;help' {
             [CompletionResult]::new('-h', 'h', [CompletionResultType]::ParameterName, 'Print help information')
             [CompletionResult]::new('--help', 'help', [CompletionResultType]::ParameterName, 'Print help information')
-            [CompletionResult]::new('-V', 'V', [CompletionResultType]::ParameterName, 'Print version information')
-            [CompletionResult]::new('--version', 'version', [CompletionResultType]::ParameterName, 'Print version information')
             break
         }
     })
diff --git a/clap_generate/tests/completions/zsh.rs b/clap_generate/tests/completions/zsh.rs
index 937946ea..062cd8f5 100644
--- a/clap_generate/tests/completions/zsh.rs
+++ b/clap_generate/tests/completions/zsh.rs
@@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
 fn build_app_with_name(s: &'static str) -> App<'static> {
     App::new(s)
         .version("3.0")
+        .setting(AppSettings::PropagateVersion)
         .about("Tests completions")
         .arg(
             Arg::new("file")
@@ -73,8 +74,6 @@ _arguments "${_arguments_options[@]}" \
 _arguments "${_arguments_options[@]}" \
 '-h[Print help information]' \
 '--help[Print help information]' \
-'-V[Print version information]' \
-'--version[Print version information]' \
 && ret=0
 ;;
         esac
@@ -195,8 +194,6 @@ _arguments "${_arguments_options[@]}" \
 _arguments "${_arguments_options[@]}" \
 '-h[Print help information]' \
 '--help[Print help information]' \
-'-V[Print version information]' \
-'--version[Print version information]' \
 && ret=0
 ;;
         esac
@@ -364,8 +361,6 @@ _my_app() {
 _arguments "${_arguments_options[@]}" \
 '-h[Print help information]' \
 '--help[Print help information]' \
-'-V[Print version information]' \
-'--version[Print version information]' \
 ":: :_my_app__second_commands" \
 "*::: :->second" \
 && ret=0
@@ -378,8 +373,16 @@ _arguments "${_arguments_options[@]}" \
         case $line[1] in
             (third)
 _arguments "${_arguments_options[@]}" \
-'--help[Print help information]' \
 '--version[Print version information]' \
+'-h[Print help information]' \
+'--help[Print help information]' \
+&& ret=0
+;;
+(help)
+_arguments "${_arguments_options[@]}" \
+'--version[Print version information]' \
+'-h[Print help information]' \
+'--help[Print help information]' \
 && ret=0
 ;;
         esac
@@ -390,8 +393,6 @@ esac
 _arguments "${_arguments_options[@]}" \
 '-h[Print help information]' \
 '--help[Print help information]' \
-'-V[Print version information]' \
-'--version[Print version information]' \
 && ret=0
 ;;
         esac
@@ -412,10 +413,16 @@ _my_app__help_commands() {
     local commands; commands=()
     _describe -t commands 'my_app help commands' commands "$@"
 }
+(( $+functions[_my_app__second__help_commands] )) ||
+_my_app__second__help_commands() {
+    local commands; commands=()
+    _describe -t commands 'my_app second help commands' commands "$@"
+}
 (( $+functions[_my_app__second_commands] )) ||
 _my_app__second_commands() {
     local commands; commands=(
 'third:' \
+'help:Print this message or the help of the given subcommand(s)' \
     )
     _describe -t commands 'my_app second commands' commands "$@"
 }
diff --git a/clap_generate/tests/generate_completions.rs b/clap_generate/tests/generate_completions.rs
index 4d75e8ec..f9446cfe 100644
--- a/clap_generate/tests/generate_completions.rs
+++ b/clap_generate/tests/generate_completions.rs
@@ -5,13 +5,8 @@ use std::io;
 #[test]
 fn generate_completions() {
     let mut app = App::new("test_app")
-        .arg(
-            Arg::new("config")
-                .short('c')
-                .conflicts_with("v")
-                .global(true),
-        )
-        .arg(Arg::new("v").short('v'))
+        .arg(Arg::new("config").short('c').global(true))
+        .arg(Arg::new("v").short('v').conflicts_with("config"))
         .subcommand(
             App::new("test")
                 .about("Subcommand")
diff --git a/clap_generate_fig/tests/completions/fig.rs b/clap_generate_fig/tests/completions/fig.rs
index 96643276..49124646 100644
--- a/clap_generate_fig/tests/completions/fig.rs
+++ b/clap_generate_fig/tests/completions/fig.rs
@@ -8,6 +8,7 @@ fn build_app() -> App<'static> {
 fn build_app_with_name(s: &'static str) -> App<'static> {
     App::new(s)
         .version("3.0")
+        .setting(AppSettings::PropagateVersion)
         .about("Tests completions")
         .arg(
             Arg::new("file")
@@ -64,10 +65,6 @@ static FIG: &str = r#"const completion: Fig.Spec = {
           name: ["-h", "--help"],
           description: "Print help information",
         },
-        {
-          name: ["-V", "--version"],
-          description: "Print version information",
-        },
       ],
     },
   ],
@@ -179,10 +176,6 @@ static FIG_SPECIAL_CMDS: &str = r#"const completion: Fig.Spec = {
           name: ["-h", "--help"],
           description: "Print help information",
         },
-        {
-          name: ["-V", "--version"],
-          description: "Print version information",
-        },
       ],
     },
   ],
@@ -413,11 +406,25 @@ static FIG_SUB_SUBCMDS: &str = r#"const completion: Fig.Spec = {
               },
             },
             {
-              name: "--help",
+              name: ["-h", "--help"],
               description: "Print help information",
             },
             {
-              name: "--version",
+              name: ["-V", "--version"],
+              description: "Print version information",
+            },
+          ],
+        },
+        {
+          name: "help",
+          description: "Print this message or the help of the given subcommand(s)",
+          options: [
+            {
+              name: ["-h", "--help"],
+              description: "Print help information",
+            },
+            {
+              name: ["-V", "--version"],
               description: "Print version information",
             },
           ],
@@ -442,10 +449,6 @@ static FIG_SUB_SUBCMDS: &str = r#"const completion: Fig.Spec = {
           name: ["-h", "--help"],
           description: "Print help information",
         },
-        {
-          name: ["-V", "--version"],
-          description: "Print version information",
-        },
       ],
     },
   ],
diff --git a/clap_generate_fig/tests/completions/mod.rs b/clap_generate_fig/tests/completions/mod.rs
index 4cfa60af..9e5c593f 100644
--- a/clap_generate_fig/tests/completions/mod.rs
+++ b/clap_generate_fig/tests/completions/mod.rs
@@ -1,4 +1,4 @@
-use clap::{App, Arg, ValueHint};
+use clap::{App, AppSettings, Arg, ValueHint};
 use clap_generate::{generate, generators::*};
 use std::fmt;
 
diff --git a/clap_generate_fig/tests/generate_completions.rs b/clap_generate_fig/tests/generate_completions.rs
index 21021717..276cc00c 100644
--- a/clap_generate_fig/tests/generate_completions.rs
+++ b/clap_generate_fig/tests/generate_completions.rs
@@ -6,13 +6,8 @@ use std::io;
 #[test]
 fn generate_completions() {
     let mut app = App::new("test_app")
-        .arg(
-            Arg::new("config")
-                .short('c')
-                .conflicts_with("v")
-                .global(true),
-        )
-        .arg(Arg::new("v").short('v'))
+        .arg(Arg::new("config").short('c').global(true))
+        .arg(Arg::new("v").short('v').conflicts_with("config"))
         .subcommand(
             App::new("test")
                 .about("Subcommand")
diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs
index 9409d732..6ba45b09 100644
--- a/src/build/app/mod.rs
+++ b/src/build/app/mod.rs
@@ -2270,6 +2270,16 @@ impl<'help> App<'help> {
         Ok(matcher.into_inner())
     }
 
+    // used in clap_generate (https://github.com/clap-rs/clap_generate)
+    #[doc(hidden)]
+    pub fn _build_all(&mut self) {
+        self._build();
+        for subcmd in self.get_subcommands_mut() {
+            subcmd._build();
+        }
+        self._build_bin_names();
+    }
+
     // used in clap_generate (https://github.com/clap-rs/clap_generate)
     #[doc(hidden)]
     pub fn _build(&mut self) {