From a43b55a0ca0feacefd76d5b40c58b877f450a218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Wed, 20 Sep 2023 10:40:57 +0200 Subject: [PATCH] Improved addon execution errorhandling (#5451) --- addons/README.md | 29 +++++ cli/cppcheckexecutor.cpp | 27 +++-- cli/cppcheckexecutor.h | 4 +- gui/checkthread.cpp | 4 +- lib/cppcheck.cpp | 148 +++++++++++++++--------- lib/cppcheck.h | 6 +- lib/errorlogger.cpp | 7 +- lib/errorlogger.h | 2 +- lib/errortypes.cpp | 41 +++---- lib/errortypes.h | 6 +- test/cli/test-other.py | 220 +++++++++++++++++++++++++++++++++++- test/testerrorlogger.cpp | 31 +++++ test/testsingleexecutor.cpp | 2 +- 13 files changed, 427 insertions(+), 100 deletions(-) diff --git a/addons/README.md b/addons/README.md index b441b22b1..34b1f7b26 100644 --- a/addons/README.md +++ b/addons/README.md @@ -10,6 +10,35 @@ Addons are scripts that analyses Cppcheck dump files to check compatibility with Checks Linux system for [year 2038 problem](https://en.wikipedia.org/wiki/Year_2038_problem) safety. This required [modified environment](https://github.com/3adev/y2038). See complete description [here](https://github.com/danmar/cppcheck/blob/main/addons/doc/y2038.txt). + [threadsafety.py](https://github.com/danmar/cppcheck/blob/main/addons/threadsafety.py) Analyse Cppcheck dump files to locate threadsafety issues like static local objects used by multiple threads. ++ [naming.py](https://github.com/danmar/cppcheck/blob/main/addons/naming.py) + Enforces naming conventions across the code. ++ [namingng.py](https://github.com/danmar/cppcheck/blob/main/addons/namingng.py) + Enforces naming conventions across the code. Enhanced version with support for type prefixes in variable and function names. ++ [findcasts.py](https://github.com/danmar/cppcheck/blob/main/addons/findcasts.py) + Locates casts in the code. ++ [misc.py](https://github.com/danmar/cppcheck/blob/main/addons/misc.py) + Performs miscellaneous checks. + +### Other files + +- doc + Additional files for documentation generation. +- tests + Contains various unit tests for the addons. +- cppcheck.py + Internal helper used by Cppcheck binary to run the addons. +- cppcheckdata.doxyfile + Configuration file for documentation generation. +- cppcheckdata.py + Helper class for reading Cppcheck dump files within an addon. +- misra_9.py + Implementation of the MISRA 9.x rules used by `misra` addon. +- naming.json + Example configuration for `namingng` addon. +- ROS_naming.json + Example configuration for the `namingng` addon enforcing the [ROS naming convention for C++ ](http://wiki.ros.org/CppStyleGuide#Files). +- runaddon.py + Internal helper used by Cppcheck binary to run the addons. ## Usage diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 747d9cb50..71ab17e18 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -540,7 +540,7 @@ bool CppCheckExecutor::tryLoadLibrary(Library& destination, const std::string& b */ // cppcheck-suppress passedByValue - used as callback so we need to preserve the signature // NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature -bool CppCheckExecutor::executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output_) +int CppCheckExecutor::executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output_) { output_.clear(); @@ -567,9 +567,12 @@ bool CppCheckExecutor::executeCommand(std::string exe, std::vector #else FILE *p = popen(cmd.c_str(), "r"); #endif + //std::cout << "invoking command '" << cmd << "'" << std::endl; if (!p) { - // TODO: read errno - return false; + // TODO: how to provide to caller? + //const int err = errno; + //std::cout << "popen() errno " << std::to_string(err) << std::endl; + return -1; } char buffer[1024]; while (fgets(buffer, sizeof(buffer), p) != nullptr) @@ -581,13 +584,19 @@ bool CppCheckExecutor::executeCommand(std::string exe, std::vector const int res = pclose(p); #endif if (res == -1) { // error occured - // TODO: read errno - return false; + // TODO: how to provide to caller? + //const int err = errno; + //std::cout << "pclose() errno " << std::to_string(err) << std::endl; + return res; } - if (res != 0) { // process failed - // TODO: need to get error details - return false; +#if !defined(WIN32) && !defined(__MINGW32__) + if (WIFEXITED(res)) { + return WEXITSTATUS(res); } - return true; + if (WIFSIGNALED(res)) { + return WTERMSIG(res); + } +#endif + return res; } diff --git a/cli/cppcheckexecutor.h b/cli/cppcheckexecutor.h index 47660c9dd..e3a040962 100644 --- a/cli/cppcheckexecutor.h +++ b/cli/cppcheckexecutor.h @@ -99,9 +99,9 @@ public: static bool tryLoadLibrary(Library& destination, const std::string& basepath, const char* filename); /** - * Execute a shell command and read the output from it. Returns true if command terminated successfully. + * Execute a shell command and read the output from it. Returns exitcode of the executed command,. */ - static bool executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output_); + static int executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output_); protected: diff --git a/gui/checkthread.cpp b/gui/checkthread.cpp index f59897897..21b05c854 100644 --- a/gui/checkthread.cpp +++ b/gui/checkthread.cpp @@ -57,7 +57,7 @@ #endif // NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature -static bool executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output) // cppcheck-suppress passedByValue +static int executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output) // cppcheck-suppress passedByValue { output.clear(); @@ -80,7 +80,7 @@ static bool executeCommand(std::string exe, std::vector args, std:: std::ofstream fout(redirect.substr(3)); fout << process.readAllStandardError().toStdString(); } - return process.exitCode() == 0; + return process.exitCode(); } diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 0feb733bf..da85ebd68 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -180,16 +180,16 @@ namespace { std::string getAddonInfo(const std::string &fileName, const std::string &exename) { if (fileName[0] == '{') { - std::istringstream in(fileName); picojson::value json; - in >> json; + const std::string err = picojson::parse(json, fileName); + (void)err; // TODO: report return parseAddonInfo(json, fileName, exename); } if (fileName.find('.') == std::string::npos) return getAddonInfo(fileName + ".py", exename); if (endsWith(fileName, ".py")) { - scriptFile = getFullPath(fileName, exename); + scriptFile = Path::fromNativeSeparators(getFullPath(fileName, exename)); if (scriptFile.empty()) return "Did not find addon " + fileName; @@ -328,11 +328,35 @@ static void createDumpFile(const Settings& settings, << "/>" << '\n'; } -static std::string executeAddon(const AddonInfo &addonInfo, - const std::string &defaultPythonExe, - const std::string &file, - const std::string &premiumArgs, - const std::function,std::string,std::string&)> &executeCommand) +static std::string detectPython(const CppCheck::ExecuteCmdFn &executeCommand) +{ +#ifdef _WIN32 + const char *py_exes[] = { "python3.exe", "python.exe" }; +#else + const char *py_exes[] = { "python3", "python" }; +#endif + for (const char* py_exe : py_exes) { + std::string out; +#ifdef _MSC_VER + // FIXME: hack to avoid debug assertion with _popen() in executeCommand() for non-existing commands + const std::string cmd = std::string(py_exe) + " --version >NUL"; + if (system(cmd.c_str()) != 0) { + // TODO: get more detailed error? + break; + } +#endif + if (executeCommand(py_exe, split("--version"), "2>&1", out) == EXIT_SUCCESS && startsWith(out, "Python ") && std::isdigit(out[7])) { + return py_exe; + } + } + return ""; +} + +static std::vector executeAddon(const AddonInfo &addonInfo, + const std::string &defaultPythonExe, + const std::string &file, + const std::string &premiumArgs, + const CppCheck::ExecuteCmdFn &executeCommand) { const std::string redirect = "2>&1"; @@ -345,28 +369,11 @@ static std::string executeAddon(const AddonInfo &addonInfo, else if (!defaultPythonExe.empty()) pythonExe = cmdFileName(defaultPythonExe); else { -#ifdef _WIN32 - const char *py_exes[] = { "python3.exe", "python.exe" }; -#else - const char *py_exes[] = { "python3", "python" }; -#endif - for (const char* py_exe : py_exes) { - std::string out; -#ifdef _MSC_VER - // FIXME: hack to avoid debug assertion with _popen() in executeCommand() for non-existing commands - const std::string cmd = std::string(py_exe) + " --version >NUL"; - if (system(cmd.c_str()) != 0) { - // TODO: get more detailed error? - break; - } -#endif - if (executeCommand(py_exe, split("--version"), redirect, out) && startsWith(out, "Python ") && std::isdigit(out[7])) { - pythonExe = py_exe; - break; - } - } - if (pythonExe.empty()) + // store in static variable so we only look this up once + static const std::string detectedPythonExe = detectPython(executeCommand); + if (detectedPythonExe.empty()) throw InternalError(nullptr, "Failed to auto detect python"); + pythonExe = detectedPythonExe; } std::string args; @@ -380,27 +387,62 @@ static std::string executeAddon(const AddonInfo &addonInfo, args += fileArg; std::string result; - if (!executeCommand(pythonExe, split(args), redirect, result)) { - std::string message("Failed to execute addon '" + addonInfo.name + "' (command: '" + pythonExe + " " + args + "'). Exitcode is nonzero."); + if (const int exitcode = executeCommand(pythonExe, split(args), redirect, result)) { + std::string message("Failed to execute addon '" + addonInfo.name + "' - exitcode is " + std::to_string(exitcode)); + std::string details = pythonExe + " " + args; if (result.size() > 2) { - message = message + "\n" + message + "\nOutput:\n" + result; - message.resize(message.find_last_not_of("\n\r")); + details += "\nOutput:\n"; + details += result; + const auto pos = details.find_last_not_of("\n\r"); + if (pos != std::string::npos) + details.resize(pos + 1); } - throw InternalError(nullptr, message); + throw InternalError(nullptr, message, details); } + std::vector addonResult; + // Validate output.. std::istringstream istr(result); std::string line; while (std::getline(istr, line)) { - if (!startsWith(line,"Checking ") && !line.empty() && line[0] != '{') { + // TODO: also bail out? + if (line.empty()) { + //std::cout << "addon '" << addonInfo.name << "' result contains empty line" << std::endl; + continue; + } + + // TODO: get rid of this + if (startsWith(line,"Checking ")) { + //std::cout << "addon '" << addonInfo.name << "' result contains 'Checking ' line" << std::endl; + continue; + } + + if (line[0] != '{') { + //std::cout << "addon '" << addonInfo.name << "' result is not a JSON" << std::endl; + result.erase(result.find_last_not_of('\n') + 1, std::string::npos); // Remove trailing newlines throw InternalError(nullptr, "Failed to execute '" + pythonExe + " " + args + "'. " + result); } + + //std::cout << "addon '" << addonInfo.name << "' result is " << line << std::endl; + + // TODO: make these failures? + picojson::value res; + const std::string err = picojson::parse(res, line); + if (!err.empty()) { + //std::cout << "addon '" << addonInfo.name << "' result is not a valid JSON (" << err << ")" << std::endl; + continue; + } + if (!res.is()) { + //std::cout << "addon '" << addonInfo.name << "' result is not a JSON object" << std::endl; + continue; + } + addonResult.emplace_back(std::move(res)); } // Valid results - return result; + return addonResult; } static std::string getDefinesFlags(const std::string &semicolonSeparatedString) @@ -413,7 +455,7 @@ static std::string getDefinesFlags(const std::string &semicolonSeparatedString) CppCheck::CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions, - std::function,std::string,std::string&)> executeCommand) + ExecuteCmdFn executeCommand) : mErrorLogger(errorLogger) , mUseGlobalSuppressions(useGlobalSuppressions) , mExecuteCommand(std::move(executeCommand)) @@ -532,7 +574,7 @@ unsigned int CppCheck::check(const std::string &path) } std::string output2; - if (!mExecuteCommand(exe,split(args2),redirect2,output2) || output2.find("TranslationUnitDecl") == std::string::npos) { + if (mExecuteCommand(exe,split(args2),redirect2,output2) != EXIT_SUCCESS || output2.find("TranslationUnitDecl") == std::string::npos) { std::cerr << "Failed to execute '" << exe << " " << args2 << " " << redirect2 << "'" << std::endl; return 0; } @@ -596,7 +638,8 @@ unsigned int CppCheck::check(const std::string &path) executeAddons(dumpFile); } catch (const InternalError &e) { - internalError(path, "Processing Clang AST dump failed: " + e.errorMessage); + const ErrorMessage errmsg = ErrorMessage::fromInternalError(e, nullptr, path, "Bailing out from analysis: Processing Clang AST dump failed"); + reportErr(errmsg); } catch (const TerminateException &) { // Analysis is terminated return mExitCode; @@ -1022,7 +1065,8 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string } catch (const std::bad_alloc &) { internalError(filename, "Checking file failed: out of memory"); } catch (const InternalError &e) { - internalError(filename, "Checking file failed: " + e.errorMessage); + const ErrorMessage errmsg = ErrorMessage::fromInternalError(e, nullptr, filename, "Bailing out from analysis: Checking file failed"); + reportErr(errmsg); } if (!mSettings.buildDir.empty()) { @@ -1448,23 +1492,14 @@ void CppCheck::executeAddons(const std::vector& files) if (addonInfo.name != "misra" && !addonInfo.ctu && endsWith(files.back(), ".ctu-info")) continue; - const std::string results = + const std::vector results = executeAddon(addonInfo, mSettings.addonPython, fileList.empty() ? files[0] : fileList, mSettings.premiumArgs, mExecuteCommand); - std::istringstream istr(results); - std::string line; const bool misraC2023 = mSettings.premiumArgs.find("--misra-c-2023") != std::string::npos; - while (std::getline(istr, line)) { - if (!startsWith(line,"{")) - continue; - - picojson::value res; - std::istringstream istr2(line); - istr2 >> res; - if (!res.is()) - continue; - + for (const picojson::value& res : results) { + // TODO: get rid of copy? + // this is a copy so we can access missing fields and get a default value picojson::object obj = res.get(); ErrorMessage errmsg; @@ -1519,7 +1554,8 @@ void CppCheck::executeAddonsWholeProgram(const std::map,std::string,std::string&)>; + /** * @brief Constructor. */ CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions, - std::function,std::string,std::string&)> executeCommand); + ExecuteCmdFn executeCommand); /** * @brief Destructor. @@ -230,7 +232,7 @@ private: AnalyzerInformation mAnalyzerInformation; /** Callback for executing a shell command (exe, args, output) */ - std::function,std::string,std::string&)> mExecuteCommand; + ExecuteCmdFn mExecuteCommand; std::ofstream mPlistFile; }; diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 99ace3684..c13dee7e7 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -233,7 +233,7 @@ static void serializeString(std::string &oss, const std::string & str) oss += str; } -ErrorMessage ErrorMessage::fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename) +ErrorMessage ErrorMessage::fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename, const std::string& msg) { if (internalError.token) assert(tokenList != nullptr); // we need to make sure we can look up the provided token @@ -253,9 +253,12 @@ ErrorMessage ErrorMessage::fromInternalError(const InternalError &internalError, ErrorMessage errmsg(std::move(locationList), tokenList ? tokenList->getSourceFilePath() : filename, Severity::error, - internalError.errorMessage, + (msg.empty() ? "" : (msg + ": ")) + internalError.errorMessage, internalError.id, Certainty::normal); + // TODO: find a better way + if (!internalError.details.empty()) + errmsg.mVerboseMessage = errmsg.mVerboseMessage + ": " + internalError.details; return errmsg; } diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 2d1bed3c4..ccc569c8c 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -198,7 +198,7 @@ public: return mSymbolNames; } - static ErrorMessage fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename); + static ErrorMessage fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename, const std::string& msg = emptyString); private: static std::string fixInvalidChars(const std::string& raw); diff --git a/lib/errortypes.cpp b/lib/errortypes.cpp index 09f6b0f73..cd9ca5771 100644 --- a/lib/errortypes.cpp +++ b/lib/errortypes.cpp @@ -18,31 +18,32 @@ #include "errortypes.h" -InternalError::InternalError(const Token *tok, std::string errorMsg, Type type) : - token(tok), errorMessage(std::move(errorMsg)), type(type) +static std::string typeToString(InternalError::Type type) { switch (type) { - case AST: - id = "internalAstError"; - break; - case SYNTAX: - id = "syntaxError"; - break; - case UNKNOWN_MACRO: - id = "unknownMacro"; - break; - case INTERNAL: - id = "cppcheckError"; - break; - case LIMIT: - id = "cppcheckLimit"; - break; - case INSTANTIATION: - id = "instantiationError"; - break; + case InternalError::Type::AST: + return "internalAstError"; + case InternalError::Type::SYNTAX: + return "syntaxError"; + case InternalError::Type::UNKNOWN_MACRO: + return "unknownMacro"; + case InternalError::Type::INTERNAL: + return "internalError"; + case InternalError::Type::LIMIT: + return "cppcheckLimit"; + case InternalError::Type::INSTANTIATION: + return "instantiationError"; } } +InternalError::InternalError(const Token *tok, std::string errorMsg, Type type) : + InternalError(tok, std::move(errorMsg), "", type) +{} + +InternalError::InternalError(const Token *tok, std::string errorMsg, std::string details, Type type) : + token(tok), errorMessage(std::move(errorMsg)), details(std::move(details)), type(type), id(typeToString(type)) +{} + std::string Severity::toString(Severity::SeverityType severity) { switch (severity) { diff --git a/lib/errortypes.h b/lib/errortypes.h index 554721ba9..2bb3a32c5 100644 --- a/lib/errortypes.h +++ b/lib/errortypes.h @@ -33,11 +33,15 @@ class Token; /** @brief Simple container to be thrown when internal error is detected. */ -struct InternalError { +struct CPPCHECKLIB InternalError { enum Type {AST, SYNTAX, UNKNOWN_MACRO, INTERNAL, LIMIT, INSTANTIATION}; + InternalError(const Token *tok, std::string errorMsg, Type type = INTERNAL); + InternalError(const Token *tok, std::string errorMsg, std::string details, Type type = INTERNAL); + const Token *token; std::string errorMessage; + std::string details; Type type; std::string id; }; diff --git a/test/cli/test-other.py b/test/cli/test-other.py index 1db9aa27f..52be3f45d 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -207,10 +207,8 @@ def test_execute_addon_failure_2(tmpdir): args = ['--addon=naming', '--addon-python=python5.x', test_file] _, _, stderr = cppcheck(args) - # /tmp/pytest-of-sshuser/pytest-215/test_execute_addon_failure_20/test.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'naming' (command: 'python5.x /mnt/s/GitHub/cppcheck-fw/addons/runaddon.py /mnt/s/GitHub/cppcheck-fw/addons/naming.py --cli /tmp/pytest-of-sshuser/pytest-215/test_execute_addon_failure_20/test.cpp.7306.dump'). Exitcode is nonzero. [internalError]\n\n^\n - # "C:\\Users\\Quotenjugendlicher\\AppData\\Local\\Temp\\pytest-of-Quotenjugendlicher\\pytest-15\\test_execute_addon_failure_20\\test.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon (command: 'python5.x S:\\GitHub\\cppcheck-fw\\bin\\debug\\addons\\runaddon.py S:\\GitHub\\cppcheck-fw\\bin\\debug\\addons\\naming.py --cli C:\\Users\\Quotenjugendlicher\\AppData\\Local\\Temp\\pytest-of-Quotenjugendlicher\\pytest-15\\test_execute_addon_failure_20\\test.cpp.9892.dump'). Exitcode is nonzero. [internalError]\n\n^\n - assert stderr.startswith('{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon \'naming\' (command: \'python5.x '.format(test_file)) - assert stderr.endswith(' [internalError]\n\n^\n') + ec = 1 if os.name == 'nt' else 127 + assert stderr == "{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'naming' - exitcode is {} [internalError]\n\n^\n".format(test_file, ec) # TODO: find a test case which always fails @@ -232,3 +230,217 @@ void f() { _, _, stderr = cppcheck(args) assert stderr == '{}:0:0: error: Bailing from out analysis: Checking file failed: converting \'1f\' to integer failed - not an integer [internalError]\n\n^\n'.format(test_file) + + +def test_addon_misra(tmpdir): + test_file = os.path.join(tmpdir, 'test.cpp') + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon=misra', '--enable=all', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == '{}:2:1: style: misra violation (use --rule-texts= to get proper output) [misra-c2012-2.3]\ntypedef int MISRA_5_6_VIOLATION;\n^\n'.format(test_file) + + +def test_addon_y2038(tmpdir): + test_file = os.path.join(tmpdir, 'test.cpp') + # TODO: trigger warning + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon=y2038', '--enable=all', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == '' + + +def test_addon_threadsafety(tmpdir): + test_file = os.path.join(tmpdir, 'test.cpp') + # TODO: trigger warning + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon=y2038', '--enable=all', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == '' + + +def test_addon_naming(tmpdir): + test_file = os.path.join(tmpdir, 'test.cpp') + # TODO: trigger warning + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon=y2038', '--enable=all', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == '' + + +def test_addon_namingng(tmpdir): + test_file = os.path.join(tmpdir, 'test.cpp') + # TODO: trigger warning + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon=y2038', '--enable=all', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == '' + + +def test_invalid_addon_json(tmpdir): + addon_file = os.path.join(tmpdir, 'addon1.json') + with open(addon_file, 'wt') as f: + f.write(""" + """) + + test_file = os.path.join(tmpdir, 'file.cpp') + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon={}'.format(addon_file), '--enable=all', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 # TODO: needs to be 1 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file), + 'Loading {} failed. syntax error at line 2 near: '.format(addon_file), + 'Loading {} failed. syntax error at line 2 near: '.format(addon_file) + ] + assert stderr == '' + + +def test_invalid_addon_py(tmpdir): + addon_file = os.path.join(tmpdir, 'addon1.py') + with open(addon_file, 'wt') as f: + f.write(""" +raise Exception() + """) + + test_file = os.path.join(tmpdir, 'file.cpp') + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon={}'.format(addon_file), '--enable=all', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 # TODO: needs to be 1 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == "{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 1 [internalError]\n\n^\n".format(test_file) + + +def test_invalid_addon_py_verbose(tmpdir): + addon_file = os.path.join(tmpdir, 'addon1.py') + with open(addon_file, 'wt') as f: + f.write(""" +raise Exception() + """) + + test_file = os.path.join(tmpdir, 'file.cpp') + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon={}'.format(addon_file), '--enable=all', '--verbose', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 # TODO: needs to be 1 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file), + 'Defines:', + 'Undefines:', + 'Includes:', + 'Platform:native' + ] + """ +/tmp/pytest-of-user/pytest-11/test_invalid_addon_py_20/file.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 1: python3 /home/user/CLionProjects/cppcheck/addons/runaddon.py /tmp/pytest-of-user/pytest-11/test_invalid_addon_py_20/addon1.py --cli /tmp/pytest-of-user/pytest-11/test_invalid_addon_py_20/file.cpp.24762.dump +Output: +Traceback (most recent call last): + File "/home/user/CLionProjects/cppcheck/addons/runaddon.py", line 8, in + runpy.run_path(addon, run_name='__main__') + File "", line 291, in run_path + File "", line 98, in _run_module_code + File "", line 88, in _run_code + File "/tmp/pytest-of-user/pytest-11/test_invalid_addon_py_20/addon1.py", line 2, in + raise Exception() +Exceptio [internalError] + """ + # /tmp/pytest-of-user/pytest-10/test_invalid_addon_py_20/file.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 256.: python3 /home/user/CLionProjects/cppcheck/addons/runaddon.py /tmp/pytest-of-user/pytest-10/test_invalid_addon_py_20/addon1.py --cli /tmp/pytest-of-user/pytest-10/test_invalid_addon_py_20/file.cpp.24637.dump + assert stderr.startswith("{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 1: ".format(test_file)) + assert stderr.count('Output:\nTraceback') + assert stderr.endswith('raise Exception()\nException [internalError]\n\n^\n') + + +def test_addon_result(tmpdir): + addon_file = os.path.join(tmpdir, 'addon1.py') + with open(addon_file, 'wt') as f: + f.write(""" +print("Checking ...") +print("") +print('{"file": "test.cpp", "linenr": 1, "column": 1, "severity": "style", "message": "msg", "addon": "addon1", "errorId": "id", "extra": ""}') +print('{"loc": [{"file": "test.cpp", "linenr": 1, "column": 1, "info": ""}], "severity": "style", "message": "msg", "addon": "addon1", "errorId": "id", "extra": ""}') + """) + + test_file = os.path.join(tmpdir, 'file.cpp') + with open(test_file, 'wt') as f: + f.write(""" +typedef int MISRA_5_6_VIOLATION; + """) + + args = ['--addon={}'.format(addon_file), '--enable=all', test_file] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0 # TODO: needs to be 1 + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == 'test.cpp:1:1: style: msg [addon1-id]\n\n^\n' diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index 14de55390..97d94dcd2 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -44,6 +44,7 @@ private: TEST_CASE(ErrorMessageConstructLocations); TEST_CASE(ErrorMessageVerbose); TEST_CASE(ErrorMessageVerboseLocations); + TEST_CASE(ErrorMessageFromInternalError); TEST_CASE(CustomFormat); TEST_CASE(CustomFormat2); TEST_CASE(CustomFormatLocations); @@ -153,6 +154,36 @@ private: ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Verbose error", msg.toString(true)); } + void ErrorMessageFromInternalError() const { + // TODO: test with token + { + InternalError internalError(nullptr, "message", InternalError::INTERNAL); + const auto msg = ErrorMessage::fromInternalError(internalError, nullptr, "file.c"); + ASSERT_EQUALS(1, msg.callStack.size()); + const auto &loc = *msg.callStack.cbegin(); + ASSERT_EQUALS(0, loc.fileIndex); + ASSERT_EQUALS(0, loc.line); + ASSERT_EQUALS(0, loc.column); + ASSERT_EQUALS("message", msg.shortMessage()); + ASSERT_EQUALS("message", msg.verboseMessage()); + ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(false)); + ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(true)); + } + { + InternalError internalError(nullptr, "message", "details", InternalError::INTERNAL); + const auto msg = ErrorMessage::fromInternalError(internalError, nullptr, "file.cpp", "msg"); + ASSERT_EQUALS(1, msg.callStack.size()); + const auto &loc = *msg.callStack.cbegin(); + ASSERT_EQUALS(0, loc.fileIndex); + ASSERT_EQUALS(0, loc.line); + ASSERT_EQUALS(0, loc.column); + ASSERT_EQUALS("msg: message", msg.shortMessage()); + ASSERT_EQUALS("msg: message: details", msg.verboseMessage()); + ASSERT_EQUALS("[file.cpp:0]: (error) msg: message", msg.toString(false)); + ASSERT_EQUALS("[file.cpp:0]: (error) msg: message: details", msg.toString(true)); + } + } + void CustomFormat() const { std::list locs(1, fooCpp5); ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); diff --git a/test/testsingleexecutor.cpp b/test/testsingleexecutor.cpp index 352b074d2..f9c2d6f55 100644 --- a/test/testsingleexecutor.cpp +++ b/test/testsingleexecutor.cpp @@ -113,7 +113,7 @@ private: executeCommandCalled = true; exe = std::move(e); args = std::move(a); - return true; + return EXIT_SUCCESS; }); cppcheck.settings() = settings;