diff --git a/Makefile b/Makefile index fa1f6624783..da21105539d 100644 --- a/Makefile +++ b/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 diff --git a/lib/analyzerinfo.cpp b/lib/analyzerinfo.cpp index dfb953c9445..f303783c737 100644 --- a/lib/analyzerinfo.cpp +++ b/lib/analyzerinfo.cpp @@ -83,14 +83,9 @@ void AnalyzerInformation::close() } } -static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t hash, std::list &errors) +bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list &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; @@ -98,6 +93,16 @@ static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t 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); diff --git a/lib/analyzerinfo.h b/lib/analyzerinfo.h index e5466906707..a67a2291a29 100644 --- a/lib/analyzerinfo.h +++ b/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 &sourcefiles, const std::string &userDefines, const std::list &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 &errors); + std::ofstream mOutputStream; std::string mAnalyzerInfoFile; }; diff --git a/test/cli/premium_test.py b/test/cli/premium_test.py index 12dd7212450..b0df148f22b 100644 --- a/test/cli/premium_test.py +++ b/test/cli/premium_test.py @@ -130,3 +130,29 @@ 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 diff --git a/test/testanalyzerinformation.cpp b/test/testanalyzerinformation.cpp index f1d94d4af3c..e6dab0fd156 100644 --- a/test/testanalyzerinformation.cpp +++ b/test/testanalyzerinformation.cpp @@ -20,8 +20,10 @@ #include "analyzerinfo.h" #include "filesettings.h" #include "fixture.h" +#include "xml.h" #include +#include 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 errorList; + tinyxml2::XMLDocument doc; + + const tinyxml2::XMLError xmlError = doc.Parse( + "" + "" + "" + "" + "" + "" + ); + ASSERT_EQUALS(tinyxml2::XML_SUCCESS, xmlError); + + ASSERT_EQUALS(false, AnalyzerInformation::skipAnalysis(doc, 100, errorList)); + ASSERT_EQUALS(0, errorList.size()); + } + + // Matching hash with premium internal error (don't skip) + { + std::list errorList; + tinyxml2::XMLDocument doc; + + const tinyxml2::XMLError xmlError = doc.Parse( + "" + "" + "" + "" + "" + "" + ); + ASSERT_EQUALS(tinyxml2::XML_SUCCESS, xmlError); + + ASSERT_EQUALS(false, AnalyzerInformation::skipAnalysis(doc, 100, errorList)); + ASSERT_EQUALS(0, errorList.size()); + } + + // Matching hash with internal error (don't skip) + { + std::list errorList; + tinyxml2::XMLDocument doc; + + const tinyxml2::XMLError xmlError = doc.Parse( + "" + "" + "" + "" + "" + "" + ); + ASSERT_EQUALS(tinyxml2::XML_SUCCESS, xmlError); + + ASSERT_EQUALS(false, AnalyzerInformation::skipAnalysis(doc, 100, errorList)); + ASSERT_EQUALS(0, errorList.size()); + } + + // Matching hash with normal error (skip) + { + std::list errorList; + tinyxml2::XMLDocument doc; + + const tinyxml2::XMLError xmlError = doc.Parse( + "" + "" + "" + "" + "" + "ptr" + "" + "" + ); + ASSERT_EQUALS(tinyxml2::XML_SUCCESS, xmlError); + + ASSERT_EQUALS(true, AnalyzerInformation::skipAnalysis(doc, 100, errorList)); + ASSERT_EQUALS(1, errorList.size()); + } + + // Matching hash with no error (skip) + { + std::list errorList; + tinyxml2::XMLDocument doc; + + const tinyxml2::XMLError xmlError = doc.Parse( + "" + "" + "" + ); + ASSERT_EQUALS(tinyxml2::XML_SUCCESS, xmlError); + + ASSERT_EQUALS(true, AnalyzerInformation::skipAnalysis(doc, 100, errorList)); + ASSERT_EQUALS(0, errorList.size()); + } + + // Different hash with normal error (don't skip) + { + std::list errorList; + tinyxml2::XMLDocument doc; + + const tinyxml2::XMLError xmlError = doc.Parse( + "" + "" + "" + "" + "" + "ptr" + "" + "" + ); + ASSERT_EQUALS(tinyxml2::XML_SUCCESS, xmlError); + + ASSERT_EQUALS(false, AnalyzerInformation::skipAnalysis(doc, 99, errorList)); + ASSERT_EQUALS(0, errorList.size()); + } + + // Empty document (don't skip) + { + std::list errorList; + tinyxml2::XMLDocument doc; + + const tinyxml2::XMLError xmlError = doc.Parse(""); + ASSERT_EQUALS(tinyxml2::XML_ERROR_EMPTY_DOCUMENT, xmlError); + + ASSERT_EQUALS(false, AnalyzerInformation::skipAnalysis(doc, 100, errorList)); + ASSERT_EQUALS(0, errorList.size()); + } + } }; REGISTER_TEST(TestAnalyzerInformation)