From bd0ef62a8f277970b7f9485fdca247e7fab212bd Mon Sep 17 00:00:00 2001
From: Jon Ross-Perkins <jperkins@google.com>
Date: Thu, 7 Dec 2023 11:06:29 -0800
Subject: [PATCH] Fix clang-tidy issues in common and testing (#3470)

Choosing to make the constructor explicit in the test, rather than
NOLINT, because it seems to better reflect how our code is usually
written (and may be more likely to trip an issue).
---
 common/struct_reflection.h        |  7 +++++--
 common/struct_reflection_test.cpp | 11 ++++++-----
 testing/file_test/line.h          |  2 +-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/common/struct_reflection.h b/common/struct_reflection.h
index 560cb5bb1574..2583e24a0681 100644
--- a/common/struct_reflection.h
+++ b/common/struct_reflection.h
@@ -36,8 +36,11 @@ namespace Internal {
 template <typename T>
 struct AnyField {
   template <typename FieldT>
+  // NOLINTNEXTLINE(google-explicit-constructor)
   operator FieldT&() const;
+
   template <typename FieldT>
+  // NOLINTNEXTLINE(google-explicit-constructor)
   operator FieldT&&() const;
 
   // Don't allow conversion to T itself. This ensures we don't match against a
@@ -52,11 +55,11 @@ struct AnyField {
 
 // Detector for whether we can list-initialize T from the given list of fields.
 template <typename T, typename... Fields>
-constexpr bool CanListInitialize(decltype(T{Fields()...})*) {
+constexpr auto CanListInitialize(decltype(T{Fields()...})* /*unused*/) -> bool {
   return true;
 }
 template <typename T, typename... Fields>
-constexpr bool CanListInitialize(...) {
+constexpr auto CanListInitialize(...) -> bool {
   return false;
 }
 
diff --git a/common/struct_reflection_test.cpp b/common/struct_reflection_test.cpp
index ec53639a61f0..619cb231e9bf 100644
--- a/common/struct_reflection_test.cpp
+++ b/common/struct_reflection_test.cpp
@@ -25,7 +25,7 @@ struct ReferenceField {
 };
 
 struct NoDefaultConstructor {
-  NoDefaultConstructor(int n) : v(n) {}
+  explicit NoDefaultConstructor(int n) : v(n) {}
   int v;
 };
 
@@ -42,8 +42,8 @@ TEST(StructReflectionTest, CanListInitialize) {
   {
     using Type = OneField;
     using Field = Internal::AnyField<Type>;
-    static_assert(Internal::CanListInitialize<Type>(0));
-    static_assert(Internal::CanListInitialize<Type, Field>(0));
+    static_assert(Internal::CanListInitialize<Type>(nullptr));
+    static_assert(Internal::CanListInitialize<Type, Field>(nullptr));
     static_assert(!Internal::CanListInitialize<Type, Field, Field>(0));
   }
 
@@ -51,7 +51,7 @@ TEST(StructReflectionTest, CanListInitialize) {
     using Type = OneFieldNoDefaultConstructor;
     using Field = Internal::AnyField<Type>;
     static_assert(!Internal::CanListInitialize<Type>(0));
-    static_assert(Internal::CanListInitialize<Type, Field>(0));
+    static_assert(Internal::CanListInitialize<Type, Field>(nullptr));
     static_assert(!Internal::CanListInitialize<Type, Field, Field>(0));
   }
 }
@@ -82,7 +82,8 @@ TEST(StructReflectionTest, TwoField) {
 
 TEST(StructReflectionTest, NoDefaultConstructor) {
   std::tuple<NoDefaultConstructor, NoDefaultConstructor> fields =
-      AsTuple(TwoFieldsNoDefaultConstructor{.x = 1, .y = 2});
+      AsTuple(TwoFieldsNoDefaultConstructor{.x = NoDefaultConstructor(1),
+                                            .y = NoDefaultConstructor(2)});
   EXPECT_EQ(std::get<0>(fields).v, 1);
   EXPECT_EQ(std::get<1>(fields).v, 2);
 }
diff --git a/testing/file_test/line.h b/testing/file_test/line.h
index e0783fed3c4f..6ceb45c25b80 100644
--- a/testing/file_test/line.h
+++ b/testing/file_test/line.h
@@ -15,7 +15,7 @@ class FileTestLineBase : public Printable<FileTestLineBase> {
  public:
   explicit FileTestLineBase(int file_number, int line_number)
       : file_number_(file_number), line_number_(line_number) {}
-  virtual ~FileTestLineBase() {}
+  virtual ~FileTestLineBase() = default;
 
   // Prints the autoupdated line.
   virtual auto Print(llvm::raw_ostream& out) const -> void = 0;