Fix first instance rule being used as rule description for all violations of that rule and other SARIF improvements

AUTHORS

@@ -411,6 +411,7 @@ Tommy Bergman
 Toralf Förster
 Troshin V.S.
 Tyson Nottingham
+Usman Majid
 Valentin Batz
 Valerii Lashmanov
 Vasily Maslyukov

Makefile

@@ -176,7 +176,7 @@ ifndef INCLUDE_FOR_CLI
 endif
 
 ifndef INCLUDE_FOR_TEST
-    INCLUDE_FOR_TEST=-Ilib -Ifrontend -Icli -isystem externals/simplecpp -isystem externals/tinyxml2
+    INCLUDE_FOR_TEST=-Ilib -Ifrontend -Icli -isystem externals/picojson -isystem externals/simplecpp -isystem externals/tinyxml2
 endif
 
 ifndef CFLAGS_FOR_TEST
@@ -249,6 +249,7 @@ LIBOBJ =      $(libcppdir)/valueflow.o \
               $(libcppdir)/programmemory.o \
               $(libcppdir)/regex.o \
               $(libcppdir)/reverseanalyzer.o \
+              $(libcppdir)/sarifreport.o \
               $(libcppdir)/settings.o \
               $(libcppdir)/standards.o \
               $(libcppdir)/summaries.o \
@@ -327,6 +328,7 @@ TESTOBJ =     test/fixture.o \
               test/testprocessexecutor.o \
               test/testprogrammemory.o \
               test/testregex.o \
+              test/testsarifreport.o \
               test/testsettings.o \
               test/testsimplifytemplate.o \
               test/testsimplifytokens.o \
@@ -638,6 +640,9 @@ $(libcppdir)/regex.o: lib/regex.cpp lib/config.h lib/regex.h
 $(libcppdir)/reverseanalyzer.o: lib/reverseanalyzer.cpp lib/addoninfo.h lib/analyzer.h lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/library.h lib/mathlib.h lib/platform.h lib/reverseanalyzer.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueptr.h lib/vfvalue.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/reverseanalyzer.cpp
 
+$(libcppdir)/sarifreport.o: lib/sarifreport.cpp externals/picojson/picojson.h lib/addoninfo.h lib/check.h lib/checkers.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/json.h lib/library.h lib/mathlib.h lib/platform.h lib/sarifreport.h lib/settings.h lib/standards.h lib/utils.h
+	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/sarifreport.cpp
+
 $(libcppdir)/settings.o: lib/settings.cpp externals/picojson/picojson.h lib/addoninfo.h lib/checkers.h lib/config.h lib/errortypes.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/summaries.h lib/suppressions.h lib/utils.h lib/vfvalue.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/settings.cpp
 
@@ -683,7 +688,7 @@ frontend/frontend.o: frontend/frontend.cpp frontend/frontend.h lib/addoninfo.h l
 cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.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/regex.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/executor.h cli/processexecutor.h cli/sehwrapper.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.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
+cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h cli/sehwrapper.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.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/sarifreport.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/executor.o: cli/executor.cpp cli/executor.h lib/addoninfo.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
@@ -854,6 +859,9 @@ test/testprogrammemory.o: test/testprogrammemory.cpp lib/addoninfo.h lib/check.h
 test/testregex.o: test/testregex.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/regex.h lib/settings.h lib/standards.h lib/utils.h test/fixture.h
 	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testregex.cpp
 
+test/testsarifreport.o: test/testsarifreport.cpp externals/picojson/picojson.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/sarifreport.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
+	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsarifreport.cpp
+
 test/testsettings.o: test/testsettings.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
 	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsettings.cpp
 

cli/cppcheckexecutor.cpp

@@ -34,6 +34,7 @@
 #include "filesettings.h"
 #include "json.h"
 #include "path.h"
+#include "sarifreport.h"
 #include "settings.h"
 #include "singleexecutor.h"
 #include "suppressions.h"
@@ -79,156 +80,6 @@
 #endif
 
 namespace {
-    class SarifReport {
-    public:
-        void addFinding(ErrorMessage msg) {
-            mFindings.push_back(std::move(msg));
-        }
-
-        picojson::array serializeRules() const {
-            picojson::array ret;
-            std::set<std::string> ruleIds;
-            for (const auto& finding : mFindings) {
-                // github only supports findings with locations
-                if (finding.callStack.empty())
-                    continue;
-                if (ruleIds.insert(finding.id).second) {
-                    picojson::object rule;
-                    rule["id"] = picojson::value(finding.id);
-                    // rule.shortDescription.text
-                    picojson::object shortDescription;
-                    shortDescription["text"] = picojson::value(finding.shortMessage());
-                    rule["shortDescription"] = picojson::value(shortDescription);
-                    // rule.fullDescription.text
-                    picojson::object fullDescription;
-                    fullDescription["text"] = picojson::value(finding.verboseMessage());
-                    rule["fullDescription"] = picojson::value(fullDescription);
-                    // rule.help.text
-                    picojson::object help;
-                    help["text"] = picojson::value(finding.verboseMessage()); // FIXME provide proper help text
-                    rule["help"] = picojson::value(help);
-                    // rule.properties.precision, rule.properties.problem.severity
-                    picojson::object properties;
-                    properties["precision"] = picojson::value(sarifPrecision(finding));
-                    const char* securitySeverity = nullptr;
-                    if (finding.severity == Severity::error && !ErrorLogger::isCriticalErrorId(finding.id))
-                        securitySeverity = "9.9"; // We see undefined behavior
-                    //else if (finding.severity == Severity::warning)
-                    //    securitySeverity = 5.1; // We see potential undefined behavior
-                    if (securitySeverity) {
-                        properties["security-severity"] = picojson::value(securitySeverity);
-                        const picojson::array tags{picojson::value("security")};
-                        properties["tags"] = picojson::value(tags);
-                    }
-                    rule["properties"] = picojson::value(properties);
-                    // rule.defaultConfiguration.level
-                    picojson::object defaultConfiguration;
-                    defaultConfiguration["level"] = picojson::value(sarifSeverity(finding));
-                    rule["defaultConfiguration"] = picojson::value(defaultConfiguration);
-
-                    ret.emplace_back(rule);
-                }
-            }
-            return ret;
-        }
-
-        static picojson::array serializeLocations(const ErrorMessage& finding) {
-            picojson::array ret;
-            for (const auto& location : finding.callStack) {
-                picojson::object physicalLocation;
-                picojson::object artifactLocation;
-                artifactLocation["uri"] = picojson::value(location.getfile(false));
-                physicalLocation["artifactLocation"] = picojson::value(artifactLocation);
-                picojson::object region;
-                region["startLine"] = picojson::value(static_cast<int64_t>(location.line < 1 ? 1 : location.line));
-                region["startColumn"] = picojson::value(static_cast<int64_t>(location.column < 1 ? 1 : location.column));
-                region["endLine"] = region["startLine"];
-                region["endColumn"] = region["startColumn"];
-                physicalLocation["region"] = picojson::value(region);
-                picojson::object loc;
-                loc["physicalLocation"] = picojson::value(physicalLocation);
-                ret.emplace_back(loc);
-            }
-            return ret;
-        }
-
-        picojson::array serializeResults() const {
-            picojson::array results;
-            for (const auto& finding : mFindings) {
-                // github only supports findings with locations
-                if (finding.callStack.empty())
-                    continue;
-                picojson::object res;
-                res["level"] = picojson::value(sarifSeverity(finding));
-                res["locations"] = picojson::value(serializeLocations(finding));
-                picojson::object message;
-                message["text"] = picojson::value(finding.shortMessage());
-                res["message"] = picojson::value(message);
-                res["ruleId"] = picojson::value(finding.id);
-                results.emplace_back(res);
-            }
-            return results;
-        }
-
-        picojson::value serializeRuns(const std::string& productName, const std::string& version) const {
-            picojson::object driver;
-            driver["name"] = picojson::value(productName);
-            driver["semanticVersion"] = picojson::value(version);
-            driver["informationUri"] = picojson::value("https://cppcheck.sourceforge.io");
-            driver["rules"] = picojson::value(serializeRules());
-            picojson::object tool;
-            tool["driver"] = picojson::value(driver);
-            picojson::object run;
-            run["tool"] = picojson::value(tool);
-            run["results"] = picojson::value(serializeResults());
-            picojson::array runs{picojson::value(run)};
-            return picojson::value(runs);
-        }
-
-        std::string serialize(std::string productName) const {
-            const auto nameAndVersion = Settings::getNameAndVersion(productName);
-            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");
-            doc["$schema"] = picojson::value("https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/schemas/sarif-schema-2.1.0.json");
-            doc["runs"] = serializeRuns(productName, version);
-
-            return picojson::value(doc).serialize(true);
-        }
-    private:
-
-        static std::string sarifSeverity(const ErrorMessage& errmsg) {
-            if (ErrorLogger::isCriticalErrorId(errmsg.id))
-                return "error";
-            switch (errmsg.severity) {
-            case Severity::error:
-            case Severity::warning:
-            case Severity::style:
-            case Severity::portability:
-            case Severity::performance:
-                return "warning";
-            case Severity::information:
-            case Severity::internal:
-            case Severity::debug:
-            case Severity::none:
-                return "note";
-            }
-            return "note";
-        }
-
-        static std::string sarifPrecision(const ErrorMessage& errmsg) {
-            if (errmsg.certainty == Certainty::inconclusive)
-                return "medium";
-            return "high";
-        }
-
-        std::vector<ErrorMessage> mFindings;
-    };
-
     class CmdLineLoggerStd : public CmdLineLogger
     {
     public:
@@ -712,18 +563,20 @@ void StdLogger::reportErr(const ErrorMessage &msg)
     msgCopy.classification = getClassification(msgCopy.guideline, mSettings.reportType);
 
     // TODO: there should be no need for verbose and default messages here
-    const std::string msgStr = msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
+    const std::string msgStr =
+        msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
 
     // Alert only about unique errors
     if (!mSettings.emitDuplicates && !mShownErrors.insert(msgStr).second)
         return;
 
-    if (mSettings.outputFormat == Settings::OutputFormat::sarif)
+    if (mSettings.outputFormat == Settings::OutputFormat::sarif) {
         mSarifReport.addFinding(std::move(msgCopy));
-    else if (mSettings.outputFormat == Settings::OutputFormat::xml)
+    } else if (mSettings.outputFormat == Settings::OutputFormat::xml) {
         reportErr(msgCopy.toXML());
-    else
+    } else {
         reportErr(msgStr);
+    }
 }
 
 /**

lib/cppcheck.vcxproj

@@ -81,6 +81,7 @@
     <ClCompile Include="programmemory.cpp" />
     <ClCompile Include="regex.cpp" />
     <ClCompile Include="reverseanalyzer.cpp" />
+    <ClCompile Include="sarifreport.cpp" />
     <ClCompile Include="settings.cpp" />
     <ClCompile Include="standards.cpp" />
     <ClCompile Include="summaries.cpp" />
@@ -158,6 +159,7 @@
     <ClInclude Include="programmemory.h" />
     <ClInclude Include="regex.h" />
     <ClInclude Include="reverseanalyzer.h" />
+    <ClInclude Include="sarifreport.h" />
     <ClInclude Include="settings.h" />
     <ClInclude Include="smallvector.h" />
     <ClInclude Include="sourcelocation.h" />