From 9d55d8485cda7077d1280f3335102ffd036240b5 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Tue, 4 Oct 2016 16:23:42 +0200 Subject: [PATCH] Clang: Show info bar for parse errors in header files ...because those errors can lead to a substantial performance/functional regression. The actual diagnostics (possibly with children) are shown as details in the info bar. The info bar can be hidden with the "Do Not Show Again" button. Re-enabling the info bar is possible with the new editor tool bar button. Change-Id: I03394ff8e3c84127946b0b791930b28a385f5a46 Reviewed-by: David Schulz --- .../documentannotationschangedmessage.cpp | 2 + .../documentannotationschangedmessage.h | 11 + .../clangbackendipcintegration.cpp | 4 +- .../clangcodemodel/clangdiagnosticmanager.cpp | 1 + .../clangdiagnostictooltipwidget.cpp | 65 ++++-- .../clangdiagnostictooltipwidget.h | 10 +- .../clangeditordocumentprocessor.cpp | 47 ++++- .../clangeditordocumentprocessor.h | 3 + src/plugins/clangcodemodel/clangtextmark.cpp | 2 +- src/plugins/cppeditor/cppeditor.cpp | 67 +++++- src/plugins/cppeditor/cppeditor.h | 12 ++ src/plugins/cppeditor/cppeditorconstants.h | 1 + src/plugins/cppeditor/cppeditordocument.h | 4 + .../cpptools/baseeditordocumentprocessor.h | 7 + .../builtineditordocumentprocessor.cpp | 5 +- .../clangrequestdocumentannotationsjob.cpp | 4 +- .../clangrequestdocumentannotationsjob.h | 1 + .../ipcsource/clangtranslationunit.cpp | 45 +++- .../ipcsource/clangtranslationunit.h | 6 +- .../clangupdatedocumentannotationsjob.cpp | 4 +- .../clangupdatedocumentannotationsjob.h | 1 + .../unittest/clangtranslationunit-test.cpp | 197 ++++++++++++++++++ .../unittest/clientserverinprocess-test.cpp | 1 + .../readandwritemessageblock-test.cpp | 1 + tests/unit/unittest/unittest.pro | 1 + 25 files changed, 458 insertions(+), 44 deletions(-) create mode 100644 tests/unit/unittest/clangtranslationunit-test.cpp diff --git a/src/libs/clangbackendipc/documentannotationschangedmessage.cpp b/src/libs/clangbackendipc/documentannotationschangedmessage.cpp index b8b4b36601c..6c7ef0d5cb6 100644 --- a/src/libs/clangbackendipc/documentannotationschangedmessage.cpp +++ b/src/libs/clangbackendipc/documentannotationschangedmessage.cpp @@ -37,6 +37,7 @@ QDebug operator<<(QDebug debug, const DocumentAnnotationsChangedMessage &message debug.nospace() << "DocumentAnnotationsChangedMessage(" << message.fileContainer() << ", " << message.diagnostics().size() + << ", " << !message.firstHeaderErrorDiagnostic().text().isEmpty() << ", " << message.highlightingMarks().size() << ", " << message.skippedPreprocessorRanges().size() << ")"; @@ -49,6 +50,7 @@ void PrintTo(const DocumentAnnotationsChangedMessage &message, ::std::ostream* o *os << "DocumentAnnotationsChangedMessage("; PrintTo(message.fileContainer(), os); *os << "," << message.diagnostics().size(); + *os << "," << !message.firstHeaderErrorDiagnostic().text().isEmpty(); *os << "," << message.highlightingMarks().size(); *os << "," << message.skippedPreprocessorRanges().size(); *os << ")"; diff --git a/src/libs/clangbackendipc/documentannotationschangedmessage.h b/src/libs/clangbackendipc/documentannotationschangedmessage.h index 556794385d8..b121fc6ecd8 100644 --- a/src/libs/clangbackendipc/documentannotationschangedmessage.h +++ b/src/libs/clangbackendipc/documentannotationschangedmessage.h @@ -41,10 +41,12 @@ public: DocumentAnnotationsChangedMessage() = default; DocumentAnnotationsChangedMessage(const FileContainer &fileContainer, const QVector &diagnostics, + const DiagnosticContainer &firstHeaderErrorDiagnostic_, const QVector &highlightingMarks, const QVector &skippedPreprocessorRanges) : fileContainer_(fileContainer), diagnostics_(diagnostics), + firstHeaderErrorDiagnostic_(firstHeaderErrorDiagnostic_), highlightingMarks_(highlightingMarks), skippedPreprocessorRanges_(skippedPreprocessorRanges) { @@ -60,6 +62,11 @@ public: return diagnostics_; } + const DiagnosticContainer &firstHeaderErrorDiagnostic() const + { + return firstHeaderErrorDiagnostic_; + } + const QVector &highlightingMarks() const { return highlightingMarks_; @@ -74,6 +81,7 @@ public: { out << message.fileContainer_; out << message.diagnostics_; + out << message.firstHeaderErrorDiagnostic_; out << message.highlightingMarks_; out << message.skippedPreprocessorRanges_; @@ -84,6 +92,7 @@ public: { in >> message.fileContainer_; in >> message.diagnostics_; + in >> message.firstHeaderErrorDiagnostic_; in >> message.highlightingMarks_; in >> message.skippedPreprocessorRanges_; @@ -95,6 +104,7 @@ public: { return first.fileContainer_ == second.fileContainer_ && first.diagnostics_ == second.diagnostics_ + && first.firstHeaderErrorDiagnostic_ == second.firstHeaderErrorDiagnostic_ && first.highlightingMarks_ == second.highlightingMarks_ && first.skippedPreprocessorRanges_ == second.skippedPreprocessorRanges_; } @@ -102,6 +112,7 @@ public: private: FileContainer fileContainer_; QVector diagnostics_; + DiagnosticContainer firstHeaderErrorDiagnostic_; QVector highlightingMarks_; QVector skippedPreprocessorRanges_; }; diff --git a/src/plugins/clangcodemodel/clangbackendipcintegration.cpp b/src/plugins/clangcodemodel/clangbackendipcintegration.cpp index 1edce8fbd43..831a3bdea62 100644 --- a/src/plugins/clangcodemodel/clangbackendipcintegration.cpp +++ b/src/plugins/clangcodemodel/clangbackendipcintegration.cpp @@ -182,7 +182,9 @@ void IpcReceiver::documentAnnotationsChanged(const DocumentAnnotationsChangedMes const QString documentProjectPartId = CppTools::CppToolsBridge::projectPartIdForFile(filePath); if (projectPartId == documentProjectPartId) { const quint32 documentRevision = message.fileContainer().documentRevision(); - processor->updateCodeWarnings(message.diagnostics(), documentRevision); + processor->updateCodeWarnings(message.diagnostics(), + message.firstHeaderErrorDiagnostic(), + documentRevision); processor->updateHighlighting(message.highlightingMarks(), message.skippedPreprocessorRanges(), documentRevision); diff --git a/src/plugins/clangcodemodel/clangdiagnosticmanager.cpp b/src/plugins/clangcodemodel/clangdiagnosticmanager.cpp index 2bfa7baac7d..52c441443d6 100644 --- a/src/plugins/clangcodemodel/clangdiagnosticmanager.cpp +++ b/src/plugins/clangcodemodel/clangdiagnosticmanager.cpp @@ -41,6 +41,7 @@ #include #include +#include #include namespace { diff --git a/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp b/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp index 52c097f305a..5b6c48c9536 100644 --- a/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp +++ b/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp @@ -129,9 +129,10 @@ enum IndentType { IndentDiagnostic, DoNotIndentDiagnostic }; QWidget *createDiagnosticLabel(const ClangBackEnd::DiagnosticContainer &diagnostic, const QString &mainFilePath, - IndentType indentType = DoNotIndentDiagnostic) + IndentType indentType = DoNotIndentDiagnostic, + bool enableClickableFixits = true) { - const bool hasFixit = !diagnostic.fixIts().isEmpty(); + const bool hasFixit = enableClickableFixits ? !diagnostic.fixIts().isEmpty() : false; const QString diagnosticText = diagnostic.text().toString().toHtmlEscaped(); const QString text = clickableLocation(mainFilePath, diagnostic.location()) + QStringLiteral(": ") @@ -159,25 +160,35 @@ class MainDiagnosticWidget : public QWidget { Q_OBJECT public: - MainDiagnosticWidget(const ClangBackEnd::DiagnosticContainer &diagnostic) + MainDiagnosticWidget(const ClangBackEnd::DiagnosticContainer &diagnostic, + const ClangCodeModel::Internal::DisplayHints &displayHints) { setContentsMargins(0, 0, 0, 0); auto *mainLayout = createLayout(); - // Set up header row: category + responsible option - const QString category = diagnostic.category(); - const QString responsibleOption = diagnostic.enableOption(); const ClangBackEnd::SourceLocationContainer location = diagnostic.location(); - auto *headerLayout = createLayout(); - headerLayout->addWidget(new QLabel(wrapInBoldTags(category)), 1); + // Set up header row: category + responsible option + if (displayHints.showMainDiagnosticHeader) { + const QString category = diagnostic.category(); + const QString responsibleOption = diagnostic.enableOption(); - auto *responsibleOptionLabel = new QLabel(wrapInColor(responsibleOption, "gray")); - headerLayout->addWidget(responsibleOptionLabel, 0); - mainLayout->addLayout(headerLayout); + 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 - mainLayout->addWidget(createDiagnosticLabel(diagnostic, location.filePath())); + const Utf8String mainFilePath = displayHints.showFileNameInMainDiagnostic + ? Utf8String() + : location.filePath(); + mainLayout->addWidget(createDiagnosticLabel(diagnostic, + mainFilePath, + DoNotIndentDiagnostic, + displayHints.enableClickableFixits)); setLayout(mainLayout); } @@ -186,26 +197,35 @@ public: void addChildrenToLayout(const QString &mainFilePath, const QVector::const_iterator first, const QVector::const_iterator last, + bool enableClickableFixits, QLayout &boxLayout) { - for (auto it = first; it != last; ++it) - boxLayout.addWidget(createDiagnosticLabel(*it, mainFilePath, IndentDiagnostic)); + for (auto it = first; it != last; ++it) { + boxLayout.addWidget(createDiagnosticLabel(*it, + mainFilePath, + IndentDiagnostic, + enableClickableFixits)); + } } void setupChildDiagnostics(const QString &mainFilePath, const QVector &diagnostics, + bool enableClickableFixits, QLayout &boxLayout) { if (diagnostics.size() <= 10) { - addChildrenToLayout(mainFilePath, diagnostics.begin(), diagnostics.end(), boxLayout); + addChildrenToLayout(mainFilePath, diagnostics.begin(), diagnostics.end(), + enableClickableFixits, boxLayout); } else { - addChildrenToLayout(mainFilePath, diagnostics.begin(), diagnostics.begin() + 7, boxLayout); + addChildrenToLayout(mainFilePath, diagnostics.begin(), diagnostics.begin() + 7, + enableClickableFixits, boxLayout); auto ellipsisLabel = new QLabel(QStringLiteral("...")); ellipsisLabel->setContentsMargins(childIndentationOnTheLeftInPixel, 0, 0, 0); boxLayout.addWidget(ellipsisLabel); - addChildrenToLayout(mainFilePath, diagnostics.end() - 3, diagnostics.end(), boxLayout); + addChildrenToLayout(mainFilePath, diagnostics.end() - 3, diagnostics.end(), + enableClickableFixits, boxLayout); } } @@ -214,13 +234,18 @@ void setupChildDiagnostics(const QString &mainFilePath, namespace ClangCodeModel { namespace Internal { -void addToolTipToLayout(const ClangBackEnd::DiagnosticContainer &diagnostic, QLayout *target) +void addToolTipToLayout(const ClangBackEnd::DiagnosticContainer &diagnostic, + QLayout *target, + const DisplayHints &displayHints) { // Set up header and text row for main diagnostic - target->addWidget(new MainDiagnosticWidget(diagnostic)); + target->addWidget(new MainDiagnosticWidget(diagnostic, displayHints)); // Set up child rows for notes - setupChildDiagnostics(diagnostic.location().filePath(), diagnostic.children(), *target); + setupChildDiagnostics(diagnostic.location().filePath(), + diagnostic.children(), + displayHints.enableClickableFixits, + *target); } } // namespace Internal diff --git a/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.h b/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.h index 600ccf22d83..839952fc774 100644 --- a/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.h +++ b/src/plugins/clangcodemodel/clangdiagnostictooltipwidget.h @@ -34,7 +34,15 @@ QT_END_NAMESPACE namespace ClangCodeModel { namespace Internal { -void addToolTipToLayout(const ClangBackEnd::DiagnosticContainer &diagnostic, QLayout *target); +struct DisplayHints { + bool showMainDiagnosticHeader = true; + bool showFileNameInMainDiagnostic = false; + bool enableClickableFixits = true; +}; + +void addToolTipToLayout(const ClangBackEnd::DiagnosticContainer &diagnostic, + QLayout *target, + const DisplayHints &displayHints = DisplayHints()); } // namespace Internal } // namespace ClangCodeModel diff --git a/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp b/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp index 99c6a670339..09a7b76b6f1 100644 --- a/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp +++ b/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp @@ -57,6 +57,8 @@ #include #include +#include +#include namespace ClangCodeModel { namespace Internal { @@ -167,14 +169,21 @@ void ClangEditorDocumentProcessor::clearProjectPart() m_projectPart.clear(); } -void ClangEditorDocumentProcessor::updateCodeWarnings(const QVector &diagnostics, - uint documentRevision) +void ClangEditorDocumentProcessor::updateCodeWarnings( + const QVector &diagnostics, + const ClangBackEnd::DiagnosticContainer &firstHeaderErrorDiagnostic, + uint documentRevision) { if (documentRevision == revision()) { m_diagnosticManager.processNewDiagnostics(diagnostics); const auto codeWarnings = m_diagnosticManager.takeExtraSelections(); const auto fixitAvailableMarkers = m_diagnosticManager.takeFixItAvailableMarkers(); - emit codeWarningsUpdated(revision(), codeWarnings, fixitAvailableMarkers); + const auto creator = creatorForHeaderErrorDiagnosticWidget(firstHeaderErrorDiagnostic); + + emit codeWarningsUpdated(revision(), + codeWarnings, + creator, + fixitAvailableMarkers); } } namespace { @@ -333,6 +342,38 @@ void ClangEditorDocumentProcessor::requestDocumentAnnotations(const QString &pro m_ipcCommunicator.requestDocumentAnnotations(fileContainer); } +static Internal::DisplayHints displayHintsForInfoBar() +{ + Internal::DisplayHints displayHints; + displayHints.showMainDiagnosticHeader = false; + displayHints.showFileNameInMainDiagnostic = true; + displayHints.enableClickableFixits = false; // Tool chain headers might be changed, so disable. + + return displayHints; +} + +CppTools::BaseEditorDocumentProcessor::HeaderErrorDiagnosticWidgetCreator +ClangEditorDocumentProcessor::creatorForHeaderErrorDiagnosticWidget( + const ClangBackEnd::DiagnosticContainer &firstHeaderErrorDiagnostic) +{ + if (firstHeaderErrorDiagnostic.text().isEmpty()) + return CppTools::BaseEditorDocumentProcessor::HeaderErrorDiagnosticWidgetCreator(); + + return [firstHeaderErrorDiagnostic]() { + auto vbox = new QVBoxLayout; + vbox->setMargin(0); + vbox->setContentsMargins(10, 0, 0, 2); + vbox->setSpacing(2); + + addToolTipToLayout(firstHeaderErrorDiagnostic, vbox, displayHintsForInfoBar()); + + auto widget = new QWidget; + widget->setLayout(vbox); + + return widget; + }; +} + static CppTools::ProjectPart projectPartForLanguageOption(CppTools::ProjectPart *projectPart) { if (projectPart) diff --git a/src/plugins/clangcodemodel/clangeditordocumentprocessor.h b/src/plugins/clangcodemodel/clangeditordocumentprocessor.h index a23d93d72fb..10355e349d7 100644 --- a/src/plugins/clangcodemodel/clangeditordocumentprocessor.h +++ b/src/plugins/clangcodemodel/clangeditordocumentprocessor.h @@ -68,6 +68,7 @@ public: void clearProjectPart(); void updateCodeWarnings(const QVector &diagnostics, + const ClangBackEnd::DiagnosticContainer &firstHeaderErrorDiagnostic, uint documentRevision); void updateHighlighting(const QVector &highlightingMarks, const QVector &skippedPreprocessorRanges, @@ -96,6 +97,8 @@ private: void registerTranslationUnitForEditor(CppTools::ProjectPart *projectPart); void updateTranslationUnitIfProjectPartExists(); void requestDocumentAnnotations(const QString &projectpartId); + HeaderErrorDiagnosticWidgetCreator creatorForHeaderErrorDiagnosticWidget( + const ClangBackEnd::DiagnosticContainer &firstHeaderErrorDiagnostic); ClangBackEnd::FileContainer fileContainerWithArguments(CppTools::ProjectPart *projectPart) const; ClangBackEnd::FileContainer fileContainerWithDocumentContent(const QString &projectpartId) const; diff --git a/src/plugins/clangcodemodel/clangtextmark.cpp b/src/plugins/clangcodemodel/clangtextmark.cpp index 137787daa64..8057d66cb61 100644 --- a/src/plugins/clangcodemodel/clangtextmark.cpp +++ b/src/plugins/clangcodemodel/clangtextmark.cpp @@ -84,7 +84,7 @@ void ClangTextMark::setIcon(ClangBackEnd::DiagnosticSeverity severity) bool ClangTextMark::addToolTipContent(QLayout *target) { - Internal::addToolTipToLayout(m_diagnostic, target); + Internal::addToolTipToLayout(m_diagnostic, target, Internal::DisplayHints()); return true; } diff --git a/src/plugins/cppeditor/cppeditor.cpp b/src/plugins/cppeditor/cppeditor.cpp index c26a8b25f1a..0492635d1a6 100644 --- a/src/plugins/cppeditor/cppeditor.cpp +++ b/src/plugins/cppeditor/cppeditor.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include @@ -77,6 +78,7 @@ #include #include #include +#include #include #include @@ -126,9 +128,13 @@ public: QSharedPointer m_declDefLink; QScopedPointer m_followSymbolUnderCursor; - QToolButton *m_preprocessorButton; + + QToolButton *m_preprocessorButton = nullptr; + QToolButton *m_headerErrorsIndicatorButton = nullptr; CppSelectionChanger m_cppSelectionChanger; + + CppEditorWidget::HeaderErrorDiagnosticWidgetCreator m_headerErrorDiagnosticWidgetCreator; }; CppEditorWidgetPrivate::CppEditorWidgetPrivate(CppEditorWidget *q) @@ -139,7 +145,6 @@ CppEditorWidgetPrivate::CppEditorWidgetPrivate(CppEditorWidget *q) , m_useSelectionsUpdater(q) , m_declDefLinkFinder(new FunctionDeclDefLinkFinder(q)) , m_followSymbolUnderCursor(new FollowSymbolUnderCursor(q)) - , m_preprocessorButton(0) , m_cppSelectionChanger() { } @@ -224,7 +229,15 @@ void CppEditorWidget::finalizeInitialization() connect(cmd, &Command::keySequenceChanged, this, &CppEditorWidget::updatePreprocessorButtonTooltip); updatePreprocessorButtonTooltip(); connect(d->m_preprocessorButton, &QAbstractButton::clicked, this, &CppEditorWidget::showPreProcessorWidget); + + d->m_headerErrorsIndicatorButton = new QToolButton(this); + d->m_headerErrorsIndicatorButton->setIcon(Utils::Icons::WARNING_TOOLBAR.pixmap()); + connect(d->m_headerErrorsIndicatorButton, &QAbstractButton::clicked, + this, &CppEditorWidget::showHeaderErrorInfoBar); + d->m_headerErrorsIndicatorButton->setEnabled(false); + insertExtraToolBarWidget(TextEditorWidget::Left, d->m_preprocessorButton); + insertExtraToolBarWidget(TextEditorWidget::Left, d->m_headerErrorsIndicatorButton); insertExtraToolBarWidget(TextEditorWidget::Left, d->m_cppEditorOutline->widget()); } @@ -287,6 +300,7 @@ void CppEditorWidget::onCppDocumentUpdated() void CppEditorWidget::onCodeWarningsUpdated(unsigned revision, const QList selections, + const HeaderErrorDiagnosticWidgetCreator &creator, const TextEditor::RefactorMarkers &refactorMarkers) { if (revision != documentRevision()) @@ -294,6 +308,9 @@ void CppEditorWidget::onCodeWarningsUpdated(unsigned revision, setExtraSelections(TextEditorWidget::CodeWarningsSelection, selections); setRefactorMarkers(refactorMarkersWithoutClangMarkers() + refactorMarkers); + + d->m_headerErrorDiagnosticWidgetCreator = creator; + updateHeaderErrorWidgets(); } void CppEditorWidget::onIfdefedOutBlocksUpdated(unsigned revision, @@ -304,6 +321,24 @@ void CppEditorWidget::onIfdefedOutBlocksUpdated(unsigned revision, setIfdefedOutBlocks(ifdefedOutBlocks); } +void CppEditorWidget::updateHeaderErrorWidgets() +{ + const Id id(Constants::ERRORS_IN_HEADER_FILES); + InfoBar *infoBar = textDocument()->infoBar(); + + infoBar->removeInfo(id); + + if (d->m_headerErrorDiagnosticWidgetCreator) { + if (infoBar->canInfoBeAdded(id)) { + addHeaderErrorInfoBarEntryAndHideIndicator(); + } else { + d->m_headerErrorsIndicatorButton->setEnabled(true); + } + } else { + d->m_headerErrorsIndicatorButton->setEnabled(false); + } +} + void CppEditorWidget::findUsages() { if (!d->m_modelManager) @@ -399,6 +434,25 @@ void CppEditorWidget::renameSymbolUnderCursorBuiltin() renameUsages(); // Rename non-local symbol or macro } +void CppEditorWidget::addHeaderErrorInfoBarEntryAndHideIndicator() const +{ + InfoBarEntry info(Constants::ERRORS_IN_HEADER_FILES, + tr("Warning: The code model could not parse an included file, " + "which might lead to slow or incorrect code completion and " + "highlighting, for example."), + InfoBarEntry::GlobalSuppressionEnabled); + info.setDetailsWidgetCreator(d->m_headerErrorDiagnosticWidgetCreator); + info.setShowDefaultCancelButton(false); + info.setSuppressionButtonInfo([this](){ + d->m_headerErrorsIndicatorButton->setEnabled(true); + }); + + InfoBar *infoBar = textDocument()->infoBar(); + infoBar->addInfo(info); + + d->m_headerErrorsIndicatorButton->setEnabled(false); +} + namespace { QList fetchProjectParts(CppTools::CppModelManager *modelManager, @@ -970,5 +1024,14 @@ void CppEditorWidget::showPreProcessorWidget() } } +void CppEditorWidget::showHeaderErrorInfoBar() +{ + const Id id(Constants::ERRORS_IN_HEADER_FILES); + QTC_CHECK(!textDocument()->infoBar()->canInfoBeAdded(id)); + + InfoBar::globallyUnsuppressInfo(id); + addHeaderErrorInfoBarEntryAndHideIndicator(); +} + } // namespace Internal } // namespace CppEditor diff --git a/src/plugins/cppeditor/cppeditor.h b/src/plugins/cppeditor/cppeditor.h index 7fb26e93c36..cce22cebcd4 100644 --- a/src/plugins/cppeditor/cppeditor.h +++ b/src/plugins/cppeditor/cppeditor.h @@ -31,6 +31,10 @@ #include +namespace Core { +class InfoBarEntry; +} + namespace CppTools { class CppEditorOutline; class RefactoringEngineInterface; @@ -87,6 +91,7 @@ public: void switchDeclarationDefinition(bool inNextSplit); void showPreProcessorWidget(); + void showHeaderErrorInfoBar(); void findUsages(); void renameSymbolUnderCursor(); @@ -108,6 +113,9 @@ protected: void slotCodeStyleSettingsChanged(const QVariant &) override; +public: + using HeaderErrorDiagnosticWidgetCreator = std::function; + private: void updateFunctionDeclDefLink(); void updateFunctionDeclDefLinkNow(); @@ -118,10 +126,12 @@ private: void onCodeWarningsUpdated(unsigned revision, const QList selections, + const HeaderErrorDiagnosticWidgetCreator &creator, const TextEditor::RefactorMarkers &refactorMarkers); void onIfdefedOutBlocksUpdated(unsigned revision, const QList ifdefedOutBlocks); + void updateHeaderErrorWidgets(); void updateSemanticInfo(const CppTools::SemanticInfo &semanticInfo, bool updateUseSelectionSynchronously = false); void updatePreprocessorButtonTooltip(); @@ -140,6 +150,8 @@ private: void renameSymbolUnderCursorClang(); void renameSymbolUnderCursorBuiltin(); + void addHeaderErrorInfoBarEntryAndHideIndicator() const; + CppTools::ProjectPart *projectPart() const; private: diff --git a/src/plugins/cppeditor/cppeditorconstants.h b/src/plugins/cppeditor/cppeditorconstants.h index 1d72be7a7a4..761822d1012 100644 --- a/src/plugins/cppeditor/cppeditorconstants.h +++ b/src/plugins/cppeditor/cppeditorconstants.h @@ -36,6 +36,7 @@ const char OPEN_DECLARATION_DEFINITION_IN_NEXT_SPLIT[] = "CppEditor.OpenDeclarat const char RENAME_SYMBOL_UNDER_CURSOR[] = "CppEditor.RenameSymbolUnderCursor"; const char FIND_USAGES[] = "CppEditor.FindUsages"; const char OPEN_PREPROCESSOR_DIALOG[] = "CppEditor.OpenPreprocessorDialog"; +const char ERRORS_IN_HEADER_FILES[] = "CppEditor.ErrorsInHeaderFiles"; const char M_REFACTORING_MENU_INSERTION_POINT[] = "CppEditor.RefactorGroup"; const char UPDATE_CODEMODEL[] = "CppEditor.UpdateCodeModel"; const char INSPECT_CPP_CODEMODEL[] = "CppEditor.InspectCppCodeModel"; diff --git a/src/plugins/cppeditor/cppeditordocument.h b/src/plugins/cppeditor/cppeditordocument.h index 953e24de3e1..985e9a41b6a 100644 --- a/src/plugins/cppeditor/cppeditordocument.h +++ b/src/plugins/cppeditor/cppeditordocument.h @@ -59,9 +59,13 @@ public: const QByteArray &defines); void scheduleProcessDocument(); +public: + using HeaderErrorDiagnosticWidgetCreator = std::function; + signals: void codeWarningsUpdated(unsigned contentsRevision, const QList selections, + const HeaderErrorDiagnosticWidgetCreator &creator, const TextEditor::RefactorMarkers &refactorMarkers); void ifdefedOutBlocksUpdated(unsigned contentsRevision, diff --git a/src/plugins/cpptools/baseeditordocumentprocessor.h b/src/plugins/cpptools/baseeditordocumentprocessor.h index da6a20ca943..3ac8a3524d9 100644 --- a/src/plugins/cpptools/baseeditordocumentprocessor.h +++ b/src/plugins/cpptools/baseeditordocumentprocessor.h @@ -37,6 +37,8 @@ #include +#include + namespace TextEditor { class TextDocument; class QuickFixOperations; @@ -69,10 +71,15 @@ public: virtual void editorDocumentTimerRestarted(); +public: + using HeaderErrorDiagnosticWidgetCreator = std::function; + signals: + // Signal interface to implement void codeWarningsUpdated(unsigned revision, const QList selections, + const HeaderErrorDiagnosticWidgetCreator &creator, const TextEditor::RefactorMarkers &refactorMarkers); void ifdefedOutBlocksUpdated(unsigned revision, diff --git a/src/plugins/cpptools/builtineditordocumentprocessor.cpp b/src/plugins/cpptools/builtineditordocumentprocessor.cpp index a7a7ac1a338..284e1af8ae9 100644 --- a/src/plugins/cpptools/builtineditordocumentprocessor.cpp +++ b/src/plugins/cpptools/builtineditordocumentprocessor.cpp @@ -310,7 +310,10 @@ void BuiltinEditorDocumentProcessor::onCodeWarningsUpdated( m_codeWarnings += toTextEditorSelections(codeWarnings, textDocument()); m_codeWarningsUpdated = true; - emit codeWarningsUpdated(revision(), m_codeWarnings, TextEditor::RefactorMarkers()); + emit codeWarningsUpdated(revision(), + m_codeWarnings, + HeaderErrorDiagnosticWidgetCreator(), + TextEditor::RefactorMarkers()); } SemanticInfo::Source BuiltinEditorDocumentProcessor::createSemanticInfoSource(bool force) const diff --git a/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.cpp b/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.cpp index 2fd806b192b..45b06c02037 100644 --- a/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.cpp +++ b/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.cpp @@ -40,7 +40,8 @@ static RequestDocumentAnnotationsJob::AsyncResult runAsyncHelper( RequestDocumentAnnotationsJob::AsyncResult asyncResult; - translationUnit.extractDocumentAnnotations(asyncResult.diagnostics, + translationUnit.extractDocumentAnnotations(asyncResult.firstHeaderErrorDiagnostic, + asyncResult.diagnostics, asyncResult.highlightingMarks, asyncResult.skippedSourceRanges); @@ -83,6 +84,7 @@ void RequestDocumentAnnotationsJob::sendAnnotations( { const DocumentAnnotationsChangedMessage message(m_pinnedFileContainer, result.diagnostics, + result.firstHeaderErrorDiagnostic, result.highlightingMarks, result.skippedSourceRanges); diff --git a/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.h b/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.h index b9ff1c822c7..e6bb4f9edca 100644 --- a/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.h +++ b/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.h @@ -36,6 +36,7 @@ namespace ClangBackEnd { struct RequestDocumentAnnotationsJobResult { + ClangBackEnd::DiagnosticContainer firstHeaderErrorDiagnostic; QVector diagnostics; QVector highlightingMarks; QVector skippedSourceRanges; diff --git a/src/tools/clangbackend/ipcsource/clangtranslationunit.cpp b/src/tools/clangbackend/ipcsource/clangtranslationunit.cpp index 2b963b21ca4..f4d3390038f 100644 --- a/src/tools/clangbackend/ipcsource/clangtranslationunit.cpp +++ b/src/tools/clangbackend/ipcsource/clangtranslationunit.cpp @@ -112,11 +112,12 @@ TranslationUnit::CodeCompletionResult TranslationUnit::complete( } void TranslationUnit::extractDocumentAnnotations( - QVector &diagnostics, + DiagnosticContainer &firstHeaderErrorDiagnostic, + QVector &mainFileDiagnostics, QVector &highlightingMarks, QVector &skippedSourceRanges) const { - diagnostics = mainFileDiagnostics(); + extractDiagnostics(firstHeaderErrorDiagnostic, mainFileDiagnostics); highlightingMarks = this->highlightingMarks().toHighlightingMarksContainers(); skippedSourceRanges = this->skippedSourceRanges().toSourceRangeContainers(); } @@ -126,15 +127,6 @@ DiagnosticSet TranslationUnit::diagnostics() const return DiagnosticSet(clang_getDiagnosticSetFromTU(m_cxTranslationUnit)); } -QVector TranslationUnit::mainFileDiagnostics() const -{ - const auto isMainFileDiagnostic = [this](const Diagnostic &diagnostic) { - return diagnostic.location().filePath() == m_filePath; - }; - - return diagnostics().toDiagnosticContainers(isMainFileDiagnostic); -} - SourceLocation TranslationUnit::sourceLocationAt(uint line,uint column) const { return SourceLocation(m_cxTranslationUnit, m_filePath, line, column); @@ -193,4 +185,35 @@ SkippedSourceRanges TranslationUnit::skippedSourceRanges() const return SkippedSourceRanges(m_cxTranslationUnit, m_filePath.constData()); } +static bool isMainFileDiagnostic(const Utf8String &mainFilePath, const Diagnostic &diagnostic) +{ + return diagnostic.location().filePath() == mainFilePath; +} + +static bool isHeaderErrorDiagnostic(const Utf8String &mainFilePath, const Diagnostic &diagnostic) +{ + const bool isCritical = diagnostic.severity() == DiagnosticSeverity::Error + || diagnostic.severity() == DiagnosticSeverity::Fatal; + return isCritical && diagnostic.location().filePath() != mainFilePath; +} + +void TranslationUnit::extractDiagnostics(DiagnosticContainer &firstHeaderErrorDiagnostic, + QVector &mainFileDiagnostics) const +{ + mainFileDiagnostics.clear(); + mainFileDiagnostics.reserve(int(diagnostics().size())); + + bool hasFirstHeaderErrorDiagnostic = false; + + for (const Diagnostic &diagnostic : diagnostics()) { + if (!hasFirstHeaderErrorDiagnostic && isHeaderErrorDiagnostic(m_filePath, diagnostic)) { + hasFirstHeaderErrorDiagnostic = true; + firstHeaderErrorDiagnostic = diagnostic.toDiagnosticContainer(); + } + + if (isMainFileDiagnostic(m_filePath, diagnostic)) + mainFileDiagnostics.push_back(diagnostic.toDiagnosticContainer()); + } +} + } // namespace ClangBackEnd diff --git a/src/tools/clangbackend/ipcsource/clangtranslationunit.h b/src/tools/clangbackend/ipcsource/clangtranslationunit.h index cf65dddc3a0..69574750923 100644 --- a/src/tools/clangbackend/ipcsource/clangtranslationunit.h +++ b/src/tools/clangbackend/ipcsource/clangtranslationunit.h @@ -76,12 +76,14 @@ public: CodeCompletionResult complete(UnsavedFiles &unsavedFiles, uint line, uint column) const; - void extractDocumentAnnotations(QVector &diagnostics, + void extractDiagnostics(DiagnosticContainer &firstHeaderErrorDiagnostic, + QVector &mainFileDiagnostics) const; + void extractDocumentAnnotations(DiagnosticContainer &firstHeaderErrorDiagnostic, + QVector &mainFileDiagnostics, QVector &highlightingMarks, QVector &skippedSourceRanges) const; DiagnosticSet diagnostics() const; - QVector mainFileDiagnostics() const; SourceLocation sourceLocationAt(uint line, uint column) const; SourceLocation sourceLocationAt(const Utf8String &filePath, uint line, uint column) const; diff --git a/src/tools/clangbackend/ipcsource/clangupdatedocumentannotationsjob.cpp b/src/tools/clangbackend/ipcsource/clangupdatedocumentannotationsjob.cpp index 4ece1fb2a0e..fee622964d8 100644 --- a/src/tools/clangbackend/ipcsource/clangupdatedocumentannotationsjob.cpp +++ b/src/tools/clangbackend/ipcsource/clangupdatedocumentannotationsjob.cpp @@ -45,7 +45,8 @@ static UpdateDocumentAnnotationsJob::AsyncResult runAsyncHelper( asyncResult.updateResult = translationUnit.update(translationUnitUpdatInput); // Collect - translationUnit.extractDocumentAnnotations(asyncResult.diagnostics, + translationUnit.extractDocumentAnnotations(asyncResult.firstHeaderErrorDiagnostic, + asyncResult.diagnostics, asyncResult.highlightingMarks, asyncResult.skippedSourceRanges); @@ -95,6 +96,7 @@ void UpdateDocumentAnnotationsJob::sendAnnotations(const AsyncResult &result) { const DocumentAnnotationsChangedMessage message(m_pinnedFileContainer, result.diagnostics, + result.firstHeaderErrorDiagnostic, result.highlightingMarks, result.skippedSourceRanges); diff --git a/src/tools/clangbackend/ipcsource/clangupdatedocumentannotationsjob.h b/src/tools/clangbackend/ipcsource/clangupdatedocumentannotationsjob.h index 5ddb9727dc0..00fe3be3110 100644 --- a/src/tools/clangbackend/ipcsource/clangupdatedocumentannotationsjob.h +++ b/src/tools/clangbackend/ipcsource/clangupdatedocumentannotationsjob.h @@ -39,6 +39,7 @@ struct UpdateDocumentAnnotationsJobResult { TranslationUnitUpdateResult updateResult; + ClangBackEnd::DiagnosticContainer firstHeaderErrorDiagnostic; QVector diagnostics; QVector highlightingMarks; QVector skippedSourceRanges; diff --git a/tests/unit/unittest/clangtranslationunit-test.cpp b/tests/unit/unittest/clangtranslationunit-test.cpp new file mode 100644 index 00000000000..10fee318ab9 --- /dev/null +++ b/tests/unit/unittest/clangtranslationunit-test.cpp @@ -0,0 +1,197 @@ +/**************************************************************************** +** +** 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 +#include +#include +#include + +#include + +#include +#include +#include +#include "gtest-qt-printing.h" + +using ClangBackEnd::DiagnosticContainer; +using ClangBackEnd::TranslationUnit; +using ClangBackEnd::TranslationUnitUpdateInput; +using ClangBackEnd::TranslationUnitUpdateResult; + +using testing::ContainerEq; +using testing::Eq; + +namespace { + +class TranslationUnit : public ::testing::Test +{ +protected: + void SetUp() override; + void TearDown() override; + + void parse(); + void reparse(); + + DiagnosticContainer createDiagnostic(const QString &text, + ClangBackEnd::DiagnosticSeverity severity, + uint line, + uint column, + const QString &filePath) const; + QVector diagnosticsFromMainFile() const; + QVector errorDiagnosticsFromHeaders() const; + +protected: + Utf8String sourceFilePath = Utf8StringLiteral(TESTDATA_DIR"/diagnostic_erroneous_source.cpp"); + Utf8String headerFilePath = Utf8StringLiteral(TESTDATA_DIR"/diagnostic_erroneous_header.h"); + CXIndex cxIndex = nullptr; + CXTranslationUnit cxTranslationUnit = nullptr; + ::TranslationUnit translationUnit{Utf8String(), sourceFilePath, cxIndex, cxTranslationUnit}; + + DiagnosticContainer extractedFirstHeaderErrorDiagnostic; + QVector extractedMainFileDiagnostics; +}; + +TEST_F(TranslationUnit, HasExpectedMainFileDiagnostics) +{ + translationUnit.extractDiagnostics(extractedFirstHeaderErrorDiagnostic, + extractedMainFileDiagnostics); + + ASSERT_THAT(extractedMainFileDiagnostics, ContainerEq(diagnosticsFromMainFile())); +} + +TEST_F(TranslationUnit, HasExpectedMainFileDiagnosticsAfterReparse) +{ + reparse(); + + translationUnit.extractDiagnostics(extractedFirstHeaderErrorDiagnostic, + extractedMainFileDiagnostics); + + ASSERT_THAT(extractedMainFileDiagnostics, ContainerEq(diagnosticsFromMainFile())); +} + +TEST_F(TranslationUnit, HasErrorDiagnosticsInHeaders) +{ + translationUnit.extractDiagnostics(extractedFirstHeaderErrorDiagnostic, + extractedMainFileDiagnostics); + + ASSERT_THAT(extractedFirstHeaderErrorDiagnostic, + Eq(errorDiagnosticsFromHeaders().first())); +} + +TEST_F(TranslationUnit, HasErrorDiagnosticsInHeadersAfterReparse) +{ + reparse(); + + translationUnit.extractDiagnostics(extractedFirstHeaderErrorDiagnostic, + extractedMainFileDiagnostics); + + ASSERT_THAT(extractedFirstHeaderErrorDiagnostic, + Eq(errorDiagnosticsFromHeaders().first())); +} + +void TranslationUnit::SetUp() +{ + parse(); +} + +void TranslationUnit::TearDown() +{ + clang_disposeTranslationUnit(cxTranslationUnit); + clang_disposeIndex(cxIndex); +} + +void TranslationUnit::parse() +{ + TranslationUnitUpdateInput parseInput; + parseInput.filePath = sourceFilePath; + parseInput.parseNeeded = true; + const TranslationUnitUpdateResult parseResult = translationUnit.update(parseInput); + ASSERT_TRUE(parseResult.hasParsed()); +} + +void TranslationUnit::reparse() +{ + TranslationUnitUpdateInput parseInput; + parseInput.filePath = sourceFilePath; + parseInput.reparseNeeded = true; + const TranslationUnitUpdateResult parseResult = translationUnit.update(parseInput); + ASSERT_TRUE(parseResult.hasReparsed()); +} + +DiagnosticContainer TranslationUnit::createDiagnostic(const QString &text, + ClangBackEnd::DiagnosticSeverity severity, + uint line, + uint column, + const QString &filePath) const +{ + return DiagnosticContainer( + text, + Utf8StringLiteral(""), + {}, + severity, + {filePath, line, column}, + {}, + {}, + {} + ); +} + +QVector TranslationUnit::diagnosticsFromMainFile() const +{ + return { + createDiagnostic( + QStringLiteral("warning: enumeration value 'Three' not handled in switch"), + ClangBackEnd::DiagnosticSeverity::Warning, + 7, + 13, + sourceFilePath), + createDiagnostic( + QStringLiteral("error: void function 'g' should not return a value"), + ClangBackEnd::DiagnosticSeverity::Error, + 15, + 5, + sourceFilePath), + createDiagnostic( + QStringLiteral("warning: using the result of an assignment as a condition without parentheses"), + ClangBackEnd::DiagnosticSeverity::Warning, + 21, + 11, + sourceFilePath), + }; +} + +QVector TranslationUnit::errorDiagnosticsFromHeaders() const +{ + return { + createDiagnostic( + QStringLiteral("error: C++ requires a type specifier for all declarations"), + ClangBackEnd::DiagnosticSeverity::Error, + 11, + 1, + headerFilePath), + }; +} + +} // anonymous namespace diff --git a/tests/unit/unittest/clientserverinprocess-test.cpp b/tests/unit/unittest/clientserverinprocess-test.cpp index 37d1e46570c..d4a1cab16dd 100644 --- a/tests/unit/unittest/clientserverinprocess-test.cpp +++ b/tests/unit/unittest/clientserverinprocess-test.cpp @@ -277,6 +277,7 @@ TEST_F(ClientServerInProcess, SendDocumentAnnotationsChangedMessage) ClangBackEnd::DocumentAnnotationsChangedMessage message(fileContainer, {diagnostic}, + {}, {highlightingMark}, QVector()); diff --git a/tests/unit/unittest/readandwritemessageblock-test.cpp b/tests/unit/unittest/readandwritemessageblock-test.cpp index d45a3a347ae..ace9effadb4 100644 --- a/tests/unit/unittest/readandwritemessageblock-test.cpp +++ b/tests/unit/unittest/readandwritemessageblock-test.cpp @@ -186,6 +186,7 @@ TEST_F(ReadAndWriteMessageBlock, CompareDocumentAnnotationsChangedMessage) CompareMessage(ClangBackEnd::DocumentAnnotationsChangedMessage(fileContainer, {diagnostic}, + {}, {highlightingMark}, QVector())); } diff --git a/tests/unit/unittest/unittest.pro b/tests/unit/unittest/unittest.pro index e847b8347fa..ce81a41a991 100644 --- a/tests/unit/unittest/unittest.pro +++ b/tests/unit/unittest/unittest.pro @@ -61,6 +61,7 @@ SOURCES += \ clangrequestdocumentannotationsjob-test.cpp \ clangsupportivetranslationunitinitializertest.cpp \ clangstring-test.cpp \ + clangtranslationunit-test.cpp \ clangtranslationunits-test.cpp \ clangupdatedocumentannotationsjob-test.cpp \ codecompletionsextractor-test.cpp \