From 8876622b6fcebc21672bc263666b858b7e152b45 Mon Sep 17 00:00:00 2001 From: Gil Date: Thu, 12 Apr 2018 14:54:53 -0700 Subject: 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 --- .../firestore/immutable/array_sorted_map_test.cc | 4 ++-- .../test/firebase/firestore/immutable/sorted_map_test.cc | 1 + .../firebase/firestore/immutable/tree_sorted_map_test.cc | 2 +- .../test/firebase/firestore/model/document_key_test.cc | 6 +++--- .../test/firebase/firestore/model/field_path_test.cc | 8 ++++---- .../test/firebase/firestore/model/field_value_test.cc | 2 +- .../test/firebase/firestore/model/resource_path_test.cc | 2 +- .../test/firebase/firestore/remote/serializer_test.cc | 2 +- Firestore/core/test/firebase/firestore/util/bits_test.cc | 16 ++++++++-------- 9 files changed, 22 insertions(+), 21 deletions(-) (limited to 'Firestore/core/test') 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 IntMap; +using IntMap = ArraySortedMap; 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(expected_size); + auto n = static_cast(expected_size); std::vector to_insert = Shuffled(Sequence(n)); std::vector 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, impl::ArraySortedMap, impl::TreeSortedMap> 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 IntMap; +using IntMap = TreeSortedMap; 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(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(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)); } } } -- cgit v1.2.3