Skip to content

generate AddonInfo only once #4958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
242 changes: 123 additions & 119 deletions Makefile

Large diffs are not rendered by default.

19 changes: 19 additions & 0 deletions cli/cppcheckexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c
if (!loadLibraries(settings))
return false;

if (!loadAddons(settings))
return false;

// Check that all include paths exist
{
for (std::list<std::string>::iterator iter = settings.includePaths.begin();
Expand Down Expand Up @@ -406,6 +409,22 @@ bool CppCheckExecutor::loadLibraries(Settings& settings)
return result;
}

bool CppCheckExecutor::loadAddons(Settings& settings)
{
bool result = true;
for (const std::string &addon: settings.addons) {
AddonInfo addonInfo;
const std::string failedToGetAddonInfo = addonInfo.getAddonInfo(addon, settings.exename);
if (!failedToGetAddonInfo.empty()) {
std::cout << failedToGetAddonInfo << std::endl;
result = false;
continue;
}
settings.addonInfos.emplace_back(std::move(addonInfo));
}
return result;
}

#ifdef _WIN32
// fix trac ticket #439 'Cppcheck reports wrong filename for filenames containing 8-bit ASCII'
static inline std::string ansiToOEM(const std::string &msg, bool doConvert)
Expand Down
7 changes: 7 additions & 0 deletions cli/cppcheckexecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,13 @@ class CppCheckExecutor : public ErrorLogger {
*/
bool loadLibraries(Settings& settings);

/**
* @brief Load addons
* @param settings Settings
* @return Returns true if successful
*/
static bool loadAddons(Settings& settings);

/**
* @brief Write the checkers report
*/
Expand Down
4 changes: 4 additions & 0 deletions gui/mainwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,7 @@ Settings MainWindow::getCppcheckSettings()

addonFilePath.replace(QChar('\\'), QChar('/'));

// TODO: use picojson to generate the JSON
QString json;
json += "{ \"script\":\"" + addonFilePath + "\"";
if (!pythonCmd.isEmpty())
Expand All @@ -1014,6 +1015,9 @@ Settings MainWindow::getCppcheckSettings()
}
json += " }";
result.addons.emplace(json.toStdString());
AddonInfo addonInfo;
addonInfo.getAddonInfo(json.toStdString(), result.exename);
result.addonInfos.emplace_back(std::move(addonInfo));
}

if (isCppcheckPremium()) {
Expand Down
132 changes: 132 additions & 0 deletions lib/addoninfo.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
* Cppcheck - A tool for static C/C++ code analysis
* Copyright (C) 2007-2023 Cppcheck team.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "addoninfo.h"

#include "path.h"
#include "utils.h"

#include <fstream>
#include <sstream>
#include <string>

#include "json.h"

static std::string getFullPath(const std::string &fileName, const std::string &exename) {
if (Path::isFile(fileName))
return fileName;

const std::string exepath = Path::getPathFromFilename(exename);
if (Path::isFile(exepath + fileName))
return exepath + fileName;
if (Path::isFile(exepath + "addons/" + fileName))
return exepath + "addons/" + fileName;

#ifdef FILESDIR
if (Path::isFile(FILESDIR + ("/" + fileName)))
return FILESDIR + ("/" + fileName);
if (Path::isFile(FILESDIR + ("/addons/" + fileName)))
return FILESDIR + ("/addons/" + fileName);
#endif
return "";
}

static std::string parseAddonInfo(AddonInfo& addoninfo, const picojson::value &json, const std::string &fileName, const std::string &exename) {
const std::string& json_error = picojson::get_last_error();
if (!json_error.empty()) {
return "Loading " + fileName + " failed. " + json_error;
}
if (!json.is<picojson::object>())
return "Loading " + fileName + " failed. Bad json.";
picojson::object obj = json.get<picojson::object>();
if (obj.count("args")) {
if (!obj["args"].is<picojson::array>())
return "Loading " + fileName + " failed. args must be array.";
for (const picojson::value &v : obj["args"].get<picojson::array>())
addoninfo.args += " " + v.get<std::string>();
}

if (obj.count("ctu")) {
// ctu is specified in the config file
if (!obj["ctu"].is<bool>())
return "Loading " + fileName + " failed. ctu must be boolean.";
addoninfo.ctu = obj["ctu"].get<bool>();
} else {
addoninfo.ctu = false;
}

if (obj.count("python")) {
// Python was defined in the config file
if (obj["python"].is<picojson::array>()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the feeling this should say if (!obj["python"].is<std::string>()) instead .. I have no idea why the old code checked for the array instead :-(

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Will have a look and add some tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code made sure it is not an array. So that is fine albeit a rather specific handling. Will still adjust though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires a lot of new tests to be written. I will do this in a follow-up PR so this can be merged.

return "Loading " + fileName +" failed. python must not be an array.";
}
addoninfo.python = obj["python"].get<std::string>();
} else {
addoninfo.python = "";
}

if (obj.count("executable")) {
if (!obj["executable"].is<std::string>())
return "Loading " + fileName + " failed. executable must be a string.";
addoninfo.executable = getFullPath(obj["executable"].get<std::string>(), fileName);
return "";
}

return addoninfo.getAddonInfo(obj["script"].get<std::string>(), exename);
}

std::string AddonInfo::getAddonInfo(const std::string &fileName, const std::string &exename) {
if (fileName[0] == '{') {
picojson::value json;
const std::string err = picojson::parse(json, fileName);
(void)err; // TODO: report
return parseAddonInfo(*this, json, fileName, exename);
}
if (fileName.find('.') == std::string::npos)
return getAddonInfo(fileName + ".py", exename);

if (endsWith(fileName, ".py")) {
scriptFile = Path::fromNativeSeparators(getFullPath(fileName, exename));
if (scriptFile.empty())
return "Did not find addon " + fileName;

std::string::size_type pos1 = scriptFile.rfind('/');
if (pos1 == std::string::npos)
pos1 = 0;
else
pos1++;
std::string::size_type pos2 = scriptFile.rfind('.');
if (pos2 < pos1)
pos2 = std::string::npos;
name = scriptFile.substr(pos1, pos2 - pos1);

runScript = getFullPath("runaddon.py", exename);

return "";
}

if (!endsWith(fileName, ".json"))
return "Failed to open addon " + fileName;

std::ifstream fin(fileName);
if (!fin.is_open())
return "Failed to open " + fileName;
picojson::value json;
fin >> json;
return parseAddonInfo(*this, json, fileName, exename);
}
38 changes: 38 additions & 0 deletions lib/addoninfo.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Cppcheck - A tool for static C/C++ code analysis
* Copyright (C) 2007-2023 Cppcheck team.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#ifndef addonInfoH
#define addonInfoH

#include "config.h"

#include <string>

struct CPPCHECKLIB AddonInfo {
std::string name;
std::string scriptFile; // addon script
std::string executable; // addon executable
std::string args; // special extra arguments
std::string python; // script interpreter
bool ctu = false;
std::string runScript;

std::string getAddonInfo(const std::string &fileName, const std::string &exename);
};

#endif // addonInfoH
130 changes: 6 additions & 124 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "cppcheck.h"

#include "addoninfo.h"
#include "check.h"
#include "checkunusedfunctions.h"
#include "clangimport.h"
Expand Down Expand Up @@ -105,122 +107,6 @@ namespace {
};
}

namespace {
struct AddonInfo {
std::string name;
std::string scriptFile; // addon script
std::string executable; // addon executable
std::string args; // special extra arguments
std::string python; // script interpreter
bool ctu = false;
std::string runScript;

static std::string getFullPath(const std::string &fileName, const std::string &exename) {
if (Path::isFile(fileName))
return fileName;

const std::string exepath = Path::getPathFromFilename(exename);
if (Path::isFile(exepath + fileName))
return exepath + fileName;
if (Path::isFile(exepath + "addons/" + fileName))
return exepath + "addons/" + fileName;

#ifdef FILESDIR
if (Path::isFile(FILESDIR + ("/" + fileName)))
return FILESDIR + ("/" + fileName);
if (Path::isFile(FILESDIR + ("/addons/" + fileName)))
return FILESDIR + ("/addons/" + fileName);
#endif
return "";
}

std::string parseAddonInfo(const picojson::value &json, const std::string &fileName, const std::string &exename) {
const std::string& json_error = picojson::get_last_error();
if (!json_error.empty()) {
return "Loading " + fileName + " failed. " + json_error;
}
if (!json.is<picojson::object>())
return "Loading " + fileName + " failed. Bad json.";
picojson::object obj = json.get<picojson::object>();
if (obj.count("args")) {
if (!obj["args"].is<picojson::array>())
return "Loading " + fileName + " failed. args must be array.";
for (const picojson::value &v : obj["args"].get<picojson::array>())
args += " " + v.get<std::string>();
}

if (obj.count("ctu")) {
// ctu is specified in the config file
if (!obj["ctu"].is<bool>())
return "Loading " + fileName + " failed. ctu must be boolean.";
ctu = obj["ctu"].get<bool>();
} else {
ctu = false;
}

if (obj.count("python")) {
// Python was defined in the config file
if (obj["python"].is<picojson::array>()) {
return "Loading " + fileName +" failed. python must not be an array.";
}
python = obj["python"].get<std::string>();
} else {
python = "";
}

if (obj.count("executable")) {
if (!obj["executable"].is<std::string>())
return "Loading " + fileName + " failed. executable must be a string.";
executable = getFullPath(obj["executable"].get<std::string>(), fileName);
return "";
}

return getAddonInfo(obj["script"].get<std::string>(), exename);
}

std::string getAddonInfo(const std::string &fileName, const std::string &exename) {
if (fileName[0] == '{') {
picojson::value 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 = Path::fromNativeSeparators(getFullPath(fileName, exename));
if (scriptFile.empty())
return "Did not find addon " + fileName;

std::string::size_type pos1 = scriptFile.rfind('/');
if (pos1 == std::string::npos)
pos1 = 0;
else
pos1++;
std::string::size_type pos2 = scriptFile.rfind('.');
if (pos2 < pos1)
pos2 = std::string::npos;
name = scriptFile.substr(pos1, pos2 - pos1);

runScript = getFullPath("runaddon.py", exename);

return "";
}

if (!endsWith(fileName, ".json"))
return "Failed to open addon " + fileName;

std::ifstream fin(fileName);
if (!fin.is_open())
return "Failed to open " + fileName;
picojson::value json;
fin >> json;
return parseAddonInfo(json, fileName, exename);
}
};
}

static std::string cmdFileName(std::string f)
{
f = Path::toNativeSeparators(f);
Expand Down Expand Up @@ -1502,14 +1388,10 @@ void CppCheck::executeAddons(const std::vector<std::string>& files)
fout << f << std::endl;
}

for (const std::string &addon : mSettings.addons) {
AddonInfo addonInfo;
const std::string &failedToGetAddonInfo = addonInfo.getAddonInfo(addon, mSettings.exename);
if (!failedToGetAddonInfo.empty()) {
reportOut(failedToGetAddonInfo, Color::FgRed);
mExitCode = 1;
continue;
}
// ensure all addons have already been resolved - TODO: remove when settings are const after creation
assert(mSettings.addonInfos.size() == mSettings.addons.size());

for (const AddonInfo &addonInfo : mSettings.addonInfos) {
if (addonInfo.name != "misra" && !addonInfo.ctu && endsWith(files.back(), ".ctu-info"))
continue;

Expand Down
Loading