improved determination if application is Premium / added TODOs

Author: firewave

cli/cmdlineparser.cpp

@@ -380,8 +380,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;
     }
 
@@ -418,7 +435,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;
         }
 
@@ -1086,7 +1103,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",
@@ -1616,13 +1633,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"
@@ -1820,7 +1839,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"
@@ -1994,6 +2013,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();
@@ -2002,12 +2022,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);
@@ -2100,8 +2114,12 @@ 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()) {
         mLogger.printError("could not load cppcheck.cfg - " + cfgErr);

cli/cmdlineparser.h

@@ -112,11 +112,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

@@ -130,6 +130,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

@@ -124,6 +124,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();
@@ -1058,6 +1059,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) {
@@ -1713,6 +1716,7 @@ void MainWindow::formatAndSetTitle(const QString &text)
         nameWithVersion += " (" + extraVersion + ")";
     }
 
+    // TODO: should not contain the version
     if (!mCppcheckCfgProductName.isEmpty())
         nameWithVersion = mCppcheckCfgProductName;
 
@@ -2240,7 +2244,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

@@ -95,6 +95,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

@@ -433,6 +433,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

@@ -298,6 +298,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;
 
@@ -384,6 +387,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

@@ -5,8 +5,9 @@
 import sys
 import pytest
 import json
+import shutil
 
-from testutils import cppcheck, assert_cppcheck, cppcheck_ex
+from testutils import cppcheck, assert_cppcheck, cppcheck_ex, __lookup_cppcheck_exe
 from xml.etree import ElementTree
 
 
@@ -1962,25 +1963,25 @@ def test_addon_lookup_ext_notfound(tmpdir):
 
 
 # TODO: test with FILESDIR
-@pytest.mark.skip
 def test_config_lookup(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
 
-    # TODO: needs to be in exepath so this is found
     config_file = os.path.join(tmpdir, 'cppcheck.cfg')
-    with open(config_file, 'wt'):
-        pass
+    with open(config_file, 'wt') as f:
+        f.write(json.dumps({}))
 
-    exitcode, stdout, stderr, exe = cppcheck_ex(['--debug-lookup=config', '--addon=misra', test_file], cwd=tmpdir)
+    exitcode, stdout, stderr, exe = cppcheck_ex(['--debug-lookup=config', test_file], cwd=tmpdir, cppcheck_exe=exe)
     exepath = os.path.dirname(exe)
     exepath_sep = exepath + os.path.sep
     assert exitcode == 0, stdout if stdout else stderr
-    lines = stdout.splitlines()
-    assert lines == [
+    assert stdout.splitlines() == [
         "looking for '{}cppcheck.cfg'".format(exepath_sep),
-        'no configuration found',
         'Checking {} ...'.format(test_file)
     ]
 
@@ -2003,6 +2004,29 @@ def test_config_lookup_notfound(tmpdir):
     ]
 
 
+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(['--debug-lookup=config', test_file], cwd=tmpdir, cppcheck_exe=exe)
+    exepath = os.path.dirname(exe)
+    exepath_sep = exepath + os.path.sep
+    assert exitcode == 1, stdout if stdout else stderr
+    assert stdout.splitlines() == [
+        "looking for '{}cppcheck.cfg'".format(exepath_sep),
+        '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:
@@ -3160,4 +3184,4 @@ def test_debug_valueflow_xml(tmp_path):  # #13606
     assert 'floatvalue' in value_elem[1].attrib
     assert value_elem[1].attrib['floatvalue'] == '1e-07'
     assert 'floatvalue' in value_elem[2].attrib
-    assert value_elem[2].attrib['floatvalue'] == '1e-07'
+    assert value_elem[2].attrib['floatvalue'] == '1e-07'

test/cli/premium_test.py

@@ -114,3 +114,11 @@ def _get_hash(s:str):
     assert hash1 != hash2
 
 
+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