From 9f3ca443e41ce1a7ff169543915708994ec5f22d Mon Sep 17 00:00:00 2001
From: Martchus <martchus@gmx.net>
Date: Tue, 15 Nov 2016 22:02:40 +0100
Subject: [PATCH] Make ArgumentParser::verifyArgs() more strict

---
 application/argumentparser.cpp | 16 ++++++++--------
 application/argumentparser.h   |  2 +-
 tests/argumentparsertests.cpp  |  6 +++---
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/application/argumentparser.cpp b/application/argumentparser.cpp
index a435d2a..319ec13 100644
--- a/application/argumentparser.cpp
+++ b/application/argumentparser.cpp
@@ -412,7 +412,7 @@ void ArgumentParser::parseArgs(int argc, const char *const *argv)
  */
 void ArgumentParser::readArgs(int argc, const char * const *argv)
 {
-    IF_DEBUG_BUILD(verifyArgs(m_mainArgs);)
+    IF_DEBUG_BUILD(verifyArgs(m_mainArgs, std::vector<char>(), std::vector<const char *>());)
     m_actualArgc = 0;
     if(argc) {
         // the first argument is the executable name
@@ -491,14 +491,12 @@ bool ArgumentParser::isUncombinableMainArgPresent() const
  *  - Verifies the sub arguments, too.
  *  - For debugging purposes only; hence only available in debug builds.
  */
-void ApplicationUtilities::ArgumentParser::verifyArgs(const ArgumentVector &args)
+void ApplicationUtilities::ArgumentParser::verifyArgs(const ArgumentVector &args, vector<char> abbreviations, vector<const char *> names)
 {
     vector<const Argument *> verifiedArgs;
     verifiedArgs.reserve(args.size());
-    vector<char> abbreviations;
-    abbreviations.reserve(args.size());
-    vector<string> names;
-    names.reserve(args.size());
+    abbreviations.reserve(abbreviations.size() + args.size());
+    names.reserve(names.size() + args.size());
     bool hasImplicit = false;
     for(const Argument *arg : args) {
         assert(find(verifiedArgs.cbegin(), verifiedArgs.cend(), arg) == verifiedArgs.cend());
@@ -508,10 +506,12 @@ void ApplicationUtilities::ArgumentParser::verifyArgs(const ArgumentVector &args
         hasImplicit |= arg->isImplicit();
         assert(!arg->abbreviation() || find(abbreviations.cbegin(), abbreviations.cend(), arg->abbreviation()) == abbreviations.cend());
         abbreviations.push_back(arg->abbreviation());
-        assert(!arg->name() || find(names.cbegin(), names.cend(), arg->name()) == names.cend());
+        assert(!arg->name() || find_if(names.cbegin(), names.cend(), [arg] (const char *name) { return !strcmp(arg->name(), name); }) == names.cend());
         assert(arg->requiredValueCount() == 0 || arg->subArguments().size() == 0);
         names.emplace_back(arg->name());
-        verifyArgs(arg->subArguments());
+    }
+    for(const Argument *arg : args) {
+        verifyArgs(arg->subArguments(), abbreviations, names);
     }
 }
 #endif
diff --git a/application/argumentparser.h b/application/argumentparser.h
index d9edb64..c07f456 100644
--- a/application/argumentparser.h
+++ b/application/argumentparser.h
@@ -231,7 +231,7 @@ public:
     bool isUncombinableMainArgPresent() const;
 
 private:
-    IF_DEBUG_BUILD(void verifyArgs(const ArgumentVector &args);)
+    IF_DEBUG_BUILD(void verifyArgs(const ArgumentVector &args, std::vector<char> abbreviations, std::vector<const char *> names);)
     void readSpecifiedArgs(ArgumentVector &args, std::size_t &index, const char *const *&argv, const char *const *end, Argument *&lastArg, const char *&argDenotation, bool completionMode = false);
     void printBashCompletion(int argc, const char * const *argv, unsigned int cursorPos, const Argument *lastDetectedArg);
     void checkConstraints(const ArgumentVector &args);
diff --git a/tests/argumentparsertests.cpp b/tests/argumentparsertests.cpp
index 140b5b2..4f29e14 100644
--- a/tests/argumentparsertests.cpp
+++ b/tests/argumentparsertests.cpp
@@ -101,17 +101,17 @@ void ArgumentParserTests::testParsing()
     outputFileArg.setCombinable(true);
     Argument printFieldNamesArg("print-field-names", '\0', "prints available field names");
     Argument displayFileInfoArg("display-file-info", 'i', "displays general file information");
+    Argument notAlbumArg("album", 'a', "should not be confused with album value");
     displayFileInfoArg.setDenotesOperation(true);
-    displayFileInfoArg.setSubArguments({&fileArg, &verboseArg});
+    displayFileInfoArg.setSubArguments({&fileArg, &verboseArg, &notAlbumArg});
     Argument fieldsArg("fields", '\0', "specifies the fields");
     fieldsArg.setRequiredValueCount(-1);
     fieldsArg.setValueNames({"title", "album", "artist", "trackpos"});
     fieldsArg.setImplicit(true);
-    Argument notAlbumArg("album", 'a', "should not be confused with album value");
     Argument displayTagInfoArg("get", 'p', "displays the values of all specified tag fields (displays all fields if none specified)");
     displayTagInfoArg.setDenotesOperation(true);
     displayTagInfoArg.setSubArguments({&fieldsArg, &filesArg, &verboseArg, &notAlbumArg});
-    parser.setMainArguments({&qtConfigArgs.qtWidgetsGuiArg(), &printFieldNamesArg, &displayTagInfoArg, &displayFileInfoArg, &helpArg, &notAlbumArg});
+    parser.setMainArguments({&qtConfigArgs.qtWidgetsGuiArg(), &printFieldNamesArg, &displayTagInfoArg, &displayFileInfoArg, &helpArg});
 
     // define some argument values
     const char *argv[] = {"tageditor", "get", "album", "title", "diskpos", "-f", "somefile"};