From 007edac1452df7abd19bbf4f2c0ae9091bf754e2 Mon Sep 17 00:00:00 2001
From: Isaac Oscar Gariano <IsaacOscar@live.com.au>
Date: Sun, 3 Aug 2025 10:07:08 +1000
Subject: [PATCH] Make argparse reject supplying a validator for boolean flags

Specifically, this commit simply makes argparse issue an error if you use the !
syntax to define a validation script on an option that does not take any
arguments. For example, "argparse foo!exit -- --foo" is now an error. This was
previously accepted, despite that fact that the code after ! would never be
executed (the ! code is only executed when an option is given a value).

Alternatively, ! validation scripts could be made to execute even when no value
was provided, but this break existing code that uses them with flags that take
optional values.
---
 doc_src/cmds/argparse.rst  | 4 +++-
 src/builtins/argparse.rs   | 8 ++++++++
 tests/checks/argparse.fish | 5 +++++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/doc_src/cmds/argparse.rst b/doc_src/cmds/argparse.rst
index cd5f052aa..96453e5a4 100644
--- a/doc_src/cmds/argparse.rst
+++ b/doc_src/cmds/argparse.rst
@@ -156,7 +156,9 @@ Each option specification consists of:
 
 - Optionally a ``&``, indicating that the option and any attached values are not to be saved in ``$argv`` or ``$argv_opts``. This does not affect the the ``_flag_`` variables.
 
-- Optionally a ``!`` followed by fish script to validate the value. Typically this will be a function to run. If the exit status is zero the value for the flag is valid. If non-zero the value is invalid. Any error messages should be written to stdout (not stderr). See the section on :ref:`Flag Value Validation <flag-value-validation>` for more information.
+- Nothing if the flag is a boolean that takes no argument, or
+
+    - ``!`` followed by fish script to validate the value. Typically this will be a function to run. If the exit status is zero the value for the flag is valid. If non-zero the value is invalid. Any error messages should be written to stdout (not stderr). See the section on :ref:`Flag Value Validation <flag-value-validation>` for more information.
 
 See the :doc:`fish_opt <fish_opt>` command for a friendlier but more verbose way to create option specifications.
 
diff --git a/src/builtins/argparse.rs b/src/builtins/argparse.rs
index c84cba249..7584fed98 100644
--- a/src/builtins/argparse.rs
+++ b/src/builtins/argparse.rs
@@ -248,6 +248,14 @@ fn parse_flag_modifiers<'args>(
     }
 
     if s.char_at(0) == '!' {
+        if opt_spec.arg_type == ArgType::NoArgument {
+            streams.err.append(wgettext_fmt!(
+                BUILTIN_ERR_INVALID_OPT_SPEC,
+                opts.name,
+                option_spec,
+                s.char_at(0)
+            ));
+        }
         s = s.slice_from(1);
         opt_spec.validation_command = s;
         // Move cursor to the end so we don't expect a long flag.
diff --git a/tests/checks/argparse.fish b/tests/checks/argparse.fish
index f471c2ee4..420236238 100644
--- a/tests/checks/argparse.fish
+++ b/tests/checks/argparse.fish
@@ -678,6 +678,11 @@ begin
     #CHECKERR: (Type 'help argparse' for related documentation)
 end
 
+begin
+    argparse f!echo hello -- -f
+    #CHECKERR: argparse: Invalid option spec 'f!echo' at char '!'
+end
+
 begin
     argparse --ignore-unknown h i -- -hoa -oia
     echo -- $argv