feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

ClangTools: make parsing diagnostics cancelable

Browse Source 
Change-Id: Ia5b4bd6f5fbb9a81888b1eaf11b956617e4b740c
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
This commit is contained in:
David Schulz
2023-06-19 10:11:45 +02:00
parent f97d9e51c0
commit deaef7da33
7 changed files with 106 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
src/plugins/clangtools/clangtool.cpp
View File

@@ -677,16 +677,6 @@ void ClangTool::startTool(ClangTool::FileSelection fileSelection,
ProjectExplorerPlugin::startRunControl(m_runControl); ProjectExplorerPlugin::startRunControl(m_runControl);
} }
Diagnostics ClangTool::read(const FilePath &logFilePath,
const QSet<FilePath> &projectFiles,
QString *errorMessage) const
{
const auto acceptFromFilePath = [projectFiles](const FilePath &filePath) {
return projectFiles.contains(filePath);
};
return readExportedDiagnostics(logFilePath, acceptFromFilePath, errorMessage);
}
FileInfos ClangTool::collectFileInfos(Project *project, FileSelection fileSelection) FileInfos ClangTool::collectFileInfos(Project *project, FileSelection fileSelection)
{ {
FileSelectionType *selectionType = std::get_if<FileSelectionType>(&fileSelection); FileSelectionType *selectionType = std::get_if<FileSelectionType>(&fileSelection);
@@ -763,21 +753,17 @@ void ClangTool::loadDiagnosticsFromFiles()
// Load files // Load files
Diagnostics diagnostics; Diagnostics diagnostics;
QString errors; QStringList errors;
for (const FilePath &filePath : filePaths) { for (const FilePath &filePath : filePaths) {
QString currentError; if (expected_str<Diagnostics> expectedDiagnostics = readExportedDiagnostics(filePath))
diagnostics << readExportedDiagnostics(filePath, {}, &currentError); diagnostics << *expectedDiagnostics;
else
if (!currentError.isEmpty()) { errors.append(expectedDiagnostics.error());
if (!errors.isEmpty())
errors.append("\n");
errors.append(currentError);
}
} }
// Show errors // Show errors
if (!errors.isEmpty()) { if (!errors.isEmpty()) {
AsynchronousMessageBox::critical(Tr::tr("Error Loading Diagnostics"), errors); AsynchronousMessageBox::critical(Tr::tr("Error Loading Diagnostics"), errors.join('\n'));
return; return;
} }

4
src/plugins/clangtools/clangtool.h
View File

@@ -65,10 +65,6 @@ public:
const RunSettings &runSettings, const RunSettings &runSettings,
const CppEditor::ClangDiagnosticConfig &diagnosticConfig); const CppEditor::ClangDiagnosticConfig &diagnosticConfig);
Diagnostics read(const Utils::FilePath &logFilePath,
const QSet<Utils::FilePath> &projectFiles,
QString *errorMessage) const;
FileInfos collectFileInfos(ProjectExplorer::Project *project, FileInfos collectFileInfos(ProjectExplorer::Project *project,
FileSelection fileSelection); FileSelection fileSelection);

33
src/plugins/clangtools/clangtoolrunner.cpp
View File

@@ -114,7 +114,6 @@ GroupItem clangToolTask(const AnalyzeInputData &input,
QString name; QString name;
FilePath executable; FilePath executable;
FilePath outputFilePath; FilePath outputFilePath;
QString errorMessage;
}; };
const TreeStorage<ClangToolStorage> storage; const TreeStorage<ClangToolStorage> storage;
@@ -186,24 +185,36 @@ GroupItem clangToolTask(const AnalyzeInputData &input,
{false, input.unit.file, data.outputFilePath, {}, input.tool, message, details}); {false, input.unit.file, data.outputFilePath, {}, input.tool, message, details});
}; };
const auto onReadSetup = [=](Async<Diagnostics> &data) { const auto onReadSetup = [=](Async<expected_str<Diagnostics>> &data) {
data.setConcurrentCallData(&readExportedDiagnostics, data.setConcurrentCallData(&parseDiagnostics,
storage->outputFilePath, storage->outputFilePath,
input.diagnosticsFilter, input.diagnosticsFilter);
&storage->errorMessage);
data.setFutureSynchronizer(ExtensionSystem::PluginManager::futureSynchronizer()); data.setFutureSynchronizer(ExtensionSystem::PluginManager::futureSynchronizer());
}; };
const auto onReadDone = [=](const Async<Diagnostics> &data) { const auto onReadDone = [=](const Async<expected_str<Diagnostics>> &data) {
if (!outputHandler) if (!outputHandler)
return; return;
outputHandler( const expected_str<Diagnostics> result = data.result();
{true, input.unit.file, storage->outputFilePath, data.result(), input.tool}); const bool success = result.has_value();
Diagnostics diagnostics;
QString error;
if (success)
diagnostics = *result;
else
error = result.error();
outputHandler({success,
input.unit.file,
storage->outputFilePath,
diagnostics,
input.tool,
error});
}; };
const auto onReadError = [=](const Async<Diagnostics> &) { const auto onReadError = [=](const Async<expected_str<Diagnostics>> &data) {
if (!outputHandler) if (!outputHandler)
return; return;
const expected_str<Diagnostics> result = data.result();
outputHandler( outputHandler(
{false, input.unit.file, storage->outputFilePath, {}, input.tool, storage->errorMessage}); {false, input.unit.file, storage->outputFilePath, {}, input.tool, result.error()});
}; };
const Group group { const Group group {
@@ -213,7 +224,7 @@ GroupItem clangToolTask(const AnalyzeInputData &input,
sequential, sequential,
finishAllAndDone, finishAllAndDone,
ProcessTask(onProcessSetup, onProcessDone, onProcessError), ProcessTask(onProcessSetup, onProcessDone, onProcessError),
AsyncTask<Diagnostics>(onReadSetup, onReadDone, onReadError) AsyncTask<expected_str<Diagnostics>>(onReadSetup, onReadDone, onReadError)
} }
}; };
return group; return group;

57
src/plugins/clangtools/clangtoolslogfilereader.cpp
View File

@@ -11,26 +11,13 @@
#include <utils/fileutils.h> #include <utils/fileutils.h>
#include <utils/textutils.h> #include <utils/textutils.h>
#include <QFuture>
#include <yaml-cpp/yaml.h> #include <yaml-cpp/yaml.h>
namespace ClangTools { namespace ClangTools {
namespace Internal { namespace Internal {
static bool checkFilePath(const Utils::FilePath &filePath, QString *errorMessage)
{
QFileInfo fi(filePath.toFileInfo());
if (!fi.exists() || !fi.isReadable()) {
if (errorMessage) {
*errorMessage
= QString(QT_TRANSLATE_NOOP("QtC::ClangTools",
"File \"%1\" does not exist or is not readable."))
.arg(filePath.toUserOutput());
}
return false;
}
return true;
}
std::optional<LineColumnInfo> byteOffsetInUtf8TextToLineColumn(const char *text, std::optional<LineColumnInfo> byteOffsetInUtf8TextToLineColumn(const char *text,
int offset, int offset,
int startLine) int startLine)
@@ -190,19 +177,25 @@ private:
} // namespace } // namespace
Diagnostics readExportedDiagnostics(const Utils::FilePath &logFilePath, void parseDiagnostics(QPromise<Utils::expected_str<Diagnostics>> &promise,
const AcceptDiagsFromFilePath &acceptFromFilePath, const Utils::FilePath &logFilePath,
QString *errorMessage) const AcceptDiagsFromFilePath &acceptFromFilePath)
{ {
if (!checkFilePath(logFilePath, errorMessage)) const Utils::expected_str<QByteArray> localFileContents = logFilePath.fileContents();
return {}; if (!localFileContents.has_value()) {
promise.addResult(Utils::make_unexpected(localFileContents.error()));
promise.future().cancel();
return;
}
FileCache fileCache; FileCache fileCache;
Diagnostics diagnostics; Diagnostics diagnostics;
try { try {
YAML::Node document = YAML::LoadFile(logFilePath.toString().toStdString()); YAML::Node document = YAML::Load(*localFileContents);
for (const auto &diagNode : document["Diagnostics"]) { for (const auto &diagNode : document["Diagnostics"]) {
if (promise.isCanceled())
return;
// Since llvm/clang 9.0 the diagnostic items are wrapped in a "DiagnosticMessage" node. // Since llvm/clang 9.0 the diagnostic items are wrapped in a "DiagnosticMessage" node.
const auto msgNode = diagNode["DiagnosticMessage"]; const auto msgNode = diagNode["DiagnosticMessage"];
const YAML::Node &node = msgNode ? msgNode : diagNode; const YAML::Node &node = msgNode ? msgNode : diagNode;
@@ -252,16 +245,24 @@ Diagnostics readExportedDiagnostics(const Utils::FilePath &logFilePath,
diagnostics.append(diag); diagnostics.append(diag);
} }
promise.addResult(diagnostics);
} catch (std::exception &e) { } catch (std::exception &e) {
if (errorMessage) { const QString errorMessage
*errorMessage = QString( = QString(QT_TRANSLATE_NOOP("QtC::ClangTools",
QT_TRANSLATE_NOOP("QtC::ClangTools", "Error: Failed to parse YAML file \"%1\": %2."))
"Error: Failed to parse YAML file \"%1\": %2.")) .arg(logFilePath.toUserOutput(), QString::fromUtf8(e.what()));
.arg(logFilePath.toUserOutput(), QString::fromUtf8(e.what())); promise.addResult(Utils::make_unexpected(errorMessage));
} promise.future().cancel();
} }
}
return diagnostics; Utils::expected_str<Diagnostics> readExportedDiagnostics(
const Utils::FilePath &logFilePath, const AcceptDiagsFromFilePath &acceptFromFilePath)
{
QPromise<Utils::expected_str<Diagnostics>> promise;
promise.start();
parseDiagnostics(promise, logFilePath, acceptFromFilePath);
return promise.future().result();
} }
} // namespace Internal } // namespace Internal

11
src/plugins/clangtools/clangtoolslogfilereader.h
View File

@@ -6,6 +6,7 @@
#include "clangtoolsdiagnostic.h" #include "clangtoolsdiagnostic.h"
#include <QPromise>
#include <optional> #include <optional>
namespace Utils { class FilePath; } namespace Utils { class FilePath; }
@@ -16,9 +17,13 @@ namespace Internal {
using AcceptDiagsFromFilePath = std::function<bool(const Utils::FilePath &)>; using AcceptDiagsFromFilePath = std::function<bool(const Utils::FilePath &)>;
// Reads diagnostics generated by "clang-tidy/clazy-standalone -export-fixes=path/to/file" // Reads diagnostics generated by "clang-tidy/clazy-standalone -export-fixes=path/to/file"
Diagnostics readExportedDiagnostics(const Utils::FilePath &logFilePath, void parseDiagnostics(QPromise<Utils::expected_str<Diagnostics>> &promise,
const AcceptDiagsFromFilePath &acceptFromFilePath, const Utils::FilePath &logFilePath,
QString *errorMessage = nullptr); const AcceptDiagsFromFilePath &acceptFromFilePath = {});
Utils::expected_str<Diagnostics> readExportedDiagnostics(
const Utils::FilePath &logFilePath,
const AcceptDiagsFromFilePath &acceptFromFilePath = {});
// Exposed for tests // Exposed for tests
struct LineColumnInfo { struct LineColumnInfo {

85
src/plugins/clangtools/readexporteddiagnosticstest.cpp
View File

@@ -31,41 +31,40 @@ ReadExportedDiagnosticsTest::ReadExportedDiagnosticsTest()
ReadExportedDiagnosticsTest::~ReadExportedDiagnosticsTest() { delete m_baseDir; } ReadExportedDiagnosticsTest::~ReadExportedDiagnosticsTest() { delete m_baseDir; }
void ReadExportedDiagnosticsTest::initTestCase() { QVERIFY(m_baseDir->isValid()); } void ReadExportedDiagnosticsTest::initTestCase() { QVERIFY(m_baseDir->isValid()); }
void ReadExportedDiagnosticsTest::init() { m_message.clear(); } void ReadExportedDiagnosticsTest::init() { }
void ReadExportedDiagnosticsTest::testNonExistingFile() void ReadExportedDiagnosticsTest::testNonExistingFile()
{ {
const Diagnostics diags = readExportedDiagnostics("nonExistingFile.yaml", {}, &m_message); const expected_str<Diagnostics> diags = readExportedDiagnostics("nonExistingFile.yaml");
QVERIFY(diags.isEmpty()); QVERIFY(!diags.has_value());
QVERIFY(!m_message.isEmpty()); QVERIFY(!diags.error().isEmpty());
} }
void ReadExportedDiagnosticsTest::testEmptyFile() void ReadExportedDiagnosticsTest::testEmptyFile()
{ {
const Diagnostics diags = readExportedDiagnostics(filePath("empty.yaml"), {}, &m_message); const expected_str<Diagnostics> diags = readExportedDiagnostics(filePath("empty.yaml"));
QVERIFY(diags.isEmpty()); QVERIFY(diags.has_value());
QVERIFY2(m_message.isEmpty(), qPrintable(m_message)); QVERIFY(diags->isEmpty());
} }
void ReadExportedDiagnosticsTest::testUnexpectedFileContents() void ReadExportedDiagnosticsTest::testUnexpectedFileContents()
{ {
const Diagnostics diags = readExportedDiagnostics(filePath("tidy.modernize-use-nullptr.cpp"), const expected_str<Diagnostics> diags = readExportedDiagnostics(
{}, &m_message); filePath("tidy.modernize-use-nullptr.cpp"));
QVERIFY(!m_message.isEmpty()); QVERIFY(!diags.has_value());
QVERIFY(diags.isEmpty()); QVERIFY(!diags.error().isEmpty());
} }
static QString appendYamlSuffix(const char *filePathFragment) static QString appendYamlSuffix(const char *filePathFragment)
{ {
const QString yamlSuffix = QLatin1String(Utils::HostOsInfo::isWindowsHost() const QString yamlSuffix = QLatin1String(HostOsInfo::isWindowsHost() ? "_win.yaml" : ".yaml");
? "_win.yaml" : ".yaml");
return filePathFragment + yamlSuffix; return filePathFragment + yamlSuffix;
} }
void ReadExportedDiagnosticsTest::testTidy() void ReadExportedDiagnosticsTest::testTidy()
{ {
const FilePath sourceFile = filePath("tidy.modernize-use-nullptr.cpp"); const FilePath sourceFile = filePath("tidy.modernize-use-nullptr.cpp");
const QString exportedFile = createFile( const FilePath exportedFile = createFile(
filePath(appendYamlSuffix("tidy.modernize-use-nullptr")).toString(), filePath(appendYamlSuffix("tidy.modernize-use-nullptr")).toString(),
sourceFile.toString()); sourceFile.toString());
Diagnostic expectedDiag; Diagnostic expectedDiag;
@@ -79,32 +78,31 @@ void ReadExportedDiagnosticsTest::testTidy()
expectedDiag.location, expectedDiag.location,
{expectedDiag.location, {sourceFile, 2, 26}}, {expectedDiag.location, {sourceFile, 2, 26}},
true}}; true}};
const Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(exportedFile), const expected_str<Diagnostics> diags = readExportedDiagnostics(exportedFile);
{}, &m_message);
QVERIFY2(m_message.isEmpty(), qPrintable(m_message)); QVERIFY(diags.has_value());
QCOMPARE(diags, {expectedDiag}); QCOMPARE(*diags, {expectedDiag});
} }
void ReadExportedDiagnosticsTest::testAcceptDiagsFromFilePaths_None() void ReadExportedDiagnosticsTest::testAcceptDiagsFromFilePaths_None()
{ {
const QString sourceFile = filePath("tidy.modernize-use-nullptr.cpp").toString(); const QString sourceFile = filePath("tidy.modernize-use-nullptr.cpp").toString();
const QString exportedFile = createFile(filePath("tidy.modernize-use-nullptr.yaml").toString(), const FilePath exportedFile = createFile(filePath("tidy.modernize-use-nullptr.yaml").toString(),
sourceFile); sourceFile);
const auto acceptNone = [](const Utils::FilePath &) { return false; }; const auto acceptNone = [](const FilePath &) { return false; };
const Diagnostics diags = readExportedDiagnostics(FilePath::fromString(exportedFile), const expected_str<Diagnostics> diags
acceptNone, &m_message); = readExportedDiagnostics(exportedFile, acceptNone);
QVERIFY2(m_message.isEmpty(), qPrintable(m_message)); QVERIFY(diags.has_value());
QVERIFY(diags.isEmpty()); QVERIFY(diags->isEmpty());
} }
// Diagnostics from clang (static) analyzer passed through via clang-tidy // Diagnostics from clang (static) analyzer passed through via clang-tidy
void ReadExportedDiagnosticsTest::testTidy_ClangAnalyzer() void ReadExportedDiagnosticsTest::testTidy_ClangAnalyzer()
{ {
const FilePath sourceFile = filePath("clang-analyzer.dividezero.cpp"); const FilePath sourceFile = filePath("clang-analyzer.dividezero.cpp");
const QString exportedFile = createFile( const FilePath exportedFile
filePath(appendYamlSuffix("clang-analyzer.dividezero")).toString(), = createFile(filePath(appendYamlSuffix("clang-analyzer.dividezero")).toString(),
sourceFile.toString()); sourceFile.toString());
Diagnostic expectedDiag; Diagnostic expectedDiag;
expectedDiag.name = "clang-analyzer-core.DivideZero"; expectedDiag.name = "clang-analyzer-core.DivideZero";
expectedDiag.location = {sourceFile, 4, 15}; expectedDiag.location = {sourceFile, 4, 15};
@@ -128,16 +126,15 @@ void ReadExportedDiagnosticsTest::testTidy_ClangAnalyzer()
false, false,
}, },
}; };
const Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(exportedFile), const expected_str<Diagnostics> diags = readExportedDiagnostics(exportedFile);
{}, &m_message); QVERIFY(diags.has_value());
QVERIFY2(m_message.isEmpty(), qPrintable(m_message)); QCOMPARE(*diags, {expectedDiag});
QCOMPARE(diags, {expectedDiag});
} }
void ReadExportedDiagnosticsTest::testClazy() void ReadExportedDiagnosticsTest::testClazy()
{ {
const FilePath sourceFile = filePath("clazy.qgetenv.cpp"); const FilePath sourceFile = filePath("clazy.qgetenv.cpp");
const QString exportedFile = createFile(filePath(appendYamlSuffix("clazy.qgetenv")).toString(), const FilePath exportedFile = createFile(filePath(appendYamlSuffix("clazy.qgetenv")).toString(),
sourceFile.toString()); sourceFile.toString());
Diagnostic expectedDiag; Diagnostic expectedDiag;
expectedDiag.name = "clazy-qgetenv"; expectedDiag.name = "clazy-qgetenv";
@@ -156,10 +153,9 @@ void ReadExportedDiagnosticsTest::testClazy()
{{sourceFile, 7, 18}, {sourceFile, 7, 29}}, {{sourceFile, 7, 18}, {sourceFile, 7, 29}},
true}, true},
}; };
const Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(exportedFile), const expected_str<Diagnostics> diags = readExportedDiagnostics(exportedFile);
{}, &m_message); QVERIFY(diags.has_value());
QVERIFY2(m_message.isEmpty(), qPrintable(m_message)); QCOMPARE(*diags, {expectedDiag});
QCOMPARE(diags, {expectedDiag});
} }
void ReadExportedDiagnosticsTest::testOffsetInvalidText() void ReadExportedDiagnosticsTest::testOffsetInvalidText()
@@ -263,25 +259,24 @@ void ReadExportedDiagnosticsTest::testOffsetMultiByteCodePoint2()
} }
// Replace FILE_PATH with a real absolute file path in the *.yaml files. // Replace FILE_PATH with a real absolute file path in the *.yaml files.
QString ReadExportedDiagnosticsTest::createFile(const QString &yamlFilePath, FilePath ReadExportedDiagnosticsTest::createFile(const QString &yamlFilePath,
const QString &filePathToInject) const const QString &filePathToInject) const
{ {
QTC_ASSERT(QDir::isAbsolutePath(filePathToInject), return QString()); QTC_ASSERT(QDir::isAbsolutePath(filePathToInject), return {});
const Utils::FilePath newFileName = m_baseDir->absolutePath( const FilePath newFileName = m_baseDir->absolutePath(QFileInfo(yamlFilePath).fileName());
QFileInfo(yamlFilePath).fileName());
Utils::FileReader reader; FileReader reader;
if (QTC_GUARD(reader.fetch(Utils::FilePath::fromString(yamlFilePath), if (QTC_GUARD(reader.fetch(FilePath::fromString(yamlFilePath),
QIODevice::ReadOnly | QIODevice::Text))) { QIODevice::ReadOnly | QIODevice::Text))) {
QByteArray contents = reader.data(); QByteArray contents = reader.data();
contents.replace("FILE_PATH", filePathToInject.toLocal8Bit()); contents.replace("FILE_PATH", filePathToInject.toLocal8Bit());
Utils::FileSaver fileSaver(newFileName, QIODevice::WriteOnly | QIODevice::Text); FileSaver fileSaver(newFileName, QIODevice::WriteOnly | QIODevice::Text);
QTC_CHECK(fileSaver.write(contents)); QTC_CHECK(fileSaver.write(contents));
QTC_CHECK(fileSaver.finalize()); QTC_CHECK(fileSaver.finalize());
} }
return newFileName.toString(); return newFileName;
} }
FilePath ReadExportedDiagnosticsTest::filePath(const QString &fileName) const FilePath ReadExportedDiagnosticsTest::filePath(const QString &fileName) const

3
src/plugins/clangtools/readexporteddiagnosticstest.h
View File

@@ -44,11 +44,10 @@ private slots:
void testOffsetMultiByteCodePoint2(); void testOffsetMultiByteCodePoint2();
private: private:
QString createFile(const QString &yamlFilePath, const QString &filePathToInject) const; Utils::FilePath createFile(const QString &yamlFilePath, const QString &filePathToInject) const;
Utils::FilePath filePath(const QString &fileName) const; Utils::FilePath filePath(const QString &fileName) const;
CppEditor::Tests::TemporaryCopiedDir * const m_baseDir; CppEditor::Tests::TemporaryCopiedDir * const m_baseDir;
QString m_message;
}; };
} // namespace ClangTools::Internal } // namespace ClangTools::Internal