[4/6] parse-options: detect ambiguous self-negation

Commit Message

René Scharfe Feb. 24, 2024, 9:29 p.m. UTC

Git currently does not detect the ambiguity of an option that starts
with "no" like --notes and its negated form if given just --n or --no.
All Git commands with such options have other negatable options, and
we detect the ambiguity with them, so that's currently only a potential
problem for scripts that use git rev-parse --parseopt.

Let's fix it nevertheless, as there's no need for that confusion.  To
detect the ambiguity we have to loosen the check in register_abbrev(),
as an option is considered an alias of itself.  Add non-matching
negation flags as a criterion to recognize an option being ambiguous
with its negated form.

And we need to keep going after finding a non-negated option as an
abbreviated candidate and perform the negation checks in the same
loop.
---
 parse-options.c               |  3 +--
 t/t1502-rev-parse-parseopt.sh | 11 +++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

--
2.44.0

Patch

diff --git a/parse-options.c b/parse-options.c
index 398ebaef14..008c0f32cf 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -363,7 +363,7 @@  static void register_abbrev(struct parse_opt_ctx_t *p,
 	if (p->flags & PARSE_OPT_KEEP_UNKNOWN_OPT)
 		return;
 	if (abbrev->option &&
-	    !is_alias(p, abbrev->option, option)) {
+	    !(abbrev->flags == flags && is_alias(p, abbrev->option, option))) {
 		/*
 		 * If this is abbreviated, it is
 		 * ambiguous. So when there is no
@@ -406,7 +406,6 @@  static enum parse_opt_result parse_long_opt(
 			if (!strncmp(long_name, arg, arg_end - arg)) {
 				register_abbrev(p, options, flags ^ opt_flags,
 						&abbrev, &ambiguous);
-				continue;
 			}
 			/* negation allowed? */
 			if (options->flags & PARSE_OPT_NONEG)
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index f0737593c3..b754b9fd74 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -322,4 +322,15 @@  check_invalid_long_option optionspec-neg --no-positive-only
 check_invalid_long_option optionspec-neg --negative
 check_invalid_long_option optionspec-neg --no-no-negative

+test_expect_success 'ambiguous: --no matches both --noble and --no-noble' '
+	cat >spec <<-\EOF &&
+	some-command [options]
+	--
+	noble The feudal switch.
+	EOF
+	test_expect_code 129 env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
+	git rev-parse --parseopt -- <spec 2>err --no &&
+	grep "error: ambiguous option: no (could be --noble or --no-noble)" err
+'
+
 test_done