CmdLineParser: various refactorings and cleanups as well as testing improvements (#5676)

cli/cmdlineparser.cpp

@@ -146,25 +146,6 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
     const bool success = parseFromArgs(argc, argv);
 
     if (success) {
-        if (getShowVersion() && !getShowErrorMessages()) {
-            if (!mSettings.cppcheckCfgProductName.empty()) {
-                mLogger.printRaw(mSettings.cppcheckCfgProductName);
-            } else {
-                const char * const extraVersion = CppCheck::extraVersion();
-                if (*extraVersion != 0)
-                    mLogger.printRaw(std::string("Cppcheck ") + CppCheck::version() + " ("+ extraVersion + ')');
-                else
-                    mLogger.printRaw(std::string("Cppcheck ") + CppCheck::version());
-            }
-        }
-
-        if (getShowErrorMessages()) {
-            XMLErrorMessagesLogger xmlLogger;
-            std::cout << ErrorMessage::getXMLHeader(mSettings.cppcheckCfgProductName);
-            CppCheck::getErrorMessages(xmlLogger);
-            std::cout << ErrorMessage::getXMLFooter() << std::endl;
-        }
-
         if (exitAfterPrinting()) {
             Settings::terminate();
             return true;
@@ -526,9 +507,16 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
 
             // print all possible error messages..
             else if (std::strcmp(argv[i], "--errorlist") == 0) {
-                mShowErrorMessages = true;
+                // TODO: make this an exclusive option
-                mSettings.xml = true;
                 mExitAfterPrint = true;
+                mSettings.loadCppcheckCfg();
+                {
+                    XMLErrorMessagesLogger xmlLogger;
+                    std::cout << ErrorMessage::getXMLHeader(mSettings.cppcheckCfgProductName);
+                    CppCheck::getErrorMessages(xmlLogger);
+                    std::cout << ErrorMessage::getXMLFooter() << std::endl;
+                }
+                return true;
             }
 
             // --error-exitcode=1
@@ -606,10 +594,10 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
 
             // Print help
             else if (std::strcmp(argv[i], "-h") == 0 || std::strcmp(argv[i], "--help") == 0) {
-                mPathNames.clear();
+                // TODO: make this an exclusive option
-                mShowHelp = true;
                 mExitAfterPrint = true;
-                break;
+                printHelp();
+                return true;
             }
 
             // Ignored paths
@@ -829,18 +817,19 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
             else if (std::strncmp(argv[i], "--plist-output=", 15) == 0) {
                 mSettings.plistOutput = Path::simplifyPath(Path::fromNativeSeparators(argv[i] + 15));
                 if (mSettings.plistOutput.empty())
-                    mSettings.plistOutput = "./";
+                    mSettings.plistOutput = ".";
-                else if (!endsWith(mSettings.plistOutput,'/'))
-                    mSettings.plistOutput += '/';
 
                 const std::string plistOutput = Path::toNativeSeparators(mSettings.plistOutput);
                 if (!Path::isDirectory(plistOutput)) {
-                    std::string message("plist folder does not exist: \"");
+                    std::string message("plist folder does not exist: '");
                     message += plistOutput;
-                    message += "\".";
+                    message += "'.";
                     mLogger.printError(message);
                     return false;
                 }
+
+                if (!endsWith(mSettings.plistOutput,'/'))
+                    mSettings.plistOutput += '/';
             }
 
             // Special Cppcheck Premium options
@@ -1163,9 +1152,18 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
                 mSettings.verbose = true;
 
             else if (std::strcmp(argv[i], "--version") == 0) {
-                mShowVersion = true;
+                // TODO: make this an exclusive parameter
                 mExitAfterPrint = true;
                 mSettings.loadCppcheckCfg();
+                if (!mSettings.cppcheckCfgProductName.empty()) {
+                    mLogger.printRaw(mSettings.cppcheckCfgProductName);
+                } else {
+                    const char * const extraVersion = CppCheck::extraVersion();
+                    if (*extraVersion != '\0')
+                        mLogger.printRaw(std::string("Cppcheck ") + CppCheck::version() + " ("+ extraVersion + ')');
+                    else
+                        mLogger.printRaw(std::string("Cppcheck ") + CppCheck::version());
+                }
                 return true;
             }
 
@@ -1231,11 +1229,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
     }
 
     if (argc <= 1) {
-        mShowHelp = true;
         mExitAfterPrint = true;
-    }
-
-    if (mShowHelp) {
         printHelp();
         return true;
     }
@@ -1630,40 +1624,40 @@ bool CmdLineParser::tryLoadLibrary(Library& destination, const std::string& base
     const Library::Error err = destination.load(basepath.c_str(), filename);
 
     if (err.errorcode == Library::ErrorCode::UNKNOWN_ELEMENT)
-        std::cout << "cppcheck: Found unknown elements in configuration file '" << filename << "': " << err.reason << std::endl;
+        mLogger.printMessage("Found unknown elements in configuration file '" + std::string(filename) + "': " + err.reason); // TODO: print as errors
     else if (err.errorcode != Library::ErrorCode::OK) {
-        std::cout << "cppcheck: Failed to load library configuration file '" << filename << "'. ";
+        std::string msg = "Failed to load library configuration file '" + std::string(filename) + "'. ";
         switch (err.errorcode) {
         case Library::ErrorCode::OK:
             break;
         case Library::ErrorCode::FILE_NOT_FOUND:
-            std::cout << "File not found";
+            msg += "File not found";
             break;
         case Library::ErrorCode::BAD_XML:
-            std::cout << "Bad XML";
+            msg += "Bad XML";
             break;
         case Library::ErrorCode::UNKNOWN_ELEMENT:
-            std::cout << "Unexpected element";
+            msg += "Unexpected element";
             break;
         case Library::ErrorCode::MISSING_ATTRIBUTE:
-            std::cout << "Missing attribute";
+            msg +="Missing attribute";
             break;
         case Library::ErrorCode::BAD_ATTRIBUTE_VALUE:
-            std::cout << "Bad attribute value";
+            msg += "Bad attribute value";
             break;
         case Library::ErrorCode::UNSUPPORTED_FORMAT:
-            std::cout << "File is of unsupported format version";
+            msg += "File is of unsupported format version";
             break;
         case Library::ErrorCode::DUPLICATE_PLATFORM_TYPE:
-            std::cout << "Duplicate platform type";
+            msg += "Duplicate platform type";
             break;
         case Library::ErrorCode::PLATFORM_TYPE_REDEFINED:
-            std::cout << "Platform type redefined";
+            msg += "Platform type redefined";
             break;
         }
         if (!err.reason.empty())
-            std::cout << " '" + err.reason + "'";
+            msg += " '" + err.reason + "'";
-        std::cout << std::endl;
+        mLogger.printMessage(msg); // TODO: print as errors
         return false;
     }
     return true;
@@ -1683,7 +1677,7 @@ bool CmdLineParser::loadLibraries(Settings& settings)
                                   "std.cfg should be available in " + cfgfolder + " or the FILESDIR "
                                   "should be configured.");
 #endif
-        std::cout << msg << " " << details << std::endl;
+        mLogger.printRaw(msg + " " + details); // TODO: do not print as raw?
         return false;
     }
 
@@ -1703,7 +1697,7 @@ bool CmdLineParser::loadAddons(Settings& settings)
         AddonInfo addonInfo;
         const std::string failedToGetAddonInfo = addonInfo.getAddonInfo(addon, settings.exename);
         if (!failedToGetAddonInfo.empty()) {
-            std::cout << failedToGetAddonInfo << std::endl;
+            mLogger.printRaw(failedToGetAddonInfo); // TODO: do not print as raw
             result = false;
             continue;
         }

cli/cmdlineparser.h

@@ -73,20 +73,6 @@ public:
      */
     bool parseFromArgs(int argc, const char* const argv[]);
 
-    /**
-     * Return if user wanted to see program version.
-     */
-    bool getShowVersion() const {
-        return mShowVersion;
-    }
-
-    /**
-     * Return if user wanted to see list of error messages.
-     */
-    bool getShowErrorMessages() const {
-        return mShowErrorMessages;
-    }
-
     /**
      * Return the path names user gave to command line.
      */
@@ -153,21 +139,21 @@ private:
      * Tries to load a library and prints warning/error messages
      * @return false, if an error occurred (except unknown XML elements)
      */
-    static bool tryLoadLibrary(Library& destination, const std::string& basepath, const char* filename);
+    bool tryLoadLibrary(Library& destination, const std::string& basepath, const char* filename);
 
     /**
      * @brief Load libraries
      * @param settings Settings
      * @return Returns true if successful
      */
-    static bool loadLibraries(Settings& settings);
+    bool loadLibraries(Settings& settings);
 
     /**
      * @brief Load addons
      * @param settings Settings
      * @return Returns true if successful
      */
-    static bool loadAddons(Settings& settings);
+    bool loadAddons(Settings& settings);
 
     CmdLineLogger &mLogger;
 
@@ -178,9 +164,6 @@ private:
     Settings &mSettings;
     Suppressions &mSuppressions;
     Suppressions &mSuppressionsNoFail;
-    bool mShowHelp{};
-    bool mShowVersion{};
-    bool mShowErrorMessages{};
     bool mExitAfterPrint{};
     std::string mVSConfig;
 };

lib/settings.cpp

@@ -40,6 +40,7 @@ Settings::Settings()
     setCheckLevelNormal();
 }
 
+// TODO: report error when the config is invalid
 void Settings::loadCppcheckCfg()
 {
     std::string fileName = Path::getPathFromFilename(exename) + "cppcheck.cfg";

test/fixture.cpp

@@ -109,12 +109,16 @@ bool TestFixture::prepareTest(const char testname[])
         } else {
             std::cout << classname << "::" << mTestname << std::endl;
         }
-        teardownTestInternal();
         return true;
     }
     return false;
 }
 
+void TestFixture::teardownTest()
+{
+    teardownTestInternal();
+}
+
 std::string TestFixture::getLocationStr(const char * const filename, const unsigned int linenr) const
 {
     return std::string(filename) + ':' + std::to_string(linenr) + '(' + classname + "::" + mTestname + ')';

test/fixture.h

@@ -62,6 +62,7 @@ protected:
 
     bool prepareTest(const char testname[]);
     virtual void prepareTestInternal() {}
+    void teardownTest();
     virtual void teardownTestInternal() {}
     std::string getLocationStr(const char * const filename, const unsigned int linenr) const;
 
@@ -254,7 +255,7 @@ extern std::ostringstream errout;
 extern std::ostringstream output;
 
 // TODO: most asserts do not actually assert i.e. do not return
-#define TEST_CASE( NAME )  do { if (prepareTest(#NAME)) { setVerbose(false); NAME(); } } while (false)
+#define TEST_CASE( NAME )  do { if (prepareTest(#NAME)) { setVerbose(false); NAME(); teardownTest(); } } while (false)
 #define ASSERT( CONDITION )  if (!assert_(__FILE__, __LINE__, (CONDITION))) return
 #define ASSERT_LOC( CONDITION, FILE_, LINE_ )  assert_(FILE_, LINE_, (CONDITION))
 #define CHECK_EQUALS( EXPECTED, ACTUAL )  assertEquals(__FILE__, __LINE__, (EXPECTED), (ACTUAL))