Fix Clang Tidy CI Target

.azure-pipelines/pipelines.yml

@@ -44,23 +44,22 @@ stages:
       parameters:
         ciTarget: $(CI_TARGET)
 
-# TODO(#849): Renable target when file issue is resolved.
-# - stage: clang_tidy
-#   dependsOn: ["check"]
-#   pool: "x64-large"
-#   jobs:
-#   - job: clang_tidy
-#     displayName: "do_ci.sh"
-#     strategy:
-#       maxParallel: 1
-#       matrix:
-#         clang_tidy:
-#           CI_TARGET: "clang_tidy"
-#     timeoutInMinutes: 120
-#     steps:
-#     - template: bazel.yml
-#       parameters:
-#         ciTarget: $(CI_TARGET)
+- stage: clang_tidy
+  dependsOn: ["check"]
+  pool: "x64-large"
+  jobs:
+  - job: clang_tidy
+    displayName: "do_ci.sh"
+    strategy:
+      maxParallel: 1
+      matrix:
+        clang_tidy:
+          CI_TARGET: "clang_tidy"
+    timeoutInMinutes: 120
+    steps:
+    - template: bazel.yml
+      parameters:
+        ciTarget: $(CI_TARGET)
 
 - stage: test_gcc
   dependsOn: ["check"]
@@ -119,7 +118,7 @@ stages:
 
 - stage: release
   dependsOn:
-  # - "clang_tidy"
+  - "clang_tidy"
   - "test_gcc"
   - "sanitizers"
   - "coverage"

.clang-tidy

@@ -1,4 +1,4 @@
-Checks:          'clang-diagnostic-*,clang-analyzer-*,abseil-*,bugprone-*,modernize-*,performance-*,readability-redundant-*,readability-braces-around-statements,-abseil-no-internal-dependencies,-modernize-use-trailing-return-type,-modernize-avoid-bind,-modernize-use-nodiscard,-modernize-concat-nested-namespaces,-bugprone-narrowing-conversions'
+Checks:          'clang-diagnostic-*,clang-analyzer-*,abseil-*,bugprone-*,modernize-*,performance-*,readability-redundant-*,readability-braces-around-statements,-abseil-no-internal-dependencies,-modernize-use-trailing-return-type,-modernize-avoid-bind,-modernize-use-nodiscard,-modernize-concat-nested-namespaces,-bugprone-narrowing-conversions,-bugprone-easily-swappable-parameters'
 
 WarningsAsErrors:  '*'
 

ci/run_clang_tidy.sh

@@ -92,7 +92,7 @@ function filter_excludes() {
 
 function run_clang_tidy() {
   echo "Running clang_tidy with NUM_CPUS: ${NUM_CPUS:-0}."
-  python3 "${LLVM_PREFIX}/share/clang/run-clang-tidy.py" \
+  python3 "${LLVM_PREFIX}/bin/run-clang-tidy" \
     -clang-tidy-binary="${CLANG_TIDY}" \
     -clang-apply-replacements-binary="${CLANG_APPLY_REPLACEMENTS}" \
     -export-fixes=${FIX_YAML} -j "${NUM_CPUS:-0}" -p "${SRCDIR}" -quiet \

source/client/stream_decoder.h

@@ -56,8 +56,7 @@ class StreamDecoder : public Envoy::Http::ResponseDecoder,
         response_body_sizes_statistic_(response_body_sizes_statistic),
         origin_latency_statistic_(origin_latency_statistic),
         request_headers_(std::move(request_headers)), connect_start_(time_source_.monotonicTime()),
-        complete_(false), measure_latencies_(measure_latencies),
-        request_body_size_(request_body_size),
+        measure_latencies_(measure_latencies), request_body_size_(request_body_size),
         downstream_address_setter_(std::make_shared<Envoy::Network::ConnectionInfoSetterImpl>(
             // The two addresses aren't used in an execution of Nighthawk.
             /* downstream_local_address = */ nullptr, /* downstream_remote_address = */ nullptr)),
@@ -118,7 +117,7 @@ class StreamDecoder : public Envoy::Http::ResponseDecoder,
   Envoy::Http::ResponseTrailerMapPtr trailer_headers_;
   const Envoy::MonotonicTime connect_start_;
   Envoy::MonotonicTime request_start_;
-  bool complete_;
+  bool complete_ = false;
   bool measure_latencies_;
   const uint32_t request_body_size_;
   Envoy::Tracing::EgressConfigImpl config_;

source/common/statistic_impl.cc

@@ -73,11 +73,11 @@ uint64_t StatisticImpl::min() const { return min_; };
 uint64_t StatisticImpl::max() const { return max_; };
 
 absl::StatusOr<std::unique_ptr<std::istream>> StatisticImpl::serializeNative() const {
-  return absl::Status(absl::StatusCode::kUnimplemented, "serializeNative not implemented.");
+  return absl::Status{absl::StatusCode::kUnimplemented, "serializeNative not implemented."};
 }
 
 absl::Status StatisticImpl::deserializeNative(std::istream&) {
-  return absl::Status(absl::StatusCode::kUnimplemented, "deserializeNative not implemented.");
+  return absl::Status{absl::StatusCode::kUnimplemented, "deserializeNative not implemented."};
 }
 
 void SimpleStatistic::addValue(uint64_t value) {
@@ -127,7 +127,7 @@ absl::Status SimpleStatistic::deserializeNative(std::istream& stream) {
   std::string tmp(std::istreambuf_iterator<char>(stream), {});
   if (!proto.ParseFromString(tmp)) {
     ENVOY_LOG(error, "Failed to read back SimpleStatistic data.");
-    return absl::Status(absl::StatusCode::kInternal, "Failed to read back SimpleStatistic data");
+    return absl::Status{absl::StatusCode::kInternal, "Failed to read back SimpleStatistic data"};
   }
   id_ = proto.id();
   count_ = proto.count();
@@ -197,7 +197,7 @@ absl::Status StreamingStatistic::deserializeNative(std::istream& stream) {
   std::string tmp(std::istreambuf_iterator<char>(stream), {});
   if (!proto.ParseFromString(tmp)) {
     ENVOY_LOG(error, "Failed to read back StreamingStatistic data.");
-    return absl::Status(absl::StatusCode::kInternal, "Failed to read back StreamingStatistic data");
+    return absl::Status{absl::StatusCode::kInternal, "Failed to read back StreamingStatistic data"};
   }
   id_ = proto.id();
   count_ = proto.count();
@@ -338,7 +338,7 @@ absl::Status HdrStatistic::deserializeNative(std::istream& stream) {
     return absl::OkStatus();
   }
   ENVOY_LOG(error, "Failed to read back HdrHistogram data.");
-  return absl::Status(absl::StatusCode::kInternal, "Failed to read back HdrHistogram data");
+  return absl::Status{absl::StatusCode::kInternal, "Failed to read back HdrHistogram data"};
 }
 
 CircllhistStatistic::CircllhistStatistic() {

source/distributor/service_impl.cc

@@ -19,19 +19,19 @@ grpc::Status validateRequest(const nighthawk::DistributedRequest& request) {
   try {
     Envoy::MessageUtil::validate(request, validation_visitor);
   } catch (const Envoy::ProtoValidationException& e) {
-    return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT, e.what());
+    return grpc::Status{grpc::StatusCode::INVALID_ARGUMENT, e.what()};
   }
 
   if (request.has_execution_request()) {
     const nighthawk::client::ExecutionRequest& execution_request = request.execution_request();
     if (!execution_request.start_request().has_options()) {
-      return grpc::Status(
+      return grpc::Status{
           grpc::StatusCode::INVALID_ARGUMENT,
-          "DistributedRequest.ExecutionRequest.StartRequest MUST have CommandLineOptions.");
+          "DistributedRequest.ExecutionRequest.StartRequest MUST have CommandLineOptions."};
     }
   } else {
-    return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT,
-                        "DistributedRequest.ExecutionRequest MUST be specified.");
+    return grpc::Status{grpc::StatusCode::INVALID_ARGUMENT,
+                        "DistributedRequest.ExecutionRequest MUST be specified."};
   }
   return grpc::Status::OK;
 }

source/sink/service_impl.cc

@@ -78,16 +78,16 @@ absl::Status mergeOutput(const nighthawk::client::Output& input_to_merge,
     // Versions probably shouldn't either. We sanity check these things here, and
     // report on error when we detect any mismatch.
     if (!MessageDifferencer::Equivalent(input_to_merge.options(), merge_target.options())) {
-      return absl::Status(absl::StatusCode::kInternal,
+      return absl::Status{absl::StatusCode::kInternal,
                           fmt::format("Options divergence detected: {} vs {}.",
                                       merge_target.options().DebugString(),
-                                      input_to_merge.options().DebugString()));
+                                      input_to_merge.options().DebugString())};
     }
     if (!MessageDifferencer::Equivalent(input_to_merge.version(), merge_target.version())) {
-      return absl::Status(absl::StatusCode::kInternal,
+      return absl::Status{absl::StatusCode::kInternal,
                           fmt::format("Version divergence detected: {} vs {}.",
                                       merge_target.version().DebugString(),
-                                      input_to_merge.version().DebugString()));
+                                      input_to_merge.version().DebugString())};
     }
   }
   // Append all input results into our own results.

test/adaptive_load/adaptive_load_client_main_test.cc

@@ -259,9 +259,9 @@ TEST(AdaptiveLoadClientMainTest, FailsIfClosingOutputFileFails) {
           true, Envoy::Api::IoErrorPtr(nullptr, [](Envoy::Api::IoError*) {})))));
   EXPECT_CALL(*mock_file, write_(_))
       .WillRepeatedly(Invoke([](absl::string_view data) -> Envoy::Api::IoCallSizeResult {
-        return Envoy::Api::IoCallSizeResult(
+        return Envoy::Api::IoCallSizeResult{
             static_cast<ssize_t>(data.length()),
-            Envoy::Api::IoErrorPtr(nullptr, [](Envoy::Api::IoError*) {}));
+            Envoy::Api::IoErrorPtr(nullptr, [](Envoy::Api::IoError*) {})};
       }));
   EXPECT_CALL(*mock_file, close_())
       .WillOnce(Return(ByMove(Envoy::Api::IoCallBoolResult(
@@ -301,9 +301,9 @@ TEST(AdaptiveLoadClientMainTest, WritesOutputProtoToFile) {
       .WillRepeatedly(Invoke(
           [&actual_outfile_contents](absl::string_view data) -> Envoy::Api::IoCallSizeResult {
             actual_outfile_contents += std::string(data);
-            return Envoy::Api::IoCallSizeResult(
+            return Envoy::Api::IoCallSizeResult{
                 static_cast<ssize_t>(data.length()),
-                Envoy::Api::IoErrorPtr(nullptr, [](Envoy::Api::IoError*) {}));
+                Envoy::Api::IoErrorPtr(nullptr, [](Envoy::Api::IoError*) {})};
           }));
 
   EXPECT_CALL(*mock_file, close_())

test/adaptive_load/fake_plugins/fake_input_variable_setter/fake_input_variable_setter.cc

@@ -5,7 +5,7 @@ namespace Nighthawk {
 namespace {
 
 absl::Status StatusFromProtoRpcStatus(const google::rpc::Status& status_proto) {
-  return absl::Status(static_cast<absl::StatusCode>(status_proto.code()), status_proto.message());
+  return absl::Status{static_cast<absl::StatusCode>(status_proto.code()), status_proto.message()};
 }
 
 } // namespace

test/adaptive_load/fake_plugins/fake_metrics_plugin/fake_metrics_plugin.cc

@@ -11,7 +11,7 @@ namespace Nighthawk {
 namespace {
 
 absl::Status GetStatusFromProtoRpcStatus(const google::rpc::Status& status_proto) {
-  return absl::Status(static_cast<absl::StatusCode>(status_proto.code()), status_proto.message());
+  return absl::Status{static_cast<absl::StatusCode>(status_proto.code()), status_proto.message()};
 }
 
 absl::StatusOr<double> StatusOrFromFakeMetricProto(

test/adaptive_load/fake_plugins/fake_step_controller/fake_step_controller.cc

@@ -9,7 +9,7 @@ namespace Nighthawk {
 namespace {
 
 absl::Status StatusFromProtoRpcStatus(const google::rpc::Status& status_proto) {
-  return absl::Status(static_cast<absl::StatusCode>(status_proto.code()), status_proto.message());
+  return absl::Status{static_cast<absl::StatusCode>(status_proto.code()), status_proto.message()};
 }
 
 } // namespace