Fix #13825 (toomanyconfigs lacks file information) (danmar#7967)

The file-less warning generated from cppcheckexecutor.cpp is removed.
the --errorlist message should match the real message.

Author: danmar

cli/cppcheckexecutor.cpp

@@ -432,10 +432,6 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
             returnValue = settings.exitCode;
     }
 
-    if (!settings.checkConfiguration) {
-        cppcheck.tooManyConfigsError("",0U);
-    }
-
     stdLogger.writeCheckersReport(supprs);
 
     if (settings.outputFormat == Settings::OutputFormat::xml) {

lib/cppcheck.cpp

@@ -857,6 +857,9 @@ std::size_t CppCheck::calculateHash(const Preprocessor& preprocessor, const std:
     toolinfo << (mSettings.severity.isEnabled(Severity::portability) ? 'p' : ' ');
     toolinfo << (mSettings.severity.isEnabled(Severity::information) ? 'i' : ' ');
     toolinfo << mSettings.userDefines;
+    toolinfo << (mSettings.checkConfiguration ? 'c' : ' '); // --check-config
+    toolinfo << (mSettings.force ? 'f' : ' ');
+    toolinfo << mSettings.maxConfigs;
     toolinfo << std::to_string(static_cast<std::uint8_t>(mSettings.checkLevel));
     for (const auto &a : mSettings.addonInfos) {
         toolinfo << a.name;
@@ -1041,6 +1044,9 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
             for (const std::string &config : configurations)
                 (void)preprocessor.getcode(config, files, false);
 
+            if (configurations.size() > mSettings.maxConfigs)
+                tooManyConfigsError(Path::toNativeSeparators(file.spath()), configurations.size());
+
             if (analyzerInformation)
                 mLogger->setAnalyzerInfo(nullptr);
             return 0;
@@ -1060,14 +1066,6 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
         }
 #endif
 
-        if (!mSettings.force && configurations.size() > mSettings.maxConfigs) {
-            if (mSettings.severity.isEnabled(Severity::information)) {
-                tooManyConfigsError(Path::toNativeSeparators(file.spath()),configurations.size());
-            } else {
-                mTooManyConfigs = true;
-            }
-        }
-
         FilesDeleter filesDeleter;
 
         // write dump file xml prolog
@@ -1092,8 +1090,18 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
 
             // Check only a few configurations (default 12), after that bail out, unless --force
             // was used.
-            if (!mSettings.force && ++checkCount > mSettings.maxConfigs)
+            if (!mSettings.force && ++checkCount > mSettings.maxConfigs) {
+                // If maxConfigs has default value then report information message that configurations are skipped.
+                // If maxConfigs does not have default value then the user is explicitly skipping configurations so
+                // the information message is not reported, the whole purpose of setting i.e. --max-configs=1 is to
+                // skip configurations. When --check-config is used then tooManyConfigs will be reported even if the
+                // value is non-default.
+                const Settings defaultSettings;
+                if (mSettings.maxConfigs == defaultSettings.maxConfigs && mSettings.severity.isEnabled(Severity::information))
+                    tooManyConfigsError(Path::toNativeSeparators(file.spath()), configurations.size());
+
                 break;
+            }
 
             std::string currentConfig;
 
@@ -1630,32 +1638,14 @@ void CppCheck::executeAddonsWholeProgram(const std::list<FileWithDetails> &files
 
 void CppCheck::tooManyConfigsError(const std::string &file, const int numberOfConfigurations)
 {
-    if (!mSettings.severity.isEnabled(Severity::information) && !mTooManyConfigs)
-        return;
-
-    mTooManyConfigs = false;
-
-    if (mSettings.severity.isEnabled(Severity::information) && file.empty())
-        return;
-
     std::list<ErrorMessage::FileLocation> loclist;
     if (!file.empty()) {
         loclist.emplace_back(file, 0, 0);
     }
 
     std::ostringstream msg;
-    msg << "Too many #ifdef configurations - cppcheck only checks " << mSettings.maxConfigs;
-    if (numberOfConfigurations > mSettings.maxConfigs)
-        msg << " of " << numberOfConfigurations << " configurations. Use --force to check all configurations.\n";
-    if (file.empty())
-        msg << " configurations. Use --force to check all configurations. For more details, use --enable=information.\n";
-    msg << "The checking of the file will be interrupted because there are too many "
-        "#ifdef configurations. Checking of all #ifdef configurations can be forced "
-        "by --force command line option or from GUI preferences. However that may "
-        "increase the checking time.";
-    if (file.empty())
-        msg << " For more details, use --enable=information.";
-
+    msg << "Too many #ifdef configurations - cppcheck only checks " << mSettings.maxConfigs
+        << " of " << numberOfConfigurations << " configurations. Use --force to check all configurations.";
 
     ErrorMessage errmsg(std::move(loclist),
                         "",
@@ -1669,8 +1659,6 @@ void CppCheck::tooManyConfigsError(const std::string &file, const int numberOfCo
 
 void CppCheck::purgedConfigurationMessage(const std::string &file, const std::string& configuration)
 {
-    mTooManyConfigs = false;
-
     if (mSettings.severity.isEnabled(Severity::information) && file.empty())
         return;
 
@@ -1699,7 +1687,6 @@ void CppCheck::getErrorMessages(ErrorLogger &errorlogger)
 
     CppCheck cppcheck(settings, supprs, errorlogger, true, nullptr);
     cppcheck.purgedConfigurationMessage("","");
-    cppcheck.mTooManyConfigs = true;
     cppcheck.tooManyConfigsError("",0U);
     // TODO: add functions to get remaining error messages
 

lib/cppcheck.h

@@ -249,9 +249,6 @@ class CPPCHECKLIB CppCheck {
 
     bool mUseGlobalSuppressions;
 
-    /** Are there too many configs? */
-    bool mTooManyConfigs{};
-
     /** File info used for whole program analysis */
     std::list<Check::FileInfo*> mFileInfo;
 

test/cli/other_test.py

@@ -3948,3 +3948,40 @@ def test_simplecpp_syntax_error(tmp_path):
         '{}:1:0: error: No header in #include [syntaxError]'.format(test_file),
         '{}:1:0: error: No header in #include [syntaxError]'.format(test_file)
     ]
+
+
+@pytest.mark.parametrize('max_configs,number_of_configs,check_config,expected_warn', [
+    # max configs = default, max configs < number of configs => warn
+    (12, 20, False, True),
+    (12, 20, True, True),
+
+    # max configs != default, max configs < number of configs => warn if --check-config
+    (6, 20, False, False),
+    (6, 20, True, True),
+
+    # max configs >= number of configs => no warning
+    (20, 20, False, False),
+    (20, 20, False, False)
+])
+def test_max_configs(tmp_path, max_configs, number_of_configs, check_config, expected_warn):
+    test_file = tmp_path / 'test.cpp'
+    with open(test_file, "w") as f:
+        for i in range(1,number_of_configs):
+            dir = 'if' if i == 1 else 'elif'
+            f.write(f'#{dir} defined(X{i})\nx = {i};\n')
+        f.write('#endif\n')
+
+    args = [f'--max-configs={max_configs}', '--enable=information', '--template=simple', str(test_file)]
+
+    if check_config:
+        args = ['--check-config'] + args
+
+    # default max configs is set to 12, warn if code contains more configurations than that
+    _, _, stderr = cppcheck(args)
+    if not expected_warn:
+        assert stderr.splitlines() == []
+    else:
+        assert stderr.splitlines() == [
+            '{}:0:0: information: Too many #ifdef configurations - cppcheck only checks {} of {} configurations. Use --force to check all configurations. [toomanyconfigs]'
+            .format(test_file, max_configs, number_of_configs)
+        ]

test/testcppcheck.cpp

@@ -81,7 +81,6 @@ class TestCppcheck : public TestFixture {
         TEST_CASE(getDumpFileContentsLibrary);
         TEST_CASE(checkPlistOutput);
         TEST_CASE(premiumResultsCache);
-        TEST_CASE(toomanyconfigs);
         TEST_CASE(purgedConfiguration);
     }
 
@@ -594,35 +593,6 @@ class TestCppcheck : public TestFixture {
         ASSERT(hash1 != hash2);
     }
 
-    void toomanyconfigs() const
-    {
-        ScopedFile test_file_a("a.c",
-                               "#if DEF_1\n"
-                               "#endif\n"
-                               "#if DEF_2\n"
-                               "#endif\n"
-                               "#if DEF_3\n"
-                               "#endif");
-
-        // this is the "simple" format
-        const auto s = dinit(Settings,
-                             $.templateFormat = templateFormat, // TODO: remove when we only longer rely on toString() in unique message handling
-                                 $.severity.enable (Severity::information);
-                             $.maxConfigs = 2);
-        Suppressions supprs;
-        ErrorLogger2 errorLogger;
-        CppCheck cppcheck(s, supprs, errorLogger, false, {});
-        ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file_a.path(), Path::identify(test_file_a.path(), false), 0)));
-        // TODO: how to properly disable these warnings?
-        errorLogger.errmsgs.erase(std::remove_if(errorLogger.errmsgs.begin(), errorLogger.errmsgs.end(), [](const ErrorMessage& msg) {
-            return msg.id == "logChecker";
-        }), errorLogger.errmsgs.end());
-        // the internal errorlist is cleared after each check() call
-        ASSERT_EQUALS(1, errorLogger.errmsgs.size());
-        const auto it = errorLogger.errmsgs.cbegin();
-        ASSERT_EQUALS("a.c:0:0: information: Too many #ifdef configurations - cppcheck only checks 2 of 4 configurations. Use --force to check all configurations. [toomanyconfigs]", it->toString(false, templateFormat, ""));
-    }
-
     void purgedConfiguration() const
     {
         ScopedFile test_file("test.cpp",