Fix redundant std::optional conversions

...proposed by clang-tidy.
Enable bugprone-optional-value-conversion check.

15 files changed, 30 insertions, 44 deletions

diff --git a/.clang-tidy b/.clang-tidy
index 4fff4a14..f97cae27 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -1,6 +1,6 @@
 FormatStyle: Google
 Checks: '*,-abseil-*,-altera-*,-android-*,-boost-*,-cert-*,-darwin-*,-fuchsia-*,-linuxkernel-*,-llvm-*,-llvmlibc-*,-mpi-*,-objc-*,-zircon-*'
-WarningsAsErrors: 'clang-diagnostic-*,-clang-diagnostic-unused-command-line-argument,clang-analyzer-*,-clang-analyzer-cplusplus.NewDeleteLeaks,-clang-analyzer-cplusplus.StringChecker,bugprone-*,-bugprone-easily-swappable-parameters,-bugprone-unchecked-optional-access,-bugprone-narrowing-conversions,-bugprone-unhandled-exception-at-new,-bugprone-empty-catch,-bugprone-optional-value-conversion,-bugprone-implicit-widening-of-multiplication-result,-bugprone-exception-escape,portability-*,hicpp-*,performance-*,-performance-avoid-endl,-performance-enum-size'
+WarningsAsErrors: 'clang-diagnostic-*,-clang-diagnostic-unused-command-line-argument,clang-analyzer-*,-clang-analyzer-cplusplus.NewDeleteLeaks,-clang-analyzer-cplusplus.StringChecker,bugprone-*,-bugprone-easily-swappable-parameters,-bugprone-unchecked-optional-access,-bugprone-narrowing-conversions,-bugprone-unhandled-exception-at-new,-bugprone-empty-catch,-bugprone-implicit-widening-of-multiplication-result,-bugprone-exception-escape,portability-*,hicpp-*,performance-*,-performance-avoid-endl,-performance-enum-size'
 CheckOptions: [
   {
     key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic,
diff --git a/src/buildtool/build_engine/expression/expression.hpp b/src/buildtool/build_engine/expression/expression.hpp
index cbfb2f24..e5788e38 100644
--- a/src/buildtool/build_engine/expression/expression.hpp
+++ b/src/buildtool/build_engine/expression/expression.hpp
@@ -152,11 +152,7 @@ class Expression {
 
     [[nodiscard]] auto At(
         std::string const& key) && -> std::optional<ExpressionPtr> {
-        auto value = std::move(*this).Map().Find(key);
-        if (value) {
-            return std::move(*value);
-        }
-        return std::nullopt;
+        return std::move(*this).Map().Find(key);
     }
 
     template <class T>
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 58059156..0bcd11cc 100644
--- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp
+++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp
@@ -383,9 +383,7 @@ auto BazelMsgFactory::CreateDirectoryDigestFromLocalTree(
         auto dir = CreateDirectory(files, dirs, symlinks);
         if (auto bytes = SerializeMessage(dir)) {
             try {
-                if (auto digest = store_dir(*bytes)) {
-                    return *digest;
-                }
+                return store_dir(*bytes);
             } catch (std::exception const& ex) {
                 Logger::Log(LogLevel::Error,
                             "storing directory failed with:\n{}",
@@ -468,9 +466,7 @@ auto BazelMsgFactory::CreateGitTreeDigestFromLocalTree(
             root, dir_reader, /*allow_upwards=*/true)) {
         if (auto tree = GitRepo::CreateShallowTree(entries)) {
             try {
-                if (auto digest = store_tree(tree->second)) {
-                    return *digest;
-                }
+                return store_tree(tree->second);
             } catch (std::exception const& ex) {
                 Logger::Log(LogLevel::Error,
                             "storing tree failed with:\n{}",
diff --git a/src/buildtool/file_system/git_repo.cpp b/src/buildtool/file_system/git_repo.cpp
index 791d5996..f3769008 100644
--- a/src/buildtool/file_system/git_repo.cpp
+++ b/src/buildtool/file_system/git_repo.cpp
@@ -1686,7 +1686,7 @@ auto GitRepo::GetObjectByPathFromTree(std::string const& tree_id,
                     GetGitCAS()->ReadObject(entry_id, /*is_hex_id=*/true)) {
                 return TreeEntryInfo{.id = entry_id,
                                      .type = entry_type,
-                                     .symlink_content = *target};
+                                     .symlink_content = std::move(target)};
             }
             Logger::Log(
                 LogLevel::Trace,
@@ -1948,11 +1948,7 @@ auto GitRepo::CreateTree(tree_entries_t const& entries) const noexcept
                         GitLastError());
             return std::nullopt;
         }
-        auto raw_id = ToRawString(oid);
-        if (not raw_id) {
-            return std::nullopt;
-        }
-        return std::move(*raw_id);
+        return ToRawString(oid);
     } catch (std::exception const& ex) {
         Logger::Log(
             LogLevel::Error, "creating tree failed with:\n{}", ex.what());
@@ -2104,9 +2100,7 @@ auto GitRepo::CreateTreeFromDirectory(std::filesystem::path const& dir,
     };
 
     if (ReadDirectory(dir, dir_read_and_store, logger)) {
-        if (auto raw_id = CreateTree(entries)) {
-            return *raw_id;
-        }
+        return CreateTree(entries);
     }
     return std::nullopt;
 #endif  // BOOTSTRAP_BUILD_TOOL
diff --git a/src/buildtool/file_system/symlinks_map/resolve_symlinks_map.cpp b/src/buildtool/file_system/symlinks_map/resolve_symlinks_map.cpp
index 86861f2f..f130df71 100644
--- a/src/buildtool/file_system/symlinks_map/resolve_symlinks_map.cpp
+++ b/src/buildtool/file_system/symlinks_map/resolve_symlinks_map.cpp
@@ -157,7 +157,7 @@ void ResolveKnownEntry(GitObjectToResolve const& obj,
                                 std::make_optional(GitRepo::TreeEntryInfo{
                                     .id = ToHexString(raw_id),
                                     .type = e.type,
-                                    .symlink_content = *target}),
+                                    .symlink_content = std::move(target)}),
                                 obj.source_cas,
                                 obj.target_cas);
                         }
diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp
index ea2f594e..51cb23d6 100644
--- a/src/buildtool/graph_traverser/graph_traverser.hpp
+++ b/src/buildtool/graph_traverser/graph_traverser.hpp
@@ -574,7 +574,7 @@ class GraphTraverser {
             return std::nullopt;
         }
 
-        return std::move(*output_paths);
+        return output_paths;
     }
 
     void PrintOutputs(
diff --git a/src/buildtool/system/system_command.hpp b/src/buildtool/system/system_command.hpp
index 6f1d80ce..ade2f54b 100644
--- a/src/buildtool/system/system_command.hpp
+++ b/src/buildtool/system/system_command.hpp
@@ -120,7 +120,7 @@ class SystemCommand {
             if (auto const err = OpenFile(stderr_file)) {
                 if (auto retval = ForkAndExecute(
                         cmd, envp, cwd, fileno(out.get()), fileno(err.get()))) {
-                    return *retval;
+                    return retval;
                 }
             }
             else {
diff --git a/src/other_tools/git_operations/git_config_settings.cpp b/src/other_tools/git_operations/git_config_settings.cpp
index c820aa32..b0fe27bc 100644
--- a/src/other_tools/git_operations/git_config_settings.cpp
+++ b/src/other_tools/git_operations/git_config_settings.cpp
@@ -327,7 +327,7 @@ auto GitConfigSettings::GetProxySettings(std::shared_ptr<git_config> const& cfg,
                                     true /*fatal*/);
                                 return std::nullopt;
                             }
-                            return proxy_info.value();
+                            return proxy_info;
                         }
                     }
                     // check the generic "http.proxy" gitconfig entry;
@@ -348,7 +348,7 @@ auto GitConfigSettings::GetProxySettings(std::shared_ptr<git_config> const& cfg,
                                     true /*fatal*/);
                                 return std::nullopt;
                             }
-                            return proxy_info.value();
+                            return proxy_info;
                         }
                         // cleanup memory
                         git_buf_dispose(&tmp_buf);
@@ -376,7 +376,7 @@ auto GitConfigSettings::GetProxySettings(std::shared_ptr<git_config> const& cfg,
                                     true /*fatal*/);
                                 return std::nullopt;
                             }
-                            return proxy_info.value();
+                            return proxy_info;
                         }
                         // check HTTPS_PROXY envariable
                         if (const char* envar = std::getenv("HTTPS_PROXY")) {
@@ -390,7 +390,7 @@ auto GitConfigSettings::GetProxySettings(std::shared_ptr<git_config> const& cfg,
                                     true /*fatal*/);
                                 return std::nullopt;
                             }
-                            return proxy_info.value();
+                            return proxy_info;
                         }
                     }
                     else if (url_scheme.value() == "http") {
@@ -406,7 +406,7 @@ auto GitConfigSettings::GetProxySettings(std::shared_ptr<git_config> const& cfg,
                                     true /*fatal*/);
                                 return std::nullopt;
                             }
-                            return proxy_info.value();
+                            return proxy_info;
                         }
                     }
                     // check all_proxy envariable
@@ -421,7 +421,7 @@ auto GitConfigSettings::GetProxySettings(std::shared_ptr<git_config> const& cfg,
                                 true /*fatal*/);
                             return std::nullopt;
                         }
-                        return proxy_info.value();
+                        return proxy_info;
                     }
                     // check ALL_PROXY envariable
                     if (const char* envar = std::getenv("ALL_PROXY")) {
@@ -435,7 +435,7 @@ auto GitConfigSettings::GetProxySettings(std::shared_ptr<git_config> const& cfg,
                                 true /*fatal*/);
                             return std::nullopt;
                         }
-                        return proxy_info.value();
+                        return proxy_info;
                     }
                 }
             }
diff --git a/src/other_tools/git_operations/git_operations.cpp b/src/other_tools/git_operations/git_operations.cpp
index 5135743f..be6d9ce1 100644
--- a/src/other_tools/git_operations/git_operations.cpp
+++ b/src/other_tools/git_operations/git_operations.cpp
@@ -63,7 +63,7 @@ auto CriticalGitOps::GitInitialCommit(GitOpParams const& crit_op_params,
         return {.git_cas = nullptr, .result = std::nullopt};
     }
     // success
-    return {.git_cas = git_repo->GetGitCAS(), .result = commit_hash.value()};
+    return {.git_cas = git_repo->GetGitCAS(), .result = std::move(commit_hash)};
 }
 
 auto CriticalGitOps::GitEnsureInit(GitOpParams const& crit_op_params,
@@ -132,7 +132,7 @@ auto CriticalGitOps::GitKeepTag(GitOpParams const& crit_op_params,
         return {.git_cas = nullptr, .result = std::nullopt};
     }
     // success
-    return {.git_cas = git_repo->GetGitCAS(), .result = *tag_result};
+    return {.git_cas = git_repo->GetGitCAS(), .result = std::move(tag_result)};
 }
 
 auto CriticalGitOps::GitGetHeadId(GitOpParams const& crit_op_params,
@@ -165,7 +165,7 @@ auto CriticalGitOps::GitGetHeadId(GitOpParams const& crit_op_params,
         return {.git_cas = nullptr, .result = std::nullopt};
     }
     // success
-    return {.git_cas = git_repo->GetGitCAS(), .result = *head_commit};
+    return {.git_cas = git_repo->GetGitCAS(), .result = std::move(head_commit)};
 }
 
 auto CriticalGitOps::GitKeepTree(GitOpParams const& crit_op_params,
@@ -208,5 +208,5 @@ auto CriticalGitOps::GitKeepTree(GitOpParams const& crit_op_params,
         return {.git_cas = nullptr, .result = std::nullopt};
     }
     // success
-    return {.git_cas = git_repo->GetGitCAS(), .result = *tag_result};
+    return {.git_cas = git_repo->GetGitCAS(), .result = std::move(tag_result)};
 }
diff --git a/src/other_tools/root_maps/root_utils.cpp b/src/other_tools/root_maps/root_utils.cpp
index 4173a00d..9dd7badf 100644
--- a/src/other_tools/root_maps/root_utils.cpp
+++ b/src/other_tools/root_maps/root_utils.cpp
@@ -28,7 +28,7 @@ auto CheckServeHasAbsentRoot(ServeApi const& serve,
                              AsyncMapConsumerLoggerPtr const& logger)
     -> std::optional<bool> {
     if (auto has_tree = serve.CheckRootTree(tree_id)) {
-        return *has_tree;
+        return has_tree;
     }
     (*logger)(fmt::format("Checking that the serve endpoint knows tree "
                           "{} failed.",
diff --git a/src/utils/archive/archive_ops.cpp b/src/utils/archive/archive_ops.cpp
index 8e70c814..14bbd648 100644
--- a/src/utils/archive/archive_ops.cpp
+++ b/src/utils/archive/archive_ops.cpp
@@ -255,7 +255,7 @@ auto ArchiveOps::CreateArchive(ArchiveType type,
         // enable the correct format for archive type
         auto res = EnableWriteFormats(a_out.get(), type);
         if (res != std::nullopt) {
-            return *res;
+            return res;
         }
         // open archive to write
         if (not FileSystemManager::CreateDirectory(destDir)) {
@@ -307,7 +307,7 @@ auto ArchiveOps::CreateArchive(ArchiveType type,
             // write entry into archive
             auto res = WriteEntry(entry.get(), a_out.get());
             if (res != std::nullopt) {
-                return *res;
+                return res;
             }
         }
     } catch (std::exception const& ex) {
@@ -343,7 +343,7 @@ auto ArchiveOps::ExtractArchive(ArchiveType type,
         // enable support for known formats
         auto res = EnableReadFormats(a_in.get(), type);
         if (res != std::nullopt) {
-            return *res;
+            return res;
         }
         // open archive for reading
         if (archive_read_open_filename(
@@ -392,7 +392,7 @@ auto ArchiveOps::ExtractArchive(ArchiveType type,
             if (archive_entry_size(entry) > 0) {
                 auto res = CopyData(a_in.get(), disk.get());
                 if (res != std::nullopt) {
-                    return *res;
+                    return res;
                 }
             }
             // finish entry writing
diff --git a/test/buildtool/file_system/git_repo.test.cpp b/test/buildtool/file_system/git_repo.test.cpp
index 73d37e1d..66293fba 100644
--- a/test/buildtool/file_system/git_repo.test.cpp
+++ b/test/buildtool/file_system/git_repo.test.cpp
@@ -78,7 +78,7 @@ class TestUtils {
                                               : (*repo_path / ".git").string()),
                         QuoteForShell(repo_path->string()));
         if (std::system(cmd.c_str()) == 0) {
-            return *repo_path;
+            return repo_path;
         }
         return std::nullopt;
     }
diff --git a/test/buildtool/file_system/resolve_symlinks_map.test.cpp b/test/buildtool/file_system/resolve_symlinks_map.test.cpp
index d1cd7692..01865f6d 100644
--- a/test/buildtool/file_system/resolve_symlinks_map.test.cpp
+++ b/test/buildtool/file_system/resolve_symlinks_map.test.cpp
@@ -103,7 +103,7 @@ auto const kBazId = std::string{"27b32561185c2825150893774953906c6daa6798"};
                                           : (*repo_path / ".git").string()),
                     QuoteForShell(repo_path->string()));
     if (std::system(cmd.c_str()) == 0) {
-        return *repo_path;
+        return repo_path;
     }
     return std::nullopt;
 }
diff --git a/test/other_tools/git_operations/critical_git_ops.test.cpp b/test/other_tools/git_operations/critical_git_ops.test.cpp
index c1494bcf..c15842e7 100644
--- a/test/other_tools/git_operations/critical_git_ops.test.cpp
+++ b/test/other_tools/git_operations/critical_git_ops.test.cpp
@@ -80,7 +80,7 @@ class TestUtilsMP {
                                               : (*repo_path / ".git").string()),
                         QuoteForShell(repo_path->string()));
         if (std::system(cmd.c_str()) == 0) {
-            return *repo_path;
+            return repo_path;
         }
         return std::nullopt;
     }
diff --git a/test/other_tools/git_operations/git_repo_remote.test.cpp b/test/other_tools/git_operations/git_repo_remote.test.cpp
index f4837b47..a4f0daa6 100644
--- a/test/other_tools/git_operations/git_repo_remote.test.cpp
+++ b/test/other_tools/git_operations/git_repo_remote.test.cpp
@@ -69,7 +69,7 @@ class TestUtils {
                                               : (*repo_path / ".git").string()),
                         QuoteForShell(repo_path->string()));
         if (std::system(cmd.c_str()) == 0) {
-            return *repo_path;
+            return repo_path;
         }
         return std::nullopt;
     }