Clang: Clear SymbolCollector before updating new project parts

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

src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h

@@ -56,6 +56,11 @@ public:
         });
     }
 
+    void clearSourceFiles()
+    {
+        m_sourceFiles.clear();
+    }
+
 private:
     FilePathIds m_sourceFiles;
     FilePathCachingInterface &m_filePathCache;

src/tools/clangrefactoringbackend/source/collectsymbolsaction.h

@@ -63,6 +63,12 @@ public:
         return m_sourceLocationEntries;
     }
 
+    void clear()
+    {
+        m_symbolEntries.clear();
+        m_sourceLocationEntries.clear();
+    }
+
 private:
     SymbolEntries m_symbolEntries;
     SourceLocationEntries m_sourceLocationEntries;

src/tools/clangrefactoringbackend/source/refactoringcompilationdatabase.cpp

@@ -46,8 +46,8 @@ RefactoringCompilationDatabase::getCompileCommands(llvm::StringRef filePath) con
 {
     std::vector<clang::tooling::CompileCommand> foundCommands;
 
-    std::copy_if(compileCommands.begin(),
+    std::copy_if(m_compileCommands.begin(),
-                 compileCommands.end(),
+                 m_compileCommands.end(),
                  std::back_inserter(foundCommands),
                  [&] (const clang::tooling::CompileCommand &compileCommand) {
         return filePath == concatFilePath(compileCommand);
@@ -60,10 +60,10 @@ std::vector<std::string>
 RefactoringCompilationDatabase::getAllFiles() const
 {
     std::vector<std::string> filePaths;
-    filePaths.reserve(compileCommands.size());
+    filePaths.reserve(m_compileCommands.size());
 
-    std::transform(compileCommands.begin(),
+    std::transform(m_compileCommands.begin(),
-                   compileCommands.end(),
+                   m_compileCommands.end(),
                    std::back_inserter(filePaths),
                    [&] (const clang::tooling::CompileCommand &compileCommand) {
           return concatFilePath(compileCommand);
@@ -75,7 +75,7 @@ RefactoringCompilationDatabase::getAllFiles() const
 std::vector<clang::tooling::CompileCommand>
 RefactoringCompilationDatabase::getAllCompileCommands() const
 {
-    return compileCommands;
+    return m_compileCommands;
 }
 
 void RefactoringCompilationDatabase::addFile(const std::string &directory,
@@ -83,7 +83,7 @@ void RefactoringCompilationDatabase::addFile(const std::string &directory,
                                              const std::vector<std::string> &commandLine)
 {
 
-    compileCommands.emplace_back(directory, fileName, commandLine, llvm::StringRef());
+    m_compileCommands.emplace_back(directory, fileName, commandLine, llvm::StringRef());
 }
 
 } // namespace ClangBackEnd

src/tools/clangrefactoringbackend/source/refactoringcompilationdatabase.h

@@ -58,7 +58,7 @@ public:
                  const std::vector<std::string> &commandLine);
 
 private:
-    std::vector<clang::tooling::CompileCommand> compileCommands;
+    std::vector<clang::tooling::CompileCommand> m_compileCommands;
 };
 
 } // namespace ClangBackEnd

src/tools/clangrefactoringbackend/source/symbolindexer.cpp

@@ -39,6 +39,8 @@ SymbolIndexer::SymbolIndexer(SymbolsCollectorInterface &symbolsCollector,
 void SymbolIndexer::updateProjectParts(V2::ProjectPartContainers &&projectParts,
                                        V2::FileContainers &&generatedFiles)
 {
+    m_symbolsCollector.clear();
+
     for (const V2::ProjectPartContainer &projectPart : projectParts)
         m_symbolsCollector.addFiles(projectPart.sourcePaths(), projectPart.arguments());
 

src/tools/clangrefactoringbackend/source/symbolscollector.cpp

@@ -35,18 +35,26 @@ SymbolsCollector::SymbolsCollector(FilePathCachingInterface &filePathCache)
 
 void SymbolsCollector::addFiles(const Utils::PathStringVector &filePaths, const Utils::SmallStringVector &arguments)
 {
-    ClangTool::addFiles(filePaths, arguments);
+    m_clangTool.addFiles(filePaths, arguments);
     m_collectMacrosSourceFileCallbacks.addSourceFiles(filePaths);
 }
 
 void SymbolsCollector::addUnsavedFiles(const V2::FileContainers &unsavedFiles)
 {
-    ClangTool::addUnsavedFiles(unsavedFiles);
+    m_clangTool.addUnsavedFiles(unsavedFiles);
+}
+
+void SymbolsCollector::clear()
+{
+    m_collectMacrosSourceFileCallbacks.clearSourceFiles();
+    m_collectSymbolsAction.clear();
+
+    m_clangTool = ClangTool();
 }
 
 void SymbolsCollector::collectSymbols()
 {
-    auto tool = createTool();
+    auto tool = m_clangTool.createTool();
 
     tool.run(clang::tooling::newFrontendActionFactory(&m_collectSymbolsAction,
                                                       &m_collectMacrosSourceFileCallbacks).get());

src/tools/clangrefactoringbackend/source/symbolscollector.h

@@ -35,7 +35,7 @@
 
 namespace ClangBackEnd {
 
-class SymbolsCollector : public ClangTool, public SymbolsCollectorInterface
+class SymbolsCollector : public SymbolsCollectorInterface
 {
 public:
     SymbolsCollector(FilePathCachingInterface &filePathCache);
@@ -45,6 +45,8 @@ public:
 
     void addUnsavedFiles(const V2::FileContainers &unsavedFiles) override;
 
+    void clear() override;
+
     void collectSymbols() override;
 
     const SymbolEntries &symbols() const override;
@@ -52,6 +54,7 @@ public:
     const FilePathIds &sourceFiles() const override;
 
 private:
+    ClangTool m_clangTool;
     CollectSymbolsAction m_collectSymbolsAction;
     CollectMacrosSourceFileCallbacks m_collectMacrosSourceFileCallbacks;
 };

src/tools/clangrefactoringbackend/source/symbolscollectorinterface.h

@@ -45,6 +45,8 @@ public:
 
     virtual void addUnsavedFiles(const V2::FileContainers &unsavedFiles) = 0;
 
+    virtual void clear() = 0;
+
     virtual void collectSymbols() = 0;
 
     virtual const SymbolEntries &symbols() const = 0;

tests/unit/unittest/mocksymbolscollector.h

@@ -42,6 +42,9 @@ public:
     MOCK_METHOD1(addUnsavedFiles,
                  void(const ClangBackEnd::V2::FileContainers &unsavedFiles));
 
+    MOCK_METHOD0(clear,
+                 void());
+
     MOCK_CONST_METHOD0(symbols,
                        const ClangBackEnd::SymbolEntries &());
 

tests/unit/unittest/symbolindexer-test.cpp

@@ -92,6 +92,13 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsAddFilesInCollector)
     indexer.updateProjectParts({projectPart1}, Utils::clone(unsaved));
 }
 
+TEST_F(SymbolIndexer, UpdateProjectPartsCallsClearInCollector)
+{
+    EXPECT_CALL(mockCollector, clear());
+
+    indexer.updateProjectParts({projectPart1}, Utils::clone(unsaved));
+}
+
 TEST_F(SymbolIndexer, UpdateProjectPartsCallsAddFilesInCollectorForEveryProjectPart)
 {
     EXPECT_CALL(mockCollector, addFiles(_, _))
@@ -145,11 +152,12 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsAddSymbolsAndSourceLocationsInStora
 
 TEST_F(SymbolIndexer, UpdateProjectPartsCallsInOrder)
 {
+    InSequence s;
+
+    EXPECT_CALL(mockCollector, clear());
     EXPECT_CALL(mockCollector, addFiles(_, _));
     EXPECT_CALL(mockCollector, addUnsavedFiles(unsaved));
     EXPECT_CALL(mockCollector, collectSymbols());
-    EXPECT_CALL(mockCollector, symbols());
-    EXPECT_CALL(mockCollector, sourceLocations());
     EXPECT_CALL(mockStorage, addSymbolsAndSourceLocations(symbolEntries, sourceLocations));
 
     indexer.updateProjectParts({projectPart1}, Utils::clone(unsaved));

tests/unit/unittest/symbolscollector-test.cpp

@@ -219,6 +219,55 @@ TEST_F(SymbolsCollector, DontDuplicateSourceFiles)
                                      filePathId(TESTDATA_DIR "/symbolscollector_header2.h")));
 }
 
+TEST_F(SymbolsCollector, ClearSourceFiles)
+{
+    collector.addFiles({TESTDATA_DIR "/symbolscollector_main.cpp"}, {"cc"});
+
+    collector.clear();
+
+    ASSERT_THAT(collector.sourceFiles(), IsEmpty());
+}
+
+TEST_F(SymbolsCollector, ClearSymbols)
+{
+    collector.addFiles({TESTDATA_DIR "/symbolscollector_main.cpp"}, {"cc"});
+    collector.collectSymbols();
+
+    collector.clear();
+
+    ASSERT_THAT(collector.symbols(), IsEmpty());
+}
+
+TEST_F(SymbolsCollector, ClearSourceLocations)
+{
+    collector.addFiles({TESTDATA_DIR "/symbolscollector_main.cpp"}, {"cc"});
+    collector.collectSymbols();
+
+    collector.clear();
+
+    ASSERT_THAT(collector.sourceLocations(), IsEmpty());
+}
+
+TEST_F(SymbolsCollector, DontCollectSymbolsAfterFilesAreCleared)
+{
+    collector.addFiles({TESTDATA_DIR "/symbolscollector_main.cpp"}, {"cc"});
+
+    collector.clear();
+    collector.collectSymbols();
+
+    ASSERT_THAT(collector.symbols(), IsEmpty());
+}
+
+TEST_F(SymbolsCollector, DontCollectSourceFilesAfterFilesAreCleared)
+{
+    collector.addFiles({TESTDATA_DIR "/symbolscollector_main.cpp"}, {"cc"});
+
+    collector.clear();
+    collector.collectSymbols();
+
+    ASSERT_THAT(collector.sourceFiles(), IsEmpty());
+}
+
 SymbolIndex SymbolsCollector::symbolIdForSymbolName(const Utils::SmallString &symbolName)
 {
     for (const auto &entry : collector.symbols()) {