From e6d16b67471d6d044c485847882ce4ac7dea365f Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Tue, 23 Jul 2019 08:28:13 +0200 Subject: [PATCH] ClangTools: Support loading exported diagnostics Add a new toolbar button to load diagnostics exported with $ clang-tidy -export-fixes=/path/to/file $ clazy-standalone -export-fixes=/path/to/file (master version) Change-Id: I8316fe0706a18222e68220ef4fbfdc7ae8d09804 Reviewed-by: David Schulz --- cmake/Findyaml-cpp.cmake | 1 + src/libs/3rdparty/yaml-cpp/yaml-cpp.qbs | 1 + src/plugins/clangtools/clangtidyclazytool.cpp | 52 +++ src/plugins/clangtools/clangtidyclazytool.h | 2 + src/plugins/clangtools/clangtools.pro | 4 +- src/plugins/clangtools/clangtools_global.h | 2 + .../clangtools/clangtoolsdiagnostic.cpp | 20 + src/plugins/clangtools/clangtoolsdiagnostic.h | 2 + .../clangtools/clangtoolslogfilereader.cpp | 240 +++++++++++- .../clangtools/clangtoolslogfilereader.h | 19 + src/plugins/debugger/debugger_global.h | 2 + .../debugger/debuggerunittestfiles.pri | 11 + tests/unit/unittest/creator_dependency.pri | 2 + .../unittest/data/clangtools/CMakeLists.txt | 26 ++ .../clangtools/clang-analyzer.dividezero.cpp | 5 + .../clangtools/clang-analyzer.dividezero.yaml | 23 ++ .../clangtools/clang.unused-parameter.cpp | 5 + .../data/clangtools/clang.unused-parameter.h | 1 + .../clangtools/clang.unused-parameter.yaml | 10 + .../data/clangtools/clazy.qgetenv.cpp | 8 + .../data/clangtools/clazy.qgetenv.yaml | 17 + .../unit/unittest/data/clangtools/empty.yaml | 0 tests/unit/unittest/data/clangtools/main.cpp | 4 + .../clangtools/tidy.modernize-use-nullptr.cpp | 2 + .../tidy.modernize-use-nullptr.yaml | 14 + .../unit/unittest/gtest-creator-printing.cpp | 38 ++ tests/unit/unittest/gtest-creator-printing.h | 14 + .../unittest/readexporteddiagnostics-test.cpp | 352 ++++++++++++++++++ tests/unit/unittest/unittest.pro | 3 +- 29 files changed, 877 insertions(+), 3 deletions(-) create mode 100644 src/plugins/debugger/debuggerunittestfiles.pri create mode 100644 tests/unit/unittest/data/clangtools/CMakeLists.txt create mode 100644 tests/unit/unittest/data/clangtools/clang-analyzer.dividezero.cpp create mode 100644 tests/unit/unittest/data/clangtools/clang-analyzer.dividezero.yaml create mode 100644 tests/unit/unittest/data/clangtools/clang.unused-parameter.cpp create mode 100644 tests/unit/unittest/data/clangtools/clang.unused-parameter.h create mode 100644 tests/unit/unittest/data/clangtools/clang.unused-parameter.yaml create mode 100644 tests/unit/unittest/data/clangtools/clazy.qgetenv.cpp create mode 100644 tests/unit/unittest/data/clangtools/clazy.qgetenv.yaml create mode 100644 tests/unit/unittest/data/clangtools/empty.yaml create mode 100644 tests/unit/unittest/data/clangtools/main.cpp create mode 100644 tests/unit/unittest/data/clangtools/tidy.modernize-use-nullptr.cpp create mode 100644 tests/unit/unittest/data/clangtools/tidy.modernize-use-nullptr.yaml create mode 100644 tests/unit/unittest/readexporteddiagnostics-test.cpp diff --git a/cmake/Findyaml-cpp.cmake b/cmake/Findyaml-cpp.cmake index 4a5fdbb649c..253cd2103d9 100644 --- a/cmake/Findyaml-cpp.cmake +++ b/cmake/Findyaml-cpp.cmake @@ -15,6 +15,7 @@ if (NOT yaml-cpp_FOUND) add_qtc_library(yaml-cpp DEFINES YAML_CPP_DLL yaml_cpp_EXPORTS INCLUDES ${YAML_SOURCE_DIR}/include + PUBLIC_DEFINES YAML_CPP_DLL PUBLIC_INCLUDES ${YAML_SOURCE_DIR}/include SOURCES ${YAML_SOURCE_DIR}/include/yaml-cpp diff --git a/src/libs/3rdparty/yaml-cpp/yaml-cpp.qbs b/src/libs/3rdparty/yaml-cpp/yaml-cpp.qbs index 7bb172afce8..9a2d3f96a00 100644 --- a/src/libs/3rdparty/yaml-cpp/yaml-cpp.qbs +++ b/src/libs/3rdparty/yaml-cpp/yaml-cpp.qbs @@ -99,6 +99,7 @@ Project { Export { Depends { name: "cpp" } cpp.includePaths: [product.sourceDirectory + "/include/"] + cpp.defines: base.concat(["YAML_CPP_DLL"]) } } } diff --git a/src/plugins/clangtools/clangtidyclazytool.cpp b/src/plugins/clangtools/clangtidyclazytool.cpp index a76f7e728dd..804da43769f 100644 --- a/src/plugins/clangtools/clangtidyclazytool.cpp +++ b/src/plugins/clangtools/clangtidyclazytool.cpp @@ -37,6 +37,8 @@ #include #include +#include +#include #include #include @@ -55,6 +57,7 @@ #include #include +#include #include using namespace Core; @@ -249,6 +252,13 @@ ClangTidyClazyTool::ClangTidyClazyTool() connect(action, &QAction::triggered, m_diagnosticView, &DetailedErrorView::goNext); m_goNext = action; + // Load diagnostics from file + action = new QAction(this); + action->setIcon(Utils::Icons::OPENFILE_TOOLBAR.icon()); + action->setToolTip(tr("Load Diagnostics from YAML Files exported with \"-export-fixes\".")); + connect(action, &QAction::triggered, this, &ClangTidyClazyTool::loadDiagnosticsFromFiles); + m_loadExported = action; + // Clear data action = new QAction(this); action->setDisabled(true); @@ -325,6 +335,7 @@ ClangTidyClazyTool::ClangTidyClazyTool() m_perspective.addToolBarAction(m_startAction); m_perspective.addToolBarAction(m_stopAction); + m_perspective.addToolBarAction(m_loadExported); m_perspective.addToolBarAction(m_clear); m_perspective.addToolBarAction(m_goBack); m_perspective.addToolBarAction(m_goNext); @@ -416,6 +427,7 @@ void ClangTidyClazyTool::updateRunActions() QString tooltipText = tr("Clang-Tidy and Clazy are still running."); m_startAction->setToolTip(tooltipText); m_stopAction->setEnabled(true); + m_loadExported->setEnabled(false); m_clear->setEnabled(false); } else { QString toolTip = tr("Start Clang-Tidy and Clazy."); @@ -430,10 +442,47 @@ void ClangTidyClazyTool::updateRunActions() m_startAction->setToolTip(toolTip); m_startAction->setEnabled(canRun); m_stopAction->setEnabled(false); + m_loadExported->setEnabled(true); m_clear->setEnabled(m_diagnosticModel->diagnostics().count()); } } +void ClangTidyClazyTool::loadDiagnosticsFromFiles() +{ + // Ask user for files + const QStringList filePaths + = QFileDialog::getOpenFileNames(Core::ICore::mainWindow(), + tr("Select YAML Files with Diagnostics"), + QDir::homePath(), + tr("YAML Files (*.yml *.yaml);;All Files (*)")); + if (filePaths.isEmpty()) + return; + + // Load files + Diagnostics diagnostics; + QString errors; + for (const QString &filePath : filePaths) { + QString currentError; + diagnostics << readExportedDiagnostics(Utils::FilePath::fromString(filePath), + {}, + ¤tError); + + if (!currentError.isEmpty()) { + if (!errors.isEmpty()) + errors.append("\n"); + errors.append(currentError); + } + } + + // Show errors + if (!errors.isEmpty()) + AsynchronousMessageBox::critical(tr("Error Loading Diagnostics"), errors); + + // Show imported + m_diagnosticModel->clear(); + onNewDiagnosticsAvailable(diagnostics); +} + void ClangTidyClazyTool::handleStateUpdate() { QTC_ASSERT(m_goBack, return); @@ -445,8 +494,11 @@ void ClangTidyClazyTool::handleStateUpdate() const int issuesVisible = m_diagnosticFilterModel->rowCount(); m_goBack->setEnabled(issuesVisible > 1); m_goNext->setEnabled(issuesVisible > 1); + m_clear->setEnabled(issuesFound > 0); m_expandCollapse->setEnabled(issuesVisible); + m_loadExported->setEnabled(!m_running); + QString message; if (m_running) { if (issuesFound) diff --git a/src/plugins/clangtools/clangtidyclazytool.h b/src/plugins/clangtools/clangtidyclazytool.h index f77fe8820a9..8834c733f2a 100644 --- a/src/plugins/clangtools/clangtidyclazytool.h +++ b/src/plugins/clangtools/clangtidyclazytool.h @@ -64,6 +64,7 @@ private: void handleStateUpdate() final; void updateRunActions(); + void loadDiagnosticsFromFiles(); DiagnosticFilterModel *m_diagnosticFilterModel = nullptr; @@ -72,6 +73,7 @@ private: QAction *m_goBack = nullptr; QAction *m_goNext = nullptr; + QAction *m_loadExported = nullptr; QAction *m_clear = nullptr; QAction *m_expandCollapse = nullptr; diff --git a/src/plugins/clangtools/clangtools.pro b/src/plugins/clangtools/clangtools.pro index 476cc90826e..cf6e62c1035 100644 --- a/src/plugins/clangtools/clangtools.pro +++ b/src/plugins/clangtools/clangtools.pro @@ -8,7 +8,9 @@ LIBS += $$LIBCLANG_LIBS INCLUDEPATH += $$LLVM_INCLUDEPATH include(../../shared/yaml-cpp/yaml-cpp_installation.pri) -!isEmpty(EXTERNAL_YAML_CPP_FOUND) { +isEmpty(EXTERNAL_YAML_CPP_FOUND) { + DEFINES += YAML_CPP_DLL +} else { LIBS += $$EXTERNAL_YAML_CPP_LIBS QMAKE_CXXFLAGS += $$EXTERNAL_YAML_CPP_CXXFLAGS } diff --git a/src/plugins/clangtools/clangtools_global.h b/src/plugins/clangtools/clangtools_global.h index 159526ec64b..0b47c30a25f 100644 --- a/src/plugins/clangtools/clangtools_global.h +++ b/src/plugins/clangtools/clangtools_global.h @@ -29,6 +29,8 @@ #if defined(CLANGTOOLS_LIBRARY) # define CLANGTOOLS_EXPORT Q_DECL_EXPORT +#elif defined(CLANGTOOLS_STATIC_LIBRARY) +# define CLANGTOOLS_EXPORT #else # define CLANGTOOLS_EXPORT Q_DECL_IMPORT #endif diff --git a/src/plugins/clangtools/clangtoolsdiagnostic.cpp b/src/plugins/clangtools/clangtoolsdiagnostic.cpp index cb1f4ead62c..5e850659bf5 100644 --- a/src/plugins/clangtools/clangtoolsdiagnostic.cpp +++ b/src/plugins/clangtools/clangtoolsdiagnostic.cpp @@ -33,6 +33,15 @@ bool ExplainingStep::isValid() const return location.isValid() && !ranges.isEmpty() && !message.isEmpty(); } +bool operator==(const ExplainingStep &lhs, const ExplainingStep &rhs) +{ + return lhs.message == rhs.message + && lhs.location == rhs.location + && lhs.ranges == rhs.ranges + && lhs.isFixIt == rhs.isFixIt + ; +} + bool Diagnostic::isValid() const { return !description.isEmpty(); @@ -46,5 +55,16 @@ quint32 qHash(const Diagnostic &diagnostic) ^ diagnostic.location.column; } +bool operator==(const Diagnostic &lhs, const Diagnostic &rhs) +{ + return lhs.description == rhs.description + && lhs.category == rhs.category + && lhs.type == rhs.type + && lhs.location == rhs.location + && lhs.explainingSteps == rhs.explainingSteps + && lhs.hasFixits == rhs.hasFixits + ; +} + } // namespace Internal } // namespace ClangTools diff --git a/src/plugins/clangtools/clangtoolsdiagnostic.h b/src/plugins/clangtools/clangtoolsdiagnostic.h index 1d2db2ba0d7..99df078fdc5 100644 --- a/src/plugins/clangtools/clangtoolsdiagnostic.h +++ b/src/plugins/clangtools/clangtoolsdiagnostic.h @@ -62,6 +62,8 @@ public: bool hasFixits = false; }; +bool operator==(const Diagnostic &lhs, const Diagnostic &rhs); + using Diagnostics = QList; quint32 qHash(const Diagnostic &diagnostic); diff --git a/src/plugins/clangtools/clangtoolslogfilereader.cpp b/src/plugins/clangtools/clangtoolslogfilereader.cpp index afca7d96179..59ea741c253 100644 --- a/src/plugins/clangtools/clangtoolslogfilereader.cpp +++ b/src/plugins/clangtools/clangtoolslogfilereader.cpp @@ -29,9 +29,9 @@ #include #include -#include #include #include +#include #include #include @@ -39,9 +39,12 @@ #include #include #include +#include #include +#include + namespace ClangTools { namespace Internal { @@ -242,5 +245,240 @@ Diagnostics readSerializedDiagnostics(const Utils::FilePath &logFilePath, return readSerializedDiagnostics_helper(logFilePath, mainFilePath, acceptFromFilePath); } +Utils::optional byteOffsetInUtf8TextToLineColumn(const char *text, + int offset, + int startLine) +{ + if (text == nullptr || offset < 0) + return {}; + + int lineCounter = startLine; + const char *lineStart = text; + + for (const char *c = text; *c != '\0'; ++c) { + // Advance to line + if (c > text && *(c - 1) == '\n') { + ++lineCounter; + lineStart = c; + } + + // Advance to column + if (c - text == offset) { + int columnCounter = 1; + c = lineStart; + while (c < text + offset && Utils::Text::utf8AdvanceCodePoint(c)) + ++columnCounter; + if (c == text + offset) + return LineColumnInfo{lineCounter, columnCounter, static_cast(lineStart - text)}; + return {}; // Ops, offset was not pointing to start of multi byte code point. + } + } + + return {}; +} + +static QString asString(const YAML::Node &node) +{ + return QString::fromStdString(node.as()); +} + +namespace { +class FileCache +{ +public: + class LineInfo { + public: + bool isValid() { return line != 0; } + int line = 0; // 1-based + int lineStartOffset = 0; + }; + + class Item { + public: + friend class FileCache; + + QByteArray fileContents() + { + if (data.isNull()) + data = readFile(filePath); + return data; + } + + LineInfo &lineInfo() { return lastLookup; } + + private: + QString filePath; + LineInfo lastLookup; + QByteArray data; + }; + + Item &item(const QString &filePath) + { + Item &i = m_cache[filePath]; + if (i.filePath.isEmpty()) + i.filePath = filePath; + return i; + } + +private: + static QByteArray readFile(const QString &filePath) + { + if (filePath.isEmpty()) + return {}; + + Utils::FileReader reader; + // Do not use QIODevice::Text as we have to deal with byte offsets. + if (reader.fetch(filePath, QIODevice::ReadOnly)) + return reader.data(); + + return {}; + } + +private: + QHash m_cache; +}; + +class Location +{ +public: + Location(const YAML::Node &node, + FileCache &fileCache, + const char *fileOffsetKey = "FileOffset", + int extraOffset = 0) + : m_node(node) + , m_fileCache(fileCache) + , m_filePath(asString(node["FilePath"])) + , m_fileOffsetKey(fileOffsetKey) + , m_extraOffset(extraOffset) + {} + + QString filePath() const { return m_filePath; } + + Debugger::DiagnosticLocation toDiagnosticLocation() const + { + FileCache::Item &cacheItem = m_fileCache.item(m_filePath); + const QByteArray fileContents = cacheItem.fileContents(); + + const char *data = fileContents.data(); + int fileOffset = m_node[m_fileOffsetKey].as() + m_extraOffset; + int startLine = 1; + + // Check cache for last lookup + FileCache::LineInfo &cachedLineInfo = cacheItem.lineInfo(); + if (cachedLineInfo.isValid() && fileOffset >= cachedLineInfo.lineStartOffset) { + // Cache hit, adjust inputs in order not to start from the beginning of the file again. + data = data + cachedLineInfo.lineStartOffset; + fileOffset = fileOffset - cachedLineInfo.lineStartOffset; + startLine = cachedLineInfo.line; + } + + // Convert + OptionalLineColumnInfo info = byteOffsetInUtf8TextToLineColumn(data, fileOffset, startLine); + if (!info) + return {m_filePath, 1, 1}; + + // Save/update lookup + int lineStartOffset = info->lineStartOffset; + if (data != fileContents.data()) + lineStartOffset += cachedLineInfo.lineStartOffset; + cachedLineInfo = FileCache::LineInfo{info->line, lineStartOffset}; + return Debugger::DiagnosticLocation{m_filePath, info->line, info->column}; + } + + static QVector toRange(const YAML::Node &node, + FileCache &fileCache) + { + // The Replacements nodes use "Offset" instead of "FileOffset" as the key name. + auto startLoc = Location(node, fileCache, "Offset"); + auto endLoc = Location(node, fileCache, "Offset", node["Length"].as()); + return {startLoc.toDiagnosticLocation(), endLoc.toDiagnosticLocation()}; + } + +private: + const YAML::Node &m_node; + FileCache &m_fileCache; + QString m_filePath; + const char *m_fileOffsetKey = nullptr; + int m_extraOffset = 0; +}; + +} // namespace + +Diagnostics readExportedDiagnostics(const Utils::FilePath &logFilePath, + const AcceptDiagsFromFilePath &acceptFromFilePath, + QString *errorMessage) +{ + if (!checkFilePath(logFilePath, errorMessage)) + return {}; + + FileCache fileCache; + Diagnostics diagnostics; + + try { + YAML::Node document = YAML::LoadFile(logFilePath.toString().toStdString()); + for (const auto &diagNode : document["Diagnostics"]) { + // clazy omits the "DiagnosticMessage" node. + const auto msgNode = diagNode["DiagnosticMessage"]; + const YAML::Node &node = msgNode ? msgNode : diagNode; + + Location loc(node, fileCache); + if (loc.filePath().isEmpty()) + continue; + if (acceptFromFilePath + && !acceptFromFilePath(Utils::FilePath::fromString(loc.filePath()))) { + continue; + } + + Diagnostic diag; + diag.location = loc.toDiagnosticLocation(); + diag.type = "warning"; + diag.description = asString(node["Message"]) + " [" + + (asString(diagNode["DiagnosticName"])) + "]"; + + // Process fixits/replacements + const YAML::Node &replacementsNode = node["Replacements"]; + for (const YAML::Node &replacementNode : replacementsNode) { + ExplainingStep step; + step.isFixIt = true; + step.message = asString(replacementNode["ReplacementText"]); + step.ranges = Location::toRange(replacementNode, fileCache); + step.location = step.ranges[0]; + + if (step.location.isValid()) + diag.explainingSteps.append(step); + } + diag.hasFixits = !diag.explainingSteps.isEmpty(); + + // Process notes + const auto notesNode = diagNode["Notes"]; + for (const YAML::Node ¬eNode : notesNode) { + Location loc(noteNode, fileCache); + // Ignore a note like + // - FileOffset: 0 + // FilePath: '' + // Message: this fix will not be applied because it overlaps with another fix + if (loc.filePath().isEmpty()) + continue; + + ExplainingStep step; + step.message = asString(noteNode["Message"]); + step.location = loc.toDiagnosticLocation(); + diag.explainingSteps.append(step); + } + + diagnostics.append(diag); + } + } catch (std::exception &e) { + if (errorMessage) { + *errorMessage = QString( + QT_TRANSLATE_NOOP("LogFileReader", + "Error: Failed to parse YAML file \"%1\": %2.")) + .arg(logFilePath.toUserOutput(), QString::fromUtf8(e.what())); + } + } + + return diagnostics; +} + } // namespace Internal } // namespace ClangTools diff --git a/src/plugins/clangtools/clangtoolslogfilereader.h b/src/plugins/clangtools/clangtoolslogfilereader.h index 6bfa2f723cf..6d53fa67c2b 100644 --- a/src/plugins/clangtools/clangtoolslogfilereader.h +++ b/src/plugins/clangtools/clangtoolslogfilereader.h @@ -25,6 +25,8 @@ #pragma once +#include + #include "clangtoolsdiagnostic.h" namespace Utils { class FilePath; } @@ -34,10 +36,27 @@ namespace Internal { using AcceptDiagsFromFilePath = std::function; +// Reads diagnostics generated by "clang -serialize-diagnostics path/to/file" Diagnostics readSerializedDiagnostics(const Utils::FilePath &logFilePath, const Utils::FilePath &mainFilePath, const AcceptDiagsFromFilePath &acceptFromFilePath, QString *errorMessage); +// Reads diagnostics generated by "clang-tidy/clazy-standalone -export-fixes=path/to/file" +Diagnostics readExportedDiagnostics(const Utils::FilePath &logFilePath, + const AcceptDiagsFromFilePath &acceptFromFilePath, + QString *errorMessage); + +// Exposed for tests +struct LineColumnInfo { + int line = 1; // 1-based + int column = 1; // 1-based + int lineStartOffset = 0; // for optimiation/caching purposes +}; +using OptionalLineColumnInfo = Utils::optional; +OptionalLineColumnInfo byteOffsetInUtf8TextToLineColumn(const char *text, + int offset, + int startLine = 1); + } // namespace Internal } // namespace ClangTools diff --git a/src/plugins/debugger/debugger_global.h b/src/plugins/debugger/debugger_global.h index fc1597cc82a..7ec10b5508c 100644 --- a/src/plugins/debugger/debugger_global.h +++ b/src/plugins/debugger/debugger_global.h @@ -29,6 +29,8 @@ #if defined(DEBUGGER_LIBRARY) # define DEBUGGER_EXPORT Q_DECL_EXPORT +#elif defined(DEBUGGER_STATIC_LIBRARY) // Abuse single files for tests +# define DEBUGGER_EXPORT #else # define DEBUGGER_EXPORT Q_DECL_IMPORT #endif diff --git a/src/plugins/debugger/debuggerunittestfiles.pri b/src/plugins/debugger/debuggerunittestfiles.pri new file mode 100644 index 00000000000..1168b5af817 --- /dev/null +++ b/src/plugins/debugger/debuggerunittestfiles.pri @@ -0,0 +1,11 @@ +shared { + DEFINES += DEBUGGER_LIBRARY +} else { + DEFINES += DEBUGGER_STATIC_LIBRARY +} + +HEADERS += \ + $$PWD/analyzer/diagnosticlocation.h \ + +SOURCES += \ + $$PWD/analyzer/diagnosticlocation.cpp \ diff --git a/tests/unit/unittest/creator_dependency.pri b/tests/unit/unittest/creator_dependency.pri index 989ffbeda7f..2e84e1cdcab 100644 --- a/tests/unit/unittest/creator_dependency.pri +++ b/tests/unit/unittest/creator_dependency.pri @@ -14,6 +14,8 @@ include($$PWD/../../../src/tools/clangpchmanagerbackend/source/clangpchmanagerba include($$PWD/../../../src/plugins/clangrefactoring/clangrefactoring-source.pri) include($$PWD/../../../src/plugins/clangpchmanager/clangpchmanager-source.pri) include($$PWD/../../../src/plugins/cpptools/cpptoolsunittestfiles.pri) +include($$PWD/../../../src/plugins/clangtools/clangtoolsunittestfiles.pri) +include($$PWD/../../../src/plugins/debugger/debuggerunittestfiles.pri) include($$PWD/../../../src/plugins/compilationdatabaseprojectmanager/compilationdatabaseunittestfiles.pri) include(cplusplus.pri) !isEmpty(LLVM_VERSION) { diff --git a/tests/unit/unittest/data/clangtools/CMakeLists.txt b/tests/unit/unittest/data/clangtools/CMakeLists.txt new file mode 100644 index 00000000000..b445f81b7dd --- /dev/null +++ b/tests/unit/unittest/data/clangtools/CMakeLists.txt @@ -0,0 +1,26 @@ +cmake_minimum_required(VERSION 3.5) + +project(clangtools LANGUAGES CXX) + +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +set(CMAKE_INCLUDE_CURRENT_DIR ON) + +set(CMAKE_AUTOUIC ON) +set(CMAKE_AUTOMOC ON) +set(CMAKE_AUTORCC ON) + +set(CMAKE_CXX_STANDARD 11) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + +find_package(Qt5 COMPONENTS Widgets REQUIRED) + +add_executable(clangtools + main.cpp + clang-analyzer.dividezero.cpp + clang.unused-parameter.cpp + clang.unused-parameter.h + clazy.qgetenv.cpp + tidy.modernize-use-nullptr.cpp +) + +target_link_libraries(clangtools PRIVATE Qt5::Widgets) diff --git a/tests/unit/unittest/data/clangtools/clang-analyzer.dividezero.cpp b/tests/unit/unittest/data/clangtools/clang-analyzer.dividezero.cpp new file mode 100644 index 00000000000..5f623dc114b --- /dev/null +++ b/tests/unit/unittest/data/clangtools/clang-analyzer.dividezero.cpp @@ -0,0 +1,5 @@ +// clang-tidy -export-fixes=clang-analyzer.dividezero.yaml "-checks=-*,clang-analyzer-core.DivideZero" clang-analyzer.dividezero.cpp +void test(int z) { + if (z == 0) + int x = 1 / z; +} diff --git a/tests/unit/unittest/data/clangtools/clang-analyzer.dividezero.yaml b/tests/unit/unittest/data/clangtools/clang-analyzer.dividezero.yaml new file mode 100644 index 00000000000..d19c5887a98 --- /dev/null +++ b/tests/unit/unittest/data/clangtools/clang-analyzer.dividezero.yaml @@ -0,0 +1,23 @@ +--- +MainSourceFile: 'FILE_PATH' +Diagnostics: + - DiagnosticName: clang-analyzer-core.DivideZero + DiagnosticMessage: + Message: Division by zero + FilePath: 'FILE_PATH' + FileOffset: 180 + Replacements: [] + Notes: + - Message: 'Assuming ''z'' is equal to 0' + FilePath: 'FILE_PATH' + FileOffset: 158 + Replacements: [] + - Message: Taking true branch + FilePath: 'FILE_PATH' + FileOffset: 154 + Replacements: [] + - Message: Division by zero + FilePath: 'FILE_PATH' + FileOffset: 180 + Replacements: [] +... diff --git a/tests/unit/unittest/data/clangtools/clang.unused-parameter.cpp b/tests/unit/unittest/data/clangtools/clang.unused-parameter.cpp new file mode 100644 index 00000000000..80d9047c52a --- /dev/null +++ b/tests/unit/unittest/data/clangtools/clang.unused-parameter.cpp @@ -0,0 +1,5 @@ +// clang-tidy -export-fixes=clang.unused-parameter.yaml clang.unused-parameter.cpp -- -Wunused-parameter +#include "clang.unused-parameter.h" + +void g(int g) {} + diff --git a/tests/unit/unittest/data/clangtools/clang.unused-parameter.h b/tests/unit/unittest/data/clangtools/clang.unused-parameter.h new file mode 100644 index 00000000000..39d55b8fcec --- /dev/null +++ b/tests/unit/unittest/data/clangtools/clang.unused-parameter.h @@ -0,0 +1 @@ +void f(int i) {} diff --git a/tests/unit/unittest/data/clangtools/clang.unused-parameter.yaml b/tests/unit/unittest/data/clangtools/clang.unused-parameter.yaml new file mode 100644 index 00000000000..6622953a2f9 --- /dev/null +++ b/tests/unit/unittest/data/clangtools/clang.unused-parameter.yaml @@ -0,0 +1,10 @@ +--- +MainSourceFile: 'FILE_PATH' +Diagnostics: + - DiagnosticName: clang-diagnostic-unused-parameter + DiagnosticMessage: + Message: 'unused parameter ''g''' + FilePath: 'FILE_PATH' + FileOffset: 153 + Replacements: [] +... diff --git a/tests/unit/unittest/data/clangtools/clazy.qgetenv.cpp b/tests/unit/unittest/data/clangtools/clazy.qgetenv.cpp new file mode 100644 index 00000000000..e04518821cb --- /dev/null +++ b/tests/unit/unittest/data/clangtools/clazy.qgetenv.cpp @@ -0,0 +1,8 @@ +// clazy-standalone -export-fixes=/tmp/clazy.qgetenv.yaml -checks=qgetenv clazy.qgetenv.cpp +#include +#include + +void g() +{ + qgetenv("Foo").isEmpty(); +} diff --git a/tests/unit/unittest/data/clangtools/clazy.qgetenv.yaml b/tests/unit/unittest/data/clangtools/clazy.qgetenv.yaml new file mode 100644 index 00000000000..a465a1ca638 --- /dev/null +++ b/tests/unit/unittest/data/clangtools/clazy.qgetenv.yaml @@ -0,0 +1,17 @@ +--- +MainSourceFile: 'FILE_PATH' +Diagnostics: + - DiagnosticName: clazy-qgetenv + Message: 'qgetenv().isEmpty() allocates. Use qEnvironmentVariableIsEmpty() instead' + FileOffset: 150 + FilePath: 'FILE_PATH' + Replacements: + - FilePath: 'FILE_PATH' + Offset: 150 + Length: 7 + ReplacementText: qEnvironmentVariableIsEmpty + - FilePath: 'FILE_PATH' + Offset: 163 + Length: 11 + ReplacementText: ')' +... diff --git a/tests/unit/unittest/data/clangtools/empty.yaml b/tests/unit/unittest/data/clangtools/empty.yaml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/unit/unittest/data/clangtools/main.cpp b/tests/unit/unittest/data/clangtools/main.cpp new file mode 100644 index 00000000000..df38a776481 --- /dev/null +++ b/tests/unit/unittest/data/clangtools/main.cpp @@ -0,0 +1,4 @@ +int main(int, char *[]) +{ + return 0; +} diff --git a/tests/unit/unittest/data/clangtools/tidy.modernize-use-nullptr.cpp b/tests/unit/unittest/data/clangtools/tidy.modernize-use-nullptr.cpp new file mode 100644 index 00000000000..fedae863306 --- /dev/null +++ b/tests/unit/unittest/data/clangtools/tidy.modernize-use-nullptr.cpp @@ -0,0 +1,2 @@ +// clang-tidy -export-fixes=tidy.modernize-use-nullptr.yaml "-checks=-*,modernize-use-nullptr" tidy.modernize-use-nullptr.cpp +int *ret_ptr() { return 0; } diff --git a/tests/unit/unittest/data/clangtools/tidy.modernize-use-nullptr.yaml b/tests/unit/unittest/data/clangtools/tidy.modernize-use-nullptr.yaml new file mode 100644 index 00000000000..7724d3bdfcb --- /dev/null +++ b/tests/unit/unittest/data/clangtools/tidy.modernize-use-nullptr.yaml @@ -0,0 +1,14 @@ +--- +MainSourceFile: 'FILE_PATH' +Diagnostics: + - DiagnosticName: modernize-use-nullptr + DiagnosticMessage: + Message: use nullptr + FilePath: 'FILE_PATH' + FileOffset: 150 + Replacements: + - FilePath: 'FILE_PATH' + Offset: 150 + Length: 1 + ReplacementText: nullptr +... diff --git a/tests/unit/unittest/gtest-creator-printing.cpp b/tests/unit/unittest/gtest-creator-printing.cpp index 385e7e8ab9a..2bbcdbaa45e 100644 --- a/tests/unit/unittest/gtest-creator-printing.cpp +++ b/tests/unit/unittest/gtest-creator-printing.cpp @@ -64,6 +64,12 @@ #include #include #include +#include +#include +#include +#include +#include +#include namespace { ClangBackEnd::FilePathCaching *filePathCache = nullptr; @@ -1303,6 +1309,38 @@ std::ostream &operator<<(std::ostream &out, const Usage &usage) } } // namespace CppTools +namespace Debugger { +std::ostream &operator<<(std::ostream &out, const DiagnosticLocation &loc) +{ + return out << "(" << loc.filePath << ", " << loc.line << ", " << loc.column << ")"; +} +} // namespace Debugger + +namespace ClangTools { +namespace Internal { +std::ostream &operator<<(std::ostream &out, const ExplainingStep &step) +{ + return out << "(" + << step.message << ", " + << step.location << ", " + << step.ranges << ", " + << step.isFixIt + << ")"; +} + +std::ostream &operator<<(std::ostream &out, const Diagnostic &diag) { + return out << "(" + << diag.description << ", " + << diag.category << ", " + << diag.type << ", " + << diag.location << ", " + << diag.explainingSteps << ", " + << diag.hasFixits + << ")"; +} +} // namespace Internal +} // namespace ClangTools + void setFilePathCache(ClangBackEnd::FilePathCaching *cache) { filePathCache = cache; diff --git a/tests/unit/unittest/gtest-creator-printing.h b/tests/unit/unittest/gtest-creator-printing.h index 948b2e911c7..2a43f7d1a13 100644 --- a/tests/unit/unittest/gtest-creator-printing.h +++ b/tests/unit/unittest/gtest-creator-printing.h @@ -328,4 +328,18 @@ class Usage; std::ostream &operator<<(std::ostream &out, const Usage &usage); } // namespace CppTools +namespace Debugger { +class DiagnosticLocation; +std::ostream &operator<<(std::ostream &out, const DiagnosticLocation &loc); +} // namespace Debugger + +namespace ClangTools { +namespace Internal { +class ExplainingStep; +class Diagnostic; +std::ostream &operator<<(std::ostream &out, const ExplainingStep &step); +std::ostream &operator<<(std::ostream &out, const Diagnostic &diag); +} // namespace Internal +} // namespace CppTools + void setFilePathCache(ClangBackEnd::FilePathCaching *filePathCache); diff --git a/tests/unit/unittest/readexporteddiagnostics-test.cpp b/tests/unit/unittest/readexporteddiagnostics-test.cpp new file mode 100644 index 00000000000..368c868233b --- /dev/null +++ b/tests/unit/unittest/readexporteddiagnostics-test.cpp @@ -0,0 +1,352 @@ +/**************************************************************************** +** +** Copyright (C) 2019 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +****************************************************************************/ + +#include "googletest.h" + +#include + +#include +#include + +#define TESTDATA TESTDATA_DIR "/clangtools/" + +using namespace ClangTools::Internal; +using Debugger::DiagnosticLocation; + +namespace { + +class ReadExportedDiagnostics : public ::testing::Test +{ +protected: + void SetUp() override + { + ASSERT_TRUE(temporaryDir.isValid()); + } + + // Replace FILE_PATH with a real absolute file path in the *.yaml files. + QString createFile(const QString &yamlFilePath, const QString &filePathToInject) + { + QTC_ASSERT(QDir::isAbsolutePath(filePathToInject), return QString()); + const QString newFileName = temporaryDir.filePath(QFileInfo(yamlFilePath).fileName()); + + Utils::FileReader reader; + if (QTC_GUARD(reader.fetch(yamlFilePath, QIODevice::ReadOnly | QIODevice::Text))) { + QByteArray contents = reader.data(); + contents.replace("FILE_PATH", filePathToInject.toLocal8Bit()); + + Utils::FileSaver fileSaver(newFileName, QIODevice::WriteOnly | QIODevice::Text); + QTC_CHECK(fileSaver.write(contents)); + QTC_CHECK(fileSaver.finalize()); + } + + return newFileName; + } + +protected: + QString errorMessage; + Utils::TemporaryDirectory temporaryDir{"clangtools-tests-XXXXXX"}; +}; + +TEST_F(ReadExportedDiagnostics, NotExistingFile) +{ + Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString("notExistingFile.yaml"), + {}, + &errorMessage); + + ASSERT_THAT(diags, IsEmpty()); + ASSERT_FALSE(errorMessage.isEmpty()); +} + +TEST_F(ReadExportedDiagnostics, EmptyFile) +{ + Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(TESTDATA "empty.yaml"), + {}, + &errorMessage); + + ASSERT_THAT(diags, IsEmpty()); + ASSERT_TRUE(errorMessage.isEmpty()); +} + +TEST_F(ReadExportedDiagnostics, UnexpectedFileContents) +{ + const QString sourceFile = TESTDATA "tidy.modernize-use-nullptr.cpp"; + + Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(sourceFile), + {}, + &errorMessage); + + ASSERT_FALSE(errorMessage.isEmpty()); + ASSERT_THAT(diags, IsEmpty()); +} + +TEST_F(ReadExportedDiagnostics, Tidy) +{ + const QString sourceFile = TESTDATA "tidy.modernize-use-nullptr.cpp"; + const QString exportedFile = createFile(TESTDATA "tidy.modernize-use-nullptr.yaml", sourceFile); + Diagnostic expectedDiag; + expectedDiag.location = {sourceFile, 2, 25}; + expectedDiag.description = "use nullptr [modernize-use-nullptr]"; + expectedDiag.type = "warning"; + expectedDiag.hasFixits = true; + expectedDiag.explainingSteps = {ExplainingStep{"nullptr", + expectedDiag.location, + {expectedDiag.location, {sourceFile, 2, 26}}, + true}}; + + Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(exportedFile), + {}, + &errorMessage); + + ASSERT_TRUE(errorMessage.isEmpty()); + ASSERT_THAT(diags, ElementsAre(expectedDiag)); +} + +TEST_F(ReadExportedDiagnostics, AcceptDiagsFromFilePaths_None) +{ + const QString sourceFile = TESTDATA "tidy.modernize-use-nullptr.cpp"; + const QString exportedFile = createFile(TESTDATA "tidy.modernize-use-nullptr.yaml", sourceFile); + const auto acceptNone = [](const Utils::FilePath &) { return false; }; + + Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(exportedFile), + acceptNone, + &errorMessage); + + ASSERT_TRUE(errorMessage.isEmpty()); + ASSERT_THAT(diags, IsEmpty()); +} + +// Diagnostics from clang passed through via clang-tidy +TEST_F(ReadExportedDiagnostics, Tidy_Clang) +{ + const QString sourceFile = TESTDATA "clang.unused-parameter.cpp"; + const QString exportedFile = createFile(TESTDATA "clang.unused-parameter.yaml", sourceFile); + Diagnostic expectedDiag; + expectedDiag.location = {sourceFile, 4, 12}; + expectedDiag.description = "unused parameter 'g' [clang-diagnostic-unused-parameter]"; + expectedDiag.type = "warning"; + expectedDiag.hasFixits = false; + + Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(exportedFile), + {}, + &errorMessage); + + ASSERT_TRUE(errorMessage.isEmpty()); + ASSERT_THAT(diags, ElementsAre(expectedDiag)); +} + +// Diagnostics from clang (static) analyzer passed through via clang-tidy +TEST_F(ReadExportedDiagnostics, Tidy_ClangAnalyzer) +{ + const QString sourceFile = TESTDATA "clang-analyzer.dividezero.cpp"; + const QString exportedFile = createFile(TESTDATA "clang-analyzer.dividezero.yaml", sourceFile); + Diagnostic expectedDiag; + expectedDiag.location = {sourceFile, 4, 15}; + expectedDiag.description = "Division by zero [clang-analyzer-core.DivideZero]"; + expectedDiag.type = "warning"; + expectedDiag.hasFixits = false; + expectedDiag.explainingSteps = { + ExplainingStep{"Assuming 'z' is equal to 0", + {sourceFile, 3, 7}, + {}, + false, + }, + ExplainingStep{"Taking true branch", + {sourceFile, 3, 3}, + {}, + false, + }, + ExplainingStep{"Division by zero", + {sourceFile, 4, 15}, + {}, + false, + }, + }; + + Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(exportedFile), + {}, + &errorMessage); + + ASSERT_TRUE(errorMessage.isEmpty()); + ASSERT_THAT(diags, ElementsAre(expectedDiag)); +} + +TEST_F(ReadExportedDiagnostics, Clazy) +{ + const QString sourceFile = TESTDATA "clazy.qgetenv.cpp"; + const QString exportedFile = createFile(TESTDATA "clazy.qgetenv.yaml", sourceFile); + Diagnostic expectedDiag; + expectedDiag.location = {sourceFile, 7, 5}; + expectedDiag.description = "qgetenv().isEmpty() allocates. Use qEnvironmentVariableIsEmpty() instead [clazy-qgetenv]"; + expectedDiag.type = "warning"; + expectedDiag.hasFixits = true; + expectedDiag.explainingSteps = { + ExplainingStep{"qEnvironmentVariableIsEmpty", + expectedDiag.location, + {expectedDiag.location, {sourceFile, 7, 12}}, + true + }, + ExplainingStep{")", + {sourceFile, 7, 18}, + {{sourceFile, 7, 18}, {sourceFile, 7, 29}}, + true}, + }; + + Diagnostics diags = readExportedDiagnostics(Utils::FilePath::fromString(exportedFile), + {}, + &errorMessage); + + ASSERT_TRUE(errorMessage.isEmpty()); + ASSERT_THAT(diags, ElementsAre(expectedDiag)); +} + +class ByteOffsetInUtf8TextToLineColumn : public ::testing::Test +{ +protected: + const char *empty = ""; + const char *asciiWord = "FOO"; + const char *asciiMultiLine = "FOO\nBAR"; + const char *asciiMultiLine_dos = "FOO\r\nBAR"; + const char *asciiEmptyMultiLine = "\n\n"; + // U+00FC - 2 code units in UTF8, 1 in UTF16 - LATIN SMALL LETTER U WITH DIAERESIS + // U+4E8C - 3 code units in UTF8, 1 in UTF16 - CJK UNIFIED IDEOGRAPH-4E8C + // U+10302 - 4 code units in UTF8, 2 in UTF16 - OLD ITALIC LETTER KE + const char *nonAsciiMultiLine = "\xc3\xbc" "\n" + "\xe4\xba\x8c" "\n" + "\xf0\x90\x8c\x82" "X"; + + // Convenience + const char *text = nullptr; +}; + +TEST_F(ByteOffsetInUtf8TextToLineColumn, InvalidText) +{ + ASSERT_FALSE(byteOffsetInUtf8TextToLineColumn(nullptr, 0)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, InvalidOffset_EmptyInput) +{ + ASSERT_FALSE(byteOffsetInUtf8TextToLineColumn(empty, 0)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, InvalidOffset_Before) +{ + ASSERT_FALSE(byteOffsetInUtf8TextToLineColumn(asciiWord, -1)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, InvalidOffset_After) +{ + ASSERT_FALSE(byteOffsetInUtf8TextToLineColumn(asciiWord, 3)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, InvalidOffset_NotFirstByteOfMultiByte) +{ + ASSERT_FALSE(byteOffsetInUtf8TextToLineColumn(nonAsciiMultiLine, 1)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, StartOfFirstLine) +{ + auto info = byteOffsetInUtf8TextToLineColumn(asciiWord, 0); + + ASSERT_TRUE(info); + ASSERT_THAT(info->line, Eq(1)); + ASSERT_THAT(info->column, Eq(1)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, EndOfFirstLine) +{ + auto info = byteOffsetInUtf8TextToLineColumn(asciiWord, 2); + + ASSERT_TRUE(info); + ASSERT_THAT(info->line, Eq(1)); + ASSERT_THAT(info->column, Eq(3)); +} + +// The invocation +// +// clang-tidy "-checks=-*,readability-braces-around-statements" /path/to/file +// +// for the code +// +// void f(bool b) +// { +// if (b) +// f(b); +// } +// +// emits +// +// 3:11: warning: statement should be inside braces [readability-braces-around-statements] +// +// The new line in the if-line is considered as column 11, which is normally not visible in the +// editor. +TEST_F(ByteOffsetInUtf8TextToLineColumn, OffsetPointingToLineSeparator_unix) +{ + auto info = byteOffsetInUtf8TextToLineColumn(asciiMultiLine, 3); + + ASSERT_TRUE(info); + ASSERT_THAT(info->line, Eq(1)); + ASSERT_THAT(info->column, Eq(4)); +} + +// For a file with dos style line endings ("\r\n"), clang-tidy points to '\r'. +TEST_F(ByteOffsetInUtf8TextToLineColumn, OffsetPointingToLineSeparator_dos) +{ + auto info = byteOffsetInUtf8TextToLineColumn(asciiMultiLine_dos, 3); + + ASSERT_TRUE(info); + ASSERT_THAT(info->line, Eq(1)); + ASSERT_THAT(info->column, Eq(4)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, StartOfSecondLine) +{ + auto info = byteOffsetInUtf8TextToLineColumn(asciiMultiLine, 4); + + ASSERT_TRUE(info); + ASSERT_THAT(info->line, Eq(2)); + ASSERT_THAT(info->column, Eq(1)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, MultiByteCodePoint1) +{ + auto info = byteOffsetInUtf8TextToLineColumn(nonAsciiMultiLine, 3); + + ASSERT_TRUE(info); + ASSERT_THAT(info->line, Eq(2)); + ASSERT_THAT(info->column, Eq(1)); +} + +TEST_F(ByteOffsetInUtf8TextToLineColumn, MultiByteCodePoint2) +{ + auto info = byteOffsetInUtf8TextToLineColumn(nonAsciiMultiLine, 11); + + ASSERT_TRUE(info); + ASSERT_THAT(info->line, Eq(3)); + ASSERT_THAT(info->column, Eq(2)); +} + +} // namespace + +#undef TESTDATA diff --git a/tests/unit/unittest/unittest.pro b/tests/unit/unittest/unittest.pro index 51361a28f10..c72d767ee0c 100644 --- a/tests/unit/unittest/unittest.pro +++ b/tests/unit/unittest/unittest.pro @@ -120,7 +120,8 @@ SOURCES += \ commandlinebuilder-test.cpp \ headerpathfilter-test.cpp \ toolchainargumentscache-test.cpp \ - modifiedtimechecker-test.cpp + modifiedtimechecker-test.cpp \ + readexporteddiagnostics-test.cpp !isEmpty(LIBCLANG_LIBS) { SOURCES += \