From bc30cc17fca61ab40688173787d4e6f6030ed3ed Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 14 Sep 2023 13:14:55 +0200 Subject: [PATCH 01/15] CppCheck: split up checks in `executeAddon()` / test-other.py: added some addon-related tests --- addons/README.md | 29 +++++++ lib/cppcheck.cpp | 18 ++++- test/cli/test-other.py | 171 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 1 deletion(-) diff --git a/addons/README.md b/addons/README.md index b441b22b19f..34b1f7b2600 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/lib/cppcheck.cpp b/lib/cppcheck.cpp index 0feb733bfae..25a61154ade 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -393,10 +393,26 @@ static std::string executeAddon(const AddonInfo &addonInfo, 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; } // Valid results diff --git a/test/cli/test-other.py b/test/cli/test-other.py index 1db9aa27f4d..ef765e447b5 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -232,3 +232,174 @@ def test_internal_error(tmpdir): _, _, 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.startswith("{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1'".format(test_file)) + assert stderr.endswith('Exitcode is nonzero. [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' From 8b6ee969ea87747cf45bfc8200fe6209f4b53f7c Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 14 Sep 2023 13:29:36 +0200 Subject: [PATCH 02/15] CppCheck: removed duplicated check --- lib/cppcheck.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 25a61154ade..6e7bbdc2436 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -1472,9 +1472,6 @@ void CppCheck::executeAddons(const std::vector& files) 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; From 5c3f0b0c88c47c777f85430cbe28448e8b3125cf Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 14 Sep 2023 13:40:49 +0200 Subject: [PATCH 03/15] CppCheck: use alias for `executeCommand` type --- lib/cppcheck.cpp | 4 ++-- lib/cppcheck.h | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 6e7bbdc2436..b252befda1b 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -332,7 +332,7 @@ 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) + const CppCheck::ExecuteCmdFn &executeCommand) { const std::string redirect = "2>&1"; @@ -429,7 +429,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)) diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 2de468d48ff..44d878f6299 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -50,12 +50,14 @@ class Tokenizer; */ class CPPCHECKLIB CppCheck : ErrorLogger { public: + using ExecuteCmdFn = std::function,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 @@ class CPPCHECKLIB CppCheck : ErrorLogger { AnalyzerInformation mAnalyzerInformation; /** Callback for executing a shell command (exe, args, output) */ - std::function,std::string,std::string&)> mExecuteCommand; + ExecuteCmdFn mExecuteCommand; std::ofstream mPlistFile; }; From a7a9e36d90e1f66ee97c2bae4d6014a045e5dcef Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 14 Sep 2023 13:53:35 +0200 Subject: [PATCH 04/15] return exitcode code from command execution --- cli/cppcheckexecutor.cpp | 20 ++++++++++---------- cli/cppcheckexecutor.h | 4 ++-- gui/checkthread.cpp | 4 ++-- lib/cppcheck.cpp | 12 ++++++------ lib/cppcheck.h | 2 +- test/cli/test-other.py | 2 +- test/testsingleexecutor.cpp | 2 +- 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 747d9cb50d5..d98f79d7887 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,10 @@ 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; } - if (res != 0) { // process failed - // TODO: need to get error details - return false; - } - return true; + return res; } diff --git a/cli/cppcheckexecutor.h b/cli/cppcheckexecutor.h index 47660c9ddbd..e3a04096299 100644 --- a/cli/cppcheckexecutor.h +++ b/cli/cppcheckexecutor.h @@ -99,9 +99,9 @@ class CppCheckExecutor : public ErrorLogger { 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 f5989789730..21b05c85462 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 b252befda1b..a341defb463 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -360,7 +360,7 @@ static std::string executeAddon(const AddonInfo &addonInfo, break; } #endif - if (executeCommand(py_exe, split("--version"), redirect, out) && startsWith(out, "Python ") && std::isdigit(out[7])) { + if (executeCommand(py_exe, split("--version"), redirect, out) == 0 && startsWith(out, "Python ") && std::isdigit(out[7])) { pythonExe = py_exe; break; } @@ -380,8 +380,8 @@ 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 + "' (command: '" + pythonExe + " " + args + "'). Exitcode is " + std::to_string(exitcode) + "."); if (result.size() > 2) { message = message + "\n" + message + "\nOutput:\n" + result; message.resize(message.find_last_not_of("\n\r")); @@ -548,7 +548,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) != 0 || output2.find("TranslationUnitDecl") == std::string::npos) { std::cerr << "Failed to execute '" << exe << " " << args2 << " " << redirect2 << "'" << std::endl; return 0; } @@ -1700,8 +1700,8 @@ void CppCheck::analyseClangTidy(const ImportProject::FileSettings &fileSettings) const std::string args = "-quiet -checks=*,-clang-analyzer-*,-llvm* \"" + fileSettings.filename + "\" -- " + allIncludes + allDefines; std::string output; - if (!mExecuteCommand(exe, split(args), emptyString, output)) { - std::cerr << "Failed to execute '" << exe << "'" << std::endl; + if (const int exitcode = mExecuteCommand(exe, split(args), emptyString, output)) { + std::cerr << "Failed to execute '" << exe << "' (exitcode: " << std::to_string(exitcode) << ")" << std::endl; return; } diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 44d878f6299..fbe73eea1a8 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -50,7 +50,7 @@ class Tokenizer; */ class CPPCHECKLIB CppCheck : ErrorLogger { public: - using ExecuteCmdFn = std::function,std::string,std::string&)>; + using ExecuteCmdFn = std::function,std::string,std::string&)>; /** * @brief Constructor. diff --git a/test/cli/test-other.py b/test/cli/test-other.py index ef765e447b5..9104c0537df 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -375,7 +375,7 @@ def test_invalid_addon_py(tmpdir): 'Checking {} ...'.format(test_file) ] assert stderr.startswith("{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1'".format(test_file)) - assert stderr.endswith('Exitcode is nonzero. [internalError]\n\n^\n') + assert stderr.endswith('Exitcode is 256. [internalError]\n\n^\n') def test_addon_result(tmpdir): diff --git a/test/testsingleexecutor.cpp b/test/testsingleexecutor.cpp index 352b074d22b..aa1f34996ee 100644 --- a/test/testsingleexecutor.cpp +++ b/test/testsingleexecutor.cpp @@ -113,7 +113,7 @@ class TestSingleExecutorBase : public TestFixture { executeCommandCalled = true; exe = std::move(e); args = std::move(a); - return true; + return 0; }); cppcheck.settings() = settings; From e6f46e449f61594478e8f13687061a9010c152c7 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 14 Sep 2023 14:02:28 +0200 Subject: [PATCH 05/15] CppCheck: parse JSON without `std::istringstream` --- lib/cppcheck.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index a341defb463..f70ce14649b 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -180,9 +180,9 @@ 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) @@ -1473,10 +1473,16 @@ void CppCheck::executeAddons(const std::vector& files) while (std::getline(istr, line)) { picojson::value res; - std::istringstream istr2(line); - istr2 >> res; - if (!res.is()) + const std::string err = picojson::parse(res, line); + if (!err.empty()) { + // TODO: report + //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; + } picojson::object obj = res.get(); From e8a6255320a350d9116dd6c0b09c99f8dd55e20e Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 14 Sep 2023 14:09:32 +0200 Subject: [PATCH 06/15] CppCheck: parse addon result only once --- lib/cppcheck.cpp | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index f70ce14649b..dd8e3f16e18 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -328,11 +328,11 @@ 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 CppCheck::ExecuteCmdFn &executeCommand) +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"; @@ -389,6 +389,8 @@ static std::string executeAddon(const AddonInfo &addonInfo, throw InternalError(nullptr, message); } + std::vector addonResult; + // Validate output.. std::istringstream istr(result); std::string line; @@ -413,10 +415,23 @@ static std::string executeAddon(const AddonInfo &addonInfo, } //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) @@ -1464,26 +1479,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)) { - picojson::value res; - const std::string err = picojson::parse(res, line); - if (!err.empty()) { - // TODO: report - //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; - } - + 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; From 950ea2d55781854bb2c3dd086a846947a371e5e0 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 21:50:11 +0200 Subject: [PATCH 07/15] InternalError: report `INTERNAL` as `internalError` --- lib/errortypes.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/errortypes.cpp b/lib/errortypes.cpp index 09f6b0f731b..a13dc56876f 100644 --- a/lib/errortypes.cpp +++ b/lib/errortypes.cpp @@ -32,7 +32,7 @@ InternalError::InternalError(const Token *tok, std::string errorMsg, Type type) id = "unknownMacro"; break; case INTERNAL: - id = "cppcheckError"; + id = "internalError"; break; case LIMIT: id = "cppcheckLimit"; From c17784efa5cd53d7add7c73945e12b115de0a17e Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 21:51:57 +0200 Subject: [PATCH 08/15] InternalError: extracted `Type` to string mapping to separate function --- lib/errortypes.cpp | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/lib/errortypes.cpp b/lib/errortypes.cpp index a13dc56876f..38ede0c7726 100644 --- a/lib/errortypes.cpp +++ b/lib/errortypes.cpp @@ -18,31 +18,28 @@ #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 = "internalError"; - 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) : + token(tok), errorMessage(std::move(errorMsg)), type(type), id(typeToString(type)) +{} + std::string Severity::toString(Severity::SeverityType severity) { switch (severity) { From a200b91a7a20d887eb8e9a314bca4b4427e9e12e Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 21:53:08 +0200 Subject: [PATCH 09/15] InternalError: added `details` field and make it possible to specify additional message --- lib/errorlogger.cpp | 7 +++++-- lib/errorlogger.h | 2 +- lib/errortypes.cpp | 6 +++++- lib/errortypes.h | 6 +++++- test/testerrorlogger.cpp | 31 +++++++++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 5 deletions(-) diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 99ace368456..c13dee7e7d9 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 2d1bed3c4f5..ccc569c8c38 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -198,7 +198,7 @@ class CPPCHECKLIB ErrorMessage { 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 38ede0c7726..cd9ca57711e 100644 --- a/lib/errortypes.cpp +++ b/lib/errortypes.cpp @@ -37,7 +37,11 @@ static std::string typeToString(InternalError::Type type) } InternalError::InternalError(const Token *tok, std::string errorMsg, Type type) : - token(tok), errorMessage(std::move(errorMsg)), type(type), id(typeToString(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) diff --git a/lib/errortypes.h b/lib/errortypes.h index 554721ba961..2bb3a32c5f8 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/testerrorlogger.cpp b/test/testerrorlogger.cpp index 14de5539047..97d94dcd2a0 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -44,6 +44,7 @@ class TestErrorLogger : public TestFixture { 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 @@ class TestErrorLogger : public TestFixture { 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); From b1f9fc7d86a1f48964d445f6db703ade74b140d2 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 21:57:30 +0200 Subject: [PATCH 10/15] CppCheck: extracted Python detection into separate function --- lib/cppcheck.cpp | 48 +++++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index dd8e3f16e18..29da6d39fab 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -328,6 +328,30 @@ static void createDumpFile(const Settings& settings, << "/>" << '\n'; } +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) == 0 && 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, @@ -345,28 +369,10 @@ static std::vector 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) == 0 && startsWith(out, "Python ") && std::isdigit(out[7])) { - pythonExe = py_exe; - break; - } - } - if (pythonExe.empty()) + const std::string detectedPythonExe = detectPython(executeCommand); + if (detectedPythonExe.empty()) throw InternalError(nullptr, "Failed to auto detect python"); + pythonExe = detectedPythonExe; } std::string args; From e08c880dbea9509eaf0bb92d7345ffbd7470e821 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 21:58:15 +0200 Subject: [PATCH 11/15] CppCheck: perform Python detection only once --- lib/cppcheck.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 29da6d39fab..3873952ba0b 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -369,7 +369,8 @@ static std::vector executeAddon(const AddonInfo &addonInfo, else if (!defaultPythonExe.empty()) pythonExe = cmdFileName(defaultPythonExe); else { - const std::string detectedPythonExe = detectPython(executeCommand); + // 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; From 655fd22ce1960c4da18581e51ba6e9bdcfbe6b07 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 22:05:10 +0200 Subject: [PATCH 12/15] CppCheck: pass additional verbose infos about execution failure as `InternalError::details` --- lib/cppcheck.cpp | 19 ++++++++------ test/cli/test-other.py | 56 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 3873952ba0b..5f7db79fa7a 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -388,12 +388,14 @@ static std::vector executeAddon(const AddonInfo &addonInfo, std::string result; if (const int exitcode = executeCommand(pythonExe, split(args), redirect, result)) { - std::string message("Failed to execute addon '" + addonInfo.name + "' (command: '" + pythonExe + " " + args + "'). Exitcode is " + std::to_string(exitcode) + "."); + 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; + details.resize(details.find_last_not_of("\n\r")); } - throw InternalError(nullptr, message); + throw InternalError(nullptr, message, details); } std::vector addonResult; @@ -634,7 +636,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; @@ -1060,7 +1063,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()) { @@ -1548,7 +1552,8 @@ void CppCheck::executeAddonsWholeProgram(const std::map + 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 + # "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 + print(stderr) + assert stderr.startswith("{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 256.: ".format(test_file)) + # TODO: string is cut off + assert stderr.count('Output:\nTraceback') + assert stderr.endswith('raise Exception()\nExceptio [internalError]\n\n^\n') def test_addon_result(tmpdir): From 2aaef14406e7e87b7b79c62873cbce07252928d7 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 22:38:59 +0200 Subject: [PATCH 13/15] corrected exitcode of command execution --- cli/cppcheckexecutor.cpp | 9 +++++++++ lib/cppcheck.cpp | 6 +++--- test/cli/test-other.py | 10 +++++----- test/testsingleexecutor.cpp | 2 +- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index d98f79d7887..71ab17e181d 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -587,7 +587,16 @@ int CppCheckExecutor::executeCommand(std::string exe, std::vector a // TODO: how to provide to caller? //const int err = errno; //std::cout << "pclose() errno " << std::to_string(err) << std::endl; + return res; } +#if !defined(WIN32) && !defined(__MINGW32__) + if (WIFEXITED(res)) { + return WEXITSTATUS(res); + } + if (WIFSIGNALED(res)) { + return WTERMSIG(res); + } +#endif return res; } diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 5f7db79fa7a..4f3346a3dc1 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -345,7 +345,7 @@ static std::string detectPython(const CppCheck::ExecuteCmdFn &executeCommand) break; } #endif - if (executeCommand(py_exe, split("--version"), "2>&1", out) == 0 && startsWith(out, "Python ") && std::isdigit(out[7])) { + if (executeCommand(py_exe, split("--version"), "2>&1", out) == EXIT_SUCCESS && startsWith(out, "Python ") && std::isdigit(out[7])) { return py_exe; } } @@ -388,7 +388,7 @@ static std::vector executeAddon(const AddonInfo &addonInfo, std::string result; 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 message("Failed to execute addon '" + addonInfo.name + "' - exitcode is " + std::to_string(exitcode)); std::string details = pythonExe + " " + args; if (result.size() > 2) { details += "\nOutput:\n"; @@ -572,7 +572,7 @@ unsigned int CppCheck::check(const std::string &path) } std::string output2; - if (mExecuteCommand(exe,split(args2),redirect2,output2) != 0 || 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; } diff --git a/test/cli/test-other.py b/test/cli/test-other.py index ff37fa530bd..bc25ad8ae2b 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -207,8 +207,8 @@ def test_execute_addon_failure_2(tmpdir): args = ['--addon=naming', '--addon-python=python5.x', test_file] _, _, stderr = cppcheck(args) - # TODO: exitcode is strange - assert stderr == "{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'naming' - exitcode is 32512. [internalError]\n\n^\n".format(test_file) + 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 @@ -372,7 +372,7 @@ def test_invalid_addon_py(tmpdir): 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 256. [internalError]\n\n^\n".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): @@ -401,7 +401,7 @@ def test_invalid_addon_py_verbose(tmpdir): '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 256.: 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 +/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 @@ -416,7 +416,7 @@ def test_invalid_addon_py_verbose(tmpdir): # /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 # "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 print(stderr) - assert stderr.startswith("{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 256.: ".format(test_file)) + assert stderr.startswith("{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 1: ".format(test_file)) # TODO: string is cut off assert stderr.count('Output:\nTraceback') assert stderr.endswith('raise Exception()\nExceptio [internalError]\n\n^\n') diff --git a/test/testsingleexecutor.cpp b/test/testsingleexecutor.cpp index aa1f34996ee..f9c2d6f55d7 100644 --- a/test/testsingleexecutor.cpp +++ b/test/testsingleexecutor.cpp @@ -113,7 +113,7 @@ class TestSingleExecutorBase : public TestFixture { executeCommandCalled = true; exe = std::move(e); args = std::move(a); - return 0; + return EXIT_SUCCESS; }); cppcheck.settings() = settings; From b17af0545b32fb78d7d5e7da6938109419332898 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 22:42:49 +0200 Subject: [PATCH 14/15] avoid cut off string in verbose command execution failure --- lib/cppcheck.cpp | 4 +++- test/cli/test-other.py | 5 +---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 4f3346a3dc1..80f4019fa76 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -393,7 +393,9 @@ static std::vector executeAddon(const AddonInfo &addonInfo, if (result.size() > 2) { details += "\nOutput:\n"; details += result; - details.resize(details.find_last_not_of("\n\r")); + const auto pos = details.find_last_not_of("\n\r"); + if (pos != std::string::npos) + details.resize(pos + 1); } throw InternalError(nullptr, message, details); } diff --git a/test/cli/test-other.py b/test/cli/test-other.py index bc25ad8ae2b..52be3f45d77 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -414,12 +414,9 @@ def test_invalid_addon_py_verbose(tmpdir): 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 - # "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 - print(stderr) assert stderr.startswith("{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 1: ".format(test_file)) - # TODO: string is cut off assert stderr.count('Output:\nTraceback') - assert stderr.endswith('raise Exception()\nExceptio [internalError]\n\n^\n') + assert stderr.endswith('raise Exception()\nException [internalError]\n\n^\n') def test_addon_result(tmpdir): From 2beca169d3ff5db13496498ca7336649707d98b0 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 15 Sep 2023 23:19:32 +0200 Subject: [PATCH 15/15] AddonInfo: fixed `name` on Windows --- lib/cppcheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 80f4019fa76..da85ebd6853 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -189,7 +189,7 @@ namespace { 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;