Clang: Extend ClangPathWatcher to communicate path changes

Change-Id: Iad2d6adc53fbc2ee32fd63b9ccdc8999d41a89da
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>

src/libs/clangsupport/clangpathwatcher.h

@@ -41,25 +41,30 @@ public:
     int id;
     FilePathId pathId;
 
-    friend bool operator==(const WatcherEntry &first, const WatcherEntry &second)
+    friend bool operator==(WatcherEntry first, WatcherEntry second)
     {
         return first.id == second.id && first.pathId == second.pathId;
     }
 
-    friend bool operator<(const WatcherEntry &first, const WatcherEntry &second)
+    friend bool operator<(WatcherEntry first, WatcherEntry second)
     {
         return std::tie(first.pathId, first.id) < std::tie(second.pathId, second.id);
     }
 
-    friend bool operator<(const WatcherEntry &entry, FilePathId pathId)
+    friend bool operator<(WatcherEntry entry, FilePathId pathId)
     {
         return entry.pathId < pathId;
     }
 
-    friend bool operator<(FilePathId pathId, const WatcherEntry &entry)
+    friend bool operator<(FilePathId pathId, WatcherEntry entry)
     {
         return pathId < entry.pathId;
     }
+
+    operator FilePathId() const
+    {
+        return pathId;
+    }
 };
 
 using WatcherEntries = std::vector<WatcherEntry>;
@@ -224,7 +229,7 @@ unittest_public:
         std::transform(watcherEntries.begin(),
                        watcherEntries.end(),
                        std::back_inserter(paths),
-                       [&] (const WatcherEntry &entry) {
+                       [&] (WatcherEntry entry) {
             return QString(m_pathCache.filePath(entry.pathId).path());
         });
 
@@ -233,7 +238,7 @@ unittest_public:
 
     template <typename Compare>
     WatcherEntries notWatchedEntries(const WatcherEntries &entries,
-                                                Compare compare) const
+                                     Compare compare) const
     {
         WatcherEntries notWatchedEntries;
         notWatchedEntries.reserve(entries.size());
@@ -255,7 +260,7 @@ unittest_public:
 
     WatcherEntries notWatchedPaths(const WatcherEntries &entries) const
     {
-        auto compare = [] (const WatcherEntry &first, const WatcherEntry &second) {
+        auto compare = [] (WatcherEntry first, WatcherEntry second) {
             return first.pathId < second.pathId;
         };
 
@@ -317,7 +322,7 @@ unittest_public:
         WatcherEntries uniqueEntries;
         uniqueEntries.reserve(pathEntries.size());
 
-        auto compare = [] (const WatcherEntry &first, const WatcherEntry &second) {
+        auto compare = [] (WatcherEntry first, WatcherEntry second) {
             return first.pathId == second.pathId;
         };
 
@@ -342,7 +347,7 @@ unittest_public:
     WatcherEntries removeIdsFromWatchedEntries(const std::vector<int> &ids)
     {
 
-        auto keep = [&] (const WatcherEntry &entry) {
+        auto keep = [&] (WatcherEntry entry) {
             return !std::binary_search(ids.begin(), ids.end(), entry.id);
         };
 
@@ -390,6 +395,20 @@ unittest_public:
         return foundEntries;
     }
 
+    FilePathIds watchedPaths(const FilePathIds &filePathIds) const
+    {
+        FilePathIds watchedFilePathIds;
+        watchedFilePathIds.reserve(filePathIds.size());
+
+        std::set_intersection(m_watchedEntries.begin(),
+                              m_watchedEntries.end(),
+                              filePathIds.begin(),
+                              filePathIds.end(),
+                              std::back_inserter(watchedFilePathIds));
+
+        return watchedFilePathIds;
+    }
+
     Utils::SmallStringVector idsForWatcherEntries(const WatcherEntries &foundEntries)
     {
         Utils::SmallStringVector ids;
@@ -398,7 +417,7 @@ unittest_public:
         std::transform(foundEntries.begin(),
                        foundEntries.end(),
                        std::back_inserter(ids),
-                       [&] (const WatcherEntry &entry) {
+                       [&] (WatcherEntry entry) {
             return Utils::SmallString(m_idCache.string(entry.id));
         });
 
@@ -414,7 +433,7 @@ unittest_public:
         return std::move(ids);
     }
 
-    void addChangedPathForFilePath(ClangBackEnd::FilePathIds &&filePathIds)
+    void addChangedPathForFilePath(FilePathIds &&filePathIds)
     {
         if (m_notifier) {
             WatcherEntries foundEntries = watchedEntriesForPaths(std::move(filePathIds));
@@ -422,6 +441,7 @@ unittest_public:
             Utils::SmallStringVector changedIds = idsForWatcherEntries(foundEntries);
 
             m_notifier->pathsWithIdsChanged(uniqueIds(std::move(changedIds)));
+            m_notifier->pathsChanged(watchedPaths(filePathIds));
         }
     }
 

src/libs/clangsupport/clangpathwatchernotifier.h

@@ -27,6 +27,8 @@
 
 #include "clangsupport_global.h"
 
+#include <filepathid.h>
+
 #include <utils/smallstringvector.h>
 
 namespace ClangBackEnd {
@@ -37,6 +39,7 @@ public:
     virtual ~ClangPathWatcherNotifier();
 
     virtual void pathsWithIdsChanged(const Utils::SmallStringVector &ids) = 0;
+    virtual void pathsChanged(const FilePathIds &filePathIds) = 0;
 };
 
 } // namespace ClangBackEnd

src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp

@@ -75,6 +75,10 @@ void PchManagerServer::pathsWithIdsChanged(const Utils::SmallStringVector &ids)
     m_fileSystemWatcher.updateIdPaths(m_pchCreator.takeProjectsIncludes());
 }
 
+void PchManagerServer::pathsChanged(const FilePathIds &/*filePathIds*/)
+{
+}
+
 void PchManagerServer::taskFinished(TaskFinishStatus status, const ProjectPartPch &projectPartPch)
 {
     if (status == TaskFinishStatus::Successfully)

src/tools/clangpchmanagerbackend/source/pchmanagerserver.h

@@ -55,6 +55,7 @@ public:
     void removePchProjectParts(RemovePchProjectPartsMessage &&message) override;
 
     void pathsWithIdsChanged(const Utils::SmallStringVector &ids) override;
+    void pathsChanged(const FilePathIds &filePathIds) override;
     void taskFinished(TaskFinishStatus status, const ProjectPartPch &projectPartPch) override;
 
 private:

tests/unit/unittest/changedfilepathcompressor-test.cpp

@@ -46,7 +46,7 @@ class ChangedFilePathCompressor : public testing::Test
 protected:
     void SetUp()
     {
-        compressor.setCallback(mockCompressor.AsStdFunction());
+        compressor.setCallback(mockCompressorCallback.AsStdFunction());
     }
 
     FilePathId filePathId(const QString &filePath)
@@ -60,11 +60,13 @@ protected:
     Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory};
     ClangBackEnd::RefactoringDatabaseInitializer<Sqlite::Database> initializer{database};
     ClangBackEnd::FilePathCaching filePathCache{database};
-    NiceMock<MockFunction<void (const ClangBackEnd::FilePathIds &filePathIds)>> mockCompressor;
+    NiceMock<MockFunction<void (const ClangBackEnd::FilePathIds &filePathIds)>> mockCompressorCallback;
     ClangBackEnd::ChangedFilePathCompressor<NiceMock<MockTimer>> compressor{filePathCache};
     NiceMock<MockTimer> &mockTimer = compressor.timer();
     QString filePath1{"filePath1"};
     QString filePath2{"filePath2"};
+    FilePathId filePathId1 = filePathId(filePath1);
+    FilePathId filePathId2 = filePathId(filePath2);
 };
 
 TEST_F(ChangedFilePathCompressor, AddFilePath)
@@ -92,12 +94,19 @@ TEST_F(ChangedFilePathCompressor, CallRestartTimerAfterAddingPath)
 
 TEST_F(ChangedFilePathCompressor, CallTimeOutAfterAddingPath)
 {
-    auto id1 = filePathId(filePath1);
-    auto id2 = filePathId(filePath2);
-    EXPECT_CALL(mockCompressor, Call(ElementsAre(id1, id2)));
+    EXPECT_CALL(mockCompressorCallback, Call(ElementsAre(filePathId1, filePathId2)));
 
     compressor.addFilePath(filePath1);
     compressor.addFilePath(filePath2);
 }
 
+TEST_F(ChangedFilePathCompressor, RemoveDuplicates)
+{
+    EXPECT_CALL(mockCompressorCallback, Call(ElementsAre(filePathId1, filePathId2)));
+
+    compressor.addFilePath(filePath1);
+    compressor.addFilePath(filePath2);
+    compressor.addFilePath(filePath1);
+}
+
 }

tests/unit/unittest/clangpathwatcher-test.cpp

@@ -44,6 +44,7 @@ using testing::NiceMock;
 
 using Watcher = ClangBackEnd::ClangPathWatcher<NiceMock<MockQFileSytemWatcher>, NiceMock<MockTimer>>;
 using ClangBackEnd::WatcherEntry;
+using ClangBackEnd::WatcherEntries;
 using ClangBackEnd::FilePath;
 using ClangBackEnd::FilePathView;
 using ClangBackEnd::FilePathId;
@@ -53,6 +54,12 @@ class ClangPathWatcher : public testing::Test
 {
 protected:
     void SetUp();
+    static WatcherEntries sorted(WatcherEntries &&entries)
+    {
+        std::stable_sort(entries.begin(), entries.end());
+
+        return entries;
+    }
 
 protected:
     NiceMock<MockFilePathCaching> filePathCache;
@@ -77,14 +84,14 @@ protected:
 
 TEST_F(ClangPathWatcher, ConvertWatcherEntriesToQStringList)
 {
-    auto convertedList = watcher.convertWatcherEntriesToQStringList({watcherEntry1, watcherEntry3});
+    auto convertedList = watcher.convertWatcherEntriesToQStringList(sorted({watcherEntry1, watcherEntry3}));
 
     ASSERT_THAT(convertedList, ElementsAre(path1QString, path2QString));
 }
 
 TEST_F(ClangPathWatcher, UniquePaths)
 {
-    auto uniqueEntries = watcher.uniquePaths({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4});
+    auto uniqueEntries = watcher.uniquePaths(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4}));
 
     ASSERT_THAT(uniqueEntries, ElementsAre(watcherEntry1, watcherEntry3));
 }
@@ -93,7 +100,7 @@ TEST_F(ClangPathWatcher, NotWatchedEntries)
 {
     watcher.addEntries({watcherEntry1, watcherEntry4});
 
-    auto newEntries = watcher.notWatchedEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4});
+    auto newEntries = watcher.notWatchedEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4}));
 
     ASSERT_THAT(newEntries, ElementsAre(watcherEntry2, watcherEntry3));
 }
@@ -158,7 +165,7 @@ TEST_F(ClangPathWatcher, ExtractSortedIdsFromConvertIdPaths)
 
 TEST_F(ClangPathWatcher, NotWatchedPaths)
 {
-    watcher.mergeToWatchedEntries({watcherEntry1});
+    watcher.mergeToWatchedEntries(sorted({watcherEntry1}));
 
     auto newEntries = watcher.notWatchedPaths({watcherEntry2, watcherEntry3});
 
@@ -167,7 +174,7 @@ TEST_F(ClangPathWatcher, NotWatchedPaths)
 
 TEST_F(ClangPathWatcher, AddedPaths)
 {
-    watcher.mergeToWatchedEntries({watcherEntry1, watcherEntry2});
+    watcher.mergeToWatchedEntries(sorted({watcherEntry1, watcherEntry2}));
 
     auto filteredEntries = watcher.filterNotWatchedPaths({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4});
 
@@ -176,16 +183,16 @@ TEST_F(ClangPathWatcher, AddedPaths)
 
 TEST_F(ClangPathWatcher, MergeEntries)
 {
-    watcher.mergeToWatchedEntries({watcherEntry1, watcherEntry4});
+    watcher.mergeToWatchedEntries(sorted({watcherEntry1, watcherEntry4}));
 
     ASSERT_THAT(watcher.watchedEntries(), ElementsAre(watcherEntry1, watcherEntry4));
 }
 
 TEST_F(ClangPathWatcher, MergeMoreEntries)
 {
-    watcher.mergeToWatchedEntries({watcherEntry1, watcherEntry4});
+    watcher.mergeToWatchedEntries(sorted({watcherEntry1, watcherEntry4}));
 
-    watcher.mergeToWatchedEntries({watcherEntry2, watcherEntry3});
+    watcher.mergeToWatchedEntries(sorted({watcherEntry2, watcherEntry3}));
 
     ASSERT_THAT(watcher.watchedEntries(), ElementsAre(watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4));
 }
@@ -234,7 +241,7 @@ TEST_F(ClangPathWatcher, DontAddNewEntriesWithDifferentIdAndSamePaths)
 
 TEST_F(ClangPathWatcher, RemoveEntriesWithId)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}));
 
     watcher.removeIdsFromWatchedEntries({ids[0]});
 
@@ -251,7 +258,7 @@ TEST_F(ClangPathWatcher, RemoveNoPathsForEmptyIds)
 
 TEST_F(ClangPathWatcher, RemoveNoPathsForOneId)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4}));
 
     EXPECT_CALL(mockQFileSytemWatcher, removePaths(_))
             .Times(0);
@@ -261,7 +268,7 @@ TEST_F(ClangPathWatcher, RemoveNoPathsForOneId)
 
 TEST_F(ClangPathWatcher, RemovePathForOneId)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3}));
 
     EXPECT_CALL(mockQFileSytemWatcher, removePaths(QStringList{path2QString}));
 
@@ -270,7 +277,7 @@ TEST_F(ClangPathWatcher, RemovePathForOneId)
 
 TEST_F(ClangPathWatcher, RemoveAllPathsForThreeId)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}));
 
     EXPECT_CALL(mockQFileSytemWatcher, removePaths(QStringList{path1QString, path2QString}));
 
@@ -279,7 +286,7 @@ TEST_F(ClangPathWatcher, RemoveAllPathsForThreeId)
 
 TEST_F(ClangPathWatcher, RemoveOnePathForTwoId)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}));
 
     EXPECT_CALL(mockQFileSytemWatcher, removePaths(QStringList{path1QString}));
 
@@ -288,7 +295,7 @@ TEST_F(ClangPathWatcher, RemoveOnePathForTwoId)
 
 TEST_F(ClangPathWatcher, NotAnymoreWatchedEntriesWithId)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}));
 
     auto oldEntries = watcher.notAnymoreWatchedEntriesWithIds({watcherEntry1, watcherEntry4}, {ids[0], ids[1]});
 
@@ -297,7 +304,7 @@ TEST_F(ClangPathWatcher, NotAnymoreWatchedEntriesWithId)
 
 TEST_F(ClangPathWatcher, RemoveUnusedEntries)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}));
 
     watcher.removeFromWatchedEntries({watcherEntry2, watcherEntry3});
 
@@ -315,7 +322,7 @@ TEST_F(ClangPathWatcher, EmptyVectorNotifyFileChange)
 
 TEST_F(ClangPathWatcher, NotifyFileChange)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}));
 
     EXPECT_CALL(notifier, pathsWithIdsChanged(ElementsAre(id2, id1)));
 
@@ -324,7 +331,7 @@ TEST_F(ClangPathWatcher, NotifyFileChange)
 
 TEST_F(ClangPathWatcher, TwoNotifyFileChanges)
 {
-    watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5});
+    watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}));
 
     EXPECT_CALL(notifier, pathsWithIdsChanged(ElementsAre(id2, id3, id1)));
 
@@ -332,6 +339,34 @@ TEST_F(ClangPathWatcher, TwoNotifyFileChanges)
     mockQFileSytemWatcher.fileChanged(path1QString);
 }
 
+TEST_F(ClangPathWatcher, NotifyForPathChanges)
+{
+    watcher.addEntries({watcherEntry1});
+
+    EXPECT_CALL(notifier, pathsChanged(ElementsAre(pathIds[0])));
+
+    mockQFileSytemWatcher.fileChanged(path1QString);
+}
+
+TEST_F(ClangPathWatcher, NoNotifyForUnwatchedPathChanges)
+{
+    watcher.addEntries({watcherEntry3});
+
+    EXPECT_CALL(notifier, pathsChanged(IsEmpty()));
+
+    mockQFileSytemWatcher.fileChanged(path1QString);
+}
+
+TEST_F(ClangPathWatcher, NoDuplicatePathChanges)
+{
+    watcher.addEntries({watcherEntry1});
+
+    EXPECT_CALL(notifier, pathsChanged(ElementsAre(pathIds[0])));
+
+    mockQFileSytemWatcher.fileChanged(path1QString);
+    mockQFileSytemWatcher.fileChanged(path1QString);
+}
+
 void ClangPathWatcher::SetUp()
 {
     ON_CALL(filePathCache, filePathId(Eq(path1)))

tests/unit/unittest/mockclangpathwatchernotifier.h

@@ -34,5 +34,8 @@ class MockClangPathWatcherNotifier : public ClangBackEnd::ClangPathWatcherNotifi
 public:
     MOCK_METHOD1(pathsWithIdsChanged,
                  void (const Utils::SmallStringVector &ids));
+
+    MOCK_METHOD1(pathsChanged,
+                 void (const ClangBackEnd::FilePathIds &filePathIds));
 };