Upgrade clang-tidy and clang-format to 11 (grpc#25590)

* Upgrade clang-tidy and clang-format to 11 * Reformat code * Fix abseil-string-find-str-contains * Fix modernize-make-unique
dmeister · Mar 4, 2021 · 377fe60 · 377fe60
1 parent dae5624
commit 377fe60
Show file tree

Hide file tree

Showing 20 changed files with 131 additions and 132 deletions.
diff --git a/.clang-format b/.clang-format
@@ -3,6 +3,7 @@ Language: Cpp
 BasedOnStyle: Google
 DerivePointerAlignment: false
 PointerAlignment: Left
+IncludeBlocks: Preserve
 ---
 Language: ObjC
 BasedOnStyle: Google

diff --git a/.clang-tidy b/.clang-tidy
@@ -31,7 +31,18 @@ Checks: '-*,
   readability-inconsistent-declaration-parameter-name,
   readability-redundant-control-flow,
   readability-redundant-smartptr-get,
-  readability-string-compare'
+  readability-string-compare,
+
+  -bugprone-branch-clone,
+  -bugprone-infinite-loop,
+  -bugprone-not-null-terminated-result,
+  -bugprone-reserved-identifier,
+  -bugprone-signed-char-misuse,
+  -bugprone-sizeof-expression,
+  -bugprone-unhandled-self-assignment,
+  -google-readability-avoid-underscore-in-googletest-name,
+  -google-upgrade-googletest-case,
+  -performance-no-automatic-move'
 WarningsAsErrors: '*'
 CheckOptions:
   - key:    readability-function-size.StatementThreshold

diff --git a/src/cpp/server/health/default_health_check_service.cc b/src/cpp/server/health/default_health_check_service.cc
@@ -243,10 +243,9 @@ bool DefaultHealthCheckService::HealthCheckServiceImpl::EncodeResponse(
       grpc_health_v1_HealthCheckResponse_new(arena.ptr());
   grpc_health_v1_HealthCheckResponse_set_status(
       response_struct,
-      status == NOT_FOUND
-          ? grpc_health_v1_HealthCheckResponse_SERVICE_UNKNOWN
-          : status == SERVING ? grpc_health_v1_HealthCheckResponse_SERVING
-                              : grpc_health_v1_HealthCheckResponse_NOT_SERVING);
+      status == NOT_FOUND ? grpc_health_v1_HealthCheckResponse_SERVICE_UNKNOWN
+      : status == SERVING ? grpc_health_v1_HealthCheckResponse_SERVING
+                          : grpc_health_v1_HealthCheckResponse_NOT_SERVING);
   size_t buf_length;
   char* buf = grpc_health_v1_HealthCheckResponse_serialize(
       response_struct, arena.ptr(), &buf_length);

diff --git a/src/objective-c/GRPCClient/private/GRPCCore/GRPCChannel.m b/src/objective-c/GRPCClient/private/GRPCCore/GRPCChannel.m
@@ -54,8 +54,8 @@ - (instancetype)initWithHost:(NSString *)host callOptions:(GRPCCallOptions *)cal
   if (_callOptions.transport != NULL) {
     id<GRPCTransportFactory> transportFactory =
         [[GRPCTransportRegistry sharedInstance] getTransportFactoryWithID:_callOptions.transport];
-    if (!
-        [transportFactory respondsToSelector:@selector(createCoreChannelFactoryWithCallOptions:)]) {
+    if (![transportFactory
+            respondsToSelector:@selector(createCoreChannelFactoryWithCallOptions:)]) {
       // impossible because we are using GRPCCore now
       [NSException raise:NSInternalInconsistencyException
                   format:@"Transport factory type is wrong"];

diff --git a/templates/tools/dockerfile/grpc_clang_format/Dockerfile.template b/templates/tools/dockerfile/grpc_clang_format/Dockerfile.template
@@ -14,14 +14,11 @@
   # See the License for the specific language governing permissions and
   # limitations under the License.
 
-  FROM debian:10
-
-  # Add buster-backports for more recent clang packages
-  RUN echo "deb http://deb.debian.org/debian buster-backports main" | tee /etc/apt/sources.list.d/buster-backports.list
+  FROM debian:bullseye
 
   # Install clang-format
-  RUN apt-get update && apt-get install -y clang-format-8
-  ENV CLANG_FORMAT=clang-format-8
+  RUN apt-get update && apt-get install -y clang-format-11
+  ENV CLANG_FORMAT=clang-format-11
 
   ADD clang_format_all_the_things.sh /
 

diff --git a/templates/tools/dockerfile/grpc_clang_tidy/Dockerfile.template b/templates/tools/dockerfile/grpc_clang_tidy/Dockerfile.template
@@ -14,14 +14,11 @@
   # See the License for the specific language governing permissions and
   # limitations under the License.
 
-  <%include file="../python_debian10.include"/>
+  FROM debian:bullseye
 
-  # Add buster-backports for more recent clang packages
-  RUN echo "deb http://deb.debian.org/debian buster-backports main" | tee /etc/apt/sources.list.d/buster-backports.list
-
-  # Install clang-tidy 7
-  RUN apt-get update && apt-get install -y clang-tidy-8 jq
-  ENV CLANG_TIDY=clang-tidy-8
+  # Install clang-tidy 11
+  RUN apt-get update && apt-get install -y clang-tidy-11 jq
+  ENV CLANG_TIDY=clang-tidy-11
 
   ADD clang_tidy_all_the_things.sh /
 

diff --git a/templates/tools/dockerfile/python_debian10.include b/templates/tools/dockerfile/python_debian10.include
diff --git a/templates/tools/dockerfile/python_debian11.include b/templates/tools/dockerfile/python_debian11.include
@@ -0,0 +1,53 @@
+FROM debian:bullseye
+
+# Install Git and basic packages.
+RUN apt-get update && apt-get install -y ${'\\'}
+  autoconf ${'\\'}
+  autotools-dev ${'\\'}
+  build-essential ${'\\'}
+  bzip2 ${'\\'}
+  ccache ${'\\'}
+  curl ${'\\'}
+  dnsutils ${'\\'}
+  gcc ${'\\'}
+  gcc-multilib ${'\\'}
+  git ${'\\'}
+  golang ${'\\'}
+  gyp ${'\\'}
+  lcov ${'\\'}
+  libc6 ${'\\'}
+  libc6-dbg ${'\\'}
+  libc6-dev ${'\\'}
+  libgtest-dev ${'\\'}
+  libtool ${'\\'}
+  make ${'\\'}
+  perl ${'\\'}
+  strace ${'\\'}
+  telnet ${'\\'}
+  unzip ${'\\'}
+  wget ${'\\'}
+  zip && apt-get clean
+
+#================
+# Build profiling
+RUN apt-get update && apt-get install -y time && apt-get clean
+
+# Install Python 3.7 from source (and installed as a default python3)
+# (Bullseye comes with Python 3.9 which isn't supported by pytype yet)
+RUN apt update && apt install -y build-essential zlib1g-dev libncurses5-dev libgdbm-dev ${'\\'}
+                                 libnss3-dev libssl-dev libreadline-dev libffi-dev
+RUN curl -O https://www.python.org/ftp/python/3.7.9/Python-3.7.9.tar.xz && ${'\\'}
+    tar -xf Python-3.7.9.tar.xz && ${'\\'}
+    cd Python-3.7.9 && ${'\\'}
+    ./configure && ${'\\'}
+    make -j 4 && ${'\\'}
+    make install
+RUN curl https://bootstrap.pypa.io/get-pip.py | python3
+
+# Install Python 2.7
+RUN apt-get update && apt-get install -y python2 python2-dev
+RUN ln -s /usr/bin/python2 /usr/bin/python
+RUN curl https://bootstrap.pypa.io/2.7/get-pip.py | python2
+
+<%include file="./gcp_api_libraries.include"/>
+<%include file="./run_tests_addons.include"/>
diff --git a/templates/tools/dockerfile/test/sanity/Dockerfile.template b/templates/tools/dockerfile/test/sanity/Dockerfile.template
@@ -14,16 +14,12 @@
   # See the License for the specific language governing permissions and
   # limitations under the License.
 
-  <%include file="../../python_debian10.include"/>
+  <%include file="../../python_debian11.include"/>
   <%include file="../../cxx_deps.include"/>
 
   #========================
   # Sanity test dependencies
-  RUN apt-get update && apt-get -t buster install -y python3.7 python3-all-dev
-  RUN curl https://bootstrap.pypa.io/get-pip.py | python3.7
 
-  # Make Python 3.7 the default Python 3 version
-  RUN update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.7 1
   RUN apt-get update && apt-get install -y ${"\\"}
         autoconf ${"\\"}
         automake ${"\\"}
@@ -34,15 +30,13 @@
   RUN python3 -m pip install simplejson mako virtualenv==16.7.9 lxml six
 
   # Upgrade Python's YAML library
+  RUN python2 -m pip install --upgrade --ignore-installed PyYAML==5.4.1 --user
   RUN python3 -m pip install --upgrade --ignore-installed PyYAML==5.4.1 --user
 
-  # Add buster-backports for more recent clang packages
-  RUN echo "deb http://deb.debian.org/debian buster-backports main" | tee /etc/apt/sources.list.d/buster-backports.list
-
   # Install clang, clang-format, and clang-tidy
-  RUN apt-get update && apt-get install -y clang clang-format-8 clang-tidy-8 jq
-  ENV CLANG_FORMAT=clang-format-8
-  ENV CLANG_TIDY=clang-tidy-8
+  RUN apt-get update && apt-get install -y clang clang-format-11 clang-tidy-11 jq
+  ENV CLANG_FORMAT=clang-format-11
+  ENV CLANG_TIDY=clang-tidy-11
 
 
   <%include file="../../bazel.include"/>

diff --git a/test/cpp/client/client_channel_stress_test.cc b/test/cpp/client/client_channel_stress_test.cc
@@ -24,6 +24,7 @@
 #include <string>
 #include <thread>
 
+#include "absl/memory/memory.h"
 #include "absl/strings/str_cat.h"
 
 #include <grpc/grpc.h>
@@ -180,8 +181,8 @@ class ClientChannelStressTest {
       port_ = grpc_pick_unused_port_or_die();
       gpr_log(GPR_INFO, "starting %s server on port %d", type_.c_str(), port_);
       grpc::internal::CondVar cond;
-      thread_.reset(new std::thread(
-          std::bind(&ServerThread::Start, this, server_host, &mu, &cond)));
+      thread_ = absl::make_unique<std::thread>(
+          std::bind(&ServerThread::Start, this, server_host, &mu, &cond));
       cond.Wait(&mu);
       gpr_log(GPR_INFO, "%s server startup complete", type_.c_str());
     }

diff --git a/test/cpp/end2end/client_callback_end2end_test.cc b/test/cpp/end2end/client_callback_end2end_test.cc
@@ -241,7 +241,7 @@ class ClientCallbackEnd2endTest
             cv.notify_one();
 #if GRPC_ALLOW_EXCEPTIONS
             if (maybe_except) {
-              throw - 1;
+              throw -1;
             }
 #else
             GPR_ASSERT(!maybe_except);

diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc
@@ -38,6 +38,7 @@
 #include <thread>
 
 #include "absl/memory/memory.h"
+#include "absl/strings/match.h"
 #include "absl/strings/str_format.h"
 
 #include "src/core/ext/filters/client_channel/backup_poller.h"
@@ -1459,13 +1460,11 @@ TEST_P(End2endTest, ExpectErrorTest) {
     EXPECT_EQ(iter->code(), s.error_code());
     EXPECT_EQ(iter->error_message(), s.error_message());
     EXPECT_EQ(iter->binary_error_details(), s.error_details());
-    EXPECT_TRUE(context.debug_error_string().find("created") !=
-                std::string::npos);
-    EXPECT_TRUE(context.debug_error_string().find("file") != std::string::npos);
-    EXPECT_TRUE(context.debug_error_string().find("line") != std::string::npos);
-    EXPECT_TRUE(context.debug_error_string().find("status") !=
-                std::string::npos);
-    EXPECT_TRUE(context.debug_error_string().find("13") != std::string::npos);
+    EXPECT_TRUE(absl::StrContains(context.debug_error_string(), "created"));
+    EXPECT_TRUE(absl::StrContains(context.debug_error_string(), "file"));
+    EXPECT_TRUE(absl::StrContains(context.debug_error_string(), "line"));
+    EXPECT_TRUE(absl::StrContains(context.debug_error_string(), "status"));
+    EXPECT_TRUE(absl::StrContains(context.debug_error_string(), "13"));
   }
 }
 

diff --git a/test/cpp/end2end/exception_test.cc b/test/cpp/end2end/exception_test.cc
@@ -41,7 +41,7 @@ class ExceptingServiceImpl : public ::grpc::testing::EchoTestService::Service {
  public:
   Status Echo(ServerContext* /*server_context*/, const EchoRequest* /*request*/,
               EchoResponse* /*response*/) override {
-    throw - 1;
+    throw -1;
   }
   Status RequestStream(ServerContext* /*context*/,
                        ServerReader<EchoRequest>* /*reader*/,

diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc
@@ -24,6 +24,7 @@
 #include <string>
 #include <thread>
 
+#include "absl/memory/memory.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_format.h"
 
@@ -696,8 +697,8 @@ class GrpclbEnd2endTest : public ::testing::Test {
       // by ServerThread::Serve from firing before the wait below is hit.
       grpc::internal::MutexLock lock(&mu);
       grpc::internal::CondVar cond;
-      thread_.reset(new std::thread(
-          std::bind(&ServerThread::Serve, this, server_host, &mu, &cond)));
+      thread_ = absl::make_unique<std::thread>(
+          std::bind(&ServerThread::Serve, this, server_host, &mu, &cond));
       cond.Wait(&mu);
       gpr_log(GPR_INFO, "%s server startup complete", type_.c_str());
     }

diff --git a/test/cpp/interop/interop_client.cc b/test/cpp/interop/interop_client.cc
@@ -23,6 +23,7 @@
 #include <type_traits>
 #include <utility>
 
+#include "absl/strings/match.h"
 #include "absl/strings/str_format.h"
 
 #include <grpc/grpc.h>
@@ -225,7 +226,7 @@ bool InteropClient::DoComputeEngineCreds(
   GPR_ASSERT(response.username().c_str() == default_service_account);
   GPR_ASSERT(!response.oauth_scope().empty());
   const char* oauth_scope_str = response.oauth_scope().c_str();
-  GPR_ASSERT(oauth_scope.find(oauth_scope_str) != std::string::npos);
+  GPR_ASSERT(absl::StrContains(oauth_scope, oauth_scope_str));
   gpr_log(GPR_DEBUG, "Large unary with compute engine creds done.");
   return true;
 }
@@ -251,7 +252,7 @@ bool InteropClient::DoOauth2AuthToken(const std::string& username,
   GPR_ASSERT(!response.oauth_scope().empty());
   GPR_ASSERT(username == response.username());
   const char* oauth_scope_str = response.oauth_scope().c_str();
-  GPR_ASSERT(oauth_scope.find(oauth_scope_str) != std::string::npos);
+  GPR_ASSERT(absl::StrContains(oauth_scope, oauth_scope_str));
   gpr_log(GPR_DEBUG, "Unary with oauth2 access token credentials done.");
   return true;
 }

diff --git a/test/cpp/qps/client_async.cc b/test/cpp/qps/client_async.cc
@@ -142,7 +142,7 @@ class ClientRpcContextUnaryImpl : public ClientRpcContext {
     if (!next_issue_) {  // ready to issue
       RunNextState(true, nullptr);
     } else {  // wait for the issue time
-      alarm_.reset(new Alarm);
+      alarm_ = absl::make_unique<Alarm>();
       alarm_->Set(cq_, next_issue_(), ClientRpcContext::tag(this));
     }
   }
@@ -371,7 +371,7 @@ class ClientRpcContextStreamingPingPongImpl : public ClientRpcContext {
           break;  // loop around, don't return
         case State::WAIT:
           next_state_ = State::READY_TO_WRITE;
-          alarm_.reset(new Alarm);
+          alarm_ = absl::make_unique<Alarm>();
           alarm_->Set(cq_, next_issue_(), ClientRpcContext::tag(this));
           return true;
         case State::READY_TO_WRITE:
@@ -556,7 +556,7 @@ class ClientRpcContextStreamingFromClientImpl : public ClientRpcContext {
           }
           break;  // loop around, don't return
         case State::WAIT:
-          alarm_.reset(new Alarm);
+          alarm_ = absl::make_unique<Alarm>();
           alarm_->Set(cq_, next_issue_(), ClientRpcContext::tag(this));
           next_state_ = State::READY_TO_WRITE;
           return true;

diff --git a/tools/distrib/run_clang_tidy.py b/tools/distrib/run_clang_tidy.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python2.7
+#!/usr/bin/env python3
 # Copyright 2017 gRPC authors.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");

diff --git a/tools/dockerfile/grpc_clang_format/Dockerfile b/tools/dockerfile/grpc_clang_format/Dockerfile
@@ -12,14 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM debian:10
-
-# Add buster-backports for more recent clang packages
-RUN echo "deb http://deb.debian.org/debian buster-backports main" | tee /etc/apt/sources.list.d/buster-backports.list
+FROM debian:bullseye
 
 # Install clang-format
-RUN apt-get update && apt-get install -y clang-format-8
-ENV CLANG_FORMAT=clang-format-8
+RUN apt-get update && apt-get install -y clang-format-11
+ENV CLANG_FORMAT=clang-format-11
 
 ADD clang_format_all_the_things.sh /