From 30e34e261a229c0b109b6f992d9f81a60359ac49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 1 Feb 2024 21:25:37 +0100 Subject: [PATCH] Fix #12389 (GUI: premiumaddon is not executed properly anymore) (#5923) --- gui/mainwindow.cpp | 86 +++++++++++++++++------- gui/mainwindow.h | 4 +- gui/projectfile.cpp | 3 + gui/test/projectfile/testprojectfile.cpp | 20 ++++++ gui/test/projectfile/testprojectfile.h | 2 + lib/addoninfo.cpp | 12 +++- 6 files changed, 98 insertions(+), 29 deletions(-) diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp index 3744f520c58..dca973e1698 100644 --- a/gui/mainwindow.cpp +++ b/gui/mainwindow.cpp @@ -486,6 +486,11 @@ void MainWindow::saveSettings() const void MainWindow::doAnalyzeProject(ImportProject p, const bool checkLibrary, const bool checkConfiguration) { + QPair checkSettingsPair = getCppcheckSettings(); + if (!checkSettingsPair.first) + return; + Settings& checkSettings = checkSettingsPair.second; + clearResults(); mIsLogfileLoaded = false; @@ -522,7 +527,6 @@ void MainWindow::doAnalyzeProject(ImportProject p, const bool checkLibrary, cons checkLockDownUI(); // lock UI while checking mUI->mResults->setCheckDirectory(checkPath); - Settings checkSettings = getCppcheckSettings(); checkSettings.force = false; checkSettings.checkLibrary = checkLibrary; checkSettings.checkConfiguration = checkConfiguration; @@ -550,9 +554,14 @@ void MainWindow::doAnalyzeProject(ImportProject p, const bool checkLibrary, cons void MainWindow::doAnalyzeFiles(const QStringList &files, const bool checkLibrary, const bool checkConfiguration) { - if (files.isEmpty()) { + if (files.isEmpty()) return; - } + + QPair checkSettingsPair = getCppcheckSettings(); + if (!checkSettingsPair.first) + return; + Settings& checkSettings = checkSettingsPair.second; + clearResults(); mIsLogfileLoaded = false; @@ -591,7 +600,6 @@ void MainWindow::doAnalyzeFiles(const QStringList &files, const bool checkLibrar checkLockDownUI(); // lock UI while checking mUI->mResults->setCheckDirectory(checkPath); - Settings checkSettings = getCppcheckSettings(); checkSettings.checkLibrary = checkLibrary; checkSettings.checkConfiguration = checkConfiguration; @@ -614,6 +622,11 @@ void MainWindow::doAnalyzeFiles(const QStringList &files, const bool checkLibrar void MainWindow::analyzeCode(const QString& code, const QString& filename) { + const QPair& checkSettingsPair = getCppcheckSettings(); + if (!checkSettingsPair.first) + return; + const Settings& checkSettings = checkSettingsPair.second; + // Initialize dummy ThreadResult as ErrorLogger ThreadResult result; result.setFiles(QStringList(filename)); @@ -628,7 +641,7 @@ void MainWindow::analyzeCode(const QString& code, const QString& filename) // Create CppCheck instance CppCheck cppcheck(result, true, nullptr); - cppcheck.settings() = getCppcheckSettings(); + cppcheck.settings() = checkSettings; // Check checkLockDownUI(); @@ -905,13 +918,12 @@ bool MainWindow::tryLoadLibrary(Library *library, const QString& filename) return true; } -void MainWindow::loadAddon(Settings &settings, const QString &filesDir, const QString &pythonCmd, const QString& addon) +QString MainWindow::loadAddon(Settings &settings, const QString &filesDir, const QString &pythonCmd, const QString& addon) { - QString addonFilePath = ProjectFile::getAddonFilePath(filesDir, addon); - if (addonFilePath.isEmpty()) - return; // TODO: report an error + const QString addonFilePath = fromNativePath(ProjectFile::getAddonFilePath(filesDir, addon)); - addonFilePath.replace(QChar('\\'), QChar('/')); + if (addonFilePath.isEmpty()) + return tr("File not found: '%1'").arg(addon); picojson::object obj; obj["script"] = picojson::value(addonFilePath.toStdString()); @@ -929,42 +941,54 @@ void MainWindow::loadAddon(Settings &settings, const QString &filesDir, const QS obj["args"] = picojson::value(arg.toStdString()); } } - picojson::value json; - json.set(std::move(obj)); - std::string json_str = json.serialize(); + + const std::string& json_str = picojson::value(obj).serialize(); AddonInfo addonInfo; - addonInfo.getAddonInfo(json_str, settings.exename); // TODO: handle error + const std::string errmsg = addonInfo.getAddonInfo(json_str, settings.exename); + if (!errmsg.empty()) + return tr("Failed to load/setup addon %1: %2").arg(addon, QString::fromStdString(errmsg)); settings.addonInfos.emplace_back(std::move(addonInfo)); - settings.addons.emplace(std::move(json_str)); + settings.addons.emplace(json_str); + + return ""; } -Settings MainWindow::getCppcheckSettings() +QPair MainWindow::getCppcheckSettings() { saveSettings(); // Save settings + Settings::terminate(true); Settings result; result.exename = QCoreApplication::applicationFilePath().toStdString(); const bool std = tryLoadLibrary(&result.library, "std.cfg"); - if (!std) - QMessageBox::critical(this, tr("Error"), tr("Failed to load %1. Your Cppcheck installation is broken. You can use --data-dir= at the command line to specify where this file is located. Please note that --data-dir is supposed to be used by installation scripts and therefore the GUI does not start when it is used, all that happens is that the setting is configured.").arg("std.cfg")); + if (!std) { + QMessageBox::critical(this, tr("Error"), tr("Failed to load %1. Your Cppcheck installation is broken. You can use --data-dir= at the command line to specify where this file is located. Please note that --data-dir is supposed to be used by installation scripts and therefore the GUI does not start when it is used, all that happens is that the setting is configured.\n\nAnalysis is aborted.").arg("std.cfg")); + return {false, {}}; + } const QString filesDir(getDataDir()); const QString pythonCmd = fromNativePath(mSettings->value(SETTINGS_PYTHON_PATH).toString()); { const QString cfgErr = QString::fromStdString(result.loadCppcheckCfg()); - if (!cfgErr.isEmpty()) - QMessageBox::critical(this, tr("Error"), tr("Failed to load %1 - %2").arg("cppcheck.cfg").arg(cfgErr)); + if (!cfgErr.isEmpty()) { + QMessageBox::critical(this, tr("Error"), tr("Failed to load %1 - %2\n\nAnalysis is aborted.").arg("cppcheck.cfg").arg(cfgErr)); + return {false, {}}; + } const auto cfgAddons = result.addons; result.addons.clear(); for (const std::string& addon : cfgAddons) { // TODO: support addons which are a script and not a file - loadAddon(result, filesDir, pythonCmd, QString::fromStdString(addon)); + const QString addonError = loadAddon(result, filesDir, pythonCmd, QString::fromStdString(addon)); + if (!addonError.isEmpty()) { + QMessageBox::critical(this, tr("Error"), tr("%1\n\nAnalysis is aborted.").arg(addonError)); + return {false, {}}; + } } } @@ -1045,7 +1069,11 @@ Settings MainWindow::getCppcheckSettings() result.checkUnknownFunctionReturn.insert(s.toStdString()); for (const QString& addon : mProjectFile->getAddons()) { - loadAddon(result, filesDir, pythonCmd, addon); + const QString addonError = loadAddon(result, filesDir, pythonCmd, addon); + if (!addonError.isEmpty()) { + QMessageBox::critical(this, tr("Error"), tr("%1\n\nAnalysis is aborted.").arg(addonError)); + return {false, {}}; + } } if (isCppcheckPremium()) { @@ -1099,7 +1127,7 @@ Settings MainWindow::getCppcheckSettings() Settings::terminate(false); - return result; + return {true, std::move(result)}; } void MainWindow::analysisDone() @@ -1223,6 +1251,11 @@ void MainWindow::reAnalyzeSelected(const QStringList& files) if (mThread->isChecking()) return; + const QPair checkSettingsPair = getCppcheckSettings(); + if (!checkSettingsPair.first) + return; + const Settings& checkSettings = checkSettingsPair.second; + // Clear details, statistics and progress mUI->mResults->clear(false); for (int i = 0; i < files.size(); ++i) @@ -1242,7 +1275,6 @@ void MainWindow::reAnalyzeSelected(const QStringList& files) // considered in "Modified Files Check" performed after "Selected Files Check" // TODO: Should we store per file CheckStartTime? QDateTime saveCheckStartTime = mThread->getCheckStartTime(); - const Settings& checkSettings = getCppcheckSettings(); mThread->check(checkSettings); mUI->mResults->setCheckSettings(checkSettings); mThread->setCheckStartTime(saveCheckStartTime); @@ -1254,6 +1286,11 @@ void MainWindow::reAnalyze(bool all) if (files.empty()) return; + const QPair& checkSettingsPair = getCppcheckSettings(); + if (!checkSettingsPair.first) + return; + const Settings& checkSettings = checkSettingsPair.second; + // Clear details, statistics and progress mUI->mResults->clear(all); @@ -1268,7 +1305,6 @@ void MainWindow::reAnalyze(bool all) qDebug() << "Rechecking project file" << mProjectFile->getFilename(); mThread->setCheckFiles(all); - const Settings& checkSettings = getCppcheckSettings(); mThread->check(checkSettings); mUI->mResults->setCheckSettings(checkSettings); } diff --git a/gui/mainwindow.h b/gui/mainwindow.h index 4b6b20f9ad3..b0f059218cf 100644 --- a/gui/mainwindow.h +++ b/gui/mainwindow.h @@ -315,7 +315,7 @@ private slots: * * @return Default cppcheck settings */ - Settings getCppcheckSettings(); + QPair getCppcheckSettings(); /** @brief Load program settings */ void loadSettings(); @@ -401,7 +401,7 @@ private slots: */ bool tryLoadLibrary(Library *library, const QString& filename); - void loadAddon(Settings &settings, const QString &filesDir, const QString &pythonCmd, const QString& addon); + QString loadAddon(Settings &settings, const QString &filesDir, const QString &pythonCmd, const QString& addon); /** * @brief Update project MRU items in File-menu. diff --git a/gui/projectfile.cpp b/gui/projectfile.cpp index f24f6eece93..393063bb49d 100644 --- a/gui/projectfile.cpp +++ b/gui/projectfile.cpp @@ -1115,6 +1115,9 @@ void ProjectFile::SafeChecks::saveToXml(QXmlStreamWriter &xmlWriter) const QString ProjectFile::getAddonFilePath(QString filesDir, const QString &addon) { + if (QFile(addon).exists()) + return addon; + if (!filesDir.endsWith("/")) filesDir += "/"; diff --git a/gui/test/projectfile/testprojectfile.cpp b/gui/test/projectfile/testprojectfile.cpp index 6e5e73b0f06..505a6e8937c 100644 --- a/gui/test/projectfile/testprojectfile.cpp +++ b/gui/test/projectfile/testprojectfile.cpp @@ -28,6 +28,7 @@ #include #include +#include #include // Mock... @@ -116,4 +117,23 @@ void TestProjectFile::loadSimpleNoroot() const QCOMPARE(defines[0], QString("FOO")); } +void TestProjectFile::getAddonFilePath() const +{ + QTemporaryDir tempdir; + QVERIFY(tempdir.isValid()); + const QString filepath(tempdir.path() + "/addon.py"); + + QFile file(filepath); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.close(); + + // Relative path to addon + QCOMPARE(ProjectFile::getAddonFilePath(tempdir.path(), "addon"), filepath); + QCOMPARE(ProjectFile::getAddonFilePath(tempdir.path(), "not exist"), QString()); + + // Absolute path to addon + QCOMPARE(ProjectFile::getAddonFilePath("/not/exist", filepath), filepath); + QCOMPARE(ProjectFile::getAddonFilePath(tempdir.path(), filepath), filepath); +} + QTEST_MAIN(TestProjectFile) diff --git a/gui/test/projectfile/testprojectfile.h b/gui/test/projectfile/testprojectfile.h index b0bc7d96969..47ed517f412 100644 --- a/gui/test/projectfile/testprojectfile.h +++ b/gui/test/projectfile/testprojectfile.h @@ -27,4 +27,6 @@ private slots: void loadSimple() const; void loadSimpleWithIgnore() const; void loadSimpleNoroot() const; + + void getAddonFilePath() const; }; diff --git a/lib/addoninfo.cpp b/lib/addoninfo.cpp index 7930f2e591c..39b56cc3c31 100644 --- a/lib/addoninfo.cpp +++ b/lib/addoninfo.cpp @@ -105,8 +105,11 @@ static std::string parseAddonInfo(AddonInfo& addoninfo, const picojson::value &j const auto& val = it->second; if (!val.is()) return "Loading " + fileName + " failed. 'executable' must be a string."; - addoninfo.executable = getFullPath(val.get(), fileName); - return ""; // TODO: why bail out? + const std::string e = val.get(); + addoninfo.executable = getFullPath(e, fileName); + if (addoninfo.executable.empty()) + addoninfo.executable = e; + return ""; // <- do not load both "executable" and "script". } } @@ -157,6 +160,11 @@ std::string AddonInfo::getAddonInfo(const std::string &fileName, const std::stri std::ifstream fin(fileName); if (!fin.is_open()) return "Failed to open " + fileName; + if (name.empty()) { + name = Path::fromNativeSeparators(fileName); + if (name.find('/') != std::string::npos) + name = name.substr(name.rfind('/') + 1); + } picojson::value json; fin >> json; return parseAddonInfo(*this, json, fileName, exename);