Fix #13832 (cppcheck build dir: do not reuse cached results if there were invalidLicense errors)

Makefile

@@ -710,7 +710,7 @@ test/options.o: test/options.cpp test/options.h
 test/test64bit.o: test/test64bit.cpp lib/addoninfo.h lib/check.h lib/check64bit.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/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/test64bit.cpp
 
-test/testanalyzerinformation.o: test/testanalyzerinformation.cpp lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.h lib/color.h lib/config.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/utils.h test/fixture.h
+test/testanalyzerinformation.o: test/testanalyzerinformation.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.h lib/color.h lib/config.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/utils.h lib/xml.h test/fixture.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testanalyzerinformation.cpp
 
 test/testassert.o: test/testassert.cpp lib/addoninfo.h lib/check.h lib/checkassert.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/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h

lib/analyzerinfo.cpp

@@ -83,21 +83,26 @@ void AnalyzerInformation::close()
     }
 }
 
-static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t hash, std::list<ErrorMessage> &errors)
+bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors)
 {
-    tinyxml2::XMLDocument doc;
-    const tinyxml2::XMLError error = doc.LoadFile(analyzerInfoFile.c_str());
-    if (error != tinyxml2::XML_SUCCESS)
-        return false;
-
-    const tinyxml2::XMLElement * const rootNode = doc.FirstChildElement();
+    const tinyxml2::XMLElement * const rootNode = analyzerInfoDoc.FirstChildElement();
     if (rootNode == nullptr)
         return false;
 
     const char *attr = rootNode->Attribute("hash");
     if (!attr || attr != std::to_string(hash))
         return false;
 
+    // Check for invalid license error or internal error, in which case we should retry analysis
+    for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
+        if (std::strcmp(e->Name(), "error") == 0 &&
+            (e->Attribute("id", "premium-invalidLicense") ||
+             e->Attribute("id", "premium-internalError") ||
+             e->Attribute("id", "internalError")
+            ))
+            return false;
+    }
+
     for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
         if (std::strcmp(e->Name(), "error") == 0)
             errors.emplace_back(e);
@@ -147,7 +152,9 @@ bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::st
 
     mAnalyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg,fileIndex);
 
-    if (skipAnalysis(mAnalyzerInfoFile, hash, errors))
+    tinyxml2::XMLDocument analyzerInfoDoc;
+    const tinyxml2::XMLError xmlError = analyzerInfoDoc.LoadFile(mAnalyzerInfoFile.c_str());
+    if (xmlError == tinyxml2::XML_SUCCESS && skipAnalysis(analyzerInfoDoc, hash, errors))
         return false;
 
     mOutputStream.open(mAnalyzerInfoFile);

lib/analyzerinfo.h

@@ -31,6 +31,10 @@
 class ErrorMessage;
 struct FileSettings;
 
+namespace tinyxml2 {
+    class XMLDocument;
+};
+
 /// @addtogroup Core
 /// @{
 
@@ -49,6 +53,8 @@ struct FileSettings;
  */
 class CPPCHECKLIB AnalyzerInformation {
 public:
+    friend class TestAnalyzerInformation;
+
     ~AnalyzerInformation();
 
     static std::string getFilesTxt(const std::list<std::string> &sourcefiles, const std::string &userDefines, const std::list<FileSettings> &fileSettings);
@@ -75,7 +81,10 @@ class CPPCHECKLIB AnalyzerInformation {
 
 protected:
     static std::string getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg, int fileIndex);
+
 private:
+    static bool skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors);
+
     std::ofstream mOutputStream;
     std::string mAnalyzerInfoFile;
 };

test/cli/premium_test.py

@@ -33,7 +33,6 @@ def __copy_cppcheck_premium(tmpdir):
 
     return exe
 
-
 def test_misra_c_builtin_style_checks(tmpdir):
     # FIXME this test does not work in ci-windows.yml (release build)
     if sys.platform == 'win32':
@@ -130,3 +129,28 @@ def test_misra_py(tmpdir):
     _, stdout, _ = cppcheck(['--enable=style', '--premium=misra-c-2012', test_file], cppcheck_exe=exe)
     assert 'misra.py' not in stdout # Did not find misra.py
     assert 'Checking' in stdout
+
+def test_invalid_license_retry(tmpdir):
+    # Trac 13832 - cppcheck build dir: do not reuse cached results if there were invalidLicense errors
+    build_dir = os.path.join(tmpdir, 'b')
+    test_file = os.path.join(tmpdir, 'test.c')
+    addon_file = os.path.join(tmpdir, 'premiumaddon.py')
+
+    os.mkdir(build_dir)
+
+    with open(test_file, 'wt') as f:
+        f.write('void foo();\n')
+
+    args = [f"--addon={addon_file}", f"--cppcheck-build-dir={build_dir}", '--xml', '--enable=all', test_file]
+
+    with open(addon_file, 'wt') as f:
+        f.write('print(\'{"addon":"premium","column":0,"errorId":"invalidLicense","extra":"","file":"Cppcheck Premium","linenr":0,"message":"Invalid license: No license file was found, contact sales@cppchecksolutions.com","severity":"error"}\')')
+
+    _, _, stderr = cppcheck(args)
+    assert 'Invalid license' in stderr
+
+    with open(addon_file, 'wt') as f:
+        f.write('')
+
+    _, _, stderr = cppcheck(args)
+    assert 'Invalid license' not in stderr

test/testanalyzerinformation.cpp

@@ -20,8 +20,10 @@
 #include "analyzerinfo.h"
 #include "filesettings.h"
 #include "fixture.h"
+#include "xml.h"
 
 #include <sstream>
+#include <tinyxml2.h>
 
 class TestAnalyzerInformation : public TestFixture, private AnalyzerInformation {
 public:
@@ -34,6 +36,7 @@ class TestAnalyzerInformation : public TestFixture, private AnalyzerInformation
         TEST_CASE(duplicateFile);
         TEST_CASE(filesTextDuplicateFile);
         TEST_CASE(parse);
+        TEST_CASE(skipAnalysis);
     }
 
     void getAnalyzerInfoFile() const {
@@ -95,6 +98,135 @@ class TestAnalyzerInformation : public TestFixture, private AnalyzerInformation
         ASSERT_EQUALS(0, info.fileIndex);
         ASSERT_EQUALS("C:/dm/cppcheck-fix-13333/test/cli/whole-program/odr1.cpp", info.sourceFile);
     }
+
+    void skipAnalysis() const {
+        // Matching hash with license error (don't skip)
+        {
+            std::list<ErrorMessage> errorList;
+            tinyxml2::XMLDocument doc;
+
+            const tinyxml2::XMLError xmlError = doc.Parse(
+                "<?xml version=\"1.0\"?>"
+                "<analyzerinfo hash=\"100\">"
+                "<error id=\"premium-invalidLicense\" severity=\"error\" msg=\"Invalid license: No license file was found, contact sales@cppchecksolutions.com\" verbose=\"Invalid license: No license file was found, contact sales@cppchecksolutions.com\" file0=\"test.c\">"
+                "<location file=\"Cppcheck Premium\" line=\"0\" column=\"0\"/>"
+                "</error>"
+                "</analyzerinfo>"
+                );
+            ASSERT_EQUALS(xmlError, tinyxml2::XML_SUCCESS);
+
+            ASSERT_EQUALS(AnalyzerInformation::skipAnalysis(doc, 100, errorList), false);
+            ASSERT_EQUALS(errorList.size(), 0);
+        }
+
+        // Matching hash with premium internal error (don't skip)
+        {
+            std::list<ErrorMessage> errorList;
+            tinyxml2::XMLDocument doc;
+
+            const tinyxml2::XMLError xmlError = doc.Parse(
+                "<?xml version=\"1.0\"?>"
+                "<analyzerinfo hash=\"100\">"
+                "<error id=\"premium-internalError\" severity=\"error\" msg=\"Something went wrong\" verbose=\"Something went wrong\" file0=\"test.c\">"
+                "<location file=\"Cppcheck\" line=\"0\" column=\"0\"/>"
+                "</error>"
+                "</analyzerinfo>"
+                );
+            ASSERT_EQUALS(xmlError, tinyxml2::XML_SUCCESS);
+
+            ASSERT_EQUALS(AnalyzerInformation::skipAnalysis(doc, 100, errorList), false);
+            ASSERT_EQUALS(errorList.size(), 0);
+        }
+
+        // Matching hash with internal error (don't skip)
+        {
+            std::list<ErrorMessage> errorList;
+            tinyxml2::XMLDocument doc;
+
+            const tinyxml2::XMLError xmlError = doc.Parse(
+                "<?xml version=\"1.0\"?>"
+                "<analyzerinfo hash=\"100\">"
+                "<error id=\"internalError\" severity=\"error\" msg=\"Something went wrong\" verbose=\"Something went wrong\" file0=\"test.c\">"
+                "<location file=\"Cppcheck\" line=\"0\" column=\"0\"/>"
+                "</error>"
+                "</analyzerinfo>"
+                );
+            ASSERT_EQUALS(xmlError, tinyxml2::XML_SUCCESS);
+
+            ASSERT_EQUALS(AnalyzerInformation::skipAnalysis(doc, 100, errorList), false);
+            ASSERT_EQUALS(errorList.size(), 0);
+        }
+
+        // Matching hash with normal error (skip)
+        {
+            std::list<ErrorMessage> errorList;
+            tinyxml2::XMLDocument doc;
+
+            const tinyxml2::XMLError xmlError = doc.Parse(
+                "<?xml version=\"1.0\"?>"
+                "<analyzerinfo hash=\"100\">"
+                "<error id=\"nullPointer\" severity=\"error\" msg=\"Null pointer dereference: ptr\" verbose=\"Null pointer dereference: ptr\" cwe=\"476\" file0=\"test.c\">"
+                "<location file=\"test.c\" line=\"4\" column=\"3\" info=\"Null pointer dereference\"/>"
+                "<location file=\"test.c\" line=\"3\" column=\"12\" info=\"Assignment &apos;ptr=NULL&apos;, assigned value is 0\"/>"
+                "<symbol>ptr</symbol>"
+                "</error>"
+                "</analyzerinfo>"
+                );
+            ASSERT_EQUALS(xmlError, tinyxml2::XML_SUCCESS);
+
+            ASSERT_EQUALS(AnalyzerInformation::skipAnalysis(doc, 100, errorList), true);
+            ASSERT_EQUALS(errorList.size(), 1);
+        }
+
+        // Matching hash with no error (skip)
+        {
+            std::list<ErrorMessage> errorList;
+            tinyxml2::XMLDocument doc;
+
+            const tinyxml2::XMLError xmlError = doc.Parse(
+                "<?xml version=\"1.0\"?>"
+                "<analyzerinfo hash=\"100\">"
+                "</analyzerinfo>"
+                );
+            ASSERT_EQUALS(xmlError, tinyxml2::XML_SUCCESS);
+
+            ASSERT_EQUALS(AnalyzerInformation::skipAnalysis(doc, 100, errorList), true);
+            ASSERT_EQUALS(errorList.size(), 0);
+        }
+
+        // Different hash with normal error (don't skip)
+        {
+            std::list<ErrorMessage> errorList;
+            tinyxml2::XMLDocument doc;
+
+            const tinyxml2::XMLError xmlError = doc.Parse(
+                "<?xml version=\"1.0\"?>"
+                "<analyzerinfo hash=\"100\">"
+                "<error id=\"nullPointer\" severity=\"error\" msg=\"Null pointer dereference: ptr\" verbose=\"Null pointer dereference: ptr\" cwe=\"476\" file0=\"test.c\">"
+                "<location file=\"test.c\" line=\"4\" column=\"3\" info=\"Null pointer dereference\"/>"
+                "<location file=\"test.c\" line=\"3\" column=\"12\" info=\"Assignment &apos;ptr=NULL&apos;, assigned value is 0\"/>"
+                "<symbol>ptr</symbol>"
+                "</error>"
+                "</analyzerinfo>"
+                );
+            ASSERT_EQUALS(xmlError, tinyxml2::XML_SUCCESS);
+
+            ASSERT_EQUALS(AnalyzerInformation::skipAnalysis(doc, 99, errorList), false);
+            ASSERT_EQUALS(errorList.size(), 0);
+        }
+
+        // Empty document (don't skip)
+        {
+            std::list<ErrorMessage> errorList;
+            tinyxml2::XMLDocument doc;
+
+            const tinyxml2::XMLError xmlError = doc.Parse("");
+            ASSERT_EQUALS(xmlError, tinyxml2::XML_ERROR_EMPTY_DOCUMENT);
+
+            ASSERT_EQUALS(AnalyzerInformation::skipAnalysis(doc, 100, errorList), false);
+            ASSERT_EQUALS(errorList.size(), 0);
+        }
+    }
 };
 
 REGISTER_TEST(TestAnalyzerInformation)