From b064b0675984838428589825dab59b66d26b0308 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Fri, 29 Jan 2016 17:17:18 +0100 Subject: [PATCH] Clang: Display also child diagnostics in tooltips ...by introducing a custom tooltip widget for diagnostics. Locations and fixits of child diagnostics are presented as clickable links, leading to that position in the editor or to the execution of that fix it. Change-Id: I83e801e22d0421dd29275e333e5dd91587885cf1 Reviewed-by: Alessandro Portale --- src/libs/utils/tooltip/tooltip.cpp | 5 + src/libs/utils/tooltip/tooltip.h | 1 + src/plugins/clangcodemodel/clangcodemodel.pro | 2 + src/plugins/clangcodemodel/clangcodemodel.qbs | 2 + .../clangdiagnostictooltipwidget.cpp | 217 ++++++++++++++++++ .../clangdiagnostictooltipwidget.h | 45 ++++ .../clangeditordocumentprocessor.cpp | 55 +---- .../clangbackend/ipcsource/diagnostic.cpp | 11 +- src/tools/clangbackend/ipcsource/diagnostic.h | 3 - .../clangbackend/ipcsource/diagnosticset.cpp | 4 +- .../clangbackend/ipcsource/diagnosticset.h | 3 +- tests/unit/unittest/diagnosticsettest.cpp | 12 - tests/unit/unittest/diagnostictest.cpp | 12 - 13 files changed, 281 insertions(+), 91 deletions(-) create mode 100644 src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp create mode 100644 src/plugins/clangcodemodel/clangdiagnostictooltipwidget.h diff --git a/src/libs/utils/tooltip/tooltip.cpp b/src/libs/utils/tooltip/tooltip.cpp index da75f9c4c6..ccdc92a764 100644 --- a/src/libs/utils/tooltip/tooltip.cpp +++ b/src/libs/utils/tooltip/tooltip.cpp @@ -206,6 +206,11 @@ void ToolTip::hide() instance()->hideTipWithDelay(); } +void ToolTip::hideImmediately() +{ + instance()->hideTipImmediately(); +} + void ToolTip::hideTipWithDelay() { if (!m_hideDelayTimer.isActive()) diff --git a/src/libs/utils/tooltip/tooltip.h b/src/libs/utils/tooltip/tooltip.h index baa95af85b..d6ad642ece 100644 --- a/src/libs/utils/tooltip/tooltip.h +++ b/src/libs/utils/tooltip/tooltip.h @@ -78,6 +78,7 @@ public: const QString &helpId = QString(), const QRect &rect = QRect()); static void move(const QPoint &pos, QWidget *w); static void hide(); + static void hideImmediately(); static bool isVisible(); static QPoint offsetFromPosition(); diff --git a/src/plugins/clangcodemodel/clangcodemodel.pro b/src/plugins/clangcodemodel/clangcodemodel.pro index 49bd6ffcc1..17a63638e5 100644 --- a/src/plugins/clangcodemodel/clangcodemodel.pro +++ b/src/plugins/clangcodemodel/clangcodemodel.pro @@ -20,6 +20,7 @@ SOURCES += \ clangcompletioncontextanalyzer.cpp \ clangdiagnosticfilter.cpp \ clangdiagnosticmanager.cpp \ + clangdiagnostictooltipwidget.cpp \ clangeditordocumentparser.cpp \ clangeditordocumentprocessor.cpp \ clangfixitoperation.cpp \ @@ -47,6 +48,7 @@ HEADERS += \ clangconstants.h \ clangdiagnosticfilter.h \ clangdiagnosticmanager.h \ + clangdiagnostictooltipwidget.h \ clangeditordocumentparser.h \ clangeditordocumentprocessor.h \ clangfixitoperation.h \ diff --git a/src/plugins/clangcodemodel/clangcodemodel.qbs b/src/plugins/clangcodemodel/clangcodemodel.qbs index 2b7c6f480a..9459aa8391 100644 --- a/src/plugins/clangcodemodel/clangcodemodel.qbs +++ b/src/plugins/clangcodemodel/clangcodemodel.qbs @@ -65,6 +65,8 @@ QtcPlugin { "clangdiagnosticfilter.h", "clangdiagnosticmanager.cpp", "clangdiagnosticmanager.h", + "clangdiagnostictooltipwidget.cpp", + "clangdiagnostictooltipwidget.h", "clangeditordocumentparser.cpp", "clangeditordocumentparser.h", "clangeditordocumentprocessor.cpp", diff --git a/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp b/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp new file mode 100644 index 0000000000..efd4dc76f6 --- /dev/null +++ b/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp @@ -0,0 +1,217 @@ +/**************************************************************************** +** +** Copyright (C) 2016 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 "clangdiagnostictooltipwidget.h" +#include "clangfixitoperation.h" + +#include + +#include + +#include +#include +#include +#include +#include + +namespace { + +const char LINK_ACTION_GOTO_LOCATION[] = "#gotoLocation"; +const char LINK_ACTION_APPLY_FIX[] = "#applyFix"; + +QString wrapInBoldTags(const QString &text) +{ + return QStringLiteral("") + text + QStringLiteral(""); +} + +QString wrapInLink(const QString &text, const QString &target) +{ + return QStringLiteral("%2").arg(target, text); +} + +QString wrapInColor(const QString &text, const QByteArray &color) +{ + return QStringLiteral("%1").arg(text, QString::fromUtf8(color)); +} + +QString fileNamePrefix(const QString &mainFilePath, + const ClangBackEnd::SourceLocationContainer &location) +{ + const QString filePath = location.filePath().toString(); + if (filePath != mainFilePath) + return QFileInfo(filePath).fileName() + QLatin1Char(':'); + + return QString(); +} + +QString locationToString(const ClangBackEnd::SourceLocationContainer &location) +{ + return QString::number(location.line()) + + QStringLiteral(":") + + QString::number(location.column()); +} + +QString clickableLocation(const QString &mainFilePath, + const ClangBackEnd::SourceLocationContainer &location) +{ + const QString filePrefix = fileNamePrefix(mainFilePath, location); + const QString lineColumn = locationToString(location); + const QString linkText = filePrefix + lineColumn; + + return wrapInLink(linkText, QLatin1String(LINK_ACTION_GOTO_LOCATION)); +} + +void openEditorAt(const ClangBackEnd::SourceLocationContainer &location) +{ + Core::EditorManager::openEditorAt(location.filePath().toString(), + int(location.line()), + int(location.column() - 1)); +} + +void applyFixit(const ClangBackEnd::SourceLocationContainer &location, + const QVector &fixits) +{ + ClangCodeModel::ClangFixItOperation operation(location.filePath(), Utf8String(), fixits); + + operation.perform(); +} + +template +LayoutType *createLayout() +{ + auto *layout = new LayoutType; + layout->setContentsMargins(0, 0, 0, 0); + layout->setSpacing(2); + + return layout; +} + +class MainDiagnosticWidget : public QWidget +{ + Q_OBJECT +public: + MainDiagnosticWidget(const ClangBackEnd::DiagnosticContainer &diagnostic) + { + setContentsMargins(0, 0, 0, 0); + auto *mainLayout = createLayout(); + + // Set up header row: category + responsible option + const QString category = diagnostic.category(); + const QString diagnosticText = diagnostic.text().toString().toHtmlEscaped(); + const QString responsibleOption = diagnostic.enableOption(); + const ClangBackEnd::SourceLocationContainer location = diagnostic.location(); + + auto *headerLayout = createLayout(); + headerLayout->addWidget(new QLabel(wrapInBoldTags(category)), 1); + + auto *responsibleOptionLabel = new QLabel(wrapInColor(responsibleOption, "gray")); + headerLayout->addWidget(responsibleOptionLabel, 0); + mainLayout->addLayout(headerLayout); + + // Set up main row: diagnostic text + const QString text = clickableLocation(location.filePath(), location) + + QStringLiteral(": ") + + diagnosticText; + auto *mainTextLabel = new QLabel(text); + mainTextLabel->setTextFormat(Qt::RichText); + QObject::connect(mainTextLabel, &QLabel::linkActivated, [location](const QString &) { + openEditorAt(location); + Utils::ToolTip::hideImmediately(); + }); + mainLayout->addWidget(mainTextLabel); + + setLayout(mainLayout); + } +}; + +QString clickableFixIt(const QString &text, bool hasFixIt) +{ + if (!hasFixIt) + return text; + + const QString notePrefix = QStringLiteral("note: "); + + QString modifiedText = text; + if (modifiedText.startsWith(notePrefix)) + modifiedText = modifiedText.mid(notePrefix.size()); + + return notePrefix + wrapInLink(modifiedText, QLatin1String(LINK_ACTION_APPLY_FIX)); +} + +QWidget *createChildDiagnosticLabel(const ClangBackEnd::DiagnosticContainer &diagnostic, + const QString &mainFilePath) +{ + const bool hasFixit = !diagnostic.fixIts().isEmpty(); + const QString diagnosticText = diagnostic.text().toString().toHtmlEscaped(); + const QString text = clickableLocation(mainFilePath, diagnostic.location()) + + QStringLiteral(": ") + + clickableFixIt(diagnosticText, hasFixit); + const ClangBackEnd::SourceLocationContainer location = diagnostic.location(); + const QVector fixits = diagnostic.fixIts(); + + auto *label = new QLabel(text); + label->setContentsMargins(10, 0, 0, 0); // indent + label->setTextFormat(Qt::RichText); + QObject::connect(label, &QLabel::linkActivated, [location, fixits](const QString &action) { + if (action == QLatin1String(LINK_ACTION_APPLY_FIX)) + applyFixit(location, fixits); + else + openEditorAt(location); + + Utils::ToolTip::hideImmediately(); + }); + + return label; +} + +} // anonymous namespace + +namespace ClangCodeModel { +namespace Internal { + +ClangDiagnosticToolTipWidget::ClangDiagnosticToolTipWidget( + const QVector &diagnostics, + QWidget *parent) + : Utils::FakeToolTip(parent) +{ + auto *mainLayout = createLayout(); + + foreach (const auto &diagnostic, diagnostics) { + // Set up header and text row of main diagnostic + mainLayout->addWidget(new MainDiagnosticWidget(diagnostic)); + + // Set up child rows + const QString mainFilePath = diagnostic.location().filePath(); + foreach (const auto &childDiagnostic, diagnostic.children()) + mainLayout->addWidget(createChildDiagnosticLabel(childDiagnostic, mainFilePath)); + } + + setLayout(mainLayout); +} + +} // namespace Internal +} // namespace ClangCodeModel + +#include "clangdiagnostictooltipwidget.moc" diff --git a/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.h b/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.h new file mode 100644 index 0000000000..4a6777e384 --- /dev/null +++ b/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.h @@ -0,0 +1,45 @@ +/**************************************************************************** +** +** Copyright (C) 2016 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. +** +****************************************************************************/ + +#pragma once + +#include + +#include + +namespace ClangCodeModel { +namespace Internal { + +class ClangDiagnosticToolTipWidget : public Utils::FakeToolTip +{ + Q_OBJECT +public: + explicit ClangDiagnosticToolTipWidget( + const QVector &diagnostics, + QWidget *parent = 0); +}; + +} // namespace Internal +} // namespace ClangCodeModel diff --git a/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp b/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp index 8cd28501f1..bda10a3166 100644 --- a/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp +++ b/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp @@ -26,6 +26,7 @@ #include "clangeditordocumentprocessor.h" #include "clangbackendipcintegration.h" +#include "clangdiagnostictooltipwidget.h" #include "clangfixitoperation.h" #include "clangfixitoperationsextractor.h" #include "clanghighlightingmarksreporter.h" @@ -239,55 +240,6 @@ bool ClangEditorDocumentProcessor::hasDiagnosticsAt(uint line, uint column) cons return m_diagnosticManager.hasDiagnosticsAt(line, column); } -namespace { - -bool isHelpfulChildDiagnostic(const ClangBackEnd::DiagnosticContainer &parentDiagnostic, - const ClangBackEnd::DiagnosticContainer &childDiagnostic) -{ - auto parentLocation = parentDiagnostic.location(); - auto childLocation = childDiagnostic.location(); - - return parentLocation == childLocation; -} - -QString diagnosticText(const ClangBackEnd::DiagnosticContainer &diagnostic) -{ - QString text = diagnostic.category().toString() - + QStringLiteral("\n\n") - + diagnostic.text().toString(); - -#ifdef QT_DEBUG - if (!diagnostic.disableOption().isEmpty()) { - text += QStringLiteral(" (disable with ") - + diagnostic.disableOption().toString() - + QStringLiteral(")"); - } -#endif - - for (auto &&childDiagnostic : diagnostic.children()) { - if (isHelpfulChildDiagnostic(diagnostic, childDiagnostic)) - text += QStringLiteral("\n ") + childDiagnostic.text().toString(); - } - - return text; -} - -QString generateTooltipText(const QVector &diagnostics) -{ - QString text; - - foreach (const ClangBackEnd::DiagnosticContainer &diagnostic, diagnostics) { - if (text.isEmpty()) - text += diagnosticText(diagnostic); - else - text += QStringLiteral("\n\n\n") + diagnosticText(diagnostic); - } - - return text; -} - -} // anonymous namespace - void ClangEditorDocumentProcessor::showDiagnosticTooltip(const QPoint &point, QWidget *parent, uint line, @@ -295,10 +247,9 @@ void ClangEditorDocumentProcessor::showDiagnosticTooltip(const QPoint &point, { const QVector diagnostics = m_diagnosticManager.diagnosticsAt(line, column); + auto *tooltipWidget = new ClangDiagnosticToolTipWidget(diagnostics, parent); - const QString tooltipText = generateTooltipText(diagnostics); - - ::Utils::ToolTip::show(point, tooltipText, parent); + ::Utils::ToolTip::show(point, tooltipWidget, parent); } ClangBackEnd::FileContainer ClangEditorDocumentProcessor::fileContainerWithArguments() const diff --git a/src/tools/clangbackend/ipcsource/diagnostic.cpp b/src/tools/clangbackend/ipcsource/diagnostic.cpp index a9ca3cae04..87af112d9f 100644 --- a/src/tools/clangbackend/ipcsource/diagnostic.cpp +++ b/src/tools/clangbackend/ipcsource/diagnostic.cpp @@ -133,7 +133,7 @@ DiagnosticSet Diagnostic::childDiagnostics() const return DiagnosticSet(clang_getChildDiagnostics(cxDiagnostic)); } -DiagnosticContainer Diagnostic::toDiagnosticContainer(const IsAcceptedDiagnostic &isAcceptedChildDiagnostic) const +DiagnosticContainer Diagnostic::toDiagnosticContainer() const { return DiagnosticContainer(text(), category(), @@ -142,14 +142,7 @@ DiagnosticContainer Diagnostic::toDiagnosticContainer(const IsAcceptedDiagnostic location().toSourceLocationContainer(), getSourceRangeContainers(), getFixItContainers(), - childDiagnostics().toDiagnosticContainers(isAcceptedChildDiagnostic)); -} - -DiagnosticContainer Diagnostic::toDiagnosticContainer() const -{ - const auto acceptAllDiagnostics = [](const Diagnostic &) { return true; }; - - return toDiagnosticContainer(acceptAllDiagnostics); + childDiagnostics().toDiagnosticContainers()); } QVector Diagnostic::getSourceRangeContainers() const diff --git a/src/tools/clangbackend/ipcsource/diagnostic.h b/src/tools/clangbackend/ipcsource/diagnostic.h index 0be58df172..76de4a3b81 100644 --- a/src/tools/clangbackend/ipcsource/diagnostic.h +++ b/src/tools/clangbackend/ipcsource/diagnostic.h @@ -71,9 +71,6 @@ public: std::vector fixIts() const; DiagnosticSet childDiagnostics() const; - using IsAcceptedDiagnostic = std::function; - DiagnosticContainer toDiagnosticContainer( - const IsAcceptedDiagnostic &isAcceptedChildDiagnostic) const; DiagnosticContainer toDiagnosticContainer() const; private: diff --git a/src/tools/clangbackend/ipcsource/diagnosticset.cpp b/src/tools/clangbackend/ipcsource/diagnosticset.cpp index 119bb60587..ce3504a114 100644 --- a/src/tools/clangbackend/ipcsource/diagnosticset.cpp +++ b/src/tools/clangbackend/ipcsource/diagnosticset.cpp @@ -88,14 +88,14 @@ QVector DiagnosticSet::toDiagnosticContainers() const } QVector DiagnosticSet::toDiagnosticContainers( - const Diagnostic::IsAcceptedDiagnostic &isAcceptedDiagnostic) const + const IsAcceptedDiagnostic &isAcceptedDiagnostic) const { QVector diagnosticContainers; diagnosticContainers.reserve(size()); for (const Diagnostic &diagnostic : *this) { if (isAcceptedDiagnostic(diagnostic)) - diagnosticContainers.push_back(diagnostic.toDiagnosticContainer(isAcceptedDiagnostic)); + diagnosticContainers.push_back(diagnostic.toDiagnosticContainer()); } return diagnosticContainers; diff --git a/src/tools/clangbackend/ipcsource/diagnosticset.h b/src/tools/clangbackend/ipcsource/diagnosticset.h index 0fa44f5b2f..2a66fa7eb5 100644 --- a/src/tools/clangbackend/ipcsource/diagnosticset.h +++ b/src/tools/clangbackend/ipcsource/diagnosticset.h @@ -67,9 +67,10 @@ public: ConstIterator begin() const; ConstIterator end() const; + using IsAcceptedDiagnostic = std::function; QVector toDiagnosticContainers() const; QVector toDiagnosticContainers( - const Diagnostic::IsAcceptedDiagnostic &isAcceptedDiagnostic) const; + const IsAcceptedDiagnostic &isAcceptedDiagnostic) const; private: DiagnosticSet(CXDiagnosticSet cxDiagnosticSet); diff --git a/tests/unit/unittest/diagnosticsettest.cpp b/tests/unit/unittest/diagnosticsettest.cpp index 2297d5178f..b6dd1fa818 100644 --- a/tests/unit/unittest/diagnosticsettest.cpp +++ b/tests/unit/unittest/diagnosticsettest.cpp @@ -160,18 +160,6 @@ TEST_F(DiagnosticSet, ToDiagnosticContainersFiltersOutTopLevelItem) ASSERT_TRUE(diagnostics.isEmpty()); } -TEST_F(DiagnosticSet, ToDiagnosticContainersFiltersOutChildren) -{ - const auto diagnosticContainerWithoutChild = expectedDiagnostic(WithoutChild); - const auto acceptMainFileDiagnostics = [this](const Diagnostic &diagnostic) { - return diagnostic.location().filePath() == translationUnitMainFile.filePath(); - }; - - const auto diagnostics = diagnosticSetWithChildren.toDiagnosticContainers(acceptMainFileDiagnostics); - - ASSERT_THAT(diagnostics, Contains(IsDiagnosticContainer(diagnosticContainerWithoutChild))); -} - DiagnosticContainer DiagnosticSet::expectedDiagnostic(DiagnosticSet::ChildMode childMode) const { QVector children; diff --git a/tests/unit/unittest/diagnostictest.cpp b/tests/unit/unittest/diagnostictest.cpp index d398e03ce9..822b473859 100644 --- a/tests/unit/unittest/diagnostictest.cpp +++ b/tests/unit/unittest/diagnostictest.cpp @@ -170,18 +170,6 @@ TEST_F(Diagnostic, toDiagnosticContainerLetChildrenThroughByDefault) ASSERT_THAT(diagnostic, IsDiagnosticContainer(diagnosticWithChild)); } -TEST_F(Diagnostic, toDiagnosticContainerFiltersOutChildren) -{ - const auto diagnosticWithoutChild = expectedDiagnostic(WithoutChild); - const auto acceptDiagnosticWithSpecificText = [](const ::Diagnostic &diagnostic) { - return diagnostic.text() != Utf8StringLiteral("note: previous declaration is here"); - }; - - const auto diagnostic = diagnosticSet.front().toDiagnosticContainer(acceptDiagnosticWithSpecificText); - - ASSERT_THAT(diagnostic, IsDiagnosticContainer(diagnosticWithoutChild)); -} - DiagnosticContainer Diagnostic::expectedDiagnostic(Diagnostic::ChildMode childMode) const { QVector children; -- GitLab