Clang: Add better diagnostic sending

We send first the current editor, next the visible editors and the end
everything else.

Change-Id: I4a7b5924ffe563d6a74251739ddedcd005ce046c
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@theqtcompany.com>

src/tools/clangbackend/ipcsource/translationunit.cpp

@@ -85,6 +85,8 @@ public:
     uint documentRevision = 0;
     bool needsToBeReparsed = false;
     bool hasNewDiagnostics = true;
+    bool isUsedByCurrentEditor = false;
+    bool isVisibleInEditor = false;
 };
 
 TranslationUnitData::TranslationUnitData(const Utf8String &filePath,
@@ -125,6 +127,26 @@ bool TranslationUnit::isNull() const
     return !d;
 }
 
+void TranslationUnit::setIsUsedByCurrentEditor(bool isUsedByCurrentEditor)
+{
+    d->isUsedByCurrentEditor = isUsedByCurrentEditor;
+}
+
+bool TranslationUnit::isUsedByCurrentEditor() const
+{
+    return d->isUsedByCurrentEditor;
+}
+
+void TranslationUnit::setIsVisibleInEditor(bool isVisibleInEditor)
+{
+    d->isVisibleInEditor = isVisibleInEditor;
+}
+
+bool TranslationUnit::isVisibleInEditor() const
+{
+    return d->isVisibleInEditor;
+}
+
 void TranslationUnit::reset()
 {
     d.reset();

src/tools/clangbackend/ipcsource/translationunit.h

@@ -86,6 +86,12 @@ public:
 
     bool isNull() const;
 
+    void setIsUsedByCurrentEditor(bool isUsedByCurrentEditor);
+    bool isUsedByCurrentEditor() const;
+
+    void setIsVisibleInEditor(bool isVisibleInEditor);
+    bool isVisibleInEditor() const;
+
     void reset();
     void reparse() const;
 

src/tools/clangbackend/ipcsource/translationunits.cpp

@@ -39,6 +39,8 @@
 
 #include <QDebug>
 
+#include <algorithm>
+
 namespace ClangBackEnd {
 
 bool operator==(const FileContainer &fileContainer, const TranslationUnit &translationUnit)
@@ -95,6 +97,18 @@ void TranslationUnits::remove(const QVector<FileContainer> &fileContainers)
     updateTranslationUnitsWithChangedDependencies(fileContainers);
 }
 
+void TranslationUnits::setCurrentEditor(const Utf8String &filePath)
+{
+    for (TranslationUnit &translationUnit : translationUnits_)
+        translationUnit.setIsUsedByCurrentEditor(translationUnit.filePath() == filePath);
+}
+
+void TranslationUnits::setVisibleEditors(const Utf8StringVector &filePaths)
+{
+    for (TranslationUnit &translationUnit : translationUnits_)
+        translationUnit.setIsVisibleInEditor(filePaths.contains(translationUnit.filePath()));
+}
+
 const TranslationUnit &TranslationUnits::translationUnit(const Utf8String &filePath, const Utf8String &projectPartId) const
 {
     checkIfProjectPartExists(projectPartId);
@@ -151,14 +165,55 @@ void TranslationUnits::updateTranslationUnitsWithChangedDependencies(const QVect
 
 DiagnosticSendState TranslationUnits::sendChangedDiagnostics()
 {
-    for (const auto &translationUnit : translationUnits_) {
-        if (translationUnit.hasNewDiagnostics()) {
-            sendDiagnosticChangedMessage(translationUnit);
-            return DiagnosticSendState::MaybeThereAreMoreDiagnostics;
-        }
+    auto diagnosticSendState = sendChangedDiagnosticsForCurrentEditor();
+    if (diagnosticSendState == DiagnosticSendState::NoDiagnosticSend)
+        diagnosticSendState = sendChangedDiagnosticsForVisibleEditors();
+    if (diagnosticSendState == DiagnosticSendState::NoDiagnosticSend)
+        diagnosticSendState = sendChangedDiagnosticsForAll();
+
+    return diagnosticSendState;
 }
 
-    return DiagnosticSendState::AllDiagnosticSend;
+template<class Predicate>
+DiagnosticSendState TranslationUnits::sendChangedDiagnostics(Predicate predicate)
+{
+    auto foundTranslationUnit = std::find_if(translationUnits_.begin(),
+                                             translationUnits_.end(),
+                                             predicate);
+
+    if (foundTranslationUnit != translationUnits().end()) {
+        sendDiagnosticChangedMessage(*foundTranslationUnit);
+        return DiagnosticSendState::MaybeThereAreMoreDiagnostics;
+    }
+
+    return DiagnosticSendState::NoDiagnosticSend;
+}
+
+DiagnosticSendState TranslationUnits::sendChangedDiagnosticsForCurrentEditor()
+{
+    auto hasDiagnosticsForCurrentEditor = [] (const TranslationUnit &translationUnit) {
+        return translationUnit.isUsedByCurrentEditor() && translationUnit.hasNewDiagnostics();
+    };
+
+    return sendChangedDiagnostics(hasDiagnosticsForCurrentEditor);
+}
+
+DiagnosticSendState TranslationUnits::sendChangedDiagnosticsForVisibleEditors()
+{
+    auto hasDiagnosticsForVisibleEditor = [] (const TranslationUnit &translationUnit) {
+        return translationUnit.isVisibleInEditor() && translationUnit.hasNewDiagnostics();
+    };
+
+    return sendChangedDiagnostics(hasDiagnosticsForVisibleEditor);
+}
+
+DiagnosticSendState TranslationUnits::sendChangedDiagnosticsForAll()
+{
+    auto hasDiagnostics = [] (const TranslationUnit &translationUnit) {
+        return translationUnit.hasNewDiagnostics();
+    };
+
+    return sendChangedDiagnostics(hasDiagnostics);
 }
 
 void TranslationUnits::setSendChangeDiagnosticsCallback(std::function<void(const DiagnosticsChangedMessage &)> &&callback)

src/tools/clangbackend/ipcsource/translationunits.h

@@ -49,8 +49,8 @@ class DiagnosticsChangedMessage;
 
 enum class DiagnosticSendState
 {
-    AllDiagnosticSend,
-    MaybeThereAreMoreDiagnostics
+    NoDiagnosticSend,
+    MaybeThereAreMoreDiagnostics,
 };
 
 class TranslationUnits
@@ -62,6 +62,9 @@ public:
     void update(const QVector<FileContainer> &fileContainers);
     void remove(const QVector<FileContainer> &fileContainers);
 
+    void setCurrentEditor(const Utf8String &filePath);
+    void setVisibleEditors(const Utf8StringVector &filePaths);
+
     const TranslationUnit &translationUnit(const Utf8String &filePath, const Utf8String &projectPartId) const;
     const TranslationUnit &translationUnit(const FileContainer &fileContainer) const;
     bool hasTranslationUnit(const Utf8String &filePath) const;
@@ -76,6 +79,9 @@ public:
     void updateTranslationUnitsWithChangedDependencies(const QVector<FileContainer> &fileContainers);
 
     DiagnosticSendState sendChangedDiagnostics();
+    DiagnosticSendState sendChangedDiagnosticsForCurrentEditor();
+    DiagnosticSendState sendChangedDiagnosticsForVisibleEditors();
+    DiagnosticSendState sendChangedDiagnosticsForAll();
 
     void setSendChangeDiagnosticsCallback(std::function<void(const DiagnosticsChangedMessage&)> &&callback);
 
@@ -99,6 +105,9 @@ private:
     void sendDiagnosticChangedMessage(const TranslationUnit &translationUnit);
     void removeTranslationUnits(const QVector<FileContainer> &fileContainers);
 
+    template<class Predicate>
+    DiagnosticSendState sendChangedDiagnostics(Predicate predicate);
+
 private:
     ClangFileSystemWatcher fileSystemWatcher;
     std::function<void(const DiagnosticsChangedMessage&)> sendDiagnosticsChangedCallback;

tests/unit/unittest/mocksenddiagnosticscallback.h

@@ -0,0 +1,53 @@
+/****************************************************************************
+**
+** Copyright (C) 2015 The Qt Company Ltd.
+** Contact: http://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 http://www.qt.io/terms-conditions.  For further information
+** use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file.  Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, The Qt Company gives you certain additional
+** rights.  These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+****************************************************************************/
+#ifndef MOCKSENDDIAGNOSTICSCALLBACK_H
+#define MOCKSENDDIAGNOSTICSCALLBACK_H
+
+#include <gmock/gmock.h>
+#include <gmock/gmock-matchers.h>
+#include <gtest/gtest.h>
+#include "gtest-qt-printing.h"
+
+class SendDiagnosticCallback
+{
+public:
+    virtual ~SendDiagnosticCallback() = default;
+
+    virtual void sendDiagnostic() = 0;
+};
+
+class MockSendDiagnosticCallback : public SendDiagnosticCallback
+{
+public:
+    MOCK_METHOD0(sendDiagnostic,
+                 void());
+};
+
+#endif // MOCKSENDDIAGNOSTICSCALLBACK_H

tests/unit/unittest/translationunitstest.cpp

@@ -43,9 +43,10 @@
 #include <unsavedfiles.h>
 #include <utf8string.h>
 
-
 #include <clang-c/Index.h>
 
+#include "mocksenddiagnosticscallback.h"
+
 #include <gmock/gmock.h>
 #include <gmock/gmock-matchers.h>
 #include <gtest/gtest.h>
@@ -54,6 +55,8 @@
 using ClangBackEnd::TranslationUnit;
 using ClangBackEnd::UnsavedFiles;
 using ClangBackEnd::ProjectPart;
+using ClangBackEnd::DiagnosticsChangedMessage;
+using ClangBackEnd::DiagnosticSendState;
 
 using testing::IsNull;
 using testing::NotNull;
@@ -81,23 +84,24 @@ class TranslationUnits : public ::testing::Test
 {
 protected:
     void SetUp() override;
+    void sendAllDiagnostics();
+    void sendAllCurrentEditorDiagnostics();
+    void sendAllVisibleEditorsDiagnostics();
 
+protected:
     ClangBackEnd::ProjectParts projects;
     ClangBackEnd::UnsavedFiles unsavedFiles;
     ClangBackEnd::TranslationUnits translationUnits{projects, unsavedFiles};
+    MockSendDiagnosticCallback mockSendDiagnosticCallback;
     const Utf8String filePath = Utf8StringLiteral(TESTDATA_DIR"/translationunits.cpp");
     const Utf8String headerPath = Utf8StringLiteral(TESTDATA_DIR"/translationunits.h");
     const Utf8String nonExistingFilePath = Utf8StringLiteral("foo.cpp");
     const Utf8String projectPartId = Utf8StringLiteral("projectPartId");
     const Utf8String nonExistingProjectPartId = Utf8StringLiteral("nonExistingProjectPartId");
+    const ClangBackEnd::FileContainer fileContainer{filePath, projectPartId};
+    const ClangBackEnd::FileContainer headerContainer{headerPath, projectPartId};
 };
 
-void TranslationUnits::SetUp()
-{
-    projects.createOrUpdate({ClangBackEnd::ProjectPartContainer(projectPartId)});
-}
-
-
 TEST_F(TranslationUnits, ThrowForGettingWithWrongFilePath)
 {
     ASSERT_THROW(translationUnits.translationUnit(nonExistingFilePath, projectPartId),
@@ -279,4 +283,193 @@ TEST_F(TranslationUnits, HasNotTranslationUnit)
     ASSERT_FALSE(translationUnits.hasTranslationUnit(filePath));
 }
 
+TEST_F(TranslationUnits, isUsedByCurrentEditor)
+{
+    translationUnits.create({fileContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+
+    translationUnits.setCurrentEditor(filePath);
+
+    ASSERT_TRUE(translationUnit.isUsedByCurrentEditor());
+}
+
+TEST_F(TranslationUnits, IsNotCurrentEditor)
+{
+    translationUnits.create({fileContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+
+    translationUnits.setCurrentEditor(headerPath);
+
+    ASSERT_FALSE(translationUnit.isUsedByCurrentEditor());
+}
+
+TEST_F(TranslationUnits, IsNotCurrentEditorAfterBeingCurrent)
+{
+    translationUnits.create({fileContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+    translationUnits.setCurrentEditor(filePath);
+
+    translationUnits.setCurrentEditor(headerPath);
+
+    ASSERT_FALSE(translationUnit.isUsedByCurrentEditor());
+}
+
+TEST_F(TranslationUnits, IsVisibleEditor)
+{
+    translationUnits.create({fileContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+
+    translationUnits.setVisibleEditors({filePath});
+
+    ASSERT_TRUE(translationUnit.isVisibleInEditor());
+}
+
+TEST_F(TranslationUnits, IsNotVisibleEditor)
+{
+    translationUnits.create({fileContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+
+    translationUnits.setVisibleEditors({headerPath});
+
+    ASSERT_FALSE(translationUnit.isVisibleInEditor());
+}
+
+TEST_F(TranslationUnits, IsNotVisibleEditorAfterBeingVisible)
+{
+    translationUnits.create({fileContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+    translationUnits.setVisibleEditors({filePath});
+
+    translationUnits.setVisibleEditors({headerPath});
+
+    ASSERT_FALSE(translationUnit.isVisibleInEditor());
+}
+
+TEST_F(TranslationUnits, DoNotSendDiagnosticsIfThereIsNothingToSend)
+{
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(0);
+
+    sendAllDiagnostics();
+}
+
+TEST_F(TranslationUnits, SendDiagnosticsAfterTranslationUnitCreation)
+{
+    translationUnits.create({fileContainer, headerContainer});
+
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(2);
+
+    sendAllDiagnostics();
+}
+
+TEST_F(TranslationUnits, DoNotSendDiagnosticsAfterGettingDiagnostics)
+{
+    translationUnits.create({fileContainer, headerContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+    translationUnit.diagnostics();
+
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(1);
+
+    sendAllDiagnostics();
+}
+
+TEST_F(TranslationUnits, SendDiagnosticsForCurrentEditor)
+{
+    translationUnits.create({fileContainer, headerContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+    translationUnit.setIsUsedByCurrentEditor(true);
+
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(1);
+
+    sendAllCurrentEditorDiagnostics();
+}
+
+TEST_F(TranslationUnits, DoNotSendDiagnosticsForCurrentEditorIfThereIsNoCurrentEditor)
+{
+    translationUnits.create({fileContainer, headerContainer});
+
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(0);
+
+    sendAllCurrentEditorDiagnostics();
+}
+
+TEST_F(TranslationUnits, DoNotSendDiagnosticsForCurrentEditorAfterGettingDiagnostics)
+{
+    translationUnits.create({fileContainer, headerContainer});
+    auto translationUnit = translationUnits.translationUnit(fileContainer);
+    translationUnit.setIsUsedByCurrentEditor(true);
+    translationUnit.diagnostics();
+
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(0);
+
+    sendAllCurrentEditorDiagnostics();
+}
+
+TEST_F(TranslationUnits, DoNotSendDiagnosticsForVisibleEditorIfThereAreNoVisibleEditors)
+{
+    translationUnits.create({fileContainer, headerContainer});
+
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(0);
+
+    translationUnits.sendChangedDiagnosticsForVisibleEditors();
+}
+
+TEST_F(TranslationUnits, SendDiagnosticsForVisibleEditors)
+{
+    translationUnits.create({fileContainer, headerContainer});
+    auto fileTranslationUnit = translationUnits.translationUnit(fileContainer);
+    fileTranslationUnit.setIsVisibleInEditor(true);
+    auto headerTranslationUnit = translationUnits.translationUnit(headerContainer);
+    headerTranslationUnit.setIsVisibleInEditor(true);
+
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(2);
+
+    sendAllVisibleEditorsDiagnostics();
+}
+
+TEST_F(TranslationUnits, SendOnlyOneDiagnosticsForVisibleEditor)
+{
+    translationUnits.create({fileContainer, headerContainer});
+    auto fileTranslationUnit = translationUnits.translationUnit(fileContainer);
+    fileTranslationUnit.setIsVisibleInEditor(true);
+    auto headerTranslationUnit = translationUnits.translationUnit(headerContainer);
+    headerTranslationUnit.setIsVisibleInEditor(true);
+    headerTranslationUnit.diagnostics();
+
+    EXPECT_CALL(mockSendDiagnosticCallback, sendDiagnostic()).Times(1);
+
+    sendAllVisibleEditorsDiagnostics();
+}
+
+void TranslationUnits::SetUp()
+{
+    projects.createOrUpdate({ClangBackEnd::ProjectPartContainer(projectPartId)});
+
+    auto callback = [&] (const DiagnosticsChangedMessage &) { mockSendDiagnosticCallback.sendDiagnostic(); };
+    translationUnits.setSendChangeDiagnosticsCallback(callback);
+}
+
+void TranslationUnits::sendAllDiagnostics()
+{
+    auto diagnosticSendState = DiagnosticSendState::MaybeThereAreMoreDiagnostics;
+
+    while (diagnosticSendState == DiagnosticSendState::MaybeThereAreMoreDiagnostics)
+        diagnosticSendState = translationUnits.sendChangedDiagnostics();
+}
+
+void TranslationUnits::sendAllCurrentEditorDiagnostics()
+{
+    auto diagnosticSendState = DiagnosticSendState::MaybeThereAreMoreDiagnostics;
+
+    while (diagnosticSendState == DiagnosticSendState::MaybeThereAreMoreDiagnostics)
+        diagnosticSendState = translationUnits.sendChangedDiagnosticsForCurrentEditor();
+}
+
+void TranslationUnits::sendAllVisibleEditorsDiagnostics()
+{
+    auto diagnosticSendState = DiagnosticSendState::MaybeThereAreMoreDiagnostics;
+
+    while (diagnosticSendState == DiagnosticSendState::MaybeThereAreMoreDiagnostics)
+        diagnosticSendState = translationUnits.sendChangedDiagnosticsForVisibleEditors();
+}
+
 }

tests/unit/unittest/unittest.pro

@@ -63,6 +63,7 @@ HEADERS += \
     mockipclient.h \
     mockipcserver.h \
     spydummy.h \
-    matcher-diagnosticcontainer.h
+    matcher-diagnosticcontainer.h \
+    mocksenddiagnosticscallback.h
 
 OTHER_FILES += $$files(data/*)