Fix readability-const-return-type,readability-delete-null-pointer,readability-simplify-boolean-expr (#25628)

.clang-tidy

@@ -45,12 +45,9 @@
 # - performance-no-automatic-move
 # - performance-unnecessary-copy-initialization
 # - performance-unnecessary-value-param
-# - readability-const-return-type
-# - readability-delete-null-pointer
 # - readability-else-after-return
 # - readability-implicit-bool-conversion
 # - readability-redundant-declaration
-# - readability-simplify-boolean-expr
 # - readability-static-definition-in-anonymous-namespace
 #
 Checks: '-*,
@@ -108,7 +105,9 @@ Checks: '-*,
   modernize-use-nullptr,
   modernize-use-override,
   modernize-use-transparent-functors,
+  readability-const-return-type,
   readability-container-size-empty,
+  readability-delete-null-pointer,
   readability-deleted-default,
   readability-function-size,
   readability-inconsistent-declaration-parameter-name,
@@ -117,6 +116,7 @@ Checks: '-*,
   readability-redundant-control-flow,
   readability-redundant-function-ptr-dereference,
   readability-redundant-smartptr-get,
+  readability-simplify-boolean-expr,
   readability-string-compare,
   readability-uniqueptr-delete-release'
 WarningsAsErrors: '*'

include/grpcpp/impl/codegen/interceptor_common.h

@@ -223,18 +223,11 @@ class InterceptorBatchMethodsImpl
   bool InterceptorsListEmpty() {
     auto* client_rpc_info = call_->client_rpc_info();
     if (client_rpc_info != nullptr) {
-      if (client_rpc_info->interceptors_.empty()) {
-        return true;
-      } else {
-        return false;
-      }
+      return client_rpc_info->interceptors_.empty();
     }
 
     auto* server_rpc_info = call_->server_rpc_info();
-    if (server_rpc_info == nullptr || server_rpc_info->interceptors_.empty()) {
-      return true;
-    }
-    return false;
+    return server_rpc_info == nullptr || server_rpc_info->interceptors_.empty();
   }
 
   // This should be used only by subclasses of CallOpSetInterface. SetCall and
@@ -419,11 +412,7 @@ class CancelInterceptorBatchMethods
  public:
   bool QueryInterceptionHookPoint(
       experimental::InterceptionHookPoints type) override {
-    if (type == experimental::InterceptionHookPoints::PRE_SEND_CANCEL) {
-      return true;
-    } else {
-      return false;
-    }
+    return type == experimental::InterceptionHookPoints::PRE_SEND_CANCEL;
   }
 
   void Proceed() override {

include/grpcpp/impl/codegen/security/auth_context.h

@@ -42,7 +42,7 @@ class AuthPropertyIterator
   AuthPropertyIterator operator++(int);
   bool operator==(const AuthPropertyIterator& rhs) const;
   bool operator!=(const AuthPropertyIterator& rhs) const;
-  const AuthProperty operator*();
+  AuthProperty operator*();
 
  protected:
   AuthPropertyIterator();

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc

@@ -501,7 +501,7 @@ void XdsClusterManagerLb::ClusterChild::ResetBackoffLocked() {
 
 void XdsClusterManagerLb::ClusterChild::DeactivateLocked() {
   // If already deactivated, don't do that again.
-  if (delayed_removal_timer_callback_pending_ == true) return;
+  if (delayed_removal_timer_callback_pending_) return;
   // Set the child weight to 0 so that future picker won't contain this child.
   // Start a timer to delete the child.
   Ref(DEBUG_LOCATION, "ClusterChild+timer").release();

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc

@@ -131,7 +131,7 @@ class XdsClusterResolverLb : public LoadBalancingPolicy {
     virtual bool disable_reresolution() = 0;
 
     // Caller must ensure that config_ is set before calling.
-    const absl::string_view GetXdsClusterResolverResourceName() const {
+    absl::string_view GetXdsClusterResolverResourceName() const {
       if (!parent_->is_xds_uri_) return parent_->server_name_;
       if (!parent_->config_->discovery_mechanisms()[index_]
                .eds_service_name.empty()) {

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc

@@ -971,11 +971,7 @@ static bool target_matches_localhost_inner(const char* name, std::string* host,
     gpr_log(GPR_ERROR, "Unable to split host and port for name: %s", name);
     return false;
   }
-  if (gpr_stricmp(host->c_str(), "localhost") == 0) {
-    return true;
-  } else {
-    return false;
-  }
+  return gpr_stricmp(host->c_str(), "localhost") == 0;
 }
 
 static bool target_matches_localhost(const char* name) {

src/core/ext/filters/workarounds/workaround_cronet_compression_filter.cc

@@ -192,7 +192,7 @@ static bool register_workaround_cronet_compression(
   if (a == nullptr) {
     return true;
   }
-  if (grpc_channel_arg_get_bool(a, false) == false) {
+  if (!grpc_channel_arg_get_bool(a, false)) {
     return true;
   }
   return grpc_channel_stack_builder_prepend_filter(

src/core/ext/transport/chttp2/transport/hpack_encoder.cc

@@ -104,7 +104,7 @@ struct SliceRefComparator {
 
 struct MetadataComparator {
   typedef grpc_mdelem Type;
-  static const grpc_mdelem Null() { return {0}; }
+  static grpc_mdelem Null() { return {0}; }
   static bool IsNull(const grpc_mdelem md) { return md.payload == 0; }
   static bool Equals(const grpc_mdelem md1, const grpc_mdelem md2) {
     return md1.payload == md2.payload;

src/core/ext/transport/cronet/transport/cronet_transport.cc

@@ -1200,7 +1200,7 @@ static enum e_op_result execute_stream_op(struct op_and_state* oas) {
       stream_state->state_op_done[OP_RECV_MESSAGE] = true;
       oas->state.state_op_done[OP_RECV_MESSAGE] = true;
       result = ACTION_TAKEN_NO_CALLBACK;
-    } else if (stream_state->rs.read_stream_closed == true) {
+    } else if (stream_state->rs.read_stream_closed) {
       /* No more data will be received */
       CRONET_LOG(GPR_DEBUG, "read stream closed");
       grpc_core::ExecCtx::Run(
@@ -1217,7 +1217,7 @@ static enum e_op_result execute_stream_op(struct op_and_state* oas) {
       stream_state->state_op_done[OP_RECV_MESSAGE] = true;
       oas->state.state_op_done[OP_RECV_MESSAGE] = true;
       result = ACTION_TAKEN_NO_CALLBACK;
-    } else if (stream_state->rs.length_field_received == false) {
+    } else if (!stream_state->rs.length_field_received) {
       if (stream_state->rs.received_bytes == GRPC_HEADER_SIZE_IN_BYTES &&
           stream_state->rs.remaining_bytes == 0) {
         /* Start a read operation for data */

src/core/lib/gpr/sync_abseil.cc

@@ -62,8 +62,7 @@ void gpr_mu_unlock(gpr_mu* mu) ABSL_NO_THREAD_SAFETY_ANALYSIS {
 
 int gpr_mu_trylock(gpr_mu* mu) {
   GPR_TIMER_SCOPE("gpr_mu_trylock", 0);
-  int ret = reinterpret_cast<absl::Mutex*>(mu)->TryLock() == true;
-  return ret;
+  return reinterpret_cast<absl::Mutex*>(mu)->TryLock();
 }
 
 /*----------------------------------------*/
@@ -89,10 +88,8 @@ int gpr_cv_wait(gpr_cv* cv, gpr_mu* mu, gpr_timespec abs_deadline) {
   abs_deadline = gpr_convert_clock_type(abs_deadline, GPR_CLOCK_REALTIME);
   timespec ts = {static_cast<decltype(ts.tv_sec)>(abs_deadline.tv_sec),
                  static_cast<decltype(ts.tv_nsec)>(abs_deadline.tv_nsec)};
-  int ret = reinterpret_cast<absl::CondVar*>(cv)->WaitWithDeadline(
-                reinterpret_cast<absl::Mutex*>(mu),
-                absl::TimeFromTimespec(ts)) == true;
-  return ret;
+  return reinterpret_cast<absl::CondVar*>(cv)->WaitWithDeadline(
+      reinterpret_cast<absl::Mutex*>(mu), absl::TimeFromTimespec(ts));
 }
 
 void gpr_cv_signal(gpr_cv* cv) {

src/core/lib/iomgr/tcp_posix.cc

@@ -1673,10 +1673,7 @@ static bool tcp_can_track_err(grpc_endpoint* ep) {
   if (getsockname(tcp->fd, &addr, &len) < 0) {
     return false;
   }
-  if (addr.sa_family == AF_INET || addr.sa_family == AF_INET6) {
-    return true;
-  }
-  return false;
+  return addr.sa_family == AF_INET || addr.sa_family == AF_INET6;
 }
 
 static const grpc_endpoint_vtable vtable = {tcp_read,

src/core/lib/surface/server.cc

@@ -877,7 +877,7 @@ grpc_call_error Server::ValidateServerRequest(
                            (rm->payload_handling == GRPC_SRM_PAYLOAD_NONE)))) {
     return GRPC_CALL_ERROR_PAYLOAD_TYPE_MISMATCH;
   }
-  if (grpc_cq_begin_op(cq_for_notification, tag) == false) {
+  if (!grpc_cq_begin_op(cq_for_notification, tag)) {
     return GRPC_CALL_ERROR_COMPLETION_QUEUE_SHUTDOWN;
   }
   return GRPC_CALL_OK;

src/core/tsi/alts/handshaker/alts_handshaker_client.cc

@@ -117,10 +117,7 @@ static void handshaker_client_send_buffer_destroy(
 
 static bool is_handshake_finished_properly(grpc_gcp_HandshakerResp* resp) {
   GPR_ASSERT(resp != nullptr);
-  if (grpc_gcp_HandshakerResp_result(resp)) {
-    return true;
-  }
-  return false;
+  return grpc_gcp_HandshakerResp_result(resp) != nullptr;
 }
 
 static void alts_grpc_handshaker_client_unref(

src/core/tsi/alts/handshaker/transport_security_common_api.cc

@@ -214,9 +214,7 @@ bool grpc_gcp_rpc_protocol_versions_check(
           ? &local_versions->min_rpc_version
           : &peer_versions->min_rpc_version;
   bool result = grpc_core::internal::grpc_gcp_rpc_protocol_version_compare(
-                    max_common_version, min_common_version) >= 0
-                    ? true
-                    : false;
+                    max_common_version, min_common_version) >= 0;
   if (result && highest_common_version != nullptr) {
     memcpy(highest_common_version, max_common_version,
            sizeof(grpc_gcp_rpc_protocol_versions_version));

src/cpp/common/auth_property_iterator.cc

@@ -61,7 +61,7 @@ bool AuthPropertyIterator::operator!=(const AuthPropertyIterator& rhs) const {
   return !operator==(rhs);
 }
 
-const AuthProperty AuthPropertyIterator::operator*() {
+AuthProperty AuthPropertyIterator::operator*() {
   return std::pair<grpc::string_ref, grpc::string_ref>(
       property_->name,
       grpc::string_ref(property_->value, property_->value_length));

src/cpp/common/channel_filter.h

@@ -60,7 +60,7 @@ class MetadataBatch {
                                               const grpc_mdelem> {
    public:
     const grpc_mdelem& operator*() const { return elem_->md; }
-    const grpc_mdelem operator->() const { return elem_->md; }
+    grpc_mdelem operator->() const { return elem_->md; }
 
     const_iterator& operator++() {
       elem_ = elem_->next;

test/core/bad_client/bad_client.cc

@@ -263,10 +263,7 @@ bool client_connection_preface_validator(grpc_slice_buffer* incoming,
   }
   const uint8_t* p = GRPC_SLICE_START_PTR(slice);
   /* Check the frame type (SETTINGS) */
-  if (*(p + 3) != 4) {
-    return false;
-  }
-  return true;
+  return *(p + 3) == 4;
 }
 
 /* connection preface and settings frame to be sent by the client */

test/core/end2end/tests/filter_init_fails.cc

@@ -169,7 +169,7 @@ static void test_server_channel_filter(grpc_end2end_test_config config) {
   CQ_EXPECT_COMPLETION(cqv, tag(1), 1);
   cq_verify(cqv);
 
-  if (g_channel_filter_init_failure == true) {
+  if (g_channel_filter_init_failure) {
     // Inproc channel returns invalid_argument and other clients return
     // unavailable.
     // Windows with sockpair returns unknown.

test/core/surface/concurrent_connectivity_test.cc

@@ -150,7 +150,7 @@ void bad_server_thread(void* vargs) {
   gpr_event_set(&args->ready, reinterpret_cast<void*>(1));
 
   gpr_mu_lock(args->mu);
-  while (args->stop.load(std::memory_order_acquire) == false) {
+  while (!args->stop.load(std::memory_order_acquire)) {
     grpc_millis deadline = grpc_core::ExecCtx::Get()->Now() + 100;
 
     grpc_pollset_worker* worker = nullptr;

test/core/tsi/alts/handshaker/alts_handshaker_client_test.cc

@@ -138,10 +138,7 @@ static grpc_gcp_HandshakerReq* deserialize_handshaker_req(
 }
 
 static bool is_recv_status_op(const grpc_op* op, size_t nops) {
-  if (nops == 1 && op->op == GRPC_OP_RECV_STATUS_ON_CLIENT) {
-    return true;
-  }
-  return false;
+  return nops == 1 && op->op == GRPC_OP_RECV_STATUS_ON_CLIENT;
 }
 
 /**

test/core/tsi/transport_security_test_lib.cc

@@ -341,11 +341,8 @@ static bool is_handshake_finished_properly(handshaker_args* args) {
   GPR_ASSERT(args != nullptr);
   GPR_ASSERT(args->fixture != nullptr);
   tsi_test_fixture* fixture = args->fixture;
-  if ((args->is_client && fixture->client_result != nullptr) ||
-      (!args->is_client && fixture->server_result != nullptr)) {
-    return true;
-  }
-  return false;
+  return (args->is_client && fixture->client_result != nullptr) ||
+         (!args->is_client && fixture->server_result != nullptr);
 }
 
 static void do_handshaker_next(handshaker_args* args) {

test/cpp/end2end/grpclb_end2end_test.cc

@@ -604,8 +604,8 @@ class GrpclbEnd2endTest : public ::testing::Test {
     response_generator_->SetReresolutionResponse(std::move(result));
   }
 
-  const std::vector<int> GetBackendPorts(size_t start_index = 0,
-                                         size_t stop_index = 0) const {
+  std::vector<int> GetBackendPorts(size_t start_index = 0,
+                                   size_t stop_index = 0) const {
     if (stop_index == 0) stop_index = backends_.size();
     std::vector<int> backend_ports;
     for (size_t i = start_index; i < stop_index; ++i) {

test/cpp/end2end/mock_test.cc

@@ -288,7 +288,7 @@ class TestServiceImpl : public EchoTestService::Service {
   }
 
  private:
-  const vector<std::string> split(const std::string& input) {
+  vector<std::string> split(const std::string& input) {
     std::string buff("");
     vector<std::string> result;
 

test/cpp/end2end/xds_end2end_test.cc

@@ -1999,8 +1999,8 @@ class XdsEnd2endTest : public ::testing::TestWithParam<TestType> {
     response_generator_->SetReresolutionResponse(std::move(result));
   }
 
-  const std::vector<int> GetBackendPorts(size_t start_index = 0,
-                                         size_t stop_index = 0) const {
+  std::vector<int> GetBackendPorts(size_t start_index = 0,
+                                   size_t stop_index = 0) const {
     if (stop_index == 0) stop_index = backends_.size();
     std::vector<int> backend_ports;
     for (size_t i = start_index; i < stop_index; ++i) {

test/cpp/interop/interop_client.cc

@@ -537,11 +537,7 @@ bool InteropClient::DoClientCompressedStreaming() {
   GPR_ASSERT(stream->WritesDone());
 
   s = stream->Finish();
-  if (!AssertStatusOk(s, context.debug_error_string())) {
-    return false;
-  }
-
-  return true;
+  return AssertStatusOk(s, context.debug_error_string());
 }
 
 bool InteropClient::DoServerCompressedStreaming() {
@@ -598,10 +594,7 @@ bool InteropClient::DoServerCompressedStreaming() {
   }
 
   Status s = stream->Finish();
-  if (!AssertStatusOk(s, context.debug_error_string())) {
-    return false;
-  }
-  return true;
+  return AssertStatusOk(s, context.debug_error_string());
 }
 
 bool InteropClient::DoResponseStreamingWithSlowConsumer() {

test/cpp/qps/driver.cc

@@ -634,9 +634,7 @@ std::unique_ptr<ScenarioResult> RunScenario(
   ReceiveFinalStatusFromServer(servers, *result);
   ShutdownServers(servers, *result);
 
-  if (g_inproc_servers != nullptr) {
-    delete g_inproc_servers;
-  }
+  delete g_inproc_servers;
 
   merged_latencies.FillProto(result->mutable_latencies());
   for (std::unordered_map<int, int64_t>::iterator it = merged_statuses.begin();

test/cpp/qps/json_run_localhost.cc

@@ -125,9 +125,8 @@ int main(int argc, char** argv) {
     }
   }
 
-  if (g_driver != nullptr) {
-    delete g_driver;
-  }
+  delete g_driver;
+
   g_driver = nullptr;
   for (int i = 0; i < kNumWorkers; ++i) {
     if (g_workers[i] != nullptr) {

test/cpp/util/cli_credentials.cc

@@ -169,7 +169,7 @@ std::shared_ptr<grpc::ChannelCredentials> CliCredentials::GetCredentials()
              : grpc::CompositeChannelCredentials(channel_creds, call_creds);
 }
 
-const std::string CliCredentials::GetCredentialUsage() const {
+std::string CliCredentials::GetCredentialUsage() const {
   return "    --ssl_target             ; Set server host for ssl validation\n"
          "    --ssl_client_cert        ; Client cert for ssl\n"
          "    --ssl_client_key         ; Client private key for ssl\n"
@@ -180,7 +180,7 @@ const std::string CliCredentials::GetCredentialUsage() const {
          " access_token=<token>\n";
 }
 
-const std::string CliCredentials::GetSslTargetNameOverride() const {
+std::string CliCredentials::GetSslTargetNameOverride() const {
   bool use_ssl = absl::GetFlag(FLAGS_channel_creds_type) == "ssl" ||
                  absl::GetFlag(FLAGS_channel_creds_type) == "gdc";
   return use_ssl ? absl::GetFlag(FLAGS_ssl_target) : "";

test/cpp/util/cli_credentials.h

@@ -29,8 +29,8 @@ class CliCredentials {
  public:
   virtual ~CliCredentials() {}
   std::shared_ptr<grpc::ChannelCredentials> GetCredentials() const;
-  virtual const std::string GetCredentialUsage() const;
-  virtual const std::string GetSslTargetNameOverride() const;
+  virtual std::string GetCredentialUsage() const;
+  virtual std::string GetSslTargetNameOverride() const;
 
  protected:
   // Returns the appropriate channel_creds_type value for the set of legacy

test/cpp/util/grpc_tool_test.cc

@@ -150,7 +150,7 @@ class TestCliCredentials final : public grpc::testing::CliCredentials {
     grpc_slice_unref(ca_slice);
     return credential_ptr;
   }
-  const std::string GetCredentialUsage() const override { return ""; }
+  std::string GetCredentialUsage() const override { return ""; }
 
  private:
   const bool secure_;
@@ -284,7 +284,7 @@ class GrpcToolTest : public ::testing::Test {
   // SetUpServer cannot be used with EXPECT_EXIT. grpc_pick_unused_port_or_die()
   // uses atexit() to free chosen ports, and it will spawn a new thread in
   // resolve_address_posix.c:192 at exit time.
-  const std::string SetUpServer(bool secure = false) {
+  std::string SetUpServer(bool secure = false) {
     std::ostringstream server_address;
     int port = grpc_pick_unused_port_or_die();
     server_address << "localhost:" << port;

test/cpp/util/proto_reflection_descriptor_database.cc

@@ -290,7 +290,7 @@ bool ProtoReflectionDescriptorDatabase::GetServices(
   return false;
 }
 
-const protobuf::FileDescriptorProto
+protobuf::FileDescriptorProto
 ProtoReflectionDescriptorDatabase::ParseFileDescriptorProtoResponse(
     const std::string& byte_fd_proto) {
   protobuf::FileDescriptorProto file_desc_proto;
@@ -310,7 +310,7 @@ void ProtoReflectionDescriptorDatabase::AddFileFromResponse(
   }
 }
 
-const std::shared_ptr<ProtoReflectionDescriptorDatabase::ClientStream>
+std::shared_ptr<ProtoReflectionDescriptorDatabase::ClientStream>
 ProtoReflectionDescriptorDatabase::GetStream() {
   if (!stream_) {
     stream_ = stub_->ServerReflectionInfo(&ctx_);

test/cpp/util/proto_reflection_descriptor_database.h

@@ -82,13 +82,13 @@ class ProtoReflectionDescriptorDatabase : public protobuf::DescriptorDatabase {
       grpc::reflection::v1alpha::ServerReflectionResponse>
       ClientStream;
 
-  const protobuf::FileDescriptorProto ParseFileDescriptorProtoResponse(
+  protobuf::FileDescriptorProto ParseFileDescriptorProtoResponse(
       const std::string& byte_fd_proto);
 
   void AddFileFromResponse(
       const grpc::reflection::v1alpha::FileDescriptorResponse& response);
 
-  const std::shared_ptr<ClientStream> GetStream();
+  std::shared_ptr<ClientStream> GetStream();
 
   bool DoOneRequest(
       const grpc::reflection::v1alpha::ServerReflectionRequest& request,