Rollup merge of #143452 - Kobzol:completions-fix, r=jieyouxu

Fix CLI completion check in `tidy`

The list of CLI completion files that were generated and that were checked by `x test tidy` was not synced. Recently, some PR only updated some of the files, which caused the rest of the files (not checked by `x test tidy`) to be dirty on `master`. This PR fixes the logic in bootstrap to always synchronize the list of completion paths.

Fixes: #143451

r? `@jieyouxu`

Author: workingjubilee

src/bootstrap/src/core/build_steps/run.rs

@@ -5,6 +5,8 @@
 
 use std::path::PathBuf;
 
+use clap_complete::{Generator, shells};
+
 use crate::core::build_steps::dist::distdir;
 use crate::core::build_steps::test;
 use crate::core::build_steps::tool::{self, SourceType, Tool};
@@ -285,36 +287,35 @@ impl Step for GenerateWindowsSys {
     }
 }
 
+/// Return tuples of (shell, file containing completions).
+pub fn get_completion_paths(builder: &Builder<'_>) -> Vec<(&'static dyn Generator, PathBuf)> {
+    vec![
+        (&shells::Bash as &'static dyn Generator, builder.src.join("src/etc/completions/x.py.sh")),
+        (&shells::Zsh, builder.src.join("src/etc/completions/x.py.zsh")),
+        (&shells::Fish, builder.src.join("src/etc/completions/x.py.fish")),
+        (&shells::PowerShell, builder.src.join("src/etc/completions/x.py.ps1")),
+        (&shells::Bash, builder.src.join("src/etc/completions/x.sh")),
+        (&shells::Zsh, builder.src.join("src/etc/completions/x.zsh")),
+        (&shells::Fish, builder.src.join("src/etc/completions/x.fish")),
+        (&shells::PowerShell, builder.src.join("src/etc/completions/x.ps1")),
+    ]
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct GenerateCompletions;
 
-macro_rules! generate_completions {
-    ( $( ( $shell:ident, $filename:expr ) ),* ) => {
-        $(
-            if let Some(comp) = get_completion($shell, &$filename) {
-                std::fs::write(&$filename, comp).expect(&format!("writing {} completion", stringify!($shell)));
-            }
-        )*
-    };
-}
-
 impl Step for GenerateCompletions {
     type Output = ();
 
     /// Uses `clap_complete` to generate shell completions.
     fn run(self, builder: &Builder<'_>) {
-        use clap_complete::shells::{Bash, Fish, PowerShell, Zsh};
-
-        generate_completions!(
-            (Bash, builder.src.join("src/etc/completions/x.py.sh")),
-            (Zsh, builder.src.join("src/etc/completions/x.py.zsh")),
-            (Fish, builder.src.join("src/etc/completions/x.py.fish")),
-            (PowerShell, builder.src.join("src/etc/completions/x.py.ps1")),
-            (Bash, builder.src.join("src/etc/completions/x.sh")),
-            (Zsh, builder.src.join("src/etc/completions/x.zsh")),
-            (Fish, builder.src.join("src/etc/completions/x.fish")),
-            (PowerShell, builder.src.join("src/etc/completions/x.ps1"))
-        );
+        for (shell, path) in get_completion_paths(builder) {
+            if let Some(comp) = get_completion(shell, &path) {
+                std::fs::write(&path, comp).unwrap_or_else(|e| {
+                    panic!("writing completion into {} failed: {e:?}", path.display())
+                });
+            }
+        }
     }
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {

src/bootstrap/src/core/build_steps/test.rs

@@ -8,12 +8,11 @@ use std::ffi::{OsStr, OsString};
 use std::path::{Path, PathBuf};
 use std::{env, fs, iter};
 
-use clap_complete::shells;
-
 use crate::core::build_steps::compile::{Std, run_cargo};
 use crate::core::build_steps::doc::DocumentationFormat;
 use crate::core::build_steps::gcc::{Gcc, add_cg_gcc_cargo_flags};
 use crate::core::build_steps::llvm::get_llvm_version;
+use crate::core::build_steps::run::get_completion_paths;
 use crate::core::build_steps::synthetic_targets::MirOptPanicAbortSyntheticTarget;
 use crate::core::build_steps::tool::{self, COMPILETEST_ALLOW_FEATURES, SourceType, Tool};
 use crate::core::build_steps::toolstate::ToolState;
@@ -1153,14 +1152,12 @@ HELP: to skip test's attempt to check tidiness, pass `--skip src/tools/tidy` to
         cmd.delay_failure().run(builder);
 
         builder.info("x.py completions check");
-        let [bash, zsh, fish, powershell] = ["x.py.sh", "x.py.zsh", "x.py.fish", "x.py.ps1"]
-            .map(|filename| builder.src.join("src/etc/completions").join(filename));
+        let completion_paths = get_completion_paths(builder);
         if builder.config.cmd.bless() {
             builder.ensure(crate::core::build_steps::run::GenerateCompletions);
-        } else if get_completion(shells::Bash, &bash).is_some()
-            || get_completion(shells::Fish, &fish).is_some()
-            || get_completion(shells::PowerShell, &powershell).is_some()
-            || crate::flags::get_completion(shells::Zsh, &zsh).is_some()
+        } else if completion_paths
+            .into_iter()
+            .any(|(shell, path)| get_completion(shell, &path).is_some())
         {
             eprintln!(
                 "x.py completions were changed; run `x.py run generate-completions` to update them"

src/bootstrap/src/core/config/flags.rs

@@ -6,6 +6,7 @@
 use std::path::{Path, PathBuf};
 
 use clap::{CommandFactory, Parser, ValueEnum};
+use clap_complete::Generator;
 #[cfg(feature = "tracing")]
 use tracing::instrument;
 
@@ -644,7 +645,7 @@ impl Subcommand {
 
 /// Returns the shell completion for a given shell, if the result differs from the current
 /// content of `path`. If `path` does not exist, always returns `Some`.
-pub fn get_completion<G: clap_complete::Generator>(shell: G, path: &Path) -> Option<String> {
+pub fn get_completion(shell: &dyn Generator, path: &Path) -> Option<String> {
     let mut cmd = Flags::command();
     let current = if !path.exists() {
         String::new()
@@ -662,7 +663,12 @@ pub fn get_completion<G: clap_complete::Generator>(shell: G, path: &Path) -> Opt
         .expect("file name should be UTF-8")
         .rsplit_once('.')
         .expect("file name should have an extension");
-    clap_complete::generate(shell, &mut cmd, bin_name, &mut buf);
+
+    // We sort of replicate `clap_complete::generate` here, because we want to call it with
+    // `&dyn Generator`, but that function requires `G: Generator` instead.
+    cmd.set_bin_name(bin_name);
+    cmd.build();
+    shell.generate(&cmd, &mut buf);
     if buf == current.as_bytes() {
         return None;
     }

src/etc/completions/x.fish

@@ -293,7 +293,7 @@ complete -c x -n "__fish_x_using_subcommand doc" -l skip-std-check-if-no-downloa
 complete -c x -n "__fish_x_using_subcommand doc" -s h -l help -d 'Print help (see more with \'--help\')'
 complete -c x -n "__fish_x_using_subcommand test" -l test-args -d 'extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)' -r
 complete -c x -n "__fish_x_using_subcommand test" -l compiletest-rustc-args -d 'extra options to pass the compiler when running compiletest tests' -r
-complete -c x -n "__fish_x_using_subcommand test" -l extra-checks -d 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell)' -r
+complete -c x -n "__fish_x_using_subcommand test" -l extra-checks -d 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell, shell:lint, cpp, cpp:fmt, spellcheck, spellcheck:fix)' -r
 complete -c x -n "__fish_x_using_subcommand test" -l compare-mode -d 'mode describing what file the actual ui output will be compared to' -r
 complete -c x -n "__fish_x_using_subcommand test" -l pass -d 'force {check,build,run}-pass tests to this mode' -r
 complete -c x -n "__fish_x_using_subcommand test" -l run -d 'whether to execute run-* tests' -r

src/etc/completions/x.ps1

@@ -339,7 +339,7 @@ Register-ArgumentCompleter -Native -CommandName 'x' -ScriptBlock {
         'x;test' {
             [CompletionResult]::new('--test-args', '--test-args', [CompletionResultType]::ParameterName, 'extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)')
             [CompletionResult]::new('--compiletest-rustc-args', '--compiletest-rustc-args', [CompletionResultType]::ParameterName, 'extra options to pass the compiler when running compiletest tests')
-            [CompletionResult]::new('--extra-checks', '--extra-checks', [CompletionResultType]::ParameterName, 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell)')
+            [CompletionResult]::new('--extra-checks', '--extra-checks', [CompletionResultType]::ParameterName, 'comma-separated list of other files types to check (accepts py, py:lint, py:fmt, shell, shell:lint, cpp, cpp:fmt, spellcheck, spellcheck:fix)')
             [CompletionResult]::new('--compare-mode', '--compare-mode', [CompletionResultType]::ParameterName, 'mode describing what file the actual ui output will be compared to')
             [CompletionResult]::new('--pass', '--pass', [CompletionResultType]::ParameterName, 'force {check,build,run}-pass tests to this mode')
             [CompletionResult]::new('--run', '--run', [CompletionResultType]::ParameterName, 'whether to execute run-* tests')

src/etc/completions/x.zsh

@@ -338,7 +338,7 @@ _arguments "${_arguments_options[@]}" : \
 _arguments "${_arguments_options[@]}" : \
 '*--test-args=[extra arguments to be passed for the test tool being used (e.g. libtest, compiletest or rustdoc)]:ARGS:_default' \
 '*--compiletest-rustc-args=[extra options to pass the compiler when running compiletest tests]:ARGS:_default' \
-'--extra-checks=[comma-separated list of other files types to check (accepts py, py\:lint, py\:fmt, shell)]:EXTRA_CHECKS:_default' \
+'--extra-checks=[comma-separated list of other files types to check (accepts py, py\:lint, py\:fmt, shell, shell\:lint, cpp, cpp\:fmt, spellcheck, spellcheck\:fix)]:EXTRA_CHECKS:_default' \
 '--compare-mode=[mode describing what file the actual ui output will be compared to]:COMPARE MODE:_default' \
 '--pass=[force {check,build,run}-pass tests to this mode]:check | build | run:_default' \
 '--run=[whether to execute run-* tests]:auto | always | never:_default' \