Fix #11824 (Option --max-configs has no effect if -D is used) (#7980)

Number of checked configurations:
cppcheck file.c => 12
cppcheck -DX file.c => 1
cppcheck -DX --max-configs=6 file.c => 6

move logic from cmdlineparser to Settings class where the code can be
reused (from GUI). The `Settings::checkAllConfigs` is removed.

Author: danmar

cli/cmdlineparser.cpp

@@ -407,8 +407,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
         }
     }
 
-    bool def = false;
-    bool maxconfigs = false;
     bool debug = false;
     bool inputAsFilter = false; // set by: --file-filter=+
 
@@ -457,8 +455,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
             if (!mSettings.userDefines.empty())
                 mSettings.userDefines += ";";
             mSettings.userDefines += define;
-
-            def = true;
         }
 
         // -E
@@ -801,8 +797,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
         }
 
         // Force checking of files that have "too many" configurations
-        else if (std::strcmp(argv[i], "-f") == 0 || std::strcmp(argv[i], "--force") == 0)
+        else if (std::strcmp(argv[i], "-f") == 0 || std::strcmp(argv[i], "--force") == 0) {
             mSettings.force = true;
+            mSettings.maxConfigsOption = Settings::maxConfigsNotAssigned;
+        }
 
         else if (std::strcmp(argv[i], "--fsigned-char") == 0)
             defaultSign = 's';
@@ -974,9 +972,8 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                 return Result::Fail;
             }
 
-            mSettings.maxConfigs = tmp;
+            mSettings.maxConfigsOption = tmp;
             mSettings.force = false;
-            maxconfigs = true;
         }
 
         // max ctu depth
@@ -1160,7 +1157,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                 return Result::Fail;
             }
 
-            mSettings.checkAllConfigurations = false;     // Can be overridden with --max-configs or --force
             std::string projectFile = argv[i]+10;
             projectType = project.import(projectFile, &mSettings, &mSuppressions);
             if (projectType == ImportProject::Type::CPPCHECK_GUI) {
@@ -1187,6 +1183,8 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                     }
                 }
             }
+            if (projectType == ImportProject::Type::COMPILE_DB)
+                mSettings.maxConfigsProject = 1;
             if (projectType == ImportProject::Type::VS_SLN || projectType == ImportProject::Type::VS_VCXPROJ) {
                 mSettings.libraries.emplace_back("windows");
             }
@@ -1591,14 +1589,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
     substituteTemplateFormatStatic(mSettings.templateFormat, !mSettings.outputFile.empty());
     substituteTemplateLocationStatic(mSettings.templateLocation, !mSettings.outputFile.empty());
 
-    if (mSettings.force || maxconfigs)
-        mSettings.checkAllConfigurations = true;
-
-    if (mSettings.force)
-        mSettings.maxConfigs = INT_MAX;
-    else if ((def || mSettings.preprocessOnly) && !maxconfigs)
-        mSettings.maxConfigs = 1U;
-
     if (debug) {
         mSettings.debugnormal = true;
         mSettings.debugvalueflow = true;

gui/mainwindow.cpp

@@ -1136,13 +1136,9 @@ bool MainWindow::getCppcheckSettings(Settings& settings, Suppressions& supprs)
             supprs.nomsg.addSuppression(suppression); // TODO: check result
         }
 
-        // Only check the given -D configuration
-        if (!defines.isEmpty())
-            settings.maxConfigs = 1;
-
         // If importing a project, only check the given configuration
-        if (!mProjectFile->getImportProject().isEmpty())
-            settings.checkAllConfigurations = false;
+        if (mProjectFile->getImportProject().endsWith("json", Qt::CaseInsensitive))
+            settings.maxConfigsProject = 1;
 
         const QString &buildDir = fromNativePath(mProjectFile->getBuildDir());
         if (!buildDir.isEmpty()) {

lib/cppcheck.cpp

@@ -859,7 +859,7 @@ std::size_t CppCheck::calculateHash(const Preprocessor& preprocessor, const std:
     toolinfo << mSettings.userDefines;
     toolinfo << (mSettings.checkConfiguration ? 'c' : ' '); // --check-config
     toolinfo << (mSettings.force ? 'f' : ' ');
-    toolinfo << mSettings.maxConfigs;
+    toolinfo << mSettings.maxConfigsOption;
     toolinfo << std::to_string(static_cast<std::uint8_t>(mSettings.checkLevel));
     for (const auto &a : mSettings.addonInfos) {
         toolinfo << a.name;
@@ -908,6 +908,8 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
     if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mUnusedFunctionsCheck)
         mUnusedFunctionsCheck.reset(new CheckUnusedFunctions());
 
+    const int maxConfigs = mSettings.getMaxConfigs();
+
     mLogger->resetExitCode();
 
     if (Settings::terminated())
@@ -1032,7 +1034,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
 
         // Get configurations..
         std::set<std::string> configurations;
-        if ((mSettings.checkAllConfigurations && mSettings.userDefines.empty()) || mSettings.force) {
+        if (maxConfigs > 1) {
             Timer::run("Preprocessor::getConfigs", mSettings.showtime, &s_timerResults, [&]() {
                 configurations = preprocessor.getConfigs();
             });
@@ -1044,7 +1046,7 @@ 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)
+            if (configurations.size() > maxConfigs)
                 tooManyConfigsError(Path::toNativeSeparators(file.spath()), configurations.size());
 
             if (analyzerInformation)
@@ -1090,14 +1092,13 @@ 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 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
+            if (!mSettings.force && ++checkCount > maxConfigs) {
+                // If maxConfigs is not assigned then report information message that configurations are skipped.
+                // If maxConfigs is assigned 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))
+                if (!mSettings.isMaxConfigsAssigned() && mSettings.severity.isEnabled(Severity::information))
                     tooManyConfigsError(Path::toNativeSeparators(file.spath()), configurations.size());
 
                 break;
@@ -1198,7 +1199,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
                     }
 
                     // Skip if we already met the same simplified token list
-                    if (mSettings.force || mSettings.maxConfigs > 1) {
+                    if (maxConfigs > 1) {
                         const std::size_t hash = tokenizer.list.calculateHash();
                         if (hashes.find(hash) != hashes.end()) {
                             if (mSettings.debugwarnings)
@@ -1644,7 +1645,7 @@ void CppCheck::tooManyConfigsError(const std::string &file, const int numberOfCo
     }
 
     std::ostringstream msg;
-    msg << "Too many #ifdef configurations - cppcheck only checks " << mSettings.maxConfigs
+    msg << "Too many #ifdef configurations - cppcheck only checks " << mSettings.getMaxConfigs()
         << " of " << numberOfConfigurations << " configurations. Use --force to check all configurations.";
 
     ErrorMessage errmsg(std::move(loclist),

lib/settings.cpp

@@ -43,6 +43,9 @@
 
 std::atomic<bool> Settings::mTerminated;
 
+const int Settings::maxConfigsNotAssigned = 0;
+const int Settings::maxConfigsDefault = 12;
+
 const char Settings::SafeChecks::XmlRootName[] = "safe-checks";
 const char Settings::SafeChecks::XmlClasses[] = "class-public";
 const char Settings::SafeChecks::XmlExternalFunctions[] = "external-functions";

lib/settings.h

@@ -136,9 +136,6 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief --cppcheck-build-dir. Always uses / as path separator. No trailing path separator. */
     std::string buildDir;
 
-    /** @brief check all configurations (false if -D or --max-configs is used */
-    bool checkAllConfigurations = true;
-
     /** Is the 'configuration checking' wanted? */
     bool checkConfiguration{};
 
@@ -294,9 +291,31 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     int loadAverage{};
 #endif
 
-    /** @brief Maximum number of configurations to check before bailing.
-        Default is 12. (--max-configs=N) */
-    int maxConfigs = 12;
+    /** --max-configs value */
+    int maxConfigsOption = 0; // "Not Assigned" value
+
+    /** max configs from --project option */
+    int maxConfigsProject = 0; // "Not Assigned" value
+
+    static const int maxConfigsNotAssigned;
+    static const int maxConfigsDefault;
+
+    bool isMaxConfigsAssigned() const {
+        return maxConfigsOption != maxConfigsNotAssigned || maxConfigsProject != maxConfigsNotAssigned;
+    }
+
+    /** @brief Maximum number of configurations to check before bailing. */
+    int getMaxConfigs() const {
+        if (force)
+            return 0x7fffffff;
+        if (maxConfigsOption != maxConfigsNotAssigned)
+            return maxConfigsOption;
+        if (maxConfigsProject != maxConfigsNotAssigned)
+            return maxConfigsProject;
+        if (!userDefines.empty())
+            return 1;
+        return maxConfigsDefault;
+    }
 
     /** @brief --max-ctu-depth */
     int maxCtuDepth = 2;

man/manual.md

@@ -287,16 +287,21 @@ To ignore certain folders in the project you can use `-i`. This will skip the an
 
     cppcheck --project=foobar.cppcheck -ifoo
 
-## CMake
+## Compilation database (cmake etc)
 
-Generate a compile database (a JSON file containing compilation commands for each source file):
+Many build systems can generate a compilation database (a JSON file containing compilation commands for each source file).
+Example `cmake` command to generate the file:
 
     cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON .
 
-The file `compile_commands.json` is created in the current folder. Now run Cppcheck like this:
+When you have a `compile_commands.json` file you can run Cppcheck like this:
 
     cppcheck --project=compile_commands.json
 
+By default only 1 configuration is checked because that is consistent with the compilation. If you want to check more configurations you can use `--max-configs` or `--force`. For example:
+
+    cppcheck --project=compile_commands.json --force
+
 To ignore certain folders you can use `-i`. This will skip analysis of source files in the `foo` folder.
 
     cppcheck --project=compile_commands.json -ifoo
@@ -338,12 +343,16 @@ To ignore certain folders in the project you can use `-i`. This will skip analys
 
 ## Other
 
-If you can generate a compile database, then it is possible to import that in Cppcheck.
+If you generate a compilation database, then it is possible to import that in Cppcheck.
+
+### Makefile
 
-In Linux you can use for instance the `bear` (build ear) utility to generate a compile database from arbitrary build tools:
+In Linux you can convert a Makefile to a compile_commands.json using for instance `bear` (build ear) utility:
 
     bear -- make
 
+If you don't use Linux; there are python scripts that converts a Makefile into a compilation database.
+
 # Preprocessor Settings
 
 If you use `--project` then Cppcheck will automatically use the preprocessor settings in the imported project file and
@@ -388,21 +397,28 @@ Example:
     cppcheck test.c
 
     # only test configuration "-DA"
-    # No bug is found (#error)
+    # No bug is found; because C is not defined the #error will cause a preprocessor error
     cppcheck -DA test.c
 
     # only test configuration "-DA -DC"
     # The first bug is found
     cppcheck -DA -DC test.c
 
-    # The configuration "-DC" is tested
+    # Test all configurations that does not define "A"
     # The last bug is found
     cppcheck -UA test.c
 
     # All configurations with "-DA" are tested
     # The two first bugs are found
     cppcheck --force -DA test.c
 
+    # only test 1 valid configuration
+    # Bug(s) will be found
+    cppcheck --max-configs=1 test.c
+
+    # test 2 valid configurations with "X" defined.
+    # Bug(s) will be found
+    cppcheck --max-configs=2 -DX test.c
 
 ## Include paths
 

test/cli/helloworld_test.py

@@ -123,7 +123,10 @@ def test_addon_with_gui_project(tmp_path):
     ret, stdout, stderr = cppcheck(args, cwd=tmp_path)
     filename = os.path.join('helloworld', 'main.c')
     assert ret == 0, stdout
-    assert stdout == 'Checking %s ...\n' % filename
+    assert stdout.strip().split('\n') == [
+        'Checking %s ...' % filename,
+        'Checking %s: SOME_CONFIG...' % filename
+    ]
     assert stderr == ('[%s:5]: (error) Division by zero.\n'
                       '[%s:4]: (style) misra violation (use --rule-texts=<file> to get proper output)\n' % (filename, filename))
 

test/cli/other_test.py

@@ -3952,8 +3952,8 @@ def test_simplecpp_syntax_error(tmp_path):
 
 @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),
+    (None, 20, False, True),
+    (None, 20, True, True),
 
     # max configs != default, max configs < number of configs => warn if --check-config
     (6, 20, False, False),
@@ -3971,7 +3971,12 @@ def test_max_configs(tmp_path, max_configs, number_of_configs, check_config, exp
             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)]
+    args = ['--enable=information', '--template=simple', str(test_file)]
+
+    if max_configs is None:
+        max_configs = 12  # default value
+    else:
+        args = [f'--max-configs={max_configs}'] + args
 
     if check_config:
         args = ['--check-config'] + args

test/testcmdlineparser.cpp

@@ -1415,7 +1415,7 @@ class TestCmdlineParser : public TestFixture {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "-f", "--max-configs=12", "file.cpp"};
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
-        ASSERT_EQUALS(12, settings->maxConfigs);
+        ASSERT_EQUALS(12, settings->maxConfigsOption);
         ASSERT_EQUALS(false, settings->force);
     }