Clang: Update symbol database if file has changed

The code in the symbol indexer is quite similar, but still different
enough to prevent easy reuse of the function.

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

src/tools/clangrefactoringbackend/source/clangrefactoringbackend-source.pri

@@ -17,7 +17,8 @@ HEADERS += \
     $$PWD/symbolsvisitorbase.h \
     $$PWD/usedmacro.h \
     $$PWD/sourcedependency.h \
-    $$PWD/filestatus.h
+    $$PWD/filestatus.h \
+    $$PWD/projectpartartefacts.h
 
 !isEmpty(LIBTOOLING_LIBS) {
 SOURCES += \

src/tools/clangrefactoringbackend/source/projectpartartefacts.h

@@ -0,0 +1,73 @@
+/****************************************************************************
+**
+** Copyright (C) 2017 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.
+**
+****************************************************************************/
+
+#pragma once
+
+#include <utils/algorithm.h>
+#include <utils/smallstringvector.h>
+
+#include <QJsonArray>
+#include <QJsonDocument>
+
+namespace ClangBackEnd {
+
+class ProjectPartArtefact
+{
+public:
+    ProjectPartArtefact(Utils::SmallStringView compilerArgumentsText,
+                        Utils::SmallStringView macroNamesText,
+                        int projectPartId)
+        : compilerArguments(toVector(compilerArgumentsText)),
+          macroNames(toVector(macroNamesText)),
+          projectPartId(projectPartId)
+    {
+    }
+
+    static
+    Utils::SmallStringVector toVector(Utils::SmallStringView jsonText)
+    {
+        QJsonDocument document = QJsonDocument::fromJson(QByteArray::fromRawData(jsonText.data(),
+                                                                                 jsonText.size()));
+
+        return Utils::transform<Utils::SmallStringVector>(document.array(), [] (const QJsonValue &value) {
+            return Utils::SmallString{value.toString()};
+        });
+    }
+
+    friend
+    bool operator==(const ProjectPartArtefact &first, const ProjectPartArtefact &second)
+    {
+        return first.compilerArguments == second.compilerArguments
+            && first.macroNames == second.macroNames;
+    }
+
+public:
+    Utils::SmallStringVector compilerArguments;
+    Utils::SmallStringVector macroNames;
+    int projectPartId = -1;
+};
+
+using ProjectPartArtefacts = std::vector<ProjectPartArtefact>;
+}

src/tools/clangrefactoringbackend/source/storagesqlitestatementfactory.h

@@ -217,6 +217,10 @@ public:
        "DELETE FROM newSourceDependencies",
        database
    };
+   ReadStatement getProjectPartCompilerArgumentsAndMacroNames{
+       "SELECT compilerArguments, macroNames, projectPartId FROM projectParts WHERE projectPartId = (SELECT projectPartId FROM projectPartsSources WHERE sourceId = ?)",
+       database
+   };
 };
 
 } // namespace ClangBackEnd

src/tools/clangrefactoringbackend/source/symbolindexer.cpp

@@ -85,7 +85,39 @@ void SymbolIndexer::pathsWithIdsChanged(const Utils::SmallStringVector &)
 
 void SymbolIndexer::pathsChanged(const FilePathIds &filePathIds)
 {
+    for (FilePathId filePathId : filePathIds)
+        updateChangedPath(filePathId);
+}
 
+void SymbolIndexer::updateChangedPath(FilePathId filePathId)
+{
+    m_symbolsCollector.clear();
+
+    const Utils::optional<ProjectPartArtefact> optionalArtefact = m_symbolStorage.fetchProjectPartArtefact(filePathId);
+
+    if (optionalArtefact) {
+        const ProjectPartArtefact &artefact = optionalArtefact.value();
+
+        m_symbolsCollector.addFiles({filePathId}, artefact.compilerArguments);
+
+        m_symbolsCollector.collectSymbols();
+
+        Sqlite::ImmediateTransaction transaction{m_transactionInterface};
+
+        m_symbolStorage.addSymbolsAndSourceLocations(m_symbolsCollector.symbols(),
+                                                     m_symbolsCollector.sourceLocations());
+
+        m_symbolStorage.updateProjectPartSources(artefact.projectPartId,
+                                                 m_symbolsCollector.sourceFiles());
+
+        m_symbolStorage.insertOrUpdateUsedMacros(m_symbolsCollector.usedMacros());
+
+        m_symbolStorage.insertFileStatuses(m_symbolsCollector.fileStatuses());
+
+        m_symbolStorage.insertOrUpdateSourceDependencies(m_symbolsCollector.sourceDependencies());
+
+        transaction.commit();
+    }
 }
 
 } // namespace ClangBackEnd

src/tools/clangrefactoringbackend/source/symbolindexer.h

@@ -50,6 +50,7 @@ public:
 
     void pathsWithIdsChanged(const Utils::SmallStringVector &ids) override;
     void pathsChanged(const FilePathIds &filePathIds) override;
+    void updateChangedPath(FilePathId filePath);
 
 private:
     SymbolsCollectorInterface &m_symbolsCollector;

src/tools/clangrefactoringbackend/source/symbolstorage.h

@@ -83,6 +83,13 @@ public:
         }
     }
 
+    Utils::optional<ProjectPartArtefact> fetchProjectPartArtefact(FilePathId sourceId) const override
+    {
+        ReadStatement &statement = m_statementFactory.getProjectPartCompilerArgumentsAndMacroNames;
+
+        return statement.template value<ProjectPartArtefact, 3>(sourceId.filePathId);
+    }
+
     void insertOrUpdateUsedMacros(const UsedMacros &usedMacros) override
     {
         WriteStatement &insertStatement = m_statementFactory.insertIntoNewUsedMacrosStatement;
@@ -112,6 +119,12 @@ public:
         ReadStatement &getProjectPartIdStatement = m_statementFactory.getProjectPartIdStatement;
         int projectPartId = getProjectPartIdStatement.template value<int>(projectPartName).value();
 
+        updateProjectPartSources(projectPartId, sourceFilePathIds);
+    }
+
+    void updateProjectPartSources(int projectPartId,
+                                  const FilePathIds &sourceFilePathIds) override
+    {
         WriteStatement &deleteStatement = m_statementFactory.deleteAllProjectPartsSourcesWithProjectPartIdStatement;
         deleteStatement.write(projectPartId);
 

src/tools/clangrefactoringbackend/source/symbolstorageinterface.h

@@ -27,6 +27,7 @@
 
 #include "filestatus.h"
 #include "projectpartentry.h"
+#include "projectpartartefacts.h"
 #include "sourcelocationentry.h"
 #include "sourcedependency.h"
 #include "symbolentry.h"
@@ -51,9 +52,12 @@ public:
                                            const Utils::SmallStringVector &macroNames) = 0;
     virtual void updateProjectPartSources(Utils::SmallStringView projectPartName,
                                           const FilePathIds &sourceFilePathIds) = 0;
+    virtual void updateProjectPartSources(int projectPartId,
+                                          const FilePathIds &sourceFilePathIds) = 0;
     virtual void insertOrUpdateUsedMacros(const UsedMacros &usedMacros) = 0;
     virtual void insertFileStatuses(const FileStatuses &fileStatuses) = 0;
     virtual void insertOrUpdateSourceDependencies(const SourceDependencies &sourceDependencies) = 0;
+    virtual Utils::optional<ProjectPartArtefact> fetchProjectPartArtefact(FilePathId sourceId) const = 0;
 };
 
 } // namespace ClangBackEnd

tests/unit/unittest/gtest-creator-printing.cpp

@@ -40,6 +40,7 @@
 #include <fulltokeninfo.h>
 #include <nativefilepath.h>
 #include <precompiledheadersupdatedmessage.h>
+#include <projectpartartefacts.h>
 #include <sourcedependency.h>
 #include <sourcelocationentry.h>
 #include <sourcelocationscontainer.h>
@@ -882,6 +883,14 @@ std::ostream &operator<<(std::ostream &out, const SourceDependency &sourceDepend
                << ")";
 }
 
+std::ostream &operator<<(std::ostream &out, const ProjectPartArtefact &projectPartArtefact)
+{
+    return out << "("
+               << projectPartArtefact.compilerArguments << ", "
+               << projectPartArtefact.macroNames
+               <<")";
+}
+
 void PrintTo(const FilePath &filePath, ::std::ostream *os)
 {
     *os << filePath;

tests/unit/unittest/gtest-creator-printing.h

@@ -26,6 +26,7 @@
 #pragma once
 
 #include <utils/smallstringio.h>
+#include <utils/optional.h>
 
 #include <clangsupport_global.h>
 
@@ -65,8 +66,24 @@ class LineColumn;
 
 std::ostream &operator<<(std::ostream &out, const LineColumn &lineColumn);
 
+template <typename Type>
+std::ostream &operator<<(std::ostream &out, const Utils::optional<Type> &optional)
+{
+    if (optional)
+        return out << "optional" << optional.value();
+    else
+        return out << "empty optional()";
+}
+
+template <typename Type>
+void PrintTo(const Utils::optional<Type> &optional, ::std::ostream *os)
+{
+    *os << optional;
+}
+
 void PrintTo(const Utils::SmallString &text, ::std::ostream *os);
 void PrintTo(const Utils::PathString &text, ::std::ostream *os);
+
 } // namespace ProjectExplorer
 
 namespace ClangBackEnd {
@@ -137,6 +154,7 @@ class ProjectPartEntry;
 class UsedMacro;
 class FileStatus;
 class SourceDependency;
+class ProjectPartArtefact;
 
 std::ostream &operator<<(std::ostream &out, const SourceLocationEntry &entry);
 std::ostream &operator<<(std::ostream &out, const IdPaths &idPaths);
@@ -207,6 +225,7 @@ std::ostream &operator<<(std::ostream &out, const ProjectPartEntry &projectPartE
 std::ostream &operator<<(std::ostream &out, const UsedMacro &usedMacro);
 std::ostream &operator<<(std::ostream &out, const FileStatus &fileStatus);
 std::ostream &operator<<(std::ostream &out, const SourceDependency &sourceDependency);
+std::ostream &operator<<(std::ostream &out, const ProjectPartArtefact &projectPartArtefact);
 
 void PrintTo(const FilePath &filePath, ::std::ostream *os);
 void PrintTo(const FilePathView &filePathView, ::std::ostream *os);

tests/unit/unittest/mocksqlitereadstatement.cpp

@@ -86,6 +86,13 @@ MockSqliteReadStatement::value<Utils::PathString>(const int &directoryId)
     return valueReturnPathString(directoryId);
 }
 
+template <>
+Utils::optional<ClangBackEnd::ProjectPartArtefact>
+MockSqliteReadStatement::value<ClangBackEnd::ProjectPartArtefact, 3>(const int& sourceId)
+{
+    return valueReturnProjectPartArtefact(sourceId);
+}
+
 template <>
 Utils::optional<Utils::SmallString>
 MockSqliteReadStatement::value<Utils::SmallString>(const int &sourceId)

tests/unit/unittest/mocksqlitereadstatement.h

@@ -31,6 +31,7 @@
 
 #include <filepathstoragesources.h>
 #include <stringcachefwd.h>
+#include <projectpartartefacts.h>
 
 #include <cpptools/usages.h>
 
@@ -78,6 +79,9 @@ public:
     MOCK_METHOD1(valueReturnSmallString,
                  Utils::optional<Utils::SmallString>(int));
 
+    MOCK_METHOD1(valueReturnProjectPartArtefact,
+                 Utils::optional<ClangBackEnd::ProjectPartArtefact>(int));
+
     template <typename ResultType,
               int ResultTypeCount = 1,
               typename... QueryType>
@@ -96,6 +100,7 @@ public:
                                    const QueryContainerType<QueryElementType> &queryValues);
 
     template <typename ResultType,
+              int ResultTypeCount = 1,
               typename... QueryTypes>
     Utils::optional<ResultType> value(const QueryTypes&... queryValues);
 
@@ -141,7 +146,9 @@ template <>
 Utils::optional<Utils::PathString>
 MockSqliteReadStatement::value<Utils::PathString>(const int&);
 
-
+template <>
+Utils::optional<ClangBackEnd::ProjectPartArtefact>
+MockSqliteReadStatement::value<ClangBackEnd::ProjectPartArtefact, 3>(const int&);
 
 template <>
 Utils::optional<Utils::SmallString>

tests/unit/unittest/mocksymbolstorage.h

@@ -44,10 +44,15 @@ public:
    MOCK_METHOD2(updateProjectPartSources,
                 void(Utils::SmallStringView projectPartName,
                      const ClangBackEnd::FilePathIds &sourceFilePathIds));
+   MOCK_METHOD2(updateProjectPartSources,
+                void(int projectPartId,
+                     const ClangBackEnd::FilePathIds &sourceFilePathIds));
    MOCK_METHOD1(insertOrUpdateUsedMacros,
                 void (const ClangBackEnd::UsedMacros &usedMacros));
    MOCK_METHOD1(insertFileStatuses,
                 void (const ClangBackEnd::FileStatuses &fileStatuses));
    MOCK_METHOD1(insertOrUpdateSourceDependencies,
                 void (const ClangBackEnd::SourceDependencies &sourceDependencies));
+   MOCK_CONST_METHOD1(fetchProjectPartArtefact,
+                      Utils::optional<ClangBackEnd::ProjectPartArtefact> (ClangBackEnd::FilePathId sourceId));
 };

tests/unit/unittest/storagesqlitestatementfactory-test.cpp

@@ -256,4 +256,10 @@ TEST_F(StorageSqliteStatementFactory, DeleteAllInNewSourceDependencies)
                 Eq("DELETE FROM newSourceDependencies"));
 }
 
+TEST_F(StorageSqliteStatementFactory, GetProjectPartCompilerArgumentsAndMacroNames)
+{
+    ASSERT_THAT(factory.getProjectPartCompilerArgumentsAndMacroNames.sqlStatement,
+                Eq("SELECT compilerArguments, macroNames, projectPartId FROM projectParts WHERE projectPartId = (SELECT projectPartId FROM projectPartsSources WHERE sourceId = ?)"));
+}
+
 }

tests/unit/unittest/symbolindexer-test.cpp

@@ -69,6 +69,7 @@ protected:
         ON_CALL(mockCollector, usedMacros()).WillByDefault(ReturnRef(usedMacros));
         ON_CALL(mockCollector, fileStatuses()).WillByDefault(ReturnRef(fileStatus));
         ON_CALL(mockCollector, sourceDependencies()).WillByDefault(ReturnRef(sourceDependencies));
+        ON_CALL(mockStorage, fetchProjectPartArtefact(_)).WillByDefault(Return(artefact));
     }
 
 protected:
@@ -97,6 +98,7 @@ protected:
     UsedMacros usedMacros{{"Foo", {1, 1}}};
     FileStatuses fileStatus{{{1, 2}, 3, 4}};
     SourceDependencies sourceDependencies{{{1, 1}, {1, 2}}, {{1, 1}, {1, 3}}};
+    ClangBackEnd::ProjectPartArtefact artefact{{"-DFOO"}, {"FOO"}, 74};
     NiceMock<MockSqliteTransactionBackend> mockSqliteTransactionBackend;
     NiceMock<MockSymbolsCollector> mockCollector;
     NiceMock<MockSymbolStorage> mockStorage;
@@ -185,8 +187,8 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsUpdateProjectPartsInStorage)
 
 TEST_F(SymbolIndexer, UpdateProjectPartsCallsUpdateProjectPartSources)
 {
-    EXPECT_CALL(mockStorage, updateProjectPartSources(Eq("project1"), ElementsAre(IsFileId(1, 1), IsFileId(42, 23))));
+    EXPECT_CALL(mockStorage, updateProjectPartSources(TypedEq<Utils::SmallStringView>("project1"), ElementsAre(IsFileId(1, 1), IsFileId(42, 23))));
-    EXPECT_CALL(mockStorage, updateProjectPartSources(Eq("project2"), ElementsAre(IsFileId(1, 1), IsFileId(42, 23))));
+    EXPECT_CALL(mockStorage, updateProjectPartSources(TypedEq<Utils::SmallStringView>("project2"), ElementsAre(IsFileId(1, 1), IsFileId(42, 23))));
 
     indexer.updateProjectParts({projectPart1, projectPart2}, Utils::clone(unsaved));
 }
@@ -225,8 +227,8 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsInOrder)
     EXPECT_CALL(mockCollector, collectSymbols());
     EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin());
     EXPECT_CALL(mockStorage, addSymbolsAndSourceLocations(symbolEntries, sourceLocations));
-    EXPECT_CALL(mockStorage, insertOrUpdateProjectPart(_, _, _));
+    EXPECT_CALL(mockStorage, insertOrUpdateProjectPart(Eq(projectPart1.projectPartId()), Eq(projectPart1.arguments()), Eq(projectPart1.macroNames())));
-    EXPECT_CALL(mockStorage, updateProjectPartSources(_, _));
+    EXPECT_CALL(mockStorage, updateProjectPartSources(TypedEq<Utils::SmallStringView>(projectPart1.projectPartId()), Eq(sourceFileIds)));
     EXPECT_CALL(mockStorage, insertOrUpdateUsedMacros(Eq(usedMacros)));
     EXPECT_CALL(mockStorage, insertFileStatuses(Eq(fileStatus)));
     EXPECT_CALL(mockStorage, insertOrUpdateSourceDependencies(Eq(sourceDependencies)));
@@ -242,4 +244,50 @@ TEST_F(SymbolIndexer, CallSetNotifier)
     ClangBackEnd::SymbolIndexer indexer{mockCollector, mockStorage, mockPathWatcher, mockFilePathCaching, mockSqliteTransactionBackend};
 }
 
+TEST_F(SymbolIndexer, PathChangedCallsFetchProjectPartArtefactInStorage)
+{
+    EXPECT_CALL(mockStorage, fetchProjectPartArtefact(sourceFileIds[0]));
+    EXPECT_CALL(mockStorage, fetchProjectPartArtefact(sourceFileIds[1]));
+
+    indexer.pathsChanged(sourceFileIds);
+}
+
+TEST_F(SymbolIndexer, UpdateChangedPathCallsInOrder)
+{
+    InSequence s;
+
+    EXPECT_CALL(mockCollector, clear());
+    EXPECT_CALL(mockStorage, fetchProjectPartArtefact(sourceFileIds[0]));
+    EXPECT_CALL(mockCollector, addFiles(ElementsAre(sourceFileIds[0]), Eq(artefact.compilerArguments)));
+    EXPECT_CALL(mockCollector, collectSymbols());
+    EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin());
+    EXPECT_CALL(mockStorage, addSymbolsAndSourceLocations(symbolEntries, sourceLocations));
+    EXPECT_CALL(mockStorage, updateProjectPartSources(artefact.projectPartId, Eq(sourceFileIds)));
+    EXPECT_CALL(mockStorage, insertOrUpdateUsedMacros(Eq(usedMacros)));
+    EXPECT_CALL(mockStorage, insertFileStatuses(Eq(fileStatus)));
+    EXPECT_CALL(mockStorage, insertOrUpdateSourceDependencies(Eq(sourceDependencies)));
+    EXPECT_CALL(mockSqliteTransactionBackend, commit());
+
+    indexer.updateChangedPath(sourceFileIds[0]);
+}
+
+TEST_F(SymbolIndexer, HandleEmptyOptionalInUpdateChangedPath)
+{
+    ON_CALL(mockStorage, fetchProjectPartArtefact(_)).WillByDefault(Return(Utils::optional<ClangBackEnd::ProjectPartArtefact>()));
+
+    EXPECT_CALL(mockCollector, clear());
+    EXPECT_CALL(mockStorage, fetchProjectPartArtefact(sourceFileIds[0]));
+    EXPECT_CALL(mockCollector, addFiles(_, _)).Times(0);
+    EXPECT_CALL(mockCollector, collectSymbols()).Times(0);
+    EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()).Times(0);
+    EXPECT_CALL(mockStorage, addSymbolsAndSourceLocations(_, _)).Times(0);
+    EXPECT_CALL(mockStorage, updateProjectPartSources(An<int>(), _)).Times(0);
+    EXPECT_CALL(mockStorage, insertOrUpdateUsedMacros(_)).Times(0);
+    EXPECT_CALL(mockStorage, insertFileStatuses(_)).Times(0);
+    EXPECT_CALL(mockStorage, insertOrUpdateSourceDependencies(_)).Times(0);
+    EXPECT_CALL(mockSqliteTransactionBackend, commit()).Times(0);
+
+    indexer.updateChangedPath(sourceFileIds[0]);
+}
+
 }

tests/unit/unittest/symbolstorage-test.cpp

@@ -35,6 +35,8 @@
 
 #include <storagesqlitestatementfactory.h>
 
+#include <utils/optional.h>
+
 namespace {
 
 using Utils::PathString;
@@ -83,10 +85,12 @@ protected:
     MockSqliteWriteStatement &syncNewSourceDependenciesStatement = statementFactory.syncNewSourceDependenciesStatement;
     MockSqliteWriteStatement &deleteOutdatedSourceDependenciesStatement = statementFactory.deleteOutdatedSourceDependenciesStatement;
     MockSqliteWriteStatement &deleteNewSourceDependenciesStatement = statementFactory.deleteNewSourceDependenciesStatement;
+    MockSqliteReadStatement &getProjectPartCompilerArgumentsAndMacroNames = statementFactory.getProjectPartCompilerArgumentsAndMacroNames;
     SymbolEntries symbolEntries{{1, {"functionUSR", "function"}},
                                 {2, {"function2USR", "function2"}}};
     SourceLocationEntries sourceLocations{{1, {1, 3}, {42, 23}, SymbolType::Declaration},
                                           {2, {1, 4}, {7, 11}, SymbolType::Declaration}};
+    ClangBackEnd::ProjectPartArtefact artefact{{"-DFOO"}, {"FOO"}, 74};
     Storage storage{statementFactory, filePathCache};
 };
 
@@ -256,5 +260,22 @@ TEST_F(SymbolStorage, InsertOrUpdateSourceDependencies)
     storage.insertOrUpdateSourceDependencies({{{1, 42}, {1, 1}}, {{1, 42}, {1, 2}}});
 }
 
+TEST_F(SymbolStorage, FetchProjectPartArtefactCallsValueInStatement)
+{
+    EXPECT_CALL(getProjectPartCompilerArgumentsAndMacroNames, valueReturnProjectPartArtefact(1))
+            .WillRepeatedly(Return(artefact));
+
+    storage.fetchProjectPartArtefact({2, 1});
+}
+
+TEST_F(SymbolStorage, FetchProjectPartArtefactReturnArtefact)
+{
+    EXPECT_CALL(getProjectPartCompilerArgumentsAndMacroNames, valueReturnProjectPartArtefact(1))
+            .WillRepeatedly(Return(artefact));
+
+    auto result = storage.fetchProjectPartArtefact({2, 1});
+
+    ASSERT_THAT(result, Eq(artefact));
+}
 }