clang-tidy fixes (envoyproxy#7318)

Signed-off-by: Derek Argueta <dereka@pinterest.com>

.clang-tidy

@@ -1,7 +1,7 @@
 Checks:          'clang-diagnostic-*,clang-analyzer-*,abseil-*,bugprone-*,modernize-*,performance-*,readability-redundant-*,readability-braces-around-statements,readability-container-size-empty'
 
 #TODO(lizan): grow this list, fix possible warnings and make more checks as error
-WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements,readability-redundant-string-cstr,bugprone-use-after-move,readability-container-size-empty'
+WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements,readability-redundant-string-cstr,bugprone-use-after-move,readability-container-size-empty,performance-faster-string-find'
 
 CheckOptions:
   - key:             bugprone-assert-side-effect.AssertMacros

include/envoy/event/timer.h

@@ -58,7 +58,7 @@ using SchedulerPtr = std::unique_ptr<Scheduler>;
  */
 class TimeSystem : public TimeSource {
 public:
-  virtual ~TimeSystem() = default;
+  ~TimeSystem() override = default;
 
   using Duration = MonotonicTime::duration;
 

include/envoy/http/header_map.h

@@ -1,9 +1,8 @@
 #pragma once
 
-#include <string.h>
-
 #include <algorithm>
 #include <cstdint>
+#include <cstring>
 #include <iostream>
 #include <memory>
 #include <string>
@@ -133,9 +132,7 @@ class HeaderString {
    *
    * @return an absl::string_view.
    */
-  absl::string_view getStringView() const {
-    return absl::string_view(buffer_.ref_, string_length_);
-  }
+  absl::string_view getStringView() const { return {buffer_.ref_, string_length_}; }
 
   /**
    * Return the string to a default state. Reference strings are not touched. Both inline/dynamic
@@ -478,7 +475,7 @@ class HeaderMap {
    * @param context supplies the context passed to iterate().
    * @return Iterate::Continue to continue iteration.
    */
-  typedef Iterate (*ConstIterateCb)(const HeaderEntry& header, void* context);
+  using ConstIterateCb = Iterate (*)(const HeaderEntry&, void*);
 
   /**
    * Iterate over a constant header map.

include/envoy/registry/registry.h

@@ -52,7 +52,7 @@ template <class Base> class FactoryRegistry {
    * Gets the current map of factory implementations. This is an ordered map for sorting reasons.
    */
   static std::map<std::string, Base*>& factories() {
-    static std::map<std::string, Base*>* factories = new std::map<std::string, Base*>;
+    static auto* factories = new std::map<std::string, Base*>;
     return *factories;
   }
 

include/envoy/stats/timespan.h

@@ -37,7 +37,7 @@ template <class TimeUnit> class TimespanWithUnit : public CompletableTimespan {
   /**
    * Complete the timespan and send the time to the histogram.
    */
-  void complete() { histogram_.recordValue(getRawDuration().count()); }
+  void complete() override { histogram_.recordValue(getRawDuration().count()); }
 
   /**
    * Get duration in the time unit since the creation of the span.

include/envoy/upstream/types.h

@@ -1,7 +1,6 @@
 #pragma once
 
-#include <stdint.h>
-
+#include <cstdint>
 #include <vector>
 
 #include "common/common/phantom.h"

source/common/access_log/access_log_formatter.cc

@@ -238,31 +238,31 @@ std::vector<FormatterProviderPtr> AccessLogFormatParser::parse(const std::string
       pos += 1;
       int command_end_position = pos + token.length();
 
-      if (token.find("REQ(") == 0) {
+      if (absl::StartsWith(token, "REQ(")) {
         std::string main_header, alternative_header;
         absl::optional<size_t> max_length;
 
         parseCommandHeader(token, ReqParamStart, main_header, alternative_header, max_length);
 
         formatters.emplace_back(FormatterProviderPtr{
             new RequestHeaderFormatter(main_header, alternative_header, max_length)});
-      } else if (token.find("RESP(") == 0) {
+      } else if (absl::StartsWith(token, "RESP(")) {
         std::string main_header, alternative_header;
         absl::optional<size_t> max_length;
 
         parseCommandHeader(token, RespParamStart, main_header, alternative_header, max_length);
 
         formatters.emplace_back(FormatterProviderPtr{
             new ResponseHeaderFormatter(main_header, alternative_header, max_length)});
-      } else if (token.find("TRAILER(") == 0) {
+      } else if (absl::StartsWith(token, "TRAILER(")) {
         std::string main_header, alternative_header;
         absl::optional<size_t> max_length;
 
         parseCommandHeader(token, TrailParamStart, main_header, alternative_header, max_length);
 
         formatters.emplace_back(FormatterProviderPtr{
             new ResponseTrailerFormatter(main_header, alternative_header, max_length)});
-      } else if (token.find(DYNAMIC_META_TOKEN) == 0) {
+      } else if (absl::StartsWith(token, DYNAMIC_META_TOKEN)) {
         std::string filter_namespace;
         absl::optional<size_t> max_length;
         std::vector<std::string> path;
@@ -271,7 +271,7 @@ std::vector<FormatterProviderPtr> AccessLogFormatParser::parse(const std::string
         parseCommand(token, start, ":", filter_namespace, path, max_length);
         formatters.emplace_back(
             FormatterProviderPtr{new DynamicMetadataFormatter(filter_namespace, path, max_length)});
-      } else if (token.find("START_TIME") == 0) {
+      } else if (absl::StartsWith(token, "START_TIME")) {
         const size_t parameters_length = pos + StartTimeParamStart + 1;
         const size_t parameters_end = command_end_position - parameters_length;
 

source/common/api/os_sys_calls_impl.cc

@@ -1,10 +1,11 @@
 #include "common/api/os_sys_calls_impl.h"
 
-#include <errno.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include <cerrno>
+
 namespace Envoy {
 namespace Api {
 

source/common/api/os_sys_calls_impl_hot_restart.cc

@@ -1,6 +1,6 @@
 #include "common/api/os_sys_calls_impl_hot_restart.h"
 
-#include <errno.h>
+#include <cerrno>
 
 namespace Envoy {
 namespace Api {

source/common/api/os_sys_calls_impl_linux.cc

@@ -4,9 +4,10 @@
 
 #include "common/api/os_sys_calls_impl_linux.h"
 
-#include <errno.h>
 #include <sched.h>
 
+#include <cerrno>
+
 namespace Envoy {
 namespace Api {
 

source/common/buffer/buffer_impl.h

@@ -507,7 +507,7 @@ class OwnedImpl : public LibEventInstance {
 
   // LibEventInstance
   Event::Libevent::BufferPtr& buffer() override { return buffer_; }
-  virtual void postProcess() override;
+  void postProcess() override;
 
   /**
    * Create a new slice at the end of the buffer, and copy the supplied content into it.

source/common/buffer/zero_copy_input_stream_impl.h

@@ -34,10 +34,10 @@ class ZeroCopyInputStreamImpl : public virtual Protobuf::io::ZeroCopyInputStream
   // Note Next() will return true with no data until more data is available if the stream is not
   // finished. It is the caller's responsibility to finish the stream or wrap with
   // LimitingInputStream before passing to protobuf code to avoid a spin loop.
-  virtual bool Next(const void** data, int* size) override;
-  virtual void BackUp(int count) override;
-  virtual bool Skip(int count) override; // Not implemented
-  virtual ProtobufTypes::Int64 ByteCount() const override { return byte_count_; }
+  bool Next(const void** data, int* size) override;
+  void BackUp(int count) override;
+  bool Skip(int count) override; // Not implemented
+  ProtobufTypes::Int64 ByteCount() const override { return byte_count_; }
 
 protected:
   Buffer::InstancePtr buffer_;

source/common/common/assert.cc

@@ -30,7 +30,7 @@ class ActionRegistrationImpl : public ActionRegistration {
 
 std::function<void()> ActionRegistrationImpl::debug_assertion_failure_record_action_;
 
-ActionRegistrationPtr setDebugAssertionFailureRecordAction(std::function<void()> action) {
+ActionRegistrationPtr setDebugAssertionFailureRecordAction(const std::function<void()>& action) {
   return std::make_unique<ActionRegistrationImpl>(action);
 }
 

source/common/common/assert.h

@@ -30,7 +30,7 @@ using ActionRegistrationPtr = std::unique_ptr<ActionRegistration>;
  * @param action The action to take when an assertion fails.
  * @return A registration object. The registration is removed when the object is destructed.
  */
-ActionRegistrationPtr setDebugAssertionFailureRecordAction(std::function<void()> action);
+ActionRegistrationPtr setDebugAssertionFailureRecordAction(const std::function<void()>& action);
 
 /**
  * Invokes the action set by setDebugAssertionFailureRecordAction, or does nothing if

source/common/common/empty_string.h

@@ -3,5 +3,5 @@
 #include <string>
 
 namespace Envoy {
-static const std::string EMPTY_STRING = "";
+static const std::string EMPTY_STRING;
 } // namespace Envoy

source/common/common/linked_object.h

@@ -76,11 +76,11 @@ template <class T> class LinkedObject {
   }
 
 protected:
-  LinkedObject() : inserted_(false) {}
+  LinkedObject() = default;
 
 private:
   typename ListType::iterator entry_;
-  bool inserted_; // iterators do not have any "invalid" value so we need this boolean for sanity
-                  // checking.
+  bool inserted_{false}; // iterators do not have any "invalid" value so we need this boolean for
+                         // sanity checking.
 };
 } // namespace Envoy

source/common/common/logger.h

@@ -74,15 +74,15 @@ class Logger {
    * but the method to log at err level is called LOGGER.error not LOGGER.err. All other level are
    * fine spdlog::info corresponds to LOGGER.info method.
    */
-  typedef enum {
+  using levels = enum {
     trace = spdlog::level::trace,
     debug = spdlog::level::debug,
     info = spdlog::level::info,
     warn = spdlog::level::warn,
     error = spdlog::level::err,
     critical = spdlog::level::critical,
     off = spdlog::level::off
-  } levels;
+  };
 
   spdlog::string_view_t levelString() const {
     return spdlog::level::level_string_views[logger_->level()];

source/common/common/matchers.h

@@ -107,7 +107,7 @@ class ListMatcher : public ValueMatcher {
 public:
   ListMatcher(const envoy::type::matcher::ListMatcher& matcher);
 
-  bool match(const ProtobufWkt::Value& value) const;
+  bool match(const ProtobufWkt::Value& value) const override;
 
 private:
   const envoy::type::matcher::ListMatcher matcher_;

source/common/common/non_copyable.h

@@ -6,7 +6,7 @@ namespace Envoy {
  */
 class NonCopyable {
 protected:
-  NonCopyable() {}
+  NonCopyable() = default;
 
 private:
   NonCopyable(const NonCopyable&);

source/common/common/perf_annotation.cc

@@ -30,7 +30,7 @@ void PerfOperation::record(absl::string_view category, absl::string_view descrip
 // The ctor is explicitly declared private to encourage clients to use getOrCreate(), at
 // least for now. Given that it's declared it must be instantiated. It's not inlined
 // because the constructor is non-trivial due to the contained unordered_map.
-PerfAnnotationContext::PerfAnnotationContext() {}
+PerfAnnotationContext::PerfAnnotationContext() = default;
 
 void PerfAnnotationContext::record(std::chrono::nanoseconds duration, absl::string_view category,
                                    absl::string_view description) {
@@ -146,7 +146,7 @@ void PerfAnnotationContext::clear() {
 }
 
 PerfAnnotationContext* PerfAnnotationContext::getOrCreate() {
-  static PerfAnnotationContext* context = new PerfAnnotationContext();
+  static auto* context = new PerfAnnotationContext();
   return context;
 }
 

source/common/common/scalar_to_byte_vector.h

@@ -1,7 +1,6 @@
 #pragma once
 
-#include <inttypes.h>
-
+#include <cinttypes>
 #include <vector>
 
 namespace Envoy {

source/common/common/stack_array.h

@@ -7,7 +7,7 @@
 #include <alloca.h>
 #endif
 
-#include <stddef.h>
+#include <cstddef>
 
 #include "common/common/assert.h"
 

source/common/common/utility.h

@@ -719,7 +719,7 @@ class InlineString : public InlineStorage {
   /**
    * @return a string_view into the InlineString.
    */
-  absl::string_view toStringView() const { return absl::string_view(data_, size_); }
+  absl::string_view toStringView() const { return {data_, size_}; }
 
   /**
    * @return the number of bytes in the string

source/common/config/metadata.cc

@@ -13,7 +13,7 @@ const ProtobufWkt::Value& Metadata::metadataValue(const envoy::api::v2::core::Me
   const ProtobufWkt::Struct* data_struct = &(filter_it->second);
   const ProtobufWkt::Value* val = nullptr;
   // go through path to select sub entries
-  for (const auto p : path) {
+  for (const auto& p : path) {
     if (nullptr == data_struct) { // sub entry not found
       return ProtobufWkt::Value::default_instance();
     }

source/common/config/well_known_names.cc

@@ -44,62 +44,62 @@ TagNameValues::TagNameValues() {
 
   // mongo.[<stat_prefix>.]collection.[<collection>.]callsite.(<callsite>.)query.<base_stat>
   addRegex(MONGO_CALLSITE,
-           "^mongo(?=\\.).*?\\.collection(?=\\.).*?\\.callsite\\.((.*?)\\.).*?query.\\w+?$",
+           R"(^mongo(?=\.).*?\.collection(?=\.).*?\.callsite\.((.*?)\.).*?query.\w+?$)",
            ".collection.");
 
   // http.[<stat_prefix>.]dynamodb.table.(<table_name>.) or
   // http.[<stat_prefix>.]dynamodb.error.(<table_name>.)*
-  addRegex(DYNAMO_TABLE, "^http(?=\\.).*?\\.dynamodb.(?:table|error)\\.((.*?)\\.)", ".dynamodb.");
+  addRegex(DYNAMO_TABLE, R"(^http(?=\.).*?\.dynamodb.(?:table|error)\.((.*?)\.))", ".dynamodb.");
 
   // mongo.[<stat_prefix>.]collection.(<collection>.)query.<base_stat>
-  addRegex(MONGO_COLLECTION, "^mongo(?=\\.).*?\\.collection\\.((.*?)\\.).*?query.\\w+?$",
+  addRegex(MONGO_COLLECTION, R"(^mongo(?=\.).*?\.collection\.((.*?)\.).*?query.\w+?$)",
            ".collection.");
 
   // mongo.[<stat_prefix>.]cmd.(<cmd>.)<base_stat>
-  addRegex(MONGO_CMD, "^mongo(?=\\.).*?\\.cmd\\.((.*?)\\.)\\w+?$", ".cmd.");
+  addRegex(MONGO_CMD, R"(^mongo(?=\.).*?\.cmd\.((.*?)\.)\w+?$)", ".cmd.");
 
   // cluster.[<route_target_cluster>.]grpc.[<grpc_service>.](<grpc_method>.)<base_stat>
-  addRegex(GRPC_BRIDGE_METHOD, "^cluster(?=\\.).*?\\.grpc(?=\\.).*\\.((.*?)\\.)\\w+?$", ".grpc.");
+  addRegex(GRPC_BRIDGE_METHOD, R"(^cluster(?=\.).*?\.grpc(?=\.).*\.((.*?)\.)\w+?$)", ".grpc.");
 
   // http.[<stat_prefix>.]user_agent.(<user_agent>.)<base_stat>
-  addRegex(HTTP_USER_AGENT, "^http(?=\\.).*?\\.user_agent\\.((.*?)\\.)\\w+?$", ".user_agent.");
+  addRegex(HTTP_USER_AGENT, R"(^http(?=\.).*?\.user_agent\.((.*?)\.)\w+?$)", ".user_agent.");
 
   // vhost.[<virtual host name>.]vcluster.(<virtual_cluster_name>.)<base_stat>
-  addRegex(VIRTUAL_CLUSTER, "^vhost(?=\\.).*?\\.vcluster\\.((.*?)\\.)\\w+?$", ".vcluster.");
+  addRegex(VIRTUAL_CLUSTER, R"(^vhost(?=\.).*?\.vcluster\.((.*?)\.)\w+?$)", ".vcluster.");
 
   // http.[<stat_prefix>.]fault.(<downstream_cluster>.)<base_stat>
-  addRegex(FAULT_DOWNSTREAM_CLUSTER, "^http(?=\\.).*?\\.fault\\.((.*?)\\.)\\w+?$", ".fault.");
+  addRegex(FAULT_DOWNSTREAM_CLUSTER, R"(^http(?=\.).*?\.fault\.((.*?)\.)\w+?$)", ".fault.");
 
   // listener.[<address>.]ssl.cipher.(<cipher>)
-  addRegex(SSL_CIPHER, "^listener(?=\\.).*?\\.ssl\\.cipher(\\.(.*?))$");
+  addRegex(SSL_CIPHER, R"(^listener(?=\.).*?\.ssl\.cipher(\.(.*?))$)");
 
   // cluster.[<cluster_name>.]ssl.ciphers.(<cipher>)
-  addRegex(SSL_CIPHER_SUITE, "^cluster(?=\\.).*?\\.ssl\\.ciphers(\\.(.*?))$", ".ssl.ciphers.");
+  addRegex(SSL_CIPHER_SUITE, R"(^cluster(?=\.).*?\.ssl\.ciphers(\.(.*?))$)", ".ssl.ciphers.");
 
   // cluster.[<route_target_cluster>.]grpc.(<grpc_service>.)*
-  addRegex(GRPC_BRIDGE_SERVICE, "^cluster(?=\\.).*?\\.grpc\\.((.*?)\\.)", ".grpc.");
+  addRegex(GRPC_BRIDGE_SERVICE, R"(^cluster(?=\.).*?\.grpc\.((.*?)\.))", ".grpc.");
 
   // tcp.(<stat_prefix>.)<base_stat>
-  addRegex(TCP_PREFIX, "^tcp\\.((.*?)\\.)\\w+?$");
+  addRegex(TCP_PREFIX, R"(^tcp\.((.*?)\.)\w+?$)");
 
   // auth.clientssl.(<stat_prefix>.)<base_stat>
-  addRegex(CLIENTSSL_PREFIX, "^auth\\.clientssl\\.((.*?)\\.)\\w+?$");
+  addRegex(CLIENTSSL_PREFIX, R"(^auth\.clientssl\.((.*?)\.)\w+?$)");
 
   // ratelimit.(<stat_prefix>.)<base_stat>
-  addRegex(RATELIMIT_PREFIX, "^ratelimit\\.((.*?)\\.)\\w+?$");
+  addRegex(RATELIMIT_PREFIX, R"(^ratelimit\.((.*?)\.)\w+?$)");
 
   // cluster.(<cluster_name>.)*
   addRegex(CLUSTER_NAME, "^cluster\\.((.*?)\\.)");
 
   // listener.[<address>.]http.(<stat_prefix>.)*
-  addRegex(HTTP_CONN_MANAGER_PREFIX, "^listener(?=\\.).*?\\.http\\.((.*?)\\.)", ".http.");
+  addRegex(HTTP_CONN_MANAGER_PREFIX, R"(^listener(?=\.).*?\.http\.((.*?)\.))", ".http.");
 
   // http.(<stat_prefix>.)*
   addRegex(HTTP_CONN_MANAGER_PREFIX, "^http\\.((.*?)\\.)");
 
   // listener.(<address>.)*
   addRegex(LISTENER_ADDRESS,
-           "^listener\\.(((?:[_.[:digit:]]*|[_\\[\\]aAbBcCdDeEfF[:digit:]]*))\\.)");
+           R"(^listener\.(((?:[_.[:digit:]]*|[_\[\]aAbBcCdDeEfF[:digit:]]*))\.))");
 
   // vhost.(<virtual host name>.)*
   addRegex(VIRTUAL_HOST, "^vhost\\.((.*?)\\.)");
@@ -108,10 +108,10 @@ TagNameValues::TagNameValues() {
   addRegex(MONGO_PREFIX, "^mongo\\.((.*?)\\.)");
 
   // http.[<stat_prefix>.]rds.(<route_config_name>.)<base_stat>
-  addRegex(RDS_ROUTE_CONFIG, "^http(?=\\.).*?\\.rds\\.((.*?)\\.)\\w+?$", ".rds.");
+  addRegex(RDS_ROUTE_CONFIG, R"(^http(?=\.).*?\.rds\.((.*?)\.)\w+?$)", ".rds.");
 
   // listener_manager.(worker_<id>.)*
-  addRegex(WORKER_ID, "^listener_manager\\.((worker_\\d+)\\.)", "listener_manager.worker_");
+  addRegex(WORKER_ID, R"(^listener_manager\.((worker_\d+)\.))", "listener_manager.worker_");
 }
 
 void TagNameValues::addRegex(const std::string& name, const std::string& regex,