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

9 files changed, 22 insertions, 21 deletions

diff --git a/Firestore/core/test/firebase/firestore/immutable/array_sorted_map_test.cc b/Firestore/core/test/firebase/firestore/immutable/array_sorted_map_test.cc
index fceab7d..6758dd5 100644
--- a/Firestore/core/test/firebase/firestore/immutable/array_sorted_map_test.cc
+++ b/Firestore/core/test/firebase/firestore/immutable/array_sorted_map_test.cc
@@ -29,7 +29,7 @@ namespace firestore {
 namespace immutable {
 namespace impl {
 
-typedef ArraySortedMap<int, int> IntMap;
+using IntMap = ArraySortedMap<int, int>;
 constexpr IntMap::size_type kFixedSize = IntMap::kFixedSize;
 
 // TODO(wilhuff): ReverseTraversal
@@ -153,7 +153,7 @@ TEST(ArraySortedMap, EmptyRemoval) {
 
 TEST(ArraySortedMap, InsertionAndRemovalOfMaxItems) {
   auto expected_size = kFixedSize;
-  int n = static_cast<int>(expected_size);
+  auto n = static_cast<int>(expected_size);
   std::vector<int> to_insert = Shuffled(Sequence(n));
   std::vector<int> to_remove = Shuffled(to_insert);
 
diff --git a/Firestore/core/test/firebase/firestore/immutable/sorted_map_test.cc b/Firestore/core/test/firebase/firestore/immutable/sorted_map_test.cc
index 44dca50..747c66b 100644
--- a/Firestore/core/test/firebase/firestore/immutable/sorted_map_test.cc
+++ b/Firestore/core/test/firebase/firestore/immutable/sorted_map_test.cc
@@ -49,6 +49,7 @@ class SortedMapTest : public ::testing::Test {
   }
 };
 
+// NOLINTNEXTLINE: must be a typedef for the gtest macros
 typedef ::testing::Types<SortedMap<int, int>,
                          impl::ArraySortedMap<int, int>,
                          impl::TreeSortedMap<int, int>>
diff --git a/Firestore/core/test/firebase/firestore/immutable/tree_sorted_map_test.cc b/Firestore/core/test/firebase/firestore/immutable/tree_sorted_map_test.cc
index 7a96b67..c03dc6c 100644
--- a/Firestore/core/test/firebase/firestore/immutable/tree_sorted_map_test.cc
+++ b/Firestore/core/test/firebase/firestore/immutable/tree_sorted_map_test.cc
@@ -25,7 +25,7 @@ namespace firestore {
 namespace immutable {
 namespace impl {
 
-typedef TreeSortedMap<int, int> IntMap;
+using IntMap = TreeSortedMap<int, int>;
 
 TEST(TreeSortedMap, EmptySize) {
   IntMap map;
diff --git a/Firestore/core/test/firebase/firestore/model/document_key_test.cc b/Firestore/core/test/firebase/firestore/model/document_key_test.cc
index 0e0df2d..619ee7f 100644
--- a/Firestore/core/test/firebase/firestore/model/document_key_test.cc
+++ b/Firestore/core/test/firebase/firestore/model/document_key_test.cc
@@ -46,7 +46,7 @@ TEST(DocumentKey, Constructor_FromPath) {
   EXPECT_EQ(key_from_path_copy.path(), path);
 
   const DocumentKey key_from_moved_path{std::move(path)};
-  EXPECT_TRUE(path.empty());
+  EXPECT_TRUE(path.empty());  // NOLINT: use after move intended
   EXPECT_FALSE(key_from_moved_path.path().empty());
   EXPECT_EQ(key_from_path_copy.path(), key_from_moved_path.path());
 }
@@ -62,7 +62,7 @@ TEST(DocumentKey, CopyAndMove) {
 
   const DocumentKey moved = std::move(key);
   EXPECT_EQ(path_string, moved.path().CanonicalString());
-  EXPECT_NE(key, moved);
+  EXPECT_NE(key, moved);  // NOLINT: use after move intended
   EXPECT_TRUE(key.path().empty());
 
   // Reassignment.
@@ -74,7 +74,7 @@ TEST(DocumentKey, CopyAndMove) {
   key = {};
   EXPECT_TRUE(key.path().empty());
   key = std::move(copied);
-  EXPECT_NE(copied, key);
+  EXPECT_NE(copied, key);  // NOLINT: use after move intended
   EXPECT_TRUE(copied.path().empty());
   EXPECT_EQ(path_string, key.path().CanonicalString());
 }
diff --git a/Firestore/core/test/firebase/firestore/model/field_path_test.cc b/Firestore/core/test/firebase/firestore/model/field_path_test.cc
index a5ae3b2..a215a96 100644
--- a/Firestore/core/test/firebase/firestore/model/field_path_test.cc
+++ b/Firestore/core/test/firebase/firestore/model/field_path_test.cc
@@ -47,7 +47,7 @@ TEST(FieldPath, Constructors) {
   EXPECT_EQ(path_from_list, copied);
   const FieldPath moved = std::move(copied);
   EXPECT_EQ(path_from_list, moved);
-  EXPECT_NE(copied, moved);
+  EXPECT_NE(copied, moved);  // NOLINT: use after move intended
   EXPECT_EQ(empty_path, copied);
 }
 
@@ -68,7 +68,7 @@ TEST(FieldPath, PopFirst) {
   const FieldPath bc{"Eros", "messages"};
   const FieldPath c{"messages"};
   const FieldPath empty;
-  const FieldPath abc_dupl{"rooms", "Eros", "messages"};
+  const FieldPath abc_dup{"rooms", "Eros", "messages"};
 
   EXPECT_NE(empty, c);
   EXPECT_NE(c, bc);
@@ -77,7 +77,7 @@ TEST(FieldPath, PopFirst) {
   EXPECT_EQ(bc, abc.PopFirst());
   EXPECT_EQ(c, abc.PopFirst(2));
   EXPECT_EQ(empty, abc.PopFirst(3));
-  EXPECT_EQ(abc_dupl, abc);
+  EXPECT_EQ(abc_dup, abc);
 }
 
 TEST(FieldPath, PopLast) {
@@ -85,7 +85,7 @@ TEST(FieldPath, PopLast) {
   const FieldPath ab{"rooms", "Eros"};
   const FieldPath a{"rooms"};
   const FieldPath empty;
-  const FieldPath abc_dupl{"rooms", "Eros", "messages"};
+  const FieldPath abc_dup{"rooms", "Eros", "messages"};
 
   EXPECT_EQ(ab, abc.PopLast());
   EXPECT_EQ(a, abc.PopLast().PopLast());
diff --git a/Firestore/core/test/firebase/firestore/model/field_value_test.cc b/Firestore/core/test/firebase/firestore/model/field_value_test.cc
index 08db76d..c5645a4 100644
--- a/Firestore/core/test/firebase/firestore/model/field_value_test.cc
+++ b/Firestore/core/test/firebase/firestore/model/field_value_test.cc
@@ -411,7 +411,7 @@ TEST(FieldValue, Move) {
   EXPECT_EQ(FieldValue::ReferenceValue(DocumentKey::FromPathString("root/abc"),
                                        &database_id),
             clone);
-  clone = null_value;
+  clone = null_value;  // NOLINT: use after move intended
   EXPECT_EQ(FieldValue::NullValue(), clone);
 
   FieldValue geo_point_value = FieldValue::GeoPointValue({1, 2});
diff --git a/Firestore/core/test/firebase/firestore/model/resource_path_test.cc b/Firestore/core/test/firebase/firestore/model/resource_path_test.cc
index 637e78e..8545884 100644
--- a/Firestore/core/test/firebase/firestore/model/resource_path_test.cc
+++ b/Firestore/core/test/firebase/firestore/model/resource_path_test.cc
@@ -47,7 +47,7 @@ TEST(ResourcePath, Constructor) {
   EXPECT_EQ(path_from_list, copied);
   const ResourcePath moved = std::move(copied);
   EXPECT_EQ(path_from_list, moved);
-  EXPECT_NE(copied, moved);
+  EXPECT_NE(copied, moved);  // NOLINT: use after move intended
   EXPECT_EQ(empty_path, copied);
 }
 
diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
index 947c2fe..addc830 100644
--- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
+++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
@@ -54,7 +54,7 @@ TEST(Serializer, CanLinkToNanopb) {
   // nanopb is concerned but that it can run at all is proof that all the
   // libraries required for nanopb to work are actually linked correctly into
   // the test.
-  pb_ostream_from_buffer(NULL, 0);
+  pb_ostream_from_buffer(nullptr, 0);
 }
 
 // Fixture for running serializer tests.
diff --git a/Firestore/core/test/firebase/firestore/util/bits_test.cc b/Firestore/core/test/firebase/firestore/util/bits_test.cc
index cb0976b..572721f 100644
--- a/Firestore/core/test/firebase/firestore/util/bits_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/bits_test.cc
@@ -69,16 +69,16 @@ TEST_F(BitsTest, Log2Random) {
   std::cout << "TestLog2Random" << std::endl;
 
   for (int i = 0; i < kNumIterations; i++) {
-    int maxbit = -1;
+    int max_bit = -1;
     uint32_t n = 0;
     while (!random_.OneIn(32u)) {
       int bit = static_cast<int>(random_.Uniform(32u));
       n |= (1U << bit);
-      maxbit = std::max(bit, maxbit);
+      max_bit = std::max(bit, max_bit);
     }
-    EXPECT_EQ(maxbit, Bits::Log2Floor(n));
+    EXPECT_EQ(max_bit, Bits::Log2Floor(n));
     if (n != 0) {
-      EXPECT_EQ(maxbit, Bits::Log2FloorNonZero(n));
+      EXPECT_EQ(max_bit, Bits::Log2FloorNonZero(n));
     }
   }
 }
@@ -87,16 +87,16 @@ TEST_F(BitsTest, Log2Random64) {
   std::cout << "TestLog2Random64" << std::endl;
 
   for (int i = 0; i < kNumIterations; i++) {
-    int maxbit = -1;
+    int max_bit = -1;
     uint64_t n = 0;
     while (!random_.OneIn(64u)) {
       int bit = static_cast<int>(random_.Uniform(64u));
       n |= (1ULL << bit);
-      maxbit = std::max(bit, maxbit);
+      max_bit = std::max(bit, max_bit);
     }
-    EXPECT_EQ(maxbit, Bits::Log2Floor64(n));
+    EXPECT_EQ(max_bit, Bits::Log2Floor64(n));
     if (n != 0) {
-      EXPECT_EQ(maxbit, Bits::Log2FloorNonZero64(n));
+      EXPECT_EQ(max_bit, Bits::Log2FloorNonZero64(n));
     }
   }
 }