review fixes, other fixes

Makefile

@@ -173,7 +173,7 @@ ifndef INCLUDE_FOR_LIB
 endif
 
 ifndef INCLUDE_FOR_CLI
-    INCLUDE_FOR_CLI=-Ilib -isystem externals/simplecpp -isystem externals/tinyxml2
+    INCLUDE_FOR_CLI=-Ilib -isystem externals/picojson -isystem externals/simplecpp -isystem externals/tinyxml2
 endif
 
 ifndef INCLUDE_FOR_TEST
@@ -762,7 +762,7 @@ $(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathli
 cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h lib/xml.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp
 
-cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/executor.h cli/processexecutor.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
+cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/executor.h cli/processexecutor.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutor.cpp
 
 cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/config.h lib/filesettings.h lib/path.h lib/platform.h lib/standards.h lib/utils.h

cli/CMakeLists.txt

@@ -12,6 +12,7 @@ if (BUILD_CLI)
     else()
         target_include_directories(cli_objs SYSTEM PRIVATE ${tinyxml2_INCLUDE_DIRS})
     endif()
+    target_externals_include_directories(cli_objs PRIVATE ${PROJECT_SOURCE_DIR}/externals/picojson/)
     target_externals_include_directories(cli_objs PRIVATE ${PROJECT_SOURCE_DIR}/externals/simplecpp/)
     if (NOT CMAKE_DISABLE_PRECOMPILE_HEADERS)
         target_precompile_headers(cli_objs PRIVATE precompiled.h)

cli/cli.vcxproj

@@ -85,7 +85,7 @@
   </PropertyGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\picojson;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <BufferSecurityCheck>true</BufferSecurityCheck>
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
       <Optimization>Disabled</Optimization>
@@ -114,7 +114,7 @@
   </ItemDefinitionGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug-PCRE|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\picojson;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <BufferSecurityCheck>true</BufferSecurityCheck>
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
       <Optimization>Disabled</Optimization>
@@ -143,7 +143,7 @@
   </ItemDefinitionGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\picojson;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <BufferSecurityCheck>false</BufferSecurityCheck>
       <Optimization>MaxSpeed</Optimization>
       <PreprocessorDefinitions>CPPCHECKLIB_IMPORT;TINYXML2_IMPORT;NDEBUG;WIN32;_CRT_SECURE_NO_WARNINGS;WIN32_LEAN_AND_MEAN;_WIN64;%(PreprocessorDefinitions)</PreprocessorDefinitions>
@@ -181,7 +181,7 @@
   </ItemDefinitionGroup>
   <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release-PCRE|x64'">
     <ClCompile>
-      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalIncludeDirectories>..\lib;..\externals;..\externals\picojson;..\externals\simplecpp;..\externals\tinyxml2;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
       <BufferSecurityCheck>false</BufferSecurityCheck>
       <Optimization>MaxSpeed</Optimization>
       <PreprocessorDefinitions>CPPCHECKLIB_IMPORT;TINYXML2_IMPORT;NDEBUG;WIN32;HAVE_RULES;_CRT_SECURE_NO_WARNINGS;WIN32_LEAN_AND_MEAN;_WIN64;%(PreprocessorDefinitions)</PreprocessorDefinitions>

cli/cmdlineparser.cpp

@@ -917,6 +917,20 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
             else if (std::strncmp(argv[i], "--output-file=", 14) == 0)
                 mSettings.outputFile = Path::simplifyPath(argv[i] + 14);
 
+            else if (std::strncmp(argv[i], "--output-format=", 16) == 0) {
+                const std::string format = argv[i] + 16;
+                if (format == "sarif")
+                    mSettings.outputFormat = Settings::OutputFormat::sarif;
+                else if (format == "xml")
+                    mSettings.outputFormat = Settings::OutputFormat::xml;
+                else {
+                    mLogger.printError("argument to '--output-format=' must be 'sarif' or 'xml'.");
+                    return Result::Fail;
+                }
+                mSettings.xml = (mSettings.outputFormat == Settings::OutputFormat::xml);
+            }
+
+
             // Experimental: limit execution time for extended valueflow analysis. basic valueflow analysis
             // is always executed.
             else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0) {
@@ -951,6 +965,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
 
             // Write results in results.plist
             else if (std::strncmp(argv[i], "--plist-output=", 15) == 0) {
+                mSettings.outputFormat = Settings::OutputFormat::plist;
                 mSettings.plistOutput = Path::simplifyPath(argv[i] + 15);
                 if (mSettings.plistOutput.empty())
                     mSettings.plistOutput = ".";
@@ -1362,12 +1377,11 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
             else if (std::strcmp(argv[i], "-v") == 0 || std::strcmp(argv[i], "--verbose") == 0)
                 mSettings.verbose = true;
 
-            else if (std::strcmp(argv[i], "--sarif") == 0)
-                mSettings.sarif = true;
-
             // Write results in results.xml
-            else if (std::strcmp(argv[i], "--xml") == 0)
+            else if (std::strcmp(argv[i], "--xml") == 0) {
                 mSettings.xml = true;
+                mSettings.outputFormat = Settings::OutputFormat::xml;
+            }
 
             // Define the XML file version (and enable XML output)
             else if (std::strncmp(argv[i], "--xml-version=", 14) == 0) {
@@ -1383,6 +1397,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                 mSettings.xml_version = tmp;
                 // Enable also XML if version is set
                 mSettings.xml = true;
+                mSettings.outputFormat = Settings::OutputFormat::xml;
             }
 
             else {
@@ -1628,6 +1643,10 @@ void CmdLineParser::printHelp() const
         "                         is 2. A larger value will mean more errors can be found\n"
         "                         but also means the analysis will be slower.\n"
         "    --output-file=<file> Write results to file, rather than standard error.\n"
+        "    --output-format=<format>\n"
+        "                        Specify the output format. The available formats are:\n"
+        "                          * sarif\n"
+        "                          * xml\n"
         "    --platform=<type>, --platform=<file>\n"
         "                         Specifies platform specific types and sizes. The\n"
         "                         available builtin platforms are:\n"

cli/cppcheckexecutor.cpp

@@ -28,14 +28,12 @@
 #include "errorlogger.h"
 #include "errortypes.h"
 #include "filesettings.h"
+#include "json.h"
 #include "settings.h"
 #include "singleexecutor.h"
 #include "suppressions.h"
 #include "utils.h"
 
-#define PICOJSON_USE_INT64
-#include "../externals/picojson/picojson.h"
-
 #if defined(HAS_THREADING_MODEL_THREAD)
 #include "threadexecutor.h"
 #endif
@@ -90,13 +88,13 @@ namespace {
                     picojson::object shortDescription;
                     shortDescription["text"] = picojson::value(finding.shortMessage());
                     rule["shortDescription"] = picojson::value(shortDescription);
-                    ret.push_back(picojson::value(rule));
+                    ret.emplace_back(rule);
                 }
             }
             return ret;
         }
 
-        picojson::array serializeLocations(const ErrorMessage& finding) const {
+        static picojson::array serializeLocations(const ErrorMessage& finding) {
             picojson::array ret;
             for (const auto& location : finding.callStack) {
                 picojson::object physicalLocation;
@@ -109,7 +107,7 @@ namespace {
                 physicalLocation["region"] = picojson::value(region);
                 picojson::object loc;
                 loc["physicalLocation"] = picojson::value(physicalLocation);
-                ret.push_back(picojson::value(loc));
+                ret.emplace_back(loc);
             }
             return ret;
         }
@@ -125,12 +123,12 @@ namespace {
                 message["text"] = picojson::value(finding.shortMessage());
                 res["message"] = picojson::value(message);
                 res["ruleId"] = picojson::value(finding.id);
-                results.push_back(picojson::value(res));
+                results.emplace_back(res);
             }
             return results;
         }
 
-        picojson::value serializeRuns(std::string productName, std::string version) const {
+        picojson::value serializeRuns(const std::string& productName, const std::string& version) const {
             picojson::object driver;
             driver["name"] = picojson::value(productName);
             driver["version"] = picojson::value(version);
@@ -147,8 +145,10 @@ namespace {
 
         std::string serialize(std::string productName) const {
             const auto nameAndVersion = Settings::getNameAndVersion(productName);
-            productName = nameAndVersion.first;
-            const std::string version = nameAndVersion.first.empty() ? CppCheck::version() : nameAndVersion.second;
+            productName = nameAndVersion.first.empty() ? "Cppcheck" : nameAndVersion.first;
+            std::string version = nameAndVersion.first.empty() ? CppCheck::version() : nameAndVersion.second;
+            if (version.find(" ") != std::string::npos)
+                version.erase(version.find(" "), std::string::npos);
 
             picojson::object doc;
             doc["version"] = picojson::value("2.1.0");
@@ -216,8 +216,8 @@ namespace {
         }
 
         ~StdLogger() override {
-            if (mSettings.sarif) {
-                std::cerr << mSarifReport.serialize(mSettings.cppcheckCfgProductName);
+            if (mSettings.outputFormat == Settings::OutputFormat::sarif) {
+                reportErr(mSarifReport.serialize(mSettings.cppcheckCfgProductName));
             }
             delete mErrorOutput;
         }
@@ -564,7 +564,7 @@ void StdLogger::reportErr(const ErrorMessage &msg)
     if (!mShownErrors.insert(msg.toString(mSettings.verbose)).second)
         return;
 
-    if (mSettings.sarif)
+    if (mSettings.outputFormat == Settings::OutputFormat::sarif)
         mSarifReport.addFinding(msg);
     else if (mSettings.xml)
         reportErr(msg.toXML());

lib/settings.h

@@ -271,6 +271,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief write results (--output-file=<file>) */
     std::string outputFile;
 
+    enum class OutputFormat : std::uint8_t {text, plist, sarif, xml};
+    OutputFormat outputFormat = OutputFormat::text;
+
     Platform platform;
 
     /** @brief pid of cppcheck. Intention is that this is set in the main process. */
@@ -441,9 +444,6 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief Is --verbose given? */
     bool verbose{};
 
-    /** @brief write SARIF results (--sarif) */
-    bool sarif{};
-
     /** @brief write XML results (--xml) */
     bool xml{};
 

releasenotes.txt

@@ -10,6 +10,8 @@ GUI:
 -
 
 Changed interface:
+- SARIF output. Use --output-format=sarif to activate this.
+- Add option --output-format=<format>. Allowed formats are sarif and xml.
 -
 
 Deprecations:

samples/incorrectLogicOperator/bad.c

@@ -0,0 +1,6 @@
+
+void foo(int x) {
+    if (x >= 0 || x <= 10) {}
+}
+
+dummy=foo();

samples/incorrectLogicOperator/good.c

@@ -0,0 +1,6 @@
+
+void foo(int x) {
+    if (x >= 0 && x <= 10) {}
+}
+
+dummy=foo();

samples/incorrectLogicOperator/out.txt

@@ -0,0 +1,3 @@
+samples/incorrectLogicOperator/bad.c:3:16: warning: Logical disjunction always evaluates to true: x >= 0 || x <= 10. [incorrectLogicOperator]
+    if (x >= 0 || x <= 10) {}
+               ^

test/cli/helloworld_test.py

@@ -4,6 +4,7 @@
 import os
 import re
 import glob
+import json
 
 from testutils import create_gui_project_file, cppcheck
 
@@ -319,3 +320,17 @@ def test_missing_include_system():  # #11283
 
     _, _, stderr = cppcheck(args, cwd=__script_dir)
     assert stderr.replace('\\', '/') == 'helloworld/main.c:1:0: information: Include file: <stdio.h> not found. Please note: Cppcheck does not need standard library headers to get proper results. [missingIncludeSystem]\n'
+
+
+def test_sarif():
+    args = [
+        '--output-format=sarif',
+        'helloworld'
+    ]
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    assert ret == 0, stdout
+    res = json.loads(stderr)
+    assert res['version'] == '2.1.0'
+    assert res['runs'][0]['results'][0]['locations'][0]['physicalLocation']['artifactLocation']['uri'] == 'helloworld/main.c'
+    assert res['runs'][0]['results'][0]['ruleId'] == 'zerodiv'
+    assert res['runs'][0]['results'][0]['message']['text'] == 'Division by zero.'

test/testcmdlineparser.cpp

@@ -208,6 +208,11 @@ class TestCmdlineParser : public TestFixture {
         TEST_CASE(maxConfigsMissingCount);
         TEST_CASE(maxConfigsInvalid);
         TEST_CASE(maxConfigsTooSmall);
+        TEST_CASE(outputFormatSarif);
+        TEST_CASE(outputFormatXml);
+        TEST_CASE(outputFormatOther);
+        TEST_CASE(outputFormatImplicitPlist);
+        TEST_CASE(outputFormatImplicitXml);
         TEST_CASE(premiumOptions1);
         TEST_CASE(premiumOptions2);
         TEST_CASE(premiumOptions3);
@@ -1247,6 +1252,41 @@ class TestCmdlineParser : public TestFixture {
         ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' must be greater than 0.\n", logger->str());
     }
 
+    void outputFormatSarif() {
+        REDIRECT;
+        const char * const argv[] = {"cppcheck", "--output-format=sarif", "file.cpp"};
+        ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
+        ASSERT_EQUALS_ENUM(Settings::OutputFormat::sarif, settings->outputFormat);
+    }
+
+    void outputFormatXml() {
+        REDIRECT;
+        const char * const argv[] = {"cppcheck", "--output-format=xml", "file.cpp"};
+        ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
+        ASSERT_EQUALS_ENUM(Settings::OutputFormat::xml, settings->outputFormat);
+    }
+
+    void outputFormatOther() {
+        REDIRECT;
+        const char * const argv[] = {"cppcheck", "--output-format=text", "file.cpp"};
+        ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
+        ASSERT_EQUALS("cppcheck: error: argument to '--output-format=' must be 'sarif' or 'xml'.\n", logger->str());
+    }
+
+    void outputFormatImplicitPlist() {
+        REDIRECT;
+        const char * const argv[] = {"cppcheck", "--plist-output=.", "file.cpp"};
+        ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
+        ASSERT_EQUALS_ENUM(Settings::OutputFormat::plist, settings->outputFormat);
+    }
+
+    void outputFormatImplicitXml() {
+        REDIRECT;
+        const char * const argv[] = {"cppcheck", "--xml", "file.cpp"};
+        ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
+        ASSERT_EQUALS_ENUM(Settings::OutputFormat::xml, settings->outputFormat);
+    }
+
     void premiumOptions1() {
         REDIRECT;
         asPremium();