diff --git a/args.hxx b/args.hxx index d4711ac..361e821 100644 --- a/args.hxx +++ b/args.hxx @@ -1441,6 +1441,45 @@ namespace args virtual void ParseValue(const std::vector &value) = 0; }; + /** (INTERNAL) Find a flag string claimed by two distinct options. + * + * When two separate options answer to the same short or long flag the + * configuration is ambiguous: Match() returns the first one it finds and + * the second option can never be reached. Returns the offending flag with + * its prefix, or an empty string when every matcher is unique. The owner + * comparison keeps a single option that is shared between groups from + * reporting itself. + */ + inline std::string FindAmbiguousFlag(const std::vector &flags, const std::string &shortPrefix, const std::string &longPrefix) + { + std::unordered_map shortOwners; + std::unordered_map longOwners; + for (const FlagBase *flag : flags) + { + for (const EitherFlag &each : flag->GetMatcher().GetFlagStrings()) + { + if (each.isShort) + { + const auto inserted = shortOwners.emplace(each.shortFlag, flag); + if (!inserted.second && inserted.first->second != flag) + { + return each.str(shortPrefix, longPrefix); + } + } + else + { + const auto inserted = longOwners.emplace(each.longFlag, flag); + if (!inserted.second && inserted.first->second != flag) + { + return each.str(shortPrefix, longPrefix); + } + } + } + } + + return {}; + } + /** Base class for value-accepting flag options */ class ValueFlagBase : public FlagBase @@ -3260,6 +3299,22 @@ namespace args } } + if (!readCompletion) + { + const std::string ambiguous = FindAmbiguousFlag(GetAllFlags(), shortprefix, longprefix); + if (!ambiguous.empty()) + { + const std::string problem = "Flag '" + ambiguous + "' is used by more than one option"; +#ifndef ARGS_NOEXCEPT + throw UsageError(problem); +#else + error = Error::Usage; + errorMsg = problem; + return end; +#endif + } + } + Validate(shortprefix, longprefix); return end; } @@ -3615,6 +3670,21 @@ namespace args } auto it = parser->Parse(args.begin(), args.end()); + + const std::string ambiguous = FindAmbiguousFlag(GetAllFlags(), parser->ShortPrefix(), parser->LongPrefix()); + if (!ambiguous.empty()) + { +#ifndef ARGS_NOEXCEPT + throw UsageError("Flag '" + ambiguous + "' is used by more than one option"); +#else + if (GetError() == Error::None) + { + error = Error::Usage; + errorMsg = "Flag '" + ambiguous + "' is used by more than one option"; + } +#endif + } + command.Validate(parser->ShortPrefix(), parser->LongPrefix()); kicked.assign(it, args.end()); diff --git a/meson.build b/meson.build index 58c2715..26e90e6 100644 --- a/meson.build +++ b/meson.build @@ -35,6 +35,7 @@ test_names = [ 'count_flag', 'custom_types', 'default_values', + 'duplicate_matcher', 'extra_positionals', 'get_assignable', 'get_program_line', @@ -57,6 +58,7 @@ test_names = [ 'noexcept_completion', 'noexcept_completion_bad_cword', 'noexcept_completion_value_oob', + 'noexcept_duplicate_matcher', 'noexcept_flag_error_no_shadow', 'noexcept_matcher_validation', 'noexcept_mode', diff --git a/test/duplicate_matcher.cxx b/test/duplicate_matcher.cxx new file mode 100644 index 0000000..46c506a --- /dev/null +++ b/test/duplicate_matcher.cxx @@ -0,0 +1,60 @@ +/* Copyright (c) Taylor Richberger + * This code is released under the license described in the LICENSE file + */ + +#include "test_common.hxx" + +#include + +#include "test_helpers.hxx" + +int main() +{ + // Two options that answer to the same short flag are ambiguous: the + // second one can never be reached, so parsing should report it. + test::require_throws_as([] { + args::ArgumentParser parser("test"); + args::ValueFlag a(parser, "a", "", {'d', "arg1"}); + args::ValueFlag b(parser, "b", "", {'d', "arg2"}); + parser.ParseArgs(std::vector{}); + }); + + // The same holds when the shared flag is the long form. + test::require_throws_as([] { + args::ArgumentParser parser("test"); + args::Flag a(parser, "a", "", {'a', "same"}); + args::Flag b(parser, "b", "", {'b', "same"}); + parser.ParseArgs(std::vector{}); + }); + + // Distinct matchers, including multi-name aliases, must not be flagged. + test::require_nothrow([] { + args::ArgumentParser parser("test"); + args::HelpFlag help(parser, "help", "", {'h', "help"}); + args::Flag verbose(parser, "verbose", "", {'v', "verbose"}); + parser.ParseArgs(std::vector{"-v"}); + }); + + // Sibling commands may reuse a flag name; only one is ever reachable. + test::require_nothrow([] { + args::ArgumentParser parser("test"); + args::Command one(parser, "one", ""); + args::Flag vone(one, "v", "", {'v'}); + args::Command two(parser, "two", ""); + args::Flag vtwo(two, "v", "", {'v'}); + parser.ParseArgs(std::vector{"one", "-v"}); + }); + + // The same ambiguity inside a subcommand's own flags is caught too. + test::require_throws_as([] { + args::ArgumentParser parser("test"); + args::Command cmd(parser, "cmd", "", [](args::Subparser &s) { + args::Flag x(s, "x", "", {'v'}); + args::Flag y(s, "y", "", {'v'}); + s.Parse(); + }); + parser.ParseArgs(std::vector{"cmd"}); + }); + + return 0; +} diff --git a/test/noexcept_duplicate_matcher.cxx b/test/noexcept_duplicate_matcher.cxx new file mode 100644 index 0000000..0702d83 --- /dev/null +++ b/test/noexcept_duplicate_matcher.cxx @@ -0,0 +1,42 @@ +/* Copyright (c) Taylor Richberger + * This code is released under the license described in the LICENSE file + */ + +#define ARGS_NOEXCEPT +#include "test_common.hxx" + +#include + +#include "test_helpers.hxx" + +int main() +{ + // A short flag shared by two options surfaces as a usage error. + { + args::ArgumentParser parser("test"); + args::Flag a(parser, "a", "", {'d', "arg1"}); + args::Flag b(parser, "b", "", {'d', "arg2"}); + parser.ParseArgs(std::vector{}); + test::require(parser.GetError() == args::Error::Usage); + } + + // And so does a shared long flag. + { + args::ArgumentParser parser("test"); + args::Flag a(parser, "a", "", {'a', "same"}); + args::Flag b(parser, "b", "", {'b', "same"}); + parser.ParseArgs(std::vector{}); + test::require(parser.GetError() == args::Error::Usage); + } + + // Unique matchers leave the parser error-free. + { + args::ArgumentParser parser("test"); + args::HelpFlag help(parser, "help", "", {'h', "help"}); + args::Flag verbose(parser, "verbose", "", {'v', "verbose"}); + parser.ParseArgs(std::vector{"-v"}); + test::require(parser.GetError() == args::Error::None); + } + + return 0; +}