Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -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






2 changes: 1 addition & 1 deletion api/test/baggage/baggage_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 += ",";
Expand Down
2 changes: 1 addition & 1 deletion api/test/baggage/baggage_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 += ",";
Expand Down
2 changes: 1 addition & 1 deletion api/test/baggage/propagation/baggage_propagator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion api/test/common/kv_properties_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
6 changes: 3 additions & 3 deletions api/test/context/context_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 2 additions & 0 deletions api/test/context/runtime_context_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand All @@ -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));
Expand Down
10 changes: 3 additions & 7 deletions api/test/logs/logger_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion api/test/logs/logger_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion api/test/metrics/meter_provider_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions api/test/nostd/shared_ptr_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);

Expand Down
12 changes: 6 additions & 6 deletions api/test/nostd/unique_ptr_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions api/test/trace/trace_state_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 += ",";
Expand Down
2 changes: 1 addition & 1 deletion examples/etw_threads/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down
7 changes: 3 additions & 4 deletions examples/grpc/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

using grpc::Channel;
using grpc::ClientContext;
using grpc::ClientReader;
using grpc::Status;

using grpc_example::Greeter;
Expand All @@ -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)
{
Expand Down Expand Up @@ -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!
Expand All @@ -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

Expand Down
9 changes: 4 additions & 5 deletions examples/grpc/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
using grpc::Server;
using grpc::ServerBuilder;
using grpc::ServerContext;
using grpc::ServerWriter;
using grpc::Status;

using grpc_example::Greeter;
Expand All @@ -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";
}
Expand Down Expand Up @@ -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);
Expand All @@ -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();
}
Expand Down
Loading