Apply changes suggested by clang-tidy 11

12 files changed, 108 insertions, 77 deletions

diff --git a/src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp b/src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp
index 7005f129..2b8cb677 100644
--- a/src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp
+++ b/src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp
@@ -174,7 +174,8 @@ class BlobContainer {
             }
 
           private:
-            static inline BazelBlob kEmpty{bazel_re::Digest{}, std::string{}};
+            static inline BazelBlob const kEmpty{bazel_re::Digest{},
+                                                 std::string{}};
             gsl::not_null<underlaying_map_t const*> blobs_;
         };
 
diff --git a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp
index ff8614f5..9534ab06 100644
--- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp
+++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp
@@ -361,6 +361,7 @@ class DirectoryTree {
     }
 
     /// \brief Convert tree to `DirectoryNodeBundle`.
+    /// NOLINTNEXTLINE(misc-no-recursion)
     [[nodiscard]] auto ToBundle(
         std::string const& root_name,
         std::optional<BazelMsgFactory::BlobStoreFunc> const& store_blob,
@@ -410,6 +411,7 @@ class DirectoryTree {
     using Node = std::variant<DirectoryTreePtr, Artifact const*>;
     std::unordered_map<std::string, Node> nodes;
 
+    // NOLINTNEXTLINE(misc-no-recursion)
     [[nodiscard]] auto AddArtifact(std::filesystem::path::iterator* begin,
                                    std::filesystem::path::iterator const& end,
                                    Artifact const* artifact) -> bool {
diff --git a/src/buildtool/execution_api/common/execution_common.hpp b/src/buildtool/execution_api/common/execution_common.hpp
index 8b6aea40..1cf198fe 100644
--- a/src/buildtool/execution_api/common/execution_common.hpp
+++ b/src/buildtool/execution_api/common/execution_common.hpp
@@ -60,7 +60,7 @@
     return dist(urandom);
 }
 
-static auto kRandomConstant = GetNonDeterministicRandomNumber();
+static auto const kRandomConstant = GetNonDeterministicRandomNumber();
 
 static void EncodeUUIDVersion4(std::string* uuid) {
     constexpr auto kVersionByte = 6UL;
diff --git a/src/buildtool/execution_api/local/config.hpp b/src/buildtool/execution_api/local/config.hpp
index 5f3a2a80..3f293b95 100644
--- a/src/buildtool/execution_api/local/config.hpp
+++ b/src/buildtool/execution_api/local/config.hpp
@@ -19,6 +19,24 @@
 
 /// \brief Store global build system configuration.
 class LocalExecutionConfig {
+    struct ConfigData {
+        // User root directory (Unix default: /home/${USER})
+        std::filesystem::path user_root{};
+
+        // Build root directory (default: empty)
+        std::filesystem::path build_root{};
+
+        // Disk cache directory (default: empty)
+        std::filesystem::path disk_cache{};
+
+        // Launcher to be prepended to action's command before executed.
+        // Default: ["env", "--"]
+        std::vector<std::string> launcher{"env", "--"};
+
+        // Persistent build directory option
+        bool keep_build_dir{false};
+    };
+
   public:
     [[nodiscard]] static auto SetBuildRoot(
         std::filesystem::path const& dir) noexcept -> bool {
@@ -28,7 +46,7 @@ class LocalExecutionConfig {
                         dir.string());
             return false;
         }
-        build_root_ = dir;
+        Data().build_root = dir;
         return true;
     }
 
@@ -40,14 +58,14 @@ class LocalExecutionConfig {
                         dir.string());
             return false;
         }
-        disk_cache_ = dir;
+        Data().disk_cache = dir;
         return true;
     }
 
     [[nodiscard]] static auto SetLauncher(
         std::vector<std::string> const& launcher) noexcept -> bool {
         try {
-            launcher_ = launcher;
+            Data().launcher = launcher;
         } catch (std::exception const& e) {
             Logger::Log(LogLevel::Error,
                         "when setting the local launcher\n{}",
@@ -59,63 +77,54 @@ class LocalExecutionConfig {
 
     [[nodiscard]] static auto SetKeepBuildDir(bool is_persistent) noexcept
         -> bool {
-        keep_build_dir_ = is_persistent;
+        Data().keep_build_dir = is_persistent;
         return true;
     }
 
     /// \brief User directory.
     [[nodiscard]] static auto GetUserDir() noexcept -> std::filesystem::path {
-        if (user_root_.empty()) {
-            user_root_ = GetUserRoot() / ".cache" / "just";
+        auto& user_root = Data().user_root;
+        if (user_root.empty()) {
+            user_root = GetUserRoot() / ".cache" / "just";
         }
-        return user_root_;
+        return user_root;
     }
 
     /// \brief Build directory, defaults to user directory if not set
     [[nodiscard]] static auto GetBuildDir() noexcept -> std::filesystem::path {
-        if (build_root_.empty()) {
+        auto& build_root = Data().build_root;
+        if (build_root.empty()) {
             return GetUserDir();
         }
-        return build_root_;
+        return build_root;
     }
 
     /// \brief Cache directory, defaults to user directory if not set
     [[nodiscard]] static auto GetCacheDir() noexcept -> std::filesystem::path {
-        if (disk_cache_.empty()) {
+        auto& disk_cache = Data().disk_cache;
+        if (disk_cache.empty()) {
             return GetBuildDir();
         }
-        return disk_cache_;
+        return disk_cache;
     }
 
     [[nodiscard]] static auto GetLauncher() noexcept
         -> std::vector<std::string> {
-        return launcher_;
+        return Data().launcher;
     }
 
     [[nodiscard]] static auto KeepBuildDir() noexcept -> bool {
-        return keep_build_dir_;
+        return Data().keep_build_dir;
     }
 
   private:
-    // User root directory (Unix default: /home/${USER})
-    static inline std::filesystem::path user_root_{};
-
-    // Build root directory (default: empty)
-    static inline std::filesystem::path build_root_{};
-
-    // Disk cache directory (default: empty)
-    static inline std::filesystem::path disk_cache_{};
-
-    // Launcher to be prepended to action's command before executed.
-    // Default: ["env", "--"]
-    static inline std::vector<std::string> launcher_{"env", "--"};
-
-    // Persistent build directory option
-    static inline bool keep_build_dir_{false};
+    [[nodiscard]] static auto Data() noexcept -> ConfigData& {
+        static ConfigData instance{};
+        return instance;
+    }
 
     /// \brief Determine user root directory
-    [[nodiscard]] static inline auto GetUserRoot() noexcept
-        -> std::filesystem::path {
+    [[nodiscard]] static auto GetUserRoot() noexcept -> std::filesystem::path {
         char const* root{nullptr};
 
 #ifdef __unix__
diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp
index 0c71a84b..d8ec8be3 100644
--- a/src/buildtool/execution_api/local/local_action.cpp
+++ b/src/buildtool/execution_api/local/local_action.cpp
@@ -184,21 +184,24 @@ auto LocalAction::CreateDirectoryStructure(
     }
 
     // create output paths
-    for (auto const& local_path : output_files_) {
-        if (not FileSystemManager::CreateDirectory(
-                (exec_path / local_path).parent_path())) {
+    auto const create_dir = [this](auto const& dir) {
+        if (not FileSystemManager::CreateDirectory(dir)) {
             logger_.Emit(LogLevel::Error, "failed to create output directory");
             return false;
         }
-    }
-    for (auto const& local_path : output_dirs_) {
-        if (not FileSystemManager::CreateDirectory(exec_path / local_path)) {
-            logger_.Emit(LogLevel::Error, "failed to create output directory");
-            return false;
-        }
-    }
-
-    return true;
+        return true;
+    };
+    return std::all_of(output_files_.begin(),
+                       output_files_.end(),
+                       [&exec_path, &create_dir](auto const& local_path) {
+                           auto dir = (exec_path / local_path).parent_path();
+                           return create_dir(dir);
+                       }) and
+           std::all_of(output_dirs_.begin(),
+                       output_dirs_.end(),
+                       [&exec_path, &create_dir](auto const& local_path) {
+                           return create_dir(exec_path / local_path);
+                       });
 }
 
 auto LocalAction::CollectOutputFile(std::filesystem::path const& exec_path,
diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp
index 96b96416..ebbb10ef 100644
--- a/src/buildtool/execution_api/local/local_api.hpp
+++ b/src/buildtool/execution_api/local/local_api.hpp
@@ -34,6 +34,7 @@ class LocalApi final : public IExecutionApi {
                                                      properties}};
     }
 
+    // NOLINTNEXTLINE(misc-no-recursion)
     [[nodiscard]] auto RetrieveToPaths(
         std::vector<Artifact::ObjectInfo> const& artifacts_info,
         std::vector<std::filesystem::path> const& output_paths) noexcept
diff --git a/src/buildtool/execution_api/local/local_storage.cpp b/src/buildtool/execution_api/local/local_storage.cpp
index c007fc1d..5e4115df 100644
--- a/src/buildtool/execution_api/local/local_storage.cpp
+++ b/src/buildtool/execution_api/local/local_storage.cpp
@@ -23,7 +23,8 @@ namespace {
     gsl::not_null<FILE*> const& stream) noexcept -> bool {
     if (auto dir = ReadDirectory(storage, tree_digest)) {
         if (auto data = BazelMsgFactory::DirectoryToString(*dir)) {
-            std::fwrite(data->data(), 1, data->size(), stream);
+            auto const& str = *data;
+            std::fwrite(str.data(), 1, str.size(), stream);
             return true;
         }
     }
@@ -71,6 +72,7 @@ auto LocalStorage::ReadTreeInfos(
     return std::nullopt;
 }
 
+// NOLINTNEXTLINE(misc-no-recursion)
 auto LocalStorage::ReadObjectInfosRecursively(
     BazelMsgFactory::InfoStoreFunc const& store_info,
     std::filesystem::path const& parent,
@@ -79,19 +81,22 @@ auto LocalStorage::ReadObjectInfosRecursively(
     if (tree_map_) {
         auto const* tree = tree_map_->GetTree(digest);
         if (tree != nullptr) {
-            for (auto const& [path, info] : *tree) {
-                try {
-                    if (IsTreeObject(info->type)
-                            ? not ReadObjectInfosRecursively(
-                                  store_info, parent / path, info->digest)
-                            : not store_info(parent / path, *info)) {
+            return std::all_of(
+                tree->begin(),
+                tree->end(),
+                // NOLINTNEXTLINE(misc-no-recursion)
+                [this, &store_info, &parent](auto const& entry) {
+                    try {
+                        auto const& [path, info] = entry;
+                        return IsTreeObject(info->type)
+                                   ? ReadObjectInfosRecursively(store_info,
+                                                                parent / path,
+                                                                info->digest)
+                                   : store_info(parent / path, *info);
+                    } catch (...) {  // satisfy clang-tidy, store_info() could
                         return false;
                     }
-                } catch (...) {  // satisfy clang-tidy, store_info() could throw
-                    return false;
-                }
-            }
-            return true;
+                });
         }
         Logger::Log(
             LogLevel::Debug, "tree {} not found in tree map", digest.hash());
diff --git a/src/buildtool/execution_api/local/local_storage.hpp b/src/buildtool/execution_api/local/local_storage.hpp
index a071a5d5..568d412d 100644
--- a/src/buildtool/execution_api/local/local_storage.hpp
+++ b/src/buildtool/execution_api/local/local_storage.hpp
@@ -52,6 +52,7 @@ class LocalStorage {
     }
 
     /// \brief Obtain blob path from digest with x-bit.
+    /// NOLINTNEXTLINE(misc-no-recursion)
     [[nodiscard]] auto BlobPath(bazel_re::Digest const& digest,
                                 bool is_executable) const noexcept
         -> std::optional<std::filesystem::path> {
@@ -91,6 +92,7 @@ class LocalStorage {
     /// \param digest        Blob digest.
     /// \param to_executable Sync direction.
     /// \returns Path to blob in target CAS.
+    /// NOLINTNEXTLINE(misc-no-recursion)
     [[nodiscard]] auto TrySyncBlob(bazel_re::Digest const& digest,
                                    bool to_executable) const noexcept
         -> std::optional<std::filesystem::path> {
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
index 05d9c357..28425400 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
@@ -53,6 +53,7 @@ auto BazelApi::CreateAction(
                                                         properties}};
 }
 
+// NOLINTNEXTLINE(misc-no-recursion)
 [[nodiscard]] auto BazelApi::RetrieveToPaths(
     std::vector<Artifact::ObjectInfo> const& artifacts_info,
     std::vector<std::filesystem::path> const& output_paths) noexcept -> bool {
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_cas_client.cpp b/src/buildtool/execution_api/remote/bazel/bazel_cas_client.cpp
index 87ceb4ae..da8b518c 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_cas_client.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_cas_client.cpp
@@ -94,6 +94,7 @@ auto BazelCasClient::BatchReadBlobs(
     return result;
 }
 
+// NOLINTNEXTLINE(misc-no-recursion)
 auto BazelCasClient::GetTree(std::string const& instance_name,
                              bazel_re::Digest const& root_digest,
                              std::int32_t page_size,
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
index ea502480..ee31d5ee 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
@@ -26,7 +26,8 @@ namespace {
     gsl::not_null<FILE*> const& stream) noexcept -> bool {
     if (auto dir = ReadDirectory(network, tree_digest)) {
         if (auto data = BazelMsgFactory::DirectoryToString(*dir)) {
-            std::fwrite(data->data(), 1, data->size(), stream);
+            auto const& str = *data;
+            std::fwrite(str.data(), 1, str.size(), stream);
             return true;
         }
     }
@@ -40,7 +41,8 @@ namespace {
     auto reader = network->IncrementalReadSingleBlob(blob_digest);
     auto data = reader.Next();
     while (data and not data->empty()) {
-        std::fwrite(data->data(), 1, data->size(), stream);
+        auto const& str = *data;
+        std::fwrite(str.data(), 1, str.size(), stream);
         data = reader.Next();
     }
     return data.has_value();
@@ -242,6 +244,7 @@ auto BazelNetwork::ReadTreeInfos(bazel_re::Digest const& tree_digest,
     return std::nullopt;
 }
 
+// NOLINTNEXTLINE(misc-no-recursion)
 auto BazelNetwork::ReadObjectInfosRecursively(
     std::optional<DirectoryMap> const& dir_map,
     BazelMsgFactory::InfoStoreFunc const& store_info,
@@ -251,21 +254,24 @@ auto BazelNetwork::ReadObjectInfosRecursively(
     if (tree_map_) {
         auto const* tree = tree_map_->GetTree(digest);
         if (tree != nullptr) {
-            for (auto const& [path, info] : *tree) {
-                try {
-                    if (IsTreeObject(info->type)
-                            ? not ReadObjectInfosRecursively(dir_map,
-                                                             store_info,
-                                                             parent / path,
-                                                             info->digest)
-                            : not store_info(parent / path, *info)) {
+            return std::all_of(
+                tree->begin(),
+                tree->end(),
+                // NOLINTNEXTLINE(misc-no-recursion)
+                [this, &dir_map, &store_info, &parent](auto const& entry) {
+                    auto const& [path, info] = entry;
+                    try {
+                        return IsTreeObject(info->type)
+                                   ? ReadObjectInfosRecursively(dir_map,
+                                                                store_info,
+                                                                parent / path,
+                                                                info->digest)
+                                   : store_info(parent / path, *info);
+                    } catch (
+                        ...) {  // satisfy clang-tidy, store_info() could throw
                         return false;
                     }
-                } catch (...) {  // satisfy clang-tidy, store_info() could throw
-                    return false;
-                }
-            }
-            return true;
+                });
         }
         Logger::Log(
             LogLevel::Debug, "tree {} not found in tree map", digest.hash());
diff --git a/src/buildtool/execution_api/remote/bazel/bytestream_client.hpp b/src/buildtool/execution_api/remote/bazel/bytestream_client.hpp
index b8823236..1bd44c42 100644
--- a/src/buildtool/execution_api/remote/bazel/bytestream_client.hpp
+++ b/src/buildtool/execution_api/remote/bazel/bytestream_client.hpp
@@ -1,6 +1,7 @@
 #ifndef INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_BAZEL_BYTESTREAM_CLIENT_HPP
 #define INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_BAZEL_BYTESTREAM_CLIENT_HPP
 
+#include <algorithm>
 #include <functional>
 #include <iomanip>
 #include <optional>
@@ -156,12 +157,11 @@ class ByteStreamClient {
         std::function<std::string(T_Input const&)> const& to_resource_name,
         std::function<std::string(T_Input const&)> const& to_data)
         const noexcept -> bool {
-        for (auto const& i : inputs) {
-            if (not Write(to_resource_name(i), to_data(i))) {
-                return false;
-            }
-        }
-        return true;
+        return std::all_of(inputs.begin(),
+                           inputs.end(),
+                           [this, &to_resource_name, &to_data](auto const& i) {
+                               return Write(to_resource_name(i), to_data(i));
+                           });
     }
 
   private: