refactor: use lock_guard instead of unique_lock when available (#398)

Follow the answer in stackoverflow
https://stackoverflow.com/a/60172828/11568166
1. `lock_guard` if you need to lock exactly 1 mutex for an entire scope.
2. `scoped_lock` if you need to lock a number of mutexes that is not
exactly 1.
3. `unique_lock` if you need to unlock within the scope of the block
(which includes use with a condition_variable).

So let clang-tidy only report `modernize-use-scoped-lock` warning when
there are multiple `lock_guard`.

Author: HuaHuaY

.clang-tidy

@@ -38,5 +38,7 @@ CheckOptions:
     value:  '_'
   - key:    readability-identifier-naming.ProtectedMemberSuffix
     value:  '_'
+  - key:    modernize-use-scoped-lock.WarnOnSingleLocks
+    value:  'false'
 
 HeaderFilterRegex: 'src/iceberg|example'

src/iceberg/catalog/memory/in_memory_catalog.cc

@@ -20,9 +20,9 @@
 #include "iceberg/catalog/memory/in_memory_catalog.h"
 
 #include <algorithm>
-#include <iterator>  // IWYU pragma: keep
+#include <iterator>
+#include <mutex>
 
-#include "iceberg/exception.h"
 #include "iceberg/table.h"
 #include "iceberg/table_metadata.h"
 #include "iceberg/util/macros.h"
@@ -337,42 +337,42 @@ std::string_view InMemoryCatalog::name() const { return catalog_name_; }
 
 Status InMemoryCatalog::CreateNamespace(
     const Namespace& ns, const std::unordered_map<std::string, std::string>& properties) {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   return root_namespace_->CreateNamespace(ns, properties);
 }
 
 Result<std::unordered_map<std::string, std::string>>
 InMemoryCatalog::GetNamespaceProperties(const Namespace& ns) const {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   return root_namespace_->GetProperties(ns);
 }
 
 Result<std::vector<Namespace>> InMemoryCatalog::ListNamespaces(
     const Namespace& ns) const {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   return root_namespace_->ListNamespaces(ns);
 }
 
 Status InMemoryCatalog::DropNamespace(const Namespace& ns) {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   return root_namespace_->DropNamespace(ns);
 }
 
 Result<bool> InMemoryCatalog::NamespaceExists(const Namespace& ns) const {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   return root_namespace_->NamespaceExists(ns);
 }
 
 Status InMemoryCatalog::UpdateNamespaceProperties(
     const Namespace& ns, const std::unordered_map<std::string, std::string>& updates,
     const std::unordered_set<std::string>& removals) {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   return root_namespace_->UpdateNamespaceProperties(ns, updates, removals);
 }
 
 Result<std::vector<TableIdentifier>> InMemoryCatalog::ListTables(
     const Namespace& ns) const {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   const auto& table_names = root_namespace_->ListTables(ns);
   ICEBERG_RETURN_UNEXPECTED(table_names);
   std::vector<TableIdentifier> table_idents;
@@ -405,12 +405,12 @@ Result<std::shared_ptr<Transaction>> InMemoryCatalog::StageCreateTable(
 }
 
 Result<bool> InMemoryCatalog::TableExists(const TableIdentifier& identifier) const {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   return root_namespace_->TableExists(identifier);
 }
 
 Status InMemoryCatalog::DropTable(const TableIdentifier& identifier, bool purge) {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   // TODO(Guotao): Delete all metadata files if purge is true.
   return root_namespace_->UnregisterTable(identifier);
 }
@@ -428,7 +428,7 @@ Result<std::unique_ptr<Table>> InMemoryCatalog::LoadTable(
 
   Result<std::string> metadata_location;
   {
-    std::unique_lock lock(mutex_);
+    std::lock_guard guard(mutex_);
     ICEBERG_ASSIGN_OR_RAISE(metadata_location,
                             root_namespace_->GetTableMetadataLocation(identifier));
   }
@@ -443,7 +443,7 @@ Result<std::unique_ptr<Table>> InMemoryCatalog::LoadTable(
 
 Result<std::shared_ptr<Table>> InMemoryCatalog::RegisterTable(
     const TableIdentifier& identifier, const std::string& metadata_file_location) {
-  std::unique_lock lock(mutex_);
+  std::lock_guard guard(mutex_);
   if (!root_namespace_->NamespaceExists(identifier.ns)) {
     return NoSuchNamespace("table namespace does not exist.");
   }

src/iceberg/catalog/rest/http_client.cc

@@ -139,7 +139,7 @@ Result<HttpResponse> HttpClient::Get(
     const ErrorHandler& error_handler) {
   cpr::Response response;
   {
-    std::scoped_lock<std::mutex> lock(session_mutex_);
+    std::lock_guard guard(session_mutex_);
     PrepareSession(path, headers, params);
     response = session_->Get();
   }
@@ -156,7 +156,7 @@ Result<HttpResponse> HttpClient::Post(
     const ErrorHandler& error_handler) {
   cpr::Response response;
   {
-    std::scoped_lock<std::mutex> lock(session_mutex_);
+    std::lock_guard guard(session_mutex_);
     PrepareSession(path, headers);
     session_->SetBody(cpr::Body{body});
     response = session_->Post();
@@ -176,7 +176,7 @@ Result<HttpResponse> HttpClient::PostForm(
   cpr::Response response;
 
   {
-    std::scoped_lock<std::mutex> lock(session_mutex_);
+    std::lock_guard guard(session_mutex_);
 
     // Override default Content-Type (application/json) with form-urlencoded
     auto form_headers = headers;
@@ -204,7 +204,7 @@ Result<HttpResponse> HttpClient::Head(
     const ErrorHandler& error_handler) {
   cpr::Response response;
   {
-    std::scoped_lock<std::mutex> lock(session_mutex_);
+    std::lock_guard guard(session_mutex_);
     PrepareSession(path, headers);
     response = session_->Head();
   }
@@ -220,7 +220,7 @@ Result<HttpResponse> HttpClient::Delete(
     const ErrorHandler& error_handler) {
   cpr::Response response;
   {
-    std::scoped_lock<std::mutex> lock(session_mutex_);
+    std::lock_guard guard(session_mutex_);
     PrepareSession(path, headers);
     response = session_->Delete();
   }

src/iceberg/table_metadata.h

@@ -419,7 +419,7 @@ class ICEBERG_EXPORT TableMetadataBuilder : public ErrorCollector {
   Result<std::unique_ptr<TableMetadata>> Build();
 
   /// \brief Destructor
-  ~TableMetadataBuilder();
+  ~TableMetadataBuilder() override;
 
   // Delete copy operations (use BuildFrom to create a new builder)
   TableMetadataBuilder(const TableMetadataBuilder&) = delete;