Clang: Improve diagnostic update timer

Start the timer after a translation unit change and stop after every
diagnostic is sent. We should decrease the interval as we are
sending the diagnostics because otherwise the sending is delayed to much.
If the file watcher is emitting a file change we should only react
to changes of files which have no editor open.

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

src/tools/clangbackend/ipcsource/clangfilesystemwatcher.cpp

@@ -90,7 +90,7 @@ void ClangFileSystemWatcher::updateTranslationUnitsWithChangedDependencies(const
 {
     translationUnits.updateTranslationUnitsWithChangedDependency(filePath);
 
-    emit fileChanged();
+    emit fileChanged(filePath);
 }
 
 } // namespace ClangBackEnd

src/tools/clangbackend/ipcsource/clangfilesystemwatcher.h

@@ -50,7 +50,7 @@ public:
     void addFiles(const QSet<Utf8String> &filePaths);
 
 signals:
-    void fileChanged();
+    void fileChanged(const Utf8String &filePath);
 
 private:
     void updateTranslationUnitsWithChangedDependencies(const QString &filePath);

src/tools/clangbackend/ipcsource/clangipcserver.cpp

@@ -59,6 +59,10 @@
 
 namespace ClangBackEnd {
 
+namespace {
+const int sendDiagnosticsTimerInterval = 300;
+}
+
 ClangIpcServer::ClangIpcServer()
     : translationUnits(projects, unsavedFiles)
 {
@@ -67,14 +71,26 @@ ClangIpcServer::ClangIpcServer()
                                                           client()->diagnosticsChanged(message);
                                                       });
 
-    sendDiagnosticsTimer.setInterval(300);
     QObject::connect(&sendDiagnosticsTimer,
                      &QTimer::timeout,
-                     [this] () { translationUnits.sendChangedDiagnostics(); });
+                     [this] () {
+                         try {
+                             auto diagnostSendState = translationUnits.sendChangedDiagnostics();
+                             if (diagnostSendState == DiagnosticSendState::MaybeThereAreMoreDiagnostics)
+                                 sendDiagnosticsTimer.setInterval(0);
+                             else
+                                 sendDiagnosticsTimer.stop();
+                         } catch (const std::exception &exception) {
+                             qWarning() << "Error in ClangIpcServer::sendDiagnosticsTimer:" << exception.what();
+                         }
+                     });
 
     QObject::connect(translationUnits.clangFileSystemWatcher(),
                      &ClangFileSystemWatcher::fileChanged,
-                     [this] () { sendDiagnosticsTimer.start(); });
+                     [this] (const Utf8String &filePath)
+                     {
+                         startSendDiagnosticTimerIfFileIsNotATranslationUnit(filePath);
+                     });
 }
 
 void ClangIpcServer::end()
@@ -91,7 +107,7 @@ void ClangIpcServer::registerTranslationUnitsForEditor(const ClangBackEnd::Regis
         if (newerFileContainers.size() > 0) {
             unsavedFiles.createOrUpdate(newerFileContainers);
             translationUnits.createOrUpdate(newerFileContainers);
-            sendDiagnosticsTimer.start();
+            sendDiagnosticsTimer.start(sendDiagnosticsTimerInterval);
         }
     } catch (const ProjectPartDoNotExistException &exception) {
         client()->projectPartsDoNotExist(ProjectPartsDoNotExistMessage(exception.projectPartIds()));
@@ -147,7 +163,7 @@ void ClangIpcServer::registerUnsavedFilesForEditor(const RegisterUnsavedFilesFor
     try {
         unsavedFiles.createOrUpdate(message.fileContainers());
         translationUnits.updateTranslationUnitsWithChangedDependencies(message.fileContainers());
-        sendDiagnosticsTimer.start();
+        sendDiagnosticsTimer.start(sendDiagnosticsTimerInterval);
     } catch (const ProjectPartDoNotExistException &exception) {
         client()->projectPartsDoNotExist(ProjectPartsDoNotExistMessage(exception.projectPartIds()));
     } catch (const std::exception &exception) {
@@ -209,4 +225,10 @@ void ClangIpcServer::requestDiagnostics(const RequestDiagnosticsMessage &message
     }
 }
 
+void ClangIpcServer::startSendDiagnosticTimerIfFileIsNotATranslationUnit(const Utf8String &filePath)
+{
+    if (!translationUnits.hasTranslationUnit(filePath))
+        sendDiagnosticsTimer.start(0);
+}
+
 }

src/tools/clangbackend/ipcsource/clangipcserver.h

@@ -61,6 +61,9 @@ public:
     void completeCode(const CompleteCodeMessage &message) override;
     void requestDiagnostics(const RequestDiagnosticsMessage &message) override;
 
+private:
+    void startSendDiagnosticTimerIfFileIsNotATranslationUnit(const Utf8String &filePath);
+
 private:
     ProjectParts projects;
     UnsavedFiles unsavedFiles;

src/tools/clangbackend/ipcsource/translationunits.cpp

@@ -101,6 +101,16 @@ const TranslationUnit &TranslationUnits::translationUnit(const FileContainer &fi
     return translationUnit(fileContainer.filePath(), fileContainer.projectPartId());
 }
 
+bool TranslationUnits::hasTranslationUnit(const Utf8String &filePath) const
+{
+    return std::any_of(translationUnits_.cbegin(),
+                       translationUnits_.cend(),
+                       [&filePath] (const TranslationUnit &translationUnit)
+    {
+        return translationUnit.filePath() == filePath;
+    });
+}
+
 const std::vector<TranslationUnit> &TranslationUnits::translationUnits() const
 {
     return translationUnits_;
@@ -128,14 +138,16 @@ void TranslationUnits::updateTranslationUnitsWithChangedDependencies(const QVect
         updateTranslationUnitsWithChangedDependency(fileContainer.filePath());
 }
 
-void TranslationUnits::sendChangedDiagnostics()
+DiagnosticSendState TranslationUnits::sendChangedDiagnostics()
 {
     for (const auto &translationUnit : translationUnits_) {
         if (translationUnit.hasNewDiagnostics()) {
             sendDiagnosticChangedMessage(translationUnit);
-            break;
+            return DiagnosticSendState::MaybeThereAreMoreDiagnostics;
         }
     }
+
+    return DiagnosticSendState::AllDiagnosticSend;
 }
 
 void TranslationUnits::setSendChangeDiagnosticsCallback(std::function<void(const DiagnosticsChangedMessage &)> &&callback)

src/tools/clangbackend/ipcsource/translationunits.h

@@ -47,6 +47,12 @@ class ProjectParts;
 class UnsavedFiles;
 class DiagnosticsChangedMessage;
 
+enum class DiagnosticSendState
+{
+    AllDiagnosticSend,
+    MaybeThereAreMoreDiagnostics
+};
+
 class TranslationUnits
 {
 public:
@@ -57,6 +63,7 @@ public:
 
     const TranslationUnit &translationUnit(const Utf8String &filePath, const Utf8String &projectPartId) const;
     const TranslationUnit &translationUnit(const FileContainer &fileContainer) const;
+    bool hasTranslationUnit(const Utf8String &filePath) const;
 
     const std::vector<TranslationUnit> &translationUnits() const;
 
@@ -67,7 +74,7 @@ public:
     void updateTranslationUnitsWithChangedDependency(const Utf8String &filePath);
     void updateTranslationUnitsWithChangedDependencies(const QVector<FileContainer> &fileContainers);
 
-    void sendChangedDiagnostics();
+    DiagnosticSendState sendChangedDiagnostics();
 
     void setSendChangeDiagnosticsCallback(std::function<void(const DiagnosticsChangedMessage&)> &&callback);
 

tests/unit/unittest/translationunitstest.cpp

@@ -238,6 +238,16 @@ TEST_F(TranslationUnits, RemoveAllValidIfExceptionIsThrown)
                                              translationUnits))));
 }
 
+TEST_F(TranslationUnits, HasTranslationUnit)
+{
+    translationUnits.createOrUpdate({{filePath, projectPartId}});
+
+    ASSERT_TRUE(translationUnits.hasTranslationUnit(filePath));
 }
 
+TEST_F(TranslationUnits, HasNotTranslationUnit)
+{
+    ASSERT_FALSE(translationUnits.hasTranslationUnit(filePath));
+}
 
+}