Add clang-tidy checks for Firestore (#1078)

* Add a .clang-tidy configuration for Firestore C++

* Fix clang-tidy warnings

  * typedef -> using
  * const ref + rvalue ref -> pass by value
  * NULL -> nullptr
  * remove useless default initializations
  * remove useless const value-type parameter declarations (definitions
    can still use them)
  * use auto instead of repeating types in a cast

* Fix typos

* Address use of static method through instance warnings

* Address use after move warnings

21 files changed, 36 insertions, 41 deletions

diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h
index 1aa76df..0a1930a 100644
--- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h
+++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h
@@ -31,10 +31,10 @@ namespace firestore {
 namespace auth {
 
 // `TokenErrorListener` is a listener that gets a token or an error.
-typedef std::function<void(util::StatusOr<Token>)> TokenListener;
+using TokenListener = std::function<void(util::StatusOr<Token>)>;
 
 // Listener notified with a User change.
-typedef std::function<void(User user)> UserChangeListener;
+using UserChangeListener = std::function<void(User user)>;
 
 /**
  * Provides methods for getting the uid and token for the current user and
diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h
index 66c3c87..0e1da31 100644
--- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h
+++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h
@@ -78,7 +78,7 @@ class FirebaseCredentialsProvider : public CredentialsProvider {
    */
   struct Contents {
     Contents(FIRApp* app, User&& user)
-        : app(app), current_user(std::move(user)), mutex() {
+        : app(app), current_user(std::move(user)) {
     }
 
     const FIRApp* app;
diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm
index 2bd3acc..ff2d5f7 100644
--- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm
+++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm
@@ -24,6 +24,7 @@
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 
 // NB: This is also defined in Firestore/Source/Public/FIRFirestoreErrors.h
+// NOLINTNEXTLINE: public constant
 NSString* const FIRFirestoreErrorDomain = @"FIRFirestoreErrorDomain";
 
 namespace firebase {
@@ -47,7 +48,7 @@ FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app)
                 std::unique_lock<std::mutex> lock(contents->mutex);
                 NSDictionary<NSString*, id>* user_info = notification.userInfo;
 
-                // ensure we're only notifiying for the current app.
+                // ensure we're only notifying for the current app.
                 FIRApp* notified_app =
                     user_info[FIRAuthStateDidChangeInternalNotificationAppKey];
                 if (![contents->app isEqual:notified_app]) {
diff --git a/Firestore/core/src/firebase/firestore/auth/token.h b/Firestore/core/src/firebase/firestore/auth/token.h
index 4b2f3aa..56084f8 100644
--- a/Firestore/core/src/firebase/firestore/auth/token.h
+++ b/Firestore/core/src/firebase/firestore/auth/token.h
@@ -42,7 +42,7 @@ namespace auth {
 // TODO(zxu123): Make this support token-type for desktop workflow.
 class Token {
  public:
-  Token(const absl::string_view token, const User& user);
+  Token(absl::string_view token, const User& user);
 
   /** The actual raw token. */
   const std::string& token() const {
diff --git a/Firestore/core/src/firebase/firestore/auth/user.cc b/Firestore/core/src/firebase/firestore/auth/user.cc
index f442d7b..4793fed 100644
--- a/Firestore/core/src/firebase/firestore/auth/user.cc
+++ b/Firestore/core/src/firebase/firestore/auth/user.cc
@@ -22,7 +22,7 @@ namespace firebase {
 namespace firestore {
 namespace auth {
 
-User::User() : uid_(), is_authenticated_(false) {
+User::User() : is_authenticated_(false) {
 }
 
 User::User(const absl::string_view uid) : uid_(uid), is_authenticated_(true) {
diff --git a/Firestore/core/src/firebase/firestore/auth/user.h b/Firestore/core/src/firebase/firestore/auth/user.h
index 3918c61..cc030cc 100644
--- a/Firestore/core/src/firebase/firestore/auth/user.h
+++ b/Firestore/core/src/firebase/firestore/auth/user.h
@@ -41,7 +41,7 @@ class User {
   User();
 
   /** Construct an authenticated user with the given UID. */
-  explicit User(const absl::string_view uid);
+  explicit User(absl::string_view uid);
 
   const std::string& uid() const {
     return uid_;
diff --git a/Firestore/core/src/firebase/firestore/core/database_info.h b/Firestore/core/src/firebase/firestore/core/database_info.h
index 2e1303e..0f97b1f 100644
--- a/Firestore/core/src/firebase/firestore/core/database_info.h
+++ b/Firestore/core/src/firebase/firestore/core/database_info.h
@@ -41,12 +41,12 @@ class DatabaseInfo {
    * @param database_id The project/database to use.
    * @param persistence_key A unique identifier for this Firestore's local
    *        storage. Usually derived from -[FIRApp appName].
-   * @param host The hostname of the datastore backend.
+   * @param host The hostname of the Firestore backend.
    * @param ssl_enabled Whether to use SSL when connecting.
    */
   DatabaseInfo(const firebase::firestore::model::DatabaseId& database_id,
-               const absl::string_view persistence_key,
-               const absl::string_view host,
+               absl::string_view persistence_key,
+               absl::string_view host,
                bool ssl_enabled);
 
   const firebase::firestore::model::DatabaseId& database_id() const {
diff --git a/Firestore/core/src/firebase/firestore/immutable/array_sorted_map.h b/Firestore/core/src/firebase/firestore/immutable/array_sorted_map.h
index 56da9e9..92fd823 100644
--- a/Firestore/core/src/firebase/firestore/immutable/array_sorted_map.h
+++ b/Firestore/core/src/firebase/firestore/immutable/array_sorted_map.h
@@ -70,8 +70,8 @@ class FixedArray {
    */
   template <typename SourceIterator>
   void append(SourceIterator src_begin, SourceIterator src_end) {
-    size_type appending = static_cast<size_type>(src_end - src_begin);
-    size_type new_size = size_ + appending;
+    auto appending = static_cast<size_type>(src_end - src_begin);
+    auto new_size = size_ + appending;
     FIREBASE_ASSERT(new_size <= fixed_size);
 
     std::copy(src_begin, src_end, end());
@@ -229,8 +229,6 @@ class ArraySortedMap : public SortedMapBase {
     }
   }
 
-  // TODO(wilhuff): indexof
-
   /** Returns true if the map contains no elements. */
   bool empty() const {
     return size() == 0;
diff --git a/Firestore/core/src/firebase/firestore/immutable/map_entry.h b/Firestore/core/src/firebase/firestore/immutable/map_entry.h
index 2130b5b..1022b06 100644
--- a/Firestore/core/src/firebase/firestore/immutable/map_entry.h
+++ b/Firestore/core/src/firebase/firestore/immutable/map_entry.h
@@ -33,7 +33,7 @@ namespace immutable {
  */
 template <typename K, typename V, typename C = std::less<K>>
 struct KeyComparator {
-  typedef std::pair<K, V> pair_type;
+  using pair_type = std::pair<K, V>;
 
   explicit KeyComparator(const C& comparator = C())
       : key_comparator_(comparator) {
diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_key.cc b/Firestore/core/src/firebase/firestore/local/leveldb_key.cc
index 03dbeae..7febe71 100644
--- a/Firestore/core/src/firebase/firestore/local/leveldb_key.cc
+++ b/Firestore/core/src/firebase/firestore/local/leveldb_key.cc
@@ -373,7 +373,7 @@ DocumentKey Reader::ReadDocumentKey() {
   }
 
   ResourcePath path{std::move(path_segments)};
-  if (ok_ && path.size() > 0 && DocumentKey::IsDocumentKey(path)) {
+  if (ok_ && !path.empty() && DocumentKey::IsDocumentKey(path)) {
     return DocumentKey{std::move(path)};
   }
 
diff --git a/Firestore/core/src/firebase/firestore/model/database_id.h b/Firestore/core/src/firebase/firestore/model/database_id.h
index e1feca9..0c0e0ec 100644
--- a/Firestore/core/src/firebase/firestore/model/database_id.h
+++ b/Firestore/core/src/firebase/firestore/model/database_id.h
@@ -44,8 +44,7 @@ class DatabaseId {
    * @param project_id The project for the database.
    * @param database_id The database in the project to use.
    */
-  DatabaseId(const absl::string_view project_id,
-             const absl::string_view database_id);
+  DatabaseId(absl::string_view project_id, absl::string_view database_id);
 
   const std::string& project_id() const {
     return project_id_;
diff --git a/Firestore/core/src/firebase/firestore/model/document.cc b/Firestore/core/src/firebase/firestore/model/document.cc
index 16548cd..ae59d15 100644
--- a/Firestore/core/src/firebase/firestore/model/document.cc
+++ b/Firestore/core/src/firebase/firestore/model/document.cc
@@ -39,7 +39,7 @@ bool Document::Equals(const MaybeDocument& other) const {
   if (other.type() != Type::Document) {
     return false;
   }
-  const Document& other_doc = static_cast<const Document&>(other);
+  auto& other_doc = static_cast<const Document&>(other);
   return MaybeDocument::Equals(other) &&
          has_local_mutations_ == other_doc.has_local_mutations_ &&
          data_ == other_doc.data_;
diff --git a/Firestore/core/src/firebase/firestore/model/field_mask.h b/Firestore/core/src/firebase/firestore/model/field_mask.h
index a9f509a..b895ab3 100644
--- a/Firestore/core/src/firebase/firestore/model/field_mask.h
+++ b/Firestore/core/src/firebase/firestore/model/field_mask.h
@@ -44,9 +44,7 @@ class FieldMask {
 
   FieldMask(std::initializer_list<FieldPath> list) : fields_{list} {
   }
-  explicit FieldMask(const std::vector<FieldPath>& fields) : fields_{fields} {
-  }
-  explicit FieldMask(std::vector<FieldPath>&& fields)
+  explicit FieldMask(std::vector<FieldPath> fields)
       : fields_{std::move(fields)} {
   }
 
diff --git a/Firestore/core/src/firebase/firestore/model/field_value.cc b/Firestore/core/src/firebase/firestore/model/field_value.cc
index 1a40331..b0519f7 100644
--- a/Firestore/core/src/firebase/firestore/model/field_value.cc
+++ b/Firestore/core/src/firebase/firestore/model/field_value.cc
@@ -38,7 +38,7 @@ namespace {
 /**
  * This deviates from the other platforms that define TypeOrder. Since
  * we already define Type for union types, we use it together with this
- * function to achive the equivalent order of types i.e.
+ * function to achieve the equivalent order of types i.e.
  *     i) if two types are comparable, then they are of equal order;
  *    ii) otherwise, their order is the same as the order of their Type.
  */
@@ -148,7 +148,8 @@ FieldValue& FieldValue::operator=(FieldValue&& value) {
       return *this;
     default:
       // We just copy over POD union types.
-      return *this = value;
+      *this = value;
+      return *this;
   }
 }
 
diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h
index c70e332..3c5af9c 100644
--- a/Firestore/core/src/firebase/firestore/model/field_value.h
+++ b/Firestore/core/src/firebase/firestore/model/field_value.h
@@ -163,7 +163,7 @@ class FieldValue {
   /**
    * Switch to the specified type, if different from the current type.
    */
-  void SwitchTo(const Type type);
+  void SwitchTo(Type type);
 
   Type tag_ = Type::Null;
   union {
diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc
index 805e7ac..befe032 100644
--- a/Firestore/core/src/firebase/firestore/remote/serializer.cc
+++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc
@@ -294,7 +294,7 @@ void Writer::WriteVarint(uint64_t value) {
  * Note that (despite the return type) this works for bool, enum, int32, int64,
  * uint32 and uint64 proto field types.
  *
- * Note: This is not expected to be called direclty, but rather only via the
+ * Note: This is not expected to be called directly, but rather only via the
  * other Decode* methods (i.e. DecodeBool, DecodeLong, etc)
  *
  * @return The decoded varint as a uint64_t.
diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.h b/Firestore/core/src/firebase/firestore/remote/serializer.h
index 3b2b667..7541ef5 100644
--- a/Firestore/core/src/firebase/firestore/remote/serializer.h
+++ b/Firestore/core/src/firebase/firestore/remote/serializer.h
@@ -72,7 +72,7 @@ class Serializer {
   // TODO(rsgowman): If we never support any output except to a vector, it may
   // make sense to have Serializer own the vector and provide an accessor rather
   // than asking the user to create it first.
-  static util::Status EncodeFieldValue(
+  util::Status EncodeFieldValue(
       const firebase::firestore::model::FieldValue& field_value,
       std::vector<uint8_t>* out_bytes);
 
@@ -84,8 +84,7 @@ class Serializer {
    * @return The model equivalent of the bytes.
    */
   // TODO(rsgowman): error handling.
-  static firebase::firestore::model::FieldValue DecodeFieldValue(
-      const uint8_t* bytes, size_t length);
+  model::FieldValue DecodeFieldValue(const uint8_t* bytes, size_t length);
 
   /**
    * @brief Converts from bytes to the model FieldValue format.
@@ -95,14 +94,13 @@ class Serializer {
    * @return The model equivalent of the bytes.
    */
   // TODO(rsgowman): error handling.
-  static firebase::firestore::model::FieldValue DecodeFieldValue(
-      const std::vector<uint8_t>& bytes) {
+  model::FieldValue DecodeFieldValue(const std::vector<uint8_t>& bytes) {
     return DecodeFieldValue(bytes.data(), bytes.size());
   }
 
  private:
   // TODO(rsgowman): We don't need the database_id_ yet (but will eventually).
-  // const firebase::firestore::model::DatabaseId& database_id_;
+  // model::DatabaseId* database_id_;
 };
 
 }  // namespace remote
diff --git a/Firestore/core/src/firebase/firestore/timestamp.cc b/Firestore/core/src/firebase/firestore/timestamp.cc
index 7d947c5..a5f0121 100644
--- a/Firestore/core/src/firebase/firestore/timestamp.cc
+++ b/Firestore/core/src/firebase/firestore/timestamp.cc
@@ -48,7 +48,7 @@ Timestamp Timestamp::Now() {
 }
 
 Timestamp Timestamp::FromTimeT(const time_t seconds_since_unix_epoch) {
-  return Timestamp(seconds_since_unix_epoch, 0);
+  return {seconds_since_unix_epoch, 0};
 }
 
 #if !defined(_STLPORT_VERSION)
diff --git a/Firestore/core/src/firebase/firestore/util/bits.h b/Firestore/core/src/firebase/firestore/util/bits.h
index fec1228..94a018f 100644
--- a/Firestore/core/src/firebase/firestore/util/bits.h
+++ b/Firestore/core/src/firebase/firestore/util/bits.h
@@ -139,23 +139,23 @@ inline int Bits::Log2FloorNonZero_Portable(uint32_t n) {
 
 // Log2Floor64() is defined in terms of Log2Floor32(), Log2FloorNonZero32()
 inline int Bits::Log2Floor64_Portable(uint64_t n) {
-  const uint32_t topbits = static_cast<uint32_t>(n >> 32);
-  if (topbits == 0) {
+  const auto top_bits = static_cast<uint32_t>(n >> 32);
+  if (top_bits == 0) {
     // Top bits are zero, so scan in bottom bits
     return Log2Floor(static_cast<uint32_t>(n));
   } else {
-    return 32 + Log2FloorNonZero(topbits);
+    return 32 + Log2FloorNonZero(top_bits);
   }
 }
 
 // Log2FloorNonZero64() is defined in terms of Log2FloorNonZero32()
 inline int Bits::Log2FloorNonZero64_Portable(uint64_t n) {
-  const uint32_t topbits = static_cast<uint32_t>(n >> 32);
-  if (topbits == 0) {
+  const auto top_bits = static_cast<uint32_t>(n >> 32);
+  if (top_bits == 0) {
     // Top bits are zero, so scan in bottom bits
     return Log2FloorNonZero(static_cast<uint32_t>(n));
   } else {
-    return 32 + Log2FloorNonZero(topbits);
+    return 32 + Log2FloorNonZero(top_bits);
   }
 }
 
diff --git a/Firestore/core/src/firebase/firestore/util/comparator_holder.h b/Firestore/core/src/firebase/firestore/util/comparator_holder.h
index 8641b0f..c7f6144 100644
--- a/Firestore/core/src/firebase/firestore/util/comparator_holder.h
+++ b/Firestore/core/src/firebase/firestore/util/comparator_holder.h
@@ -47,7 +47,7 @@ class ComparatorHolder {
 template <typename C>
 class ComparatorHolder<C, true> : private C {
  protected:
-  explicit ComparatorHolder(const C&) noexcept {
+  explicit ComparatorHolder(const C& /* comparator */) noexcept {
   }
 
   const C& comparator() const noexcept {
diff --git a/Firestore/core/src/firebase/firestore/util/comparison.cc b/Firestore/core/src/firebase/firestore/util/comparison.cc
index b346277..5ac4c27 100644
--- a/Firestore/core/src/firebase/firestore/util/comparison.cc
+++ b/Firestore/core/src/firebase/firestore/util/comparison.cc
@@ -78,7 +78,7 @@ ComparisonResult CompareMixedNumber(double double_value, int64_t int64_value) {
 
   // At this point the long representations are equal but this could be due to
   // rounding.
-  double int64_as_double = static_cast<double>(int64_value);
+  auto int64_as_double = static_cast<double>(int64_value);
   return Compare<double>(double_value, int64_as_double);
 }