CMake: Clean up API of FileApiParser

Clean up the API of FileApiParser in preparation of moving
the FileSystemWatcher out of that class and into FileApiReader.

Change-Id: I5618252dd98a39f6a70f56b50fb50327e8b43e84
Reviewed-by: Eike Ziller <eike.ziller@qt.io>

src/plugins/cmakeprojectmanager/fileapiparser.cpp

@@ -47,10 +47,17 @@ const char CMAKE_RELATIVE_QUERY_PATH[] = ".cmake/api/v1/query";
 
 static Q_LOGGING_CATEGORY(cmakeFileApi, "qtc.cmake.fileApi", QtWarningMsg);
 
+const QStringList CMAKE_QUERY_FILENAMES = {"cache-v2", "codemodel-v2", "cmakeFiles-v1"};
+
 // --------------------------------------------------------------------
 // Helper:
 // --------------------------------------------------------------------
 
+static FilePath cmakeReplyDirectory(const FilePath &buildDirectory)
+{
+    return buildDirectory.pathAppended(CMAKE_RELATIVE_REPLY_PATH);
+}
+
 static void reportFileApiSetupFailure()
 {
     Core::MessageManager::write(QCoreApplication::translate(
@@ -499,7 +506,7 @@ static std::vector<Configuration> readCodemodelFile(const QString &codemodelFile
 std::vector<FileApiDetails::FragmentInfo> extractFragments(const QJsonObject &obj)
 {
     const QJsonArray fragments = obj.value("commandFragments").toArray();
-    return Utils::transform<std::vector>(fragments, [](const QJsonValue &v) {
+    return transform<std::vector>(fragments, [](const QJsonValue &v) {
         const QJsonObject o = v.toObject();
         return FileApiDetails::FragmentInfo{o.value("fragment").toString(),
                                             o.value("role").toString()};
@@ -807,49 +814,38 @@ QString FileApiDetails::ReplyFileContents::jsonFile(const QString &kind, const Q
 // FileApi:
 // --------------------------------------------------------------------
 
-FileApiParser::FileApiParser(const FilePath &sourceDirectory, const FilePath &buildDirectory)
+FileApiParser::FileApiParser(const FilePath &buildDirectory) : m_buildDirectory(buildDirectory)
-    : m_sourceDirectory(sourceDirectory)
-    , m_buildDirectory(buildDirectory)
 {
-    setupCMakeFileApi();
+    setupCMakeFileApi(buildDirectory, m_watcher);
 
     QObject::connect(&m_watcher,
                      &FileSystemWatcher::directoryChanged,
                      this,
                      &FileApiParser::replyDirectoryHasChanged);
-
-    m_watcher.addDirectory(cmakeReplyDirectory().toString(), FileSystemWatcher::WatchAllChanges);
-}
-
-FilePath FileApiParser::cmakeReplyDirectory() const
-{
-    return m_buildDirectory.pathAppended(CMAKE_RELATIVE_REPLY_PATH);
 }
 
 FileApiParser::~FileApiParser() = default;
 
-void FileApiParser::setupCMakeFileApi() const
+bool FileApiParser::setupCMakeFileApi(const FilePath &buildDirectory, Utils::FileSystemWatcher &watcher)
 {
-    const QDir buildDir = QDir(m_buildDirectory.toString());
+    const QDir buildDir = QDir(buildDirectory.toString());
-    const QString relativeQueryPath = QString::fromLatin1(CMAKE_RELATIVE_QUERY_PATH);
-
-    buildDir.mkpath(relativeQueryPath);
     buildDir.mkpath(
         QString::fromLatin1(CMAKE_RELATIVE_REPLY_PATH)); // So that we have a directory to watch!
 
+    const QString relativeQueryPath = QString::fromLatin1(CMAKE_RELATIVE_QUERY_PATH);
+    buildDir.mkpath(relativeQueryPath);
+
     QDir queryDir = buildDir;
     queryDir.cd(relativeQueryPath);
 
     if (!queryDir.exists()) {
         reportFileApiSetupFailure();
-        return;
+        return false;
     }
     QTC_ASSERT(queryDir.exists(), );
 
     bool failedBefore = false;
-    for (const QString &fileName : cmakeQueryFileNames()) {
+    for (const QString &filePath : cmakeQueryFilePaths(buildDirectory)) {
-        const QString filePath = queryDir.filePath(fileName);
-
         QFile f(filePath);
         if (!f.exists()) {
             f.open(QFile::WriteOnly);
@@ -861,6 +857,9 @@ void FileApiParser::setupCMakeFileApi() const
             reportFileApiSetupFailure();
         }
     }
+
+    watcher.addDirectory(cmakeReplyDirectory(buildDirectory).toString(), FileSystemWatcher::WatchAllChanges);
+    return true;
 }
 
 static QStringList uniqueTargetFiles(const std::vector<Configuration> &configs)
@@ -910,9 +909,9 @@ FileApiData FileApiParser::parseData(const QFileInfo &replyFileInfo, QString &er
     return result;
 }
 
-QFileInfo FileApiParser::scanForCMakeReplyFile() const
+QFileInfo FileApiParser::scanForCMakeReplyFile(const FilePath &buildDirectory)
 {
-    QDir replyDir(cmakeReplyDirectory().toString());
+    QDir replyDir(cmakeReplyDirectory(buildDirectory).toString());
     if (!replyDir.exists())
         return {};
 
@@ -922,16 +921,10 @@ QFileInfo FileApiParser::scanForCMakeReplyFile() const
     return fis.isEmpty() ? QFileInfo() : fis.last();
 }
 
-QStringList FileApiParser::cmakeQueryFileNames() const
+QStringList FileApiParser::cmakeQueryFilePaths(const Utils::FilePath &buildDirectory)
 {
-    return {"cache-v2", "codemodel-v2", "cmakeFiles-v1"};
+    QDir queryDir(QDir::cleanPath(buildDirectory.pathAppended(CMAKE_RELATIVE_QUERY_PATH).toString()));
-}
+    return transform(CMAKE_QUERY_FILENAMES,
-
-QStringList FileApiParser::cmakeQueryFilePaths() const
-{
-    QDir queryDir(QDir::cleanPath(m_sourceDirectory.toString() + "/"
-                                  + QString::fromLatin1(CMAKE_RELATIVE_QUERY_PATH)));
-    return transform(cmakeQueryFileNames(),
                      [&queryDir](const QString &name) { return queryDir.absoluteFilePath(name); });
 }
 
@@ -942,8 +935,8 @@ void FileApiParser::setParsedReplyFilePath(const QString &filePath)
 
 void FileApiParser::replyDirectoryHasChanged(const QString &directory) const
 {
-    if (directory == cmakeReplyDirectory().toString()) {
+    if (directory == cmakeReplyDirectory(m_buildDirectory).toString()) {
-        QFileInfo fi = scanForCMakeReplyFile();
+        QFileInfo fi = scanForCMakeReplyFile(m_buildDirectory);
         if (fi.isFile() && fi.filePath() != m_lastParsedReplyFile) {
             emit dirty();
         }

src/plugins/cmakeprojectmanager/fileapiparser.h

@@ -246,29 +246,25 @@ class FileApiParser final : public QObject
     Q_OBJECT
 
 public:
-    FileApiParser(const Utils::FilePath &sourceDirectory, const Utils::FilePath &buildDirectory);
+    FileApiParser(const Utils::FilePath &buildDirectory);
     ~FileApiParser() final;
 
-    Utils::FilePath cmakeReplyDirectory() const;
-    QFileInfo scanForCMakeReplyFile() const;
-
-    QStringList cmakeQueryFileNames() const;
-    QStringList cmakeQueryFilePaths() const;
-
     void setParsedReplyFilePath(const QString &filePath);
 
     static FileApiData parseData(const QFileInfo &replyFileInfo, QString &errorMessage);
 
+    static bool setupCMakeFileApi(const Utils::FilePath &buildDirectory,
+                                  Utils::FileSystemWatcher &watcher);
+
+    static QStringList cmakeQueryFilePaths(const Utils::FilePath &buildDirectory);
+
+    static QFileInfo scanForCMakeReplyFile(const Utils::FilePath &buildDirectory);
+
 signals:
-    void dataAvailable() const;
-    void errorOccurred(const QString &message) const;
     void dirty() const;
 
 private:
-    void setupCMakeFileApi() const;
+    Utils::FilePath m_buildDirectory;
-
-    const Utils::FilePath &m_sourceDirectory;
-    const Utils::FilePath &m_buildDirectory;
 
     void replyDirectoryHasChanged(const QString &directory) const;
     Utils::FileSystemWatcher m_watcher;

src/plugins/cmakeprojectmanager/fileapireader.cpp

@@ -71,7 +71,7 @@ void FileApiReader::setParameters(const BuildDirParameters &p)
 
     resetData();
 
-    m_fileApi = std::make_unique<FileApiParser>(m_parameters.sourceDirectory, m_parameters.workDirectory);
+    m_fileApi = std::make_unique<FileApiParser>(m_parameters.workDirectory);
     connect(m_fileApi.get(), &FileApiParser::dirty, this, [this]() {
         if (!m_isParsing)
             emit dirty();
@@ -106,13 +106,13 @@ void FileApiReader::parse(bool forceCMakeRun, bool forceConfiguration)
         return;
     }
 
-    const QFileInfo replyFi = m_fileApi->scanForCMakeReplyFile();
+    const QFileInfo replyFi = m_fileApi->scanForCMakeReplyFile(m_parameters.workDirectory);
     // Only need to update when one of the following conditions is met:
     //  * The user forces the update,
     //  * There is no reply file,
     //  * One of the cmakefiles is newer than the replyFile and the user asked
     //    for creator to run CMake as needed,
-    //  * A query files are newer than the reply file
+    //  * A query file is newer than the reply file
     const bool mustUpdate = forceCMakeRun || !replyFi.exists()
                             || (m_parameters.cmakeTool() && m_parameters.cmakeTool()->isAutoRun()
                                 && anyOf(m_cmakeFiles,
@@ -120,7 +120,7 @@ void FileApiReader::parse(bool forceCMakeRun, bool forceConfiguration)
                                              return f.toFileInfo().lastModified()
                                                     > replyFi.lastModified();
                                          }))
-                            || anyOf(m_fileApi->cmakeQueryFilePaths(), [&replyFi](const QString &qf) {
+                            || anyOf(FileApiParser::cmakeQueryFilePaths(m_parameters.workDirectory), [&replyFi](const QString &qf) {
                                    return QFileInfo(qf).lastModified() > replyFi.lastModified();
                                });
 
@@ -267,7 +267,7 @@ void FileApiReader::cmakeFinishedState(int code, QProcess::ExitStatus status)
 
     m_cmakeProcess.release()->deleteLater();
 
-    endState(m_fileApi->scanForCMakeReplyFile());
+    endState(m_fileApi->scanForCMakeReplyFile(m_parameters.workDirectory));
 }
 
 } // namespace Internal