[Code Health] clang-tidy cleanup, part 1 (#2990)

.clang-tidy

@@ -0,0 +1,69 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+Checks: >
+  -*,
+  abseil-*,
+  -abseil-string-find-str-contains,
+  bugprone-*,
+  -bugprone-easily-swappable-parameters,
+  -bugprone-implicit-widening-of-multiplication-result,
+  -bugprone-inc-dec-in-conditions,
+  -bugprone-narrowing-conversions,
+  -bugprone-unchecked-optional-access,
+  -bugprone-unhandled-exception-at-new,
+  -bugprone-unused-local-non-trivial-variable,
+  -bugprone-unused-return-value,
+  google-*,
+  -google-build-using-namespace,
+  -google-default-arguments,
+  -google-explicit-constructor,
+  -google-readability-avoid-underscore-in-googletest-name,
+  -google-readability-braces-around-statements,
+  -google-readability-namespace-comments,
+  -google-readability-todo,
+  -google-runtime-references,
+  misc-*,
+  -misc-const-correctness,
+  -misc-include-cleaner,
+  -misc-non-private-member-variables-in-classes,
+  -misc-unused-alias-decls,
+  -misc-use-anonymous-namespace,
+  performance-*,
+  -performance-move-const-arg,
+  portability-*
+#  readability-*,
+#  -readability-convert-member-functions-to-static,
+#  -readability-else-after-return,
+#  -readability-function-cognitive-complexity,
+#  -readability-identifier-length,
+#  -readability-implicit-bool-conversion,
+#  -readability-isolate-declaration,
+#  -readability-magic-numbers,
+#  -readability-named-parameter,
+#  -readability-redundant-*,
+#  -readability-string-compare,
+#  cppcoreguidelines-*,
+#  -cppcoreguidelines-avoid-c-arrays,
+#  -cppcoreguidelines-avoid-magic-numbers,
+#  -cppcoreguidelines-init-variables,
+#  -cppcoreguidelines-macro-usage,
+#  -cppcoreguidelines-non-private-member-variables-in-classes,
+#  -cppcoreguidelines-pro-*,
+#  modernize-*,
+#  -modernize-use-default-member-init,
+#  -modernize-use-nodiscard,
+#  -modernize-use-trailing-return-type,
+#  -modernize-avoid-c-arrays,
+#  -modernize-use-using
+
+# Use existing clang-format for formatting the code
+# FormatStyle: 'file'
+
+# TODO: include checks: readability, cppcoreguidelines, modernize , google-readability-namespace-comments, google-readability-avoid-underscore-in-googletest-name, performance-move-const-arg
+
+
+
+
+
+

api/test/baggage/baggage_benchmark.cc

@@ -22,7 +22,7 @@ std::string header_with_custom_entries(size_t num_entries)
   {
     std::string key   = "ADecentlyLargekey" + std::to_string(i);
     std::string value = "ADecentlyLargeValue" + std::to_string(i);
-    header += key + "=" + value;
+    header.append(key).append("=").append(value);
     if (i != num_entries - 1)
     {
       header += ",";

api/test/baggage/baggage_test.cc

@@ -19,7 +19,7 @@ std::string header_with_custom_entries(size_t num_entries)
   {
     std::string key   = "key" + std::to_string(i);
     std::string value = "value" + std::to_string(i);
-    header += key + "=" + value;
+    header.append(key).append("=").append(value);
     if (i != num_entries - 1)
     {
       header += ",";

api/test/baggage/propagation/baggage_propagator_test.cc

@@ -62,7 +62,7 @@ TEST(BaggagePropagatorTest, ExtractAndInjectBaggage)
       {"invalid_header", ""},                                 // invalid header
       {very_large_baggage_header, ""}};  // baggage header larger than allowed size.
 
-  for (auto baggage : baggages)
+  for (const auto &baggage : baggages)
   {
     BaggageCarrierTest carrier1;
     carrier1.headers_[baggage::kBaggageHeader.data()] = baggage.first;

api/test/common/kv_properties_test.cc

@@ -31,7 +31,7 @@ TEST(EntryTest, KeyValueConstruction)
 TEST(EntryTest, Copy)
 {
   KeyValueProperties::Entry e("test_key", "test_value");
-  KeyValueProperties::Entry copy(e);
+  const KeyValueProperties::Entry &copy(e);
   EXPECT_EQ(copy.GetKey(), e.GetKey());
   EXPECT_EQ(copy.GetValue(), e.GetValue());
 }

api/test/context/context_test.cc

@@ -105,8 +105,8 @@ TEST(ContextTest, ContextCopyOperator)
       {"foo_key", static_cast<int64_t>(456)},
       {"other_key", static_cast<int64_t>(789)}};
 
-  context::Context test_context   = context::Context(test_map);
-  context::Context copied_context = test_context;
+  context::Context test_context          = context::Context(test_map);
+  const context::Context &copied_context = test_context;
 
   EXPECT_EQ(nostd::get<int64_t>(copied_context.GetValue("test_key")), 123);
   EXPECT_EQ(nostd::get<int64_t>(copied_context.GetValue("foo_key")), 456);
@@ -135,7 +135,7 @@ TEST(ContextTest, ContextCopyCompare)
 {
   std::map<std::string, context::ContextValue> map_test = {{"test_key", static_cast<int64_t>(123)}};
   context::Context context_test                         = context::Context(map_test);
-  context::Context copied_test                          = context_test;
+  const context::Context &copied_test                   = context_test;
   EXPECT_TRUE(context_test == copied_test);
 }
 

api/test/context/runtime_context_test.cc

@@ -113,6 +113,7 @@ TEST(RuntimeContextTest, DetachOutOfOrder)
   indices.push_back(3);
 
   std::vector<context::Context> contexts;
+  contexts.reserve(indices.size());
   for (auto i : indices)
   {
     contexts.push_back(context::Context("index", static_cast<int64_t>(i)));
@@ -122,6 +123,7 @@ TEST(RuntimeContextTest, DetachOutOfOrder)
   {
     std::vector<nostd::unique_ptr<context::Token>> tokens;
 
+    tokens.reserve(contexts.size());
     for (auto &c : contexts)
     {
       tokens.push_back(context::RuntimeContext::Attach(c));

api/test/logs/logger_benchmark.cc

@@ -16,13 +16,8 @@
 #include <benchmark/benchmark.h>
 
 using opentelemetry::logs::EventId;
-using opentelemetry::logs::Logger;
-using opentelemetry::logs::LoggerProvider;
 using opentelemetry::logs::Provider;
 using opentelemetry::logs::Severity;
-using opentelemetry::nostd::shared_ptr;
-using opentelemetry::nostd::span;
-using opentelemetry::nostd::string_view;
 
 namespace common  = opentelemetry::common;
 namespace nostd   = opentelemetry::nostd;
@@ -66,7 +61,7 @@ class Barrier
 static void ThreadRoutine(Barrier &barrier,
                           benchmark::State &state,
                           int thread_id,
-                          std::function<void()> func)
+                          const std::function<void()> &func)
 {
   barrier.Wait();
 
@@ -87,14 +82,15 @@ static void ThreadRoutine(Barrier &barrier,
   barrier.Wait();
 }
 
-void MultiThreadRunner(benchmark::State &state, std::function<void()> func)
+void MultiThreadRunner(benchmark::State &state, const std::function<void()> &func)
 {
   int num_threads = std::thread::hardware_concurrency();
 
   Barrier barrier(num_threads);
 
   std::vector<std::thread> threads;
 
+  threads.reserve(num_threads);
   for (int i = 0; i < num_threads; i++)
   {
     threads.emplace_back(ThreadRoutine, std::ref(barrier), std::ref(state), i, func);

api/test/logs/logger_test.cc

@@ -16,7 +16,6 @@ using opentelemetry::logs::LoggerProvider;
 using opentelemetry::logs::Provider;
 using opentelemetry::logs::Severity;
 using opentelemetry::nostd::shared_ptr;
-using opentelemetry::nostd::span;
 using opentelemetry::nostd::string_view;
 namespace common = opentelemetry::common;
 namespace nostd  = opentelemetry::nostd;

api/test/metrics/meter_provider_test.cc

@@ -6,7 +6,6 @@
 #include "opentelemetry/metrics/provider.h"
 #include "opentelemetry/nostd/shared_ptr.h"
 
-using opentelemetry::metrics::Meter;
 using opentelemetry::metrics::MeterProvider;
 using opentelemetry::metrics::NoopMeterProvider;
 using opentelemetry::metrics::Provider;

api/test/nostd/shared_ptr_test.cc

@@ -63,7 +63,7 @@ TEST(SharedPtrTest, MoveConstruction)
   auto value = new int{123};
   shared_ptr<int> ptr1{value};
   shared_ptr<int> ptr2{std::move(ptr1)};
-  EXPECT_EQ(ptr1.get(), nullptr);
+  EXPECT_EQ(ptr1.get(), nullptr);  // NOLINT
   EXPECT_EQ(ptr2.get(), value);
 }
 
@@ -72,7 +72,7 @@ TEST(SharedPtrTest, MoveConstructionFromDifferentType)
   auto value = new int{123};
   shared_ptr<int> ptr1{value};
   shared_ptr<const int> ptr2{std::move(ptr1)};
-  EXPECT_EQ(ptr1.get(), nullptr);
+  EXPECT_EQ(ptr1.get(), nullptr);  // NOLINT
   EXPECT_EQ(ptr2.get(), value);
 }
 
@@ -81,14 +81,14 @@ TEST(SharedPtrTest, MoveConstructionFromStdSharedPtr)
   auto value = new int{123};
   std::shared_ptr<int> ptr1{value};
   shared_ptr<int> ptr2{std::move(ptr1)};
-  EXPECT_EQ(ptr1.get(), nullptr);
+  EXPECT_EQ(ptr1.get(), nullptr);  // NOLINT
   EXPECT_EQ(ptr2.get(), value);
 }
 
 TEST(SharedPtrTest, Destruction)
 {
   bool was_destructed;
-  shared_ptr<A>{new A{was_destructed}};
+  shared_ptr<A>{new A{was_destructed}};  // NOLINT
   EXPECT_TRUE(was_destructed);
 }
 
@@ -173,7 +173,7 @@ static void SharedPtrTest_Sort(size_t size = 10)
   auto nums2 = nums;
 
   std::sort(nums.begin(), nums.end(),
-            [](shared_ptr<const int> a, shared_ptr<const int> b) { return *a < *b; });
+            [](const shared_ptr<const int> &a, const shared_ptr<const int> &b) { return *a < *b; });
 
   EXPECT_NE(nums, nums2);
 

api/test/nostd/unique_ptr_test.cc

@@ -49,7 +49,7 @@ TEST(UniquePtrTest, MoveConstruction)
   auto value = new int{123};
   unique_ptr<int> ptr1{value};
   unique_ptr<int> ptr2{std::move(ptr1)};
-  EXPECT_EQ(ptr1.get(), nullptr);
+  EXPECT_EQ(ptr1.get(), nullptr);  // NOLINT
   EXPECT_EQ(ptr2.get(), value);
 }
 
@@ -58,7 +58,7 @@ TEST(UniquePtrTest, MoveConstructionFromDifferentType)
   auto value = new int{123};
   unique_ptr<int> ptr1{value};
   unique_ptr<const int> ptr2{std::move(ptr1)};
-  EXPECT_EQ(ptr1.get(), nullptr);
+  EXPECT_EQ(ptr1.get(), nullptr);  // NOLINT
   EXPECT_EQ(ptr2.get(), value);
 }
 
@@ -67,14 +67,14 @@ TEST(UniquePtrTest, MoveConstructionFromStdUniquePtr)
   auto value = new int{123};
   std::unique_ptr<int> ptr1{value};
   unique_ptr<int> ptr2{std::move(ptr1)};
-  EXPECT_EQ(ptr1.get(), nullptr);
+  EXPECT_EQ(ptr1.get(), nullptr);  // NOLINT
   EXPECT_EQ(ptr2.get(), value);
 }
 
 TEST(UniquePtrTest, Destruction)
 {
   bool was_destructed;
-  unique_ptr<A>{new A{was_destructed}};
+  unique_ptr<A>{new A{was_destructed}};  // NOLINT
   EXPECT_TRUE(was_destructed);
 }
 
@@ -83,13 +83,13 @@ TEST(UniquePtrTest, StdUniquePtrConversionOperator)
   auto value = new int{123};
   unique_ptr<int> ptr1{value};
   std::unique_ptr<int> ptr2{std::move(ptr1)};
-  EXPECT_EQ(ptr1.get(), nullptr);
+  EXPECT_EQ(ptr1.get(), nullptr);  // NOLINT
   EXPECT_EQ(ptr2.get(), value);
 
   value = new int{456};
   ptr1  = unique_ptr<int>{value};
   ptr2  = std::move(ptr1);
-  EXPECT_EQ(ptr1.get(), nullptr);
+  EXPECT_EQ(ptr1.get(), nullptr);  // NOLINT
   EXPECT_EQ(ptr2.get(), value);
 
   ptr2 = nullptr;

api/test/trace/trace_state_test.cc

@@ -20,7 +20,7 @@ const char *kLongString =
 
 // -------------------------- TraceState class tests ---------------------------
 
-std::string create_ts_return_header(std::string header)
+std::string create_ts_return_header(const std::string &header)
 {
   auto ts = TraceState::FromHeader(header);
   return ts->ToHeader();
@@ -34,7 +34,7 @@ std::string header_with_max_members()
   {
     std::string key   = "key" + std::to_string(i);
     std::string value = "value" + std::to_string(i);
-    header += key + "=" + value;
+    header.append(key).append("=").append(value);
     if (i != max_members - 1)
     {
       header += ",";

examples/etw_threads/main.cc

@@ -113,7 +113,7 @@ void beep()
 // Max number of threads to spawn
 constexpr int kMaxThreads = 10;
 
-int main(int arc, char **argv)
+int main(int /*arc*/, char ** /*argv*/)
 {
   std::thread pool[kMaxThreads];
 

examples/grpc/client.cc

@@ -20,7 +20,6 @@
 
 using grpc::Channel;
 using grpc::ClientContext;
-using grpc::ClientReader;
 using grpc::Status;
 
 using grpc_example::Greeter;
@@ -34,7 +33,7 @@ using namespace opentelemetry::trace;
 class GreeterClient
 {
 public:
-  GreeterClient(std::shared_ptr<Channel> channel) : stub_(Greeter::NewStub(channel)) {}
+  GreeterClient(const std::shared_ptr<Channel> &channel) : stub_(Greeter::NewStub(channel)) {}
 
   std::string Greet(std::string ip, uint16_t port)
   {
@@ -77,7 +76,7 @@ class GreeterClient
     }
     else
     {
-      std::cout << status.error_code() << ": " << status.error_message() << std::endl;
+      std::cout << status.error_code() << ": " << status.error_message() << '\n';
       span->SetStatus(StatusCode::kError);
       span->SetAttribute(SemanticConventions::kRpcGrpcStatusCode, status.error_code());
       // Make sure to end your spans!
@@ -95,7 +94,7 @@ void RunClient(uint16_t port)
   GreeterClient greeter(
       grpc::CreateChannel("0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials()));
   std::string response = greeter.Greet("0.0.0.0", port);
-  std::cout << "grpc_server says: " << response << std::endl;
+  std::cout << "grpc_server says: " << response << '\n';
 }
 }  // namespace
 

examples/grpc/server.cc

@@ -27,7 +27,6 @@
 using grpc::Server;
 using grpc::ServerBuilder;
 using grpc::ServerContext;
-using grpc::ServerWriter;
 using grpc::Status;
 
 using grpc_example::Greeter;
@@ -49,7 +48,7 @@ class GreeterServer final : public Greeter::Service
                const GreetRequest *request,
                GreetResponse *response) override
   {
-    for (auto elem : context->client_metadata())
+    for (const auto &elem : context->client_metadata())
     {
       std::cout << "ELEM: " << elem.first << " " << elem.second << "\n";
     }
@@ -78,8 +77,8 @@ class GreeterServer final : public Greeter::Service
     // Fetch and parse whatever HTTP headers we can from the gRPC request.
     span->AddEvent("Processing client attributes");
 
-    std::string req = request->request();
-    std::cout << std::endl << "grpc_client says: " << req << std::endl;
+    const std::string &req = request->request();
+    std::cout << '\n' << "grpc_client says: " << req << '\n';
     std::string message = "The pleasure is mine.";
     // Send response to client
     response->set_response(message);
@@ -102,7 +101,7 @@ void RunServer(uint16_t port)
   builder.AddListeningPort(address, grpc::InsecureServerCredentials());
 
   std::unique_ptr<Server> server(builder.BuildAndStart());
-  std::cout << "Server listening on port: " << address << std::endl;
+  std::cout << "Server listening on port: " << address << '\n';
   server->Wait();
   server->Shutdown();
 }