improved determination if application is Premium / added TODOs

Author: firewave

cli/cmdlineparser.cpp

@@ -321,8 +321,25 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
     // default to --check-level=normal from CLI for now
     mSettings.setCheckLevel(Settings::CheckLevel::normal);
 
+    // TODO: this is duplicated in gui/mainwindows.cpp
+    {
+        // only read cppcheck.cfg if the product name has not been set yet
+        // this is hack so we can unit test this - should never happen in production
+        if (mSettings.cppcheckCfgProductName.empty())
+        {
+            // load config to determine if we are premium - do not apply any other settings
+            Settings s;
+            s.exename = mSettings.exename;
+            Suppressions supprs;
+            // TODO: errorhandling
+            Settings::loadCppcheckCfg(s, supprs, mSettings.debuglookup || mSettings.debuglookupConfig);
+            mSettings.cppcheckCfgProductName = s.cppcheckCfgProductName;
+        }
+        mSettings.premium = startsWith(mSettings.cppcheckCfgProductName, "Cppcheck Premium");
+    }
+
     if (argc <= 1) {
-        printHelp();
+        printHelp(mSettings.premium);
         return Result::Exit;
     }
 
@@ -359,7 +376,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
 
         // Print help
         if (std::strcmp(argv[i], "-h") == 0 || std::strcmp(argv[i], "--help") == 0) {
-            printHelp();
+            printHelp(mSettings.premium);
             return Result::Exit;
         }
 
@@ -1084,7 +1101,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
         }
 
         // Special Cppcheck Premium options
-        else if ((std::strncmp(argv[i], "--premium=", 10) == 0 || std::strncmp(argv[i], "--premium-", 10) == 0) && isCppcheckPremium()) {
+            else if ((std::strncmp(argv[i], "--premium=", 10) == 0 || std::strncmp(argv[i], "--premium-", 10) == 0) && mSettings.premium) {
             // valid options --premium=..
             const std::set<std::string> valid{
                 "autosar",
@@ -1681,13 +1698,15 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
     return Result::Success;
 }
 
-void CmdLineParser::printHelp() const
+void CmdLineParser::printHelp(bool premium) const
 {
-    const std::string manualUrl(isCppcheckPremium() ?
-                                "https://cppcheck.sourceforge.io/manual.pdf" :
-                                "https://files.cppchecksolutions.com/manual.pdf");
+    // TODO: fetch URL from config like product name?
+    const std::string manualUrl(premium ?
+                                "https://files.cppchecksolutions.com/manual.pdf" :
+                                "https://cppcheck.sourceforge.io/manual.pdf");
 
     std::ostringstream oss;
+    // TODO: display product name
     oss << "Cppcheck - A tool for static C/C++ code analysis\n"
         "\n"
         "Syntax:\n"
@@ -1888,7 +1907,7 @@ void CmdLineParser::printHelp() const
         "    --plist-output=<path>\n"
         "                         Generate Clang-plist output files in folder.\n";
 
-    if (isCppcheckPremium()) {
+    if (premium) {
         oss <<
             "    --premium=<option>\n"
             "                         Coding standards:\n"
@@ -2071,6 +2090,7 @@ void CmdLineParser::printHelp() const
 }
 
 std::string CmdLineParser::getVersion() const {
+    // TODO: this should not contain the version - it should set the extraVersion
     if (!mSettings.cppcheckCfgProductName.empty())
         return mSettings.cppcheckCfgProductName;
     const char * const extraVersion = CppCheck::extraVersion();
@@ -2079,12 +2099,6 @@ std::string CmdLineParser::getVersion() const {
     return std::string("Cppcheck ") + CppCheck::version();
 }
 
-bool CmdLineParser::isCppcheckPremium() const {
-    if (mSettings.cppcheckCfgProductName.empty())
-        Settings::loadCppcheckCfg(mSettings, mSuppressions, mSettings.debuglookup || mSettings.debuglookupConfig);
-    return startsWith(mSettings.cppcheckCfgProductName, "Cppcheck Premium");
-}
-
 bool CmdLineParser::tryLoadLibrary(Library& destination, const std::string& basepath, const char* filename, bool debug)
 {
     const Library::Error err = destination.load(basepath.c_str(), filename, debug);
@@ -2177,10 +2191,15 @@ bool CmdLineParser::loadAddons(Settings& settings)
 
 bool CmdLineParser::loadCppcheckCfg()
 {
-    if (!mSettings.cppcheckCfgProductName.empty())
-        return true;
+    if (!mSettings.settingsFiles.empty())
+    {
+        // should never happen - programming error
+        mLogger.printError("cppcheck.cfg has already been loaded from " + mSettings.settingsFiles[0]);
+        return false;
+    }
     const std::string cfgErr = Settings::loadCppcheckCfg(mSettings, mSuppressions, mSettings.debuglookup || mSettings.debuglookupConfig);
     if (!cfgErr.empty()) {
+        // TODO: log full path
         mLogger.printError("could not load cppcheck.cfg - " + cfgErr);
         return false;
     }

cli/cmdlineparser.h

@@ -124,11 +124,9 @@ class CmdLineParser {
     /**
      * Print help text to the console.
      */
-    void printHelp() const;
+    void printHelp(bool premium) const;
 
 private:
-    bool isCppcheckPremium() const;
-
     template<typename T>
     bool parseNumberArg(const char* const arg, std::size_t offset, T& num, bool mustBePositive = false)
     {

gui/main.cpp

@@ -124,6 +124,7 @@ static void ShowUsage()
 
 static void ShowVersion()
 {
+// TODO: should only *not* show a dialog when we are on a commnd-line
 #if defined(_WIN32)
     AboutDialog *dlg = new AboutDialog(CppCheck::version(), CppCheck::extraVersion(), 0);
     dlg->exec();

gui/mainwindow.cpp

@@ -131,6 +131,7 @@ MainWindow::MainWindow(TranslationHandler* th, QSettings* settings) :
     mSelectLanguageActions(new QActionGroup(this)),
     mSelectReportActions(new QActionGroup(this))
 {
+    // TODO: this is duplicated in cli/cmdlineparser.cpp
     {
         Settings tempSettings;
         tempSettings.exename = QCoreApplication::applicationFilePath().toStdString();
@@ -1087,6 +1088,8 @@ bool MainWindow::getCppcheckSettings(Settings& settings, Suppressions& supprs)
             return false;
         }
 
+        settings.premium = startsWith(settings.cppcheckCfgProductName, "Cppcheck Premium");
+
         const auto cfgAddons = settings.addons;
         settings.addons.clear();
         for (const std::string& addon : cfgAddons) {
@@ -1750,6 +1753,7 @@ void MainWindow::formatAndSetTitle(const QString &text)
         nameWithVersion += " (" + extraVersion + ")";
     }
 
+    // TODO: should not contain the version
     if (!mCppcheckCfgProductName.isEmpty())
         nameWithVersion = mCppcheckCfgProductName;
 
@@ -2278,7 +2282,9 @@ void MainWindow::replyFinished(QNetworkReply *reply) {
     const QString str = reply->readAll();
     qDebug() << "Response: " << str;
     if (reply->url().fileName() == "version.txt") {
+        // TODO: lacks extra version
         QString nameWithVersion = QString("Cppcheck %1").arg(CppCheck::version());
+        // TODO: this should not contain the version
         if (!mCppcheckCfgProductName.isEmpty())
             nameWithVersion = mCppcheckCfgProductName;
         const int appVersion = getVersion(nameWithVersion);

gui/xmlreportv2.cpp

@@ -94,6 +94,7 @@ void XmlReportV2::writeHeader()
 {
     const auto nameAndVersion = Settings::getNameAndVersion(mProductName.toStdString());
     const QString name = QString::fromStdString(nameAndVersion.first);
+    // TODO: lacks extraVersion
     const QString version = nameAndVersion.first.empty() ? CppCheck::version() : QString::fromStdString(nameAndVersion.second);
 
     mXmlWriter->setAutoFormatting(true);

lib/errorlogger.cpp

@@ -437,6 +437,7 @@ std::string ErrorMessage::getXMLHeader(std::string productName, int xmlVersion)
 {
     const auto nameAndVersion = Settings::getNameAndVersion(productName);
     productName = nameAndVersion.first;
+    // TODO: lacks extra version
     const std::string version = nameAndVersion.first.empty() ? CppCheck::version() : nameAndVersion.second;
 
     tinyxml2::XMLPrinter printer;

lib/settings.cpp

@@ -67,7 +67,6 @@ Settings::Settings()
 
 std::string Settings::loadCppcheckCfg(Settings& settings, Suppressions& suppressions, bool debug)
 {
-    // TODO: this always needs to be run *after* the Settings has been filled
     static const std::string cfgFilename = "cppcheck.cfg";
     std::string fileName;
 #ifdef FILESDIR
@@ -167,6 +166,8 @@ std::string Settings::loadCppcheckCfg(Settings& settings, Suppressions& suppress
         }
     }
 
+    settings.settingsFiles.emplace_back(std::move(fileName));
+
     return "";
 }
 

lib/settings.h

@@ -312,6 +312,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief plist output (--plist-output=<dir>) */
     std::string plistOutput;
 
+    /** @brief Are we Cppcheck Premium */
+    bool premium{};
+
     /** @brief Extra arguments for Cppcheck Premium addon */
     std::string premiumArgs;
 
@@ -398,6 +401,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
 
     SafeChecks safeChecks;
 
+    /** @brief the files we successfully loaded settings from */
+    std::vector<std::string> settingsFiles;
+
     SimpleEnableGroup<Severity> severity;
     SimpleEnableGroup<Certainty> certainty;
     SimpleEnableGroup<Checks> checks;

test/cli/other_test.py

@@ -7,8 +7,9 @@
 import glob
 import json
 import subprocess
+import shutil
 
-from testutils import cppcheck, assert_cppcheck
+from testutils import cppcheck, assert_cppcheck, cppcheck_ex, __lookup_cppcheck_exe
 from xml.etree import ElementTree
 
 
@@ -1701,6 +1702,26 @@ def test_cpp_probe_2(tmpdir):
     assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=[])
 
 
+def test_config_invalid(tmpdir):
+    # cppcheck.cfg needs to be next to executable
+    exe = shutil.copy2(__lookup_cppcheck_exe(), tmpdir)
+    shutil.copytree(os.path.join(os.path.dirname(__lookup_cppcheck_exe()), 'cfg'), os.path.join(tmpdir, 'cfg'))
+
+    test_file = os.path.join(tmpdir, 'test.c')
+    with open(test_file, 'wt'):
+        pass
+
+    config_file = os.path.join(tmpdir, 'cppcheck.cfg')
+    with open(config_file, 'wt'):
+        pass
+
+    exitcode, stdout, stderr, exe = cppcheck_ex([test_file], cwd=tmpdir, cppcheck_exe=exe)
+    assert exitcode == 1, stdout if stdout else stderr
+    assert stdout.splitlines() == [
+        'cppcheck: error: could not load cppcheck.cfg - not a valid JSON - syntax error at line 1 near: '
+    ]
+
+
 def test_checkers_report(tmpdir):
     test_file = os.path.join(tmpdir, 'test.c')
     with open(test_file, 'wt') as f:

test/cli/premium_test.py

@@ -26,8 +26,7 @@ def __copy_cppcheck_premium(tmpdir):
                 {
                     "addons": [],
                     "productName": "NAME",
-                    "about": "NAME",
-                    "safety": true
+                    "about": "NAME"
                 }
                 """.replace('NAME', __PRODUCT_NAME))
 
@@ -45,29 +44,29 @@ def test_misra_c_builtin_style_checks(tmpdir):
 
     exe = __copy_cppcheck_premium(tmpdir)
 
-    exitcode, _, stderr = cppcheck(['--premium=autosar', '--xml', test_file], cppcheck_exe=exe)
+    exitcode, _, stderr = cppcheck(['--premium=autosar', '--premium=safety', '--xml', test_file], cppcheck_exe=exe)
     assert exitcode == 0
     assert 'id="unusedVariable"' in stderr
     assert 'id="checkersReport"' in stderr
 
-    exitcode, _, stderr = cppcheck(['--premium=autosar', '--premium=safety-off', '--xml', test_file], cppcheck_exe=exe)
+    exitcode, _, stderr = cppcheck(['--premium=autosar', '--xml', test_file], cppcheck_exe=exe)
     assert exitcode == 0
     assert 'id="unusedVariable"' in stderr
     assert 'id="checkersReport"' not in stderr
 
-    exitcode, _, stderr = cppcheck(['--xml-version=3', test_file], cppcheck_exe=exe)
+    exitcode, _, stderr = cppcheck(['--xml-version=3', '--premium=safety', test_file], cppcheck_exe=exe)
     assert exitcode == 0
     assert '<safety/>' in stderr
 
-    exitcode, _, stderr = cppcheck(['--xml-version=3', '--premium=safety-off', test_file], cppcheck_exe=exe)
+    exitcode, _, stderr = cppcheck(['--xml-version=3', test_file], cppcheck_exe=exe)
     assert exitcode == 0
     assert '<safety/>' not in stderr
 
-    exitcode, _, stderr = cppcheck(['--xml-version=3', '--inline-suppr', test_file], cppcheck_exe=exe)
+    exitcode, _, stderr = cppcheck(['--xml-version=3', '--premium=safety', '--inline-suppr', test_file], cppcheck_exe=exe)
     assert exitcode == 0
     assert '<inline-suppr/>' in stderr
 
-    exitcode, _, stderr = cppcheck(['--xml-version=3', '--suppress=foo', test_file], cppcheck_exe=exe)
+    exitcode, _, stderr = cppcheck(['--xml-version=3', '--premium=safety', '--suppress=foo', test_file], cppcheck_exe=exe)
     assert exitcode == 0
     assert '<suppression errorId="foo" inline="false" />' in stderr
 
@@ -156,3 +155,13 @@ def test_invalid_license_retry(tmpdir):
 
     _, _, stderr = cppcheck(args)
     assert 'Invalid license' not in stderr
+
+
+def test_help(tmpdir):
+    exe = __copy_cppcheck_premium(tmpdir)
+
+    exitcode, stdout, _ = cppcheck(['--help'], cppcheck_exe=exe)
+    assert exitcode == 0
+    assert stdout.startswith('Cppcheck ')  # check for product name - TODO: should be "Cppcheck Premium"
+    assert '--premium=' in stdout, stdout  # check for premium option
+    assert 'cppchecksolutions.com' in stdout, stdout  # check for premium help link