Utils: Cleanup searchInX functions

Also move some often used types into new file "utiltypes.h"

Change-Id: I3f152d1dc2f96ba0259ad6c098d9ac5ee03a59f1
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: hjk <hjk@qt.io>

src/libs/utils/environment.cpp

@@ -4,6 +4,7 @@
 #include "environment.h"
 
 #include "algorithm.h"
+#include "filepath.h"
 #include "qtcassert.h"
 
 #include <QDir>
@@ -130,36 +131,44 @@ void Environment::setupEnglishOutput()
     m_dict.set("LANGUAGE", "en_US:en");
 }
 
-static FilePath searchInDirectory(const QStringList &execs,
+using SearchResultCallback = std::function<IterationPolicy(const FilePath &)>;
+
+static IterationPolicy searchInDirectory(const SearchResultCallback &resultCallback,
+                                         const FilePaths &execs,
                                          const FilePath &directory,
-                                  QSet<FilePath> &alreadyChecked)
+                                         QSet<FilePath> &alreadyCheckedDirectories,
+                                         const FilePathPredicate &filter = {})
 {
-    const int checkedCount = alreadyChecked.count();
+    // Compare the initial size of the set with the size after insertion to check if the directory
-    alreadyChecked.insert(directory);
+    // was already checked.
+    const int initialCount = alreadyCheckedDirectories.count();
+    alreadyCheckedDirectories.insert(directory);
+    const bool wasAlreadyChecked = alreadyCheckedDirectories.count() == initialCount;
 
-    if (directory.isEmpty() || alreadyChecked.count() == checkedCount)
+    if (directory.isEmpty() || wasAlreadyChecked)
-        return FilePath();
+        return IterationPolicy::Continue;
 
-    for (const QString &exec : execs) {
+    for (const FilePath &exec : execs) {
-        const FilePath filePath = directory.pathAppended(exec);
+        const FilePath filePath = directory / exec.path();
-        if (filePath.isExecutableFile())
+        if (filePath.isExecutableFile() && (!filter || filter(filePath))) {
-            return filePath;
+            if (resultCallback(filePath) == IterationPolicy::Stop)
+                return IterationPolicy::Stop;
         }
-    return FilePath();
+    }
+    return IterationPolicy::Continue;
 }
 
-static QStringList appendExeExtensions(const Environment &env, const QString &executable)
+static FilePaths appendExeExtensions(const Environment &env, const FilePath &executable)
 {
-    QStringList execs(executable);
+    FilePaths execs{executable};
     if (env.osType() == OsTypeWindows) {
-        const QFileInfo fi(executable);
         // Check all the executable extensions on windows:
         // PATHEXT is only used if the executable has no extension
-        if (fi.suffix().isEmpty()) {
+        if (executable.suffix().isEmpty()) {
             const QStringList extensions = env.expandedValueForKey("PATHEXT").split(';');
 
             for (const QString &ext : extensions)
-                execs << executable + ext.toLower();
+                execs << executable.stringAppended(ext.toLower());
         }
     }
     return execs;
@@ -170,99 +179,101 @@ QString Environment::expandedValueForKey(const QString &key) const
     return expandVariables(m_dict.value(key));
 }
 
-static FilePath searchInDirectoriesHelper(const Environment &env,
+static void searchInDirectoriesHelper(const SearchResultCallback &resultCallback,
+                                      const Environment &env,
                                       const QString &executable,
                                       const FilePaths &dirs,
-                                          const Environment::PathFilter &func,
+                                      const FilePathPredicate &func,
                                       bool usePath)
 {
     if (executable.isEmpty())
-        return FilePath();
+        return;
 
-    const QString exec = QDir::cleanPath(env.expandVariables(executable));
+    const FilePath exec = FilePath::fromUserInput(QDir::cleanPath(env.expandVariables(executable)));
-    const QFileInfo fi(exec);
+    const FilePaths execs = appendExeExtensions(env, exec);
 
-    const QStringList execs = appendExeExtensions(env, exec);
+    if (exec.isAbsolutePath()) {
-
+        for (const FilePath &path : execs) {
-    if (fi.isAbsolute()) {
+            if (path.isExecutableFile() && (!func || func(path)))
-        for (const QString &path : execs) {
+                if (resultCallback(path) == IterationPolicy::Stop)
-            QFileInfo pfi = QFileInfo(path);
+                    return;
-            if (pfi.isFile() && pfi.isExecutable())
-                return FilePath::fromString(path);
         }
-        return FilePath::fromString(exec);
+        return;
     }
 
-    QSet<FilePath> alreadyChecked;
+    QSet<FilePath> alreadyCheckedDirectories;
     for (const FilePath &dir : dirs) {
-        FilePath tmp = searchInDirectory(execs, dir, alreadyChecked);
+        if (searchInDirectory(resultCallback, execs, dir, alreadyCheckedDirectories, func)
-        if (!tmp.isEmpty() && (!func || func(tmp)))
+            == IterationPolicy::Stop)
-            return tmp;
+            return;
     }
 
     if (usePath) {
-        if (executable.contains('/'))
+        QTC_ASSERT(!executable.contains('/'), return);
-            return FilePath();
 
         for (const FilePath &p : env.path()) {
-            FilePath tmp = searchInDirectory(execs, p, alreadyChecked);
+            if (searchInDirectory(resultCallback, execs, p, alreadyCheckedDirectories, func)
-            if (!tmp.isEmpty() && (!func || func(tmp)))
+                == IterationPolicy::Stop)
-                return tmp;
+                return;
         }
     }
-    return FilePath();
+    return;
 }
 
 FilePath Environment::searchInDirectories(const QString &executable,
                                           const FilePaths &dirs,
-                                          const PathFilter &func) const
+                                          const FilePathPredicate &func) const
 {
-    return searchInDirectoriesHelper(*this, executable, dirs, func, false);
+    FilePath result;
+    searchInDirectoriesHelper(
+        [&result](const FilePath &path) {
+            result = path;
+            return IterationPolicy::Stop;
+        },
+        *this,
+        executable,
+        dirs,
+        func,
+        false);
+
+    return result;
 }
 
 FilePath Environment::searchInPath(const QString &executable,
                                    const FilePaths &additionalDirs,
-                                   const PathFilter &func) const
+                                   const FilePathPredicate &func) const
 {
-    return searchInDirectoriesHelper(*this, executable, additionalDirs, func, true);
+    FilePath result;
+    searchInDirectoriesHelper(
+        [&result](const FilePath &path) {
+            result = path;
+            return IterationPolicy::Stop;
+        },
+        *this,
+        executable,
+        additionalDirs,
+        func,
+        true);
+
+    return result;
 }
 
 FilePaths Environment::findAllInPath(const QString &executable,
                                      const FilePaths &additionalDirs,
-                                        const Environment::PathFilter &func) const
+                                     const FilePathPredicate &func) const
 {
-    if (executable.isEmpty())
-        return {};
-
-    const QString exec = QDir::cleanPath(expandVariables(executable));
-    const QFileInfo fi(exec);
-
-    const QStringList execs = appendExeExtensions(*this, exec);
-
-    if (fi.isAbsolute()) {
-        for (const QString &path : execs) {
-            QFileInfo pfi = QFileInfo(path);
-            if (pfi.isFile() && pfi.isExecutable())
-                return {FilePath::fromString(path)};
-        }
-        return {FilePath::fromString(exec)};
-    }
-
     QSet<FilePath> result;
-    QSet<FilePath> alreadyChecked;
+    searchInDirectoriesHelper(
-    for (const FilePath &dir : additionalDirs) {
+        [&result](const FilePath &path) {
-        FilePath tmp = searchInDirectory(execs, dir, alreadyChecked);
+            result.insert(path);
-        if (!tmp.isEmpty() && (!func || func(tmp)))
+            return IterationPolicy::Continue;
-            result << tmp;
+        },
-    }
+        *this,
+        executable,
+        additionalDirs,
+        func,
+        true);
 
-    if (!executable.contains('/')) {
-        for (const FilePath &p : path()) {
-            FilePath tmp = searchInDirectory(execs, p, alreadyChecked);
-            if (!tmp.isEmpty() && (!func || func(tmp)))
-                result << tmp;
-        }
-    }
     return result.values();
 }
 

src/libs/utils/environment.h

@@ -8,6 +8,7 @@
 #include "environmentfwd.h"
 #include "filepath.h"
 #include "namevaluedictionary.h"
+#include "utiltypes.h"
 
 #include <functional>
 #include <optional>
@@ -56,16 +57,15 @@ public:
 
     void setupEnglishOutput();
 
-    using PathFilter = std::function<bool(const FilePath &)>;
     FilePath searchInPath(const QString &executable,
                           const FilePaths &additionalDirs = FilePaths(),
-                          const PathFilter &func = PathFilter()) const;
+                          const FilePathPredicate &func = {}) const;
     FilePath searchInDirectories(const QString &executable,
                                  const FilePaths &dirs,
-                                 const PathFilter &func = {}) const;
+                                 const FilePathPredicate &func = {}) const;
     FilePaths findAllInPath(const QString &executable,
                             const FilePaths &additionalDirs = {},
-                            const PathFilter &func = {}) const;
+                            const FilePathPredicate &func = {}) const;
 
     FilePaths path() const;
     FilePaths pathListValue(const QString &varName) const;

src/libs/utils/filepath.cpp

@@ -1451,7 +1451,7 @@ FilePath FilePath::withNewPath(const QString &newPath) const
         assert(fullPath == FilePath::fromUrl("docker://123/usr/bin/make"))
     \endcode
 */
-FilePath FilePath::searchInDirectories(const FilePaths &dirs, const PathFilter &filter) const
+FilePath FilePath::searchInDirectories(const FilePaths &dirs, const FilePathPredicate &filter) const
 {
     if (isAbsolutePath())
         return *this;
@@ -1460,7 +1460,7 @@ FilePath FilePath::searchInDirectories(const FilePaths &dirs, const PathFilter &
 
 FilePath FilePath::searchInPath(const FilePaths &additionalDirs,
                                 PathAmending amending,
-                                const PathFilter &filter) const
+                                const FilePathPredicate &filter) const
 {
     if (isAbsolutePath())
         return *this;

src/libs/utils/filepath.h

@@ -8,6 +8,7 @@
 #include "expected.h"
 #include "filepathinfo.h"
 #include "osspecificaspects.h"
+#include "utiltypes.h"
 
 #include <QDir>
 #include <QDirIterator>
@@ -51,8 +52,6 @@ public:
 
 using FilePaths = QList<class FilePath>;
 
-enum class IterationPolicy { Stop, Continue };
-
 class QTCREATOR_UTILS_EXPORT FilePath
 {
 public:
@@ -159,7 +158,7 @@ public:
     [[nodiscard]] FilePath relativeChildPath(const FilePath &parent) const;
     [[nodiscard]] FilePath relativePathFrom(const FilePath &anchor) const;
     [[nodiscard]] FilePath searchInDirectories(const FilePaths &dirs,
-                                               const PathFilter &filter = {}) const;
+                                               const FilePathPredicate &filter = {}) const;
     [[nodiscard]] Environment deviceEnvironment() const;
     [[nodiscard]] FilePath onDevice(const FilePath &deviceTemplate) const;
     [[nodiscard]] FilePath withNewPath(const QString &newPath) const;
@@ -182,7 +181,7 @@ public:
     enum PathAmending { AppendToPath, PrependToPath };
     [[nodiscard]] FilePath searchInPath(const FilePaths &additionalDirs = {},
                                         PathAmending = AppendToPath,
-                                        const PathFilter &filter = {}) const;
+                                        const FilePathPredicate &filter = {}) const;
 
     enum MatchScope { ExactMatchOnly, WithExeSuffix, WithBatSuffix,
                       WithExeOrBatSuffix, WithAnySuffix };

src/libs/utils/filesearch.cpp

@@ -9,6 +9,7 @@
 #include "qtcassert.h"
 #include "stringutils.h"
 #include "utilstr.h"
+#include "utiltypes.h"
 
 #include <QLoggingCategory>
 #include <QMutex>
@@ -496,8 +497,7 @@ static bool isFileIncluded(const QList<QRegularExpression> &filterRegs,
     return isIncluded && (exclusionRegs.isEmpty() || !matches(exclusionRegs, filePath));
 }
 
-std::function<bool(const FilePath &)> filterFileFunction(const QStringList &filters,
+FilePathPredicate filterFileFunction(const QStringList &filters, const QStringList &exclusionFilters)
-                                                         const QStringList &exclusionFilters)
 {
     const QList<QRegularExpression> filterRegs = filtersToRegExps(filters);
     const QList<QRegularExpression> exclusionRegs = filtersToRegExps(exclusionFilters);

src/libs/utils/utiltypes.h

@@ -0,0 +1,14 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0+ OR GPL-3.0 WITH Qt-GPL-exception-1.0
+
+#pragma once
+
+#include <functional>
+
+namespace Utils {
+class FilePath;
+
+enum class IterationPolicy { Stop, Continue };
+
+using FilePathPredicate = std::function<bool(const FilePath &)>;
+} // namespace Utils

src/plugins/projectexplorer/buildsettingspropertiespage.cpp

@@ -283,7 +283,7 @@ void BuildSettingsWidget::cloneConfiguration()
     bc->setDisplayName(name);
     const FilePath buildDirectory = bc->buildDirectory();
     if (buildDirectory != m_target->project()->projectDirectory()) {
-        const std::function<bool(const FilePath &)> isBuildDirOk = [this](const FilePath &candidate) {
+        const FilePathPredicate isBuildDirOk = [this](const FilePath &candidate) {
             if (candidate.exists())
                 return false;
             return !anyOf(m_target->buildConfigurations(), [&candidate](const BuildConfiguration *bc) {

tests/auto/utils/filepath/tst_filepath.cpp

@@ -107,15 +107,21 @@ private slots:
     void tmp();
     void tmp_data();
 
+    void searchInWithFilter();
+
 private:
     QTemporaryDir tempDir;
     QString rootPath;
+    QString exeExt;
 };
 
-static void touch(const QDir &dir, const QString &filename, bool fill)
+static void touch(const QDir &dir, const QString &filename, bool fill, bool executable = false)
 {
     QFile file(dir.absoluteFilePath(filename));
     file.open(QIODevice::WriteOnly);
+    if (executable)
+        file.setPermissions(file.permissions() | QFileDevice::ExeUser);
+
     if (fill) {
         QRandomGenerator *random = QRandomGenerator::global();
         for (int i = 0; i < 10; ++i)
@@ -126,7 +132,7 @@ static void touch(const QDir &dir, const QString &filename, bool fill)
 
 void tst_filepath::initTestCase()
 {
-    // initialize test for tst_fileutiles::relativePath*()
+    // initialize test for tst_filepath::relativePath*()
     QVERIFY(tempDir.isValid());
     rootPath = tempDir.path();
     QDir dir(rootPath);
@@ -141,6 +147,38 @@ void tst_filepath::initTestCase()
 
     // initialize test for tst_filepath::asyncLocalCopy()
     touch(dir, "x/y/fileToCopy.txt", true);
+
+// initialize test for tst_filepath::searchIn()
+#ifdef Q_OS_WIN
+    exeExt = ".exe";
+#endif
+
+    dir.mkpath("s/1");
+    dir.mkpath("s/2");
+    touch(dir, "s/1/testexe" + exeExt, false, true);
+    touch(dir, "s/2/testexe" + exeExt, false, true);
+}
+
+void tst_filepath::searchInWithFilter()
+{
+    const FilePaths dirs = {FilePath::fromUserInput(rootPath) / "s" / "1",
+                            FilePath::fromUserInput(rootPath) / "s" / "2"};
+
+    FilePath exe = FilePath::fromUserInput("testexe" + exeExt)
+                       .searchInDirectories(dirs, [](const FilePath &path) {
+                           return path.path().contains("/2/");
+                       });
+
+    QVERIFY(!exe.path().endsWith("/1/testexe" + exeExt)
+            && exe.path().endsWith("/2/testexe" + exeExt));
+
+    FilePath exe2 = FilePath::fromUserInput("testexe" + exeExt)
+                        .searchInDirectories(dirs, [](const FilePath &path) {
+                            return path.path().contains("/1/");
+                        });
+
+    QVERIFY(!exe2.path().endsWith("/2/testexe" + exeExt)
+            && exe2.path().endsWith("/1/testexe" + exeExt));
 }
 
 void tst_filepath::isEmpty_data()