lib/cli: first unit test implemented

- argumentWaitingForValue was never reset after the value had been retrieved
- made it so the ArgumentsParser constructor can accept const char**
- correct usage of substr() and actually use the correct position in the first place
- fix a typo where I tried to dereference `this` with `-` instead of `->`

scripts/build.sh

@@ -72,7 +72,7 @@ echo "Building tools..."
 mkdir -pv bin/tools
 # add compile commands to this array
 COMPILE_COMMANDS=(
-    "$CXX_WITH_FLAGS src/tools/dumpnbt.cpp -Lbin/lib -l:nbt.so -o bin/tools/dumpnbt",
+    "$CXX_WITH_FLAGS src/tools/dumpnbt.cpp -Lbin/lib -l:nbt.so -o bin/tools/dumpnbt"
     "$CXX_WITH_FLAGS src/tools/hexnet.cpp -Ldependencies/sockpp-0.7.1/build -l:libsockpp.so -Idependencies/sockpp-0.7.1/include -o bin/tools/hexnet"
 )
 for command in ${!COMPILE_COMMANDS[@]}; do

scripts/test.sh

@@ -38,6 +38,7 @@ echo "Building tests..."
 # add compile commands to this array
 COMPILE_COMMANDS=(
     "$CXX_WITH_FLAGS src/test/nbt_helpers.cpp -Lbin/lib -l:nbt.so -o bin/test/nbt_helpers"
+    "$CXX_WITH_FLAGS src/test/cli_argument_parser.cpp -Lbin/lib -l:cli.so -o bin/test/cli_argument_parser"
 )
 for command in ${!COMPILE_COMMANDS[@]}; do
     echo "${COMPILE_COMMANDS[command]}"

src/lib/cli.cpp

@@ -46,14 +46,14 @@ namespace CLI {
     PositionalArgument::PositionalArgument() {
         this->present = false;
     }
-    PositionalArgument::PositionalArgument(std::string description, std::string placeholder) {
+    PositionalArgument::PositionalArgument(std::string placeholder, std::string description) {
         this->description = description;
         this->placeholder = placeholder;
         this->present = false;
     }
 
     // using int here bc that's how main() is defined
-    ArgumentsParser::ArgumentsParser(int argc, char* argv[], std::vector<Flag> flags, std::vector<UnpositionalArgument> unpositionalArguments, std::vector<PositionalArgument> positionalArguments) {
+    ArgumentsParser::ArgumentsParser(int argc, const char* argv[], std::vector<Flag> flags, std::vector<UnpositionalArgument> unpositionalArguments, std::vector<PositionalArgument> positionalArguments) {
         this->wrongUsage = false;
         this->wrongUsageMessages = std::vector<std::string>();
         this->programName = std::string(argv[0]);
@@ -99,7 +99,7 @@ namespace CLI {
                     if (position==std::string::npos) {
                         // no =value
                         //is argument or flag?
-                        std::string argumentName = argument.substr(2,argument.length());
+                        std::string argumentName = argument.substr(2,argument.length()-2);
                         if (flagsByLongName.contains(argumentName)) {
                             // flag
                             flagsByLongName[argumentName]->present = true;
@@ -113,8 +113,8 @@ namespace CLI {
                         }
                     } else {
                         // has =value
-                        std::string value = argument.substr(position, argument.length());
-                        std::string argumentName = argument.substr(2, position);
+                        std::string value = argument.substr(position+1, argument.length()-position-1);
+                        std::string argumentName = argument.substr(2, position-2);
                         if (argumentsByLongName.contains(argumentName)) {
                             argumentsByLongName[argumentName]->present = true;
                             argumentsByLongName[argumentName]->value = value;
@@ -140,12 +140,12 @@ namespace CLI {
                                 argumentWaitingForValue = argumentsByShortName[argument[i]];
                             } else {
                                 //assume the rest of the argv is a concatenated argument value
-                                argumentsByShortName[argument[i]]->value = argument.substr(i+1, argument.length());
+                                argumentsByShortName[argument[i]]->value = argument.substr(i+1, argument.length()-i-1);
                                 break;
                             }
                         } else {
                             this->wrongUsage = true;
-                            this->wrongUsageMessages.push_back(std::string("Unknown argument or flag(s): ")+argument.substr(i, argument.length()));
+                            this->wrongUsageMessages.push_back(std::string("Unknown argument or flag(s): ")+argument.substr(i, argument.length()-i));
                             // err on the side of caution to ensure that
                             // no unwanted options get activated on programs
                             // that deal gracefully with unrecognized command
@@ -169,6 +169,7 @@ namespace CLI {
                 } else {
                     // value for unpositional argument
                     argumentWaitingForValue->value = argument;
+                    argumentWaitingForValue = nullptr;
                 }
             }
         }
@@ -190,14 +191,7 @@ namespace CLI {
         }
     }
 
-    ErrorOr<std::string> ArgumentsParser::getProgramName() {
-        if (this->wrongUsage) {
-            return ErrorOr<std::string>(true, ErrorCodes::WRONG_USAGE, this->programName);
-        }
-        return ErrorOr<std::string>(this->programName);
-    }
-
-    ErrorOr<bool> ArgumentsParser::getFlag(int argc, char* argv[], char shortName) {
+    ErrorOr<bool> ArgumentsParser::getFlag(char shortName) {
         if (!this->flagsByShortName.contains(shortName)) return ErrorOr<bool>(true, ErrorCodes::UNKNOWN_KEY, false);
         if (this->wrongUsage) {
             return ErrorOr<bool>(true, ErrorCodes::WRONG_USAGE, this->flagsByShortName[shortName]->present);
@@ -205,7 +199,7 @@ namespace CLI {
         return ErrorOr<bool>(this->flagsByShortName[shortName]->present);
     }
 
-    ErrorOr<bool> ArgumentsParser::getFlag(int argc, char* argv[], std::string longName) {
+    ErrorOr<bool> ArgumentsParser::getFlag(std::string longName) {
         if (!this->flagsByLongName.contains(longName)) return ErrorOr<bool>(true, ErrorCodes::UNKNOWN_KEY, false);
         if (this->wrongUsage) {
             return ErrorOr<bool>(true, ErrorCodes::WRONG_USAGE, this->flagsByLongName[longName]->present);
@@ -213,7 +207,7 @@ namespace CLI {
         return ErrorOr<bool>(this->flagsByLongName[longName]->present);
     }
 
-    ErrorOr<std::string> ArgumentsParser::getPositionalArgument(int argc, char* argv[], std::vector<CLI::PositionalArgument>::size_type position){
+    ErrorOr<std::string> ArgumentsParser::getPositionalArgument(std::vector<CLI::PositionalArgument>::size_type position){
         if (position >= this->positionalArguments.size()) return ErrorOr<std::string>(true, ErrorCodes::OUT_OF_RANGE, std::string(""));
         if (this->wrongUsage) {
             if (this->positionalArguments.at(position).present) return ErrorOr<std::string>(true, ErrorCodes::WRONG_USAGE, this->positionalArguments.at(position).value);
@@ -222,9 +216,9 @@ namespace CLI {
         return ErrorOr<std::string>(this->positionalArguments.at(position).value);
     }
 
-    ErrorOr<std::string> ArgumentsParser::getUnpositionalArgument(int argc, char* argv[], char shortName) {
+    ErrorOr<std::string> ArgumentsParser::getUnpositionalArgument(char shortName) {
         if (!this->argumentsByShortName.contains(shortName)) return ErrorOr<std::string>(true, ErrorCodes::UNKNOWN_KEY, std::string(""));
-        if (this-wrongUsage) {
+        if (this->wrongUsage) {
             if (this->argumentsByShortName[shortName]->present) return ErrorOr<std::string>(true, ErrorCodes::WRONG_USAGE, this->argumentsByShortName[shortName]->value);
             else return ErrorOr<std::string>(true, ErrorCodes::NOT_PRESENT, std::string(""));
         }
@@ -233,9 +227,9 @@ namespace CLI {
         else return ErrorOr<std::string>(false, ErrorCodes::NOT_PRESENT, std::string(""));
     }
 
-    ErrorOr<std::string> ArgumentsParser::getUnpositionalArgument(int argc, char* argv[], std::string longName) {
+    ErrorOr<std::string> ArgumentsParser::getUnpositionalArgument(std::string longName) {
         if (!this->argumentsByLongName.contains(longName)) return ErrorOr<std::string>(true, ErrorCodes::UNKNOWN_KEY, std::string(""));
-        if (this-wrongUsage) {
+        if (this->wrongUsage) {
             if (this->argumentsByLongName[longName]->present) return ErrorOr<std::string>(true, ErrorCodes::WRONG_USAGE, this->argumentsByLongName[longName]->value);
             else return ErrorOr<std::string>(true, ErrorCodes::NOT_PRESENT, std::string(""));
         }

src/lib/cli.h++

@@ -58,32 +58,33 @@ namespace CLI {
         std::string value;
 
         PositionalArgument();
-        PositionalArgument(std::string description, std::string placeholder);
+        PositionalArgument(std::string placeholder, std::string description);
     };
 
     class ArgumentsParser {
-        bool wrongUsage;
-        std::vector<std::string> wrongUsageMessages;
+        private:
+            std::map<char, Flag*> flagsByShortName;
+            std::map<std::string, Flag*> flagsByLongName;
+            std::map<char, UnpositionalArgument*> argumentsByShortName;
+            std::map<std::string, UnpositionalArgument*> argumentsByLongName;
+            std::vector<PositionalArgument> positionalArguments;
 
-        std::string programName;
-        std::map<char, Flag*> flagsByShortName;
-        std::map<std::string, Flag*> flagsByLongName;
-        std::map<char, UnpositionalArgument*> argumentsByShortName;
-        std::map<std::string, UnpositionalArgument*> argumentsByLongName;
-        std::vector<PositionalArgument> positionalArguments;
+        public:
+            std::string programName;
+            bool wrongUsage;
+            std::vector<std::string> wrongUsageMessages;
 
-        // using int here bc that's how main() is defined
-        ArgumentsParser(int argc, char* argv[], std::vector<Flag> flags, std::vector<UnpositionalArgument> unpositionalArguments, std::vector<PositionalArgument> positionalArguments);
-        ~ArgumentsParser();
+            // using int here bc that's how main() is defined
+            ArgumentsParser(int argc, const char* argv[], std::vector<Flag> flags, std::vector<UnpositionalArgument> unpositionalArguments, std::vector<PositionalArgument> positionalArguments);
+            ~ArgumentsParser();
 
-        ErrorOr<std::string> getProgramName();
-        ErrorOr<bool> getFlag(int argc, char* argv[], char shortName);
-        ErrorOr<bool> getFlag(int argc, char* argv[], std::string longName);
-        ErrorOr<std::string> getPositionalArgument(int argc, char* argv[], std::vector<CLI::PositionalArgument>::size_type position);
-        ErrorOr<std::string> getUnpositionalArgument(int argc, char* argv[], char shortName);
-        ErrorOr<std::string> getUnpositionalArgument(int argc, char* argv[], std::string longName);
+            ErrorOr<bool> getFlag(char shortName);
+            ErrorOr<bool> getFlag(std::string longName);
+            ErrorOr<std::string> getPositionalArgument(std::vector<CLI::PositionalArgument>::size_type position);
+            ErrorOr<std::string> getUnpositionalArgument(char shortName);
+            ErrorOr<std::string> getUnpositionalArgument(std::string longName);
 
-        std::string getUsage();
+            std::string getUsage();
     };
 
 }

src/test/cli_argument_parser.cpp

@@ -0,0 +1,159 @@
+// Copyright 2022, FOSS-VG Developers and Contributers
+//
+// This program is free software: you can redistribute it and/or modify it
+// under the terms of the GNU Affero General Public License as published
+// by the Free Software Foundation, version 3.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied
+// warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+// See the GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// version 3 along with this program.
+// If not, see https://www.gnu.org/licenses/agpl-3.0.en.html
+
+#include <string>
+#include <iostream>
+#include <vector>
+#include "assert.h++"
+#include "../lib/error.h++"
+
+#include "../lib/cli.h++"
+
+int main(int argc, char* argv[]) {
+    std::cout << "################################################################################" << std::endl;
+    std::cout << "CLI argument parser tests" << std::endl;
+    std::cout << "################################################################################" << std::endl;
+
+    // Valid parameter test ############################################
+    // This test exercises the basic functionality of the arguments parser
+    // with many variations of valid input on a single command line.
+    //
+    // simulated command line:
+    // test -0 -ab -12345 --long-flag -cconcatenated -d separate-value -efdouble-concatenated "positional argument 0" -gh concatenated-separate-value --long-argument-with-value-included="included value" --long-argument-with-value-separated "separate value" "positional argument 1" "positional argument 2"
+
+    std::vector<CLI::Flag> flags;
+    //flags.push_back(CLI::Flag(shortName, longName, description));
+    flags.push_back(CLI::Flag('0', "00000", "a short flag on its own"));
+    flags.push_back(CLI::Flag('a', "aaaaa", "concatenated short flags"));
+    flags.push_back(CLI::Flag('b', "bbbbb", "concatenated short flags"));
+    flags.push_back(CLI::Flag('1', "11111", "more than two concatenated short flags"));
+    flags.push_back(CLI::Flag('2', "22222", "more than two concatenated short flags"));
+    flags.push_back(CLI::Flag('3', "33333", "more than two concatenated short flags"));
+    flags.push_back(CLI::Flag('4', "44444", "more than two concatenated short flags"));
+    flags.push_back(CLI::Flag('5', "55555", "more than two concatenated short flags"));
+    flags.push_back(CLI::Flag('l', "long-flag", "long flags"));
+    flags.push_back(CLI::Flag('e', "eeeee", "short flags concatenated with an argument that is concatenated with its value"));
+    flags.push_back(CLI::Flag('g', "ggggg", "short flags concatenated with an argument that has a separate value"));
+    flags.push_back(CLI::Flag('6', "66666", "unused flag"));
+
+    std::vector<CLI::UnpositionalArgument> unpositionalArguments;
+    unpositionalArguments.push_back(CLI::UnpositionalArgument('c', "ccccc", "VALUE", "short argument concatenated with its value"));
+    unpositionalArguments.push_back(CLI::UnpositionalArgument('d', "ddddd", " VALUE", "short argument with separate value"));
+    unpositionalArguments.push_back(CLI::UnpositionalArgument('f', "fffff", "VALUE", "short argument concatenated with a flag and its value"));
+    unpositionalArguments.push_back(CLI::UnpositionalArgument('h', "hhhhh", " VALUE", "short argument concatenated with a flag with separate value"));
+    unpositionalArguments.push_back(CLI::UnpositionalArgument('x', "long-argument-with-value-included", "VALUE", "long argument with its value included using ="));
+    unpositionalArguments.push_back(CLI::UnpositionalArgument('y', "long-argument-with-value-separated", " VALUE", "long argument with separate value"));
+    unpositionalArguments.push_back(CLI::UnpositionalArgument('z', "zzzzz", "NOPE", "unused argument"));
+
+    std::vector<CLI::PositionalArgument> positionalArguments;
+    positionalArguments.push_back(CLI::PositionalArgument("argument0", "positional argument between optional parameters"));
+    positionalArguments.push_back(CLI::PositionalArgument("argument1", "positional argument"));
+    positionalArguments.push_back(CLI::PositionalArgument("argument2", "positional argument"));
+
+    const char** validParameterList = new const char*[17];
+    validParameterList[ 0] = "test";
+    validParameterList[ 1] = "-0";
+    validParameterList[ 2] = "-ab";
+    validParameterList[ 3] = "-12345";
+    validParameterList[ 4] = "--long-flag";
+    validParameterList[ 5] = "-cconcatenated";
+    validParameterList[ 6] = "-d";
+    validParameterList[ 7] = "separate-value";
+    validParameterList[ 8] = "-efdouble-concatenated";
+    validParameterList[ 9] = "positional argument 0";
+    validParameterList[10] = "-gh";
+    validParameterList[11] = "concatenated-separate-value";
+    validParameterList[12] = "--long-argument-with-value-included=included value";
+    validParameterList[13] = "--long-argument-with-value-separated";
+    validParameterList[14] = "separate value";
+    validParameterList[15] = "positional argument 1";
+    validParameterList[16] = "positional argument 2";
+    int validParameterCount = 17;
+
+    CLI::ArgumentsParser parser = CLI::ArgumentsParser(validParameterCount, validParameterList, flags, unpositionalArguments, positionalArguments);
+
+    ASSERT(!parser.wrongUsage);
+    ASSERT(parser.programName == std::string("test"));
+    ASSERT(!parser.getFlag('0').isError);
+    ASSERT(parser.getFlag('0').value);
+    ASSERT(parser.getFlag(std::string("00000")).value);
+    ASSERT(!parser.getFlag('a').isError);
+    ASSERT(parser.getFlag('a').value);
+    ASSERT(parser.getFlag(std::string("aaaaa")).value);
+    ASSERT(!parser.getFlag('b').isError);
+    ASSERT(parser.getFlag('b').value);
+    ASSERT(parser.getFlag(std::string("bbbbb")).value);
+    ASSERT(!parser.getFlag('1').isError);
+    ASSERT(parser.getFlag('1').value);
+    ASSERT(parser.getFlag(std::string("11111")).value);
+    ASSERT(!parser.getFlag('2').isError);
+    ASSERT(parser.getFlag('2').value);
+    ASSERT(parser.getFlag(std::string("22222")).value);
+    ASSERT(!parser.getFlag('3').isError);
+    ASSERT(parser.getFlag('3').value);
+    ASSERT(parser.getFlag(std::string("33333")).value);
+    ASSERT(!parser.getFlag('4').isError);
+    ASSERT(parser.getFlag('4').value);
+    ASSERT(parser.getFlag(std::string("44444")).value);
+    ASSERT(!parser.getFlag('5').isError);
+    ASSERT(parser.getFlag('5').value);
+    ASSERT(parser.getFlag(std::string("55555")).value);
+    ASSERT(!parser.getFlag(std::string("long-flag")).isError);
+    ASSERT(parser.getFlag('l').value);
+    ASSERT(parser.getFlag(std::string("long-flag")).value);
+    ASSERT(!parser.getFlag('6').isError);
+    ASSERT(!parser.getFlag('6').value);
+    ASSERT(!parser.getFlag(std::string("66666")).value);
+    ASSERT(!parser.getUnpositionalArgument('c').isError);
+    ASSERT(parser.getUnpositionalArgument('c').value==std::string("concatenated"));
+    ASSERT(parser.getUnpositionalArgument(std::string("ccccc")).value==std::string("concatenated"));
+    ASSERT(!parser.getUnpositionalArgument('d').isError);
+    ASSERT(parser.getUnpositionalArgument('d').value==std::string("separate-value"));
+    ASSERT(parser.getUnpositionalArgument(std::string("ddddd")).value==std::string("separate-value"));
+    ASSERT(!parser.getFlag('e').isError);
+    ASSERT(parser.getFlag('e').value);
+    ASSERT(parser.getFlag(std::string("eeeee")).value);
+    ASSERT(!parser.getUnpositionalArgument('f').isError);
+    ASSERT(parser.getUnpositionalArgument('f').value==std::string("double-concatenated"));
+    ASSERT(parser.getUnpositionalArgument(std::string("fffff")).value==std::string("double-concatenated"));
+    ASSERT(!parser.getPositionalArgument(0).isError);
+    ASSERT(parser.getPositionalArgument(0).value=="positional argument 0");
+    ASSERT(!parser.getFlag('g').isError);
+    ASSERT(parser.getFlag('g').value);
+    ASSERT(parser.getFlag(std::string("ggggg")).value);
+    ASSERT(!parser.getUnpositionalArgument('h').isError);
+    ASSERT(parser.getUnpositionalArgument('h').value==std::string("concatenated-separate-value"));
+    ASSERT(parser.getUnpositionalArgument(std::string("hhhhh")).value==std::string("concatenated-separate-value"));
+    ASSERT(!parser.getUnpositionalArgument('x').isError);
+    ASSERT(parser.getUnpositionalArgument('x').value==std::string("included value"));
+    ASSERT(parser.getUnpositionalArgument(std::string("long-argument-with-value-included")).value==std::string("included value"));
+    ASSERT(!parser.getUnpositionalArgument('y').isError);
+    ASSERT(parser.getUnpositionalArgument('y').value==std::string("separate value"));
+    ASSERT(parser.getUnpositionalArgument(std::string("long-argument-with-value-separated")).value==std::string("separate value"));
+    ASSERT(!parser.getPositionalArgument(1).isError);
+    ASSERT(parser.getPositionalArgument(1).value=="positional argument 1");
+    ASSERT(!parser.getPositionalArgument(2).isError);
+    ASSERT(parser.getPositionalArgument(2).value=="positional argument 2");
+    ASSERT(!parser.getUnpositionalArgument('z').isError);
+    ASSERT(parser.getUnpositionalArgument('z').errorCode == ErrorCodes::NOT_PRESENT);
+
+    std::cout << "Passed valid input test." << std::endl;
+
+    //TODO add tests for invalid input
+    std::cerr << "TODO: invalid input tests" << std::endl;
+
+    delete validParameterList;
+    return 0;
+}

src/test/nbt_helpers.cpp

@@ -23,6 +23,9 @@
 #include "../lib/error.h++"
 
 int main(){
+    std::cout << "################################################################################" << std::endl;
+    std::cout << "NBT helper tests" << std::endl;
+    std::cout << "################################################################################" << std::endl;
 
     // used for all integer tests
     uint8_t dataForIntTest[] = {30, 31, 32, 33, 34, 35, 36, 37, 38, 39};