Skip to content

Commit

Permalink
Enable readability-identifier-naming in clang-tidy (#1383)
Browse files Browse the repository at this point in the history
  • Loading branch information
PragmaTwice authored Apr 13, 2023
1 parent 060612a commit 0f7f0b2
Show file tree
Hide file tree
Showing 28 changed files with 168 additions and 143 deletions.
27 changes: 26 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# refer to https://clang.llvm.org/extra/clang-tidy/checks/list.html
Checks: -*, clang-analyzer-core.*, clang-analyzer-cplusplus.*, clang-analyzer-deadcode.*, clang-analyzer-nullability.*, clang-analyzer-security.*, clang-analyzer-unix.*, clang-analyzer-valist.*, cppcoreguidelines-init-variables, cppcoreguidelines-macro-usage, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-narrowing-conversions, cppcoreguidelines-no-malloc, cppcoreguidelines-prefer-member-initializer, cppcoreguidelines-special-member-functions, cppcoreguidelines-slicing, google-build-explicit-make-pair, google-default-arguments, google-explicit-constructor, modernize-avoid-bind, modernize-loop-convert, modernize-macro-to-enum, modernize-make-shared, modernize-make-unique, modernize-pass-by-value, modernize-redundant-void-arg, modernize-return-braced-init-list, modernize-use-auto, modernize-use-bool-literals, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, modernize-use-nullptr, modernize-use-override, modernize-use-using, performance-faster-string-find, performance-for-range-copy, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-inefficient-vector-operation, performance-move-const-arg, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, performance-unnecessary-value-param, readability-avoid-const-params-in-decls, readability-const-return-type, readability-convert-member-functions-to-static, readability-make-member-function-const, readability-redundant-access-specifiers, readability-redundant-control-flow, readability-redundant-declaration, readability-redundant-member-init, readability-redundant-string-cstr, readability-redundant-string-init, readability-simplify-boolean-expr, readability-simplify-subscript-expr, readability-string-compare
Checks: -*, clang-analyzer-core.*, clang-analyzer-cplusplus.*, clang-analyzer-deadcode.*, clang-analyzer-nullability.*, clang-analyzer-security.*, clang-analyzer-unix.*, clang-analyzer-valist.*, cppcoreguidelines-init-variables, cppcoreguidelines-macro-usage, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-narrowing-conversions, cppcoreguidelines-no-malloc, cppcoreguidelines-prefer-member-initializer, cppcoreguidelines-special-member-functions, cppcoreguidelines-slicing, google-build-explicit-make-pair, google-default-arguments, google-explicit-constructor, modernize-avoid-bind, modernize-loop-convert, modernize-macro-to-enum, modernize-make-shared, modernize-make-unique, modernize-pass-by-value, modernize-redundant-void-arg, modernize-return-braced-init-list, modernize-use-auto, modernize-use-bool-literals, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, modernize-use-nullptr, modernize-use-override, modernize-use-using, performance-faster-string-find, performance-for-range-copy, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-inefficient-vector-operation, performance-move-const-arg, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, performance-unnecessary-value-param, readability-avoid-const-params-in-decls, readability-const-return-type, readability-convert-member-functions-to-static, readability-make-member-function-const, readability-redundant-access-specifiers, readability-redundant-control-flow, readability-redundant-declaration, readability-redundant-member-init, readability-redundant-string-cstr, readability-redundant-string-init, readability-simplify-boolean-expr, readability-simplify-subscript-expr, readability-string-compare, readability-identifier-naming

WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand, google-*, performance-*, cppcoreguidelines-*, modernize-*, readability-*

Expand All @@ -10,3 +10,28 @@ CheckOptions:
value: True
- key: performance-move-const-arg.CheckTriviallyCopyableMove
value: False
- key: readability-identifier-naming.LocalVariableCase
value: lower_case
- key: readability-identifier-naming.StructCase
value: CamelCase
- key: readability-identifier-naming.ClassCase
value: CamelCase
- key: readability-identifier-naming.UnionCase
value: CamelCase
- key: readability-identifier-naming.ParameterCase
value: lower_case
# FIXME: enable these options
# - key: readability-identifier-naming.PublicMethodCase
# value: CamelCase
# - key: readability-identifier-naming.PrivateMethodCase
# value: camelBack
# - key: readability-identifier-naming.ProtectedMethodCase
# value: camelBack
- key: readability-identifier-naming.LocalConstantCase
value: aNy_CasE # FIXME: use a more strictly case
- key: readability-identifier-naming.ClassMemberCase
value: lower_case
- key: readability-identifier-naming.ClassMemberSuffix
value: _
- key: readability-identifier-naming.ClassConstantCase
value: aNy_CasE # FIXME: use a more strictly case
6 changes: 3 additions & 3 deletions src/cluster/cluster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ Status Cluster::LoadClusterNodes(const std::string &file_path) {
}

int64_t version = -1;
std::string id, nodesInfo;
std::string id, nodes_info;
std::string line;
while (!file.eof()) {
std::getline(file, line);
Expand All @@ -638,12 +638,12 @@ Status Cluster::LoadClusterNodes(const std::string &file_path) {
return {Status::NotOK, errInvalidNodeID};
}
} else if (key == "node") {
nodesInfo.append(parsed->second + "\n");
nodes_info.append(parsed->second + "\n");
} else {
return {Status::NotOK, fmt::format("unknown key: {}", key)};
}
}
return SetClusterNodes(nodesInfo, version, false);
return SetClusterNodes(nodes_info, version, false);
}

Status Cluster::ParseClusterNodes(const std::string &nodes_str, ClusterNodes *nodes,
Expand Down
4 changes: 2 additions & 2 deletions src/commands/cmd_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -669,8 +669,8 @@ class CommandHello final : public Commander {
const std::string &opt = args_[next_arg];
if (opt == "AUTH" && more_args != 0) {
const auto &user_password = args_[next_arg + 1];
auto authResult = AuthenticateUser(conn, svr->GetConfig(), user_password);
switch (authResult) {
auto auth_result = AuthenticateUser(conn, svr->GetConfig(), user_password);
switch (auth_result) {
case AuthResult::INVALID_PASSWORD:
return {Status::NotOK, "invalid password"};
case AuthResult::NO_REQUIRE_PASS:
Expand Down
6 changes: 3 additions & 3 deletions src/common/cron.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
#include "parse_util.h"

std::string Scheduler::ToString() const {
auto param2String = [](int n) -> std::string { return n == -1 ? "*" : std::to_string(n); };
return param2String(minute) + " " + param2String(hour) + " " + param2String(mday) + " " + param2String(month) + " " +
param2String(wday);
auto param2string = [](int n) -> std::string { return n == -1 ? "*" : std::to_string(n); };
return param2string(minute) + " " + param2string(hour) + " " + param2string(mday) + " " + param2string(month) + " " +
param2string(wday);
}

Status Cron::SetScheduleTime(const std::vector<std::string> &args) {
Expand Down
2 changes: 1 addition & 1 deletion src/common/sha1.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ By Steve Reid <steve@edmweb.com>

#include <cstdint>

struct SHA1_CTX {
struct SHA1_CTX { // NOLINT
uint32_t state[5];
uint32_t count[2];
unsigned char buffer[64];
Expand Down
20 changes: 10 additions & 10 deletions src/config/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,18 @@ constexpr const char *errBlobDbNotEnabled = "Must set rocksdb.enable_blob_files
constexpr const char *errLevelCompactionDynamicLevelBytesNotSet =
"Must set rocksdb.level_compaction_dynamic_level_bytes yes first.";

configEnum compression_types[] = {
ConfigEnum compression_types[] = {
{"no", rocksdb::CompressionType::kNoCompression}, {"snappy", rocksdb::CompressionType::kSnappyCompression},
{"lz4", rocksdb::CompressionType::kLZ4Compression}, {"zstd", rocksdb::CompressionType::kZSTD},
{"zlib", rocksdb::CompressionType::kZlibCompression}, {nullptr, 0}};

configEnum supervised_modes[] = {{"no", kSupervisedNone},
ConfigEnum supervised_modes[] = {{"no", kSupervisedNone},
{"auto", kSupervisedAutoDetect},
{"upstart", kSupervisedUpStart},
{"systemd", kSupervisedSystemd},
{nullptr, 0}};

configEnum log_levels[] = {{"info", google::INFO},
ConfigEnum log_levels[] = {{"info", google::INFO},
{"warning", google::WARNING},
{"error", google::ERROR},
{"fatal", google::FATAL},
Expand All @@ -67,15 +67,15 @@ std::string trimRocksDBPrefix(std::string s) {
return s.substr(8, s.size() - 8);
}

int configEnumGetValue(configEnum *ce, const char *name) {
int configEnumGetValue(ConfigEnum *ce, const char *name) {
while (ce->name != nullptr) {
if (strcasecmp(ce->name, name) == 0) return ce->val;
ce++;
}
return INT_MIN;
}

const char *configEnumGetName(configEnum *ce, int val) {
const char *configEnumGetName(ConfigEnum *ce, int val) {
while (ce->name != nullptr) {
if (ce->val == val) return ce->name;
ce++;
Expand Down Expand Up @@ -700,8 +700,8 @@ Status Config::finish() {
if (backup_dir.empty()) backup_dir = dir + "/backup";
if (log_dir.empty()) log_dir = dir;
if (pidfile.empty()) pidfile = dir + "/kvrocks.pid";
std::vector<std::string> createDirs = {dir};
for (const auto &name : createDirs) {
std::vector<std::string> create_dirs = {dir};
for (const auto &name : create_dirs) {
auto s = rocksdb::Env::Default()->CreateDirIfMissing(name);
if (!s.ok()) return {Status::NotOK, s.ToString()};
}
Expand Down Expand Up @@ -821,9 +821,9 @@ Status Config::Rewrite() {
new_config[iter.first] = iter.second->ToString();
}

std::string namespacePrefix = "namespace.";
std::string namespace_prefix = "namespace.";
for (const auto &iter : tokens) {
new_config[namespacePrefix + iter.second] = iter.first;
new_config[namespace_prefix + iter.second] = iter.first;
}

std::ifstream file(path_);
Expand All @@ -837,7 +837,7 @@ Status Config::Rewrite() {
continue;
}
auto kv = std::move(*parsed);
if (Util::HasPrefix(kv.first, namespacePrefix)) {
if (Util::HasPrefix(kv.first, namespace_prefix)) {
// Ignore namespace fields here since we would always rewrite them
continue;
}
Expand Down
10 changes: 5 additions & 5 deletions src/config/config_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ using IntField = IntegerField<int>;
using UInt32Field = IntegerField<uint32_t>;
using Int64Field = IntegerField<int64_t>;

struct configEnum {
struct ConfigEnum {
const char *name;
const int val;
};

enum configType { SingleConfig, MultiConfig };

int configEnumGetValue(configEnum *ce, const char *name);
const char *configEnumGetName(configEnum *ce, int val);
int configEnumGetValue(ConfigEnum *ce, const char *name);
const char *configEnumGetName(ConfigEnum *ce, int val);

class ConfigField {
public:
Expand Down Expand Up @@ -186,7 +186,7 @@ class YesNoField : public ConfigField {

class EnumField : public ConfigField {
public:
EnumField(int *receiver, configEnum *enums, int e) : receiver_(receiver), enums_(enums) { *receiver_ = e; }
EnumField(int *receiver, ConfigEnum *enums, int e) : receiver_(receiver), enums_(enums) { *receiver_ = e; }
~EnumField() override = default;
std::string ToString() override { return configEnumGetName(enums_, *receiver_); }
Status ToNumber(int64_t *n) override {
Expand All @@ -204,5 +204,5 @@ class EnumField : public ConfigField {

private:
int *receiver_;
configEnum *enums_ = nullptr;
ConfigEnum *enums_ = nullptr;
};
6 changes: 3 additions & 3 deletions src/server/redis_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ Status Request::Tokenize(evbuffer *input) {
while (true) {
switch (state_) {
case ArrayLen: {
bool isOnlyLF = true;
bool is_only_lf = true;
// We don't use the `EVBUFFER_EOL_CRLF_STRICT` here since only LF is allowed in INLINE protocol.
// So we need to search LF EOL and figure out current line has CR or not.
UniqueEvbufReadln line(input, EVBUFFER_EOL_LF);
if (line && line.length > 0 && line[line.length - 1] == '\r') {
// remove `\r` if exists
--line.length;
isOnlyLF = false;
is_only_lf = false;
}

if (!line || line.length <= 0) {
Expand All @@ -75,7 +75,7 @@ Status Request::Tokenize(evbuffer *input) {
}

multi_bulk_len_ = *parse_result;
if (isOnlyLF || multi_bulk_len_ > (int64_t)PROTO_MULTI_MAX_SIZE) {
if (is_only_lf || multi_bulk_len_ > (int64_t)PROTO_MULTI_MAX_SIZE) {
return {Status::NotOK, "Protocol error: invalid multibulk length"};
}

Expand Down
6 changes: 3 additions & 3 deletions src/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1498,9 +1498,9 @@ Status Server::LookupAndCreateCommand(const std::string &cmd_name, std::unique_p
return {Status::RedisUnknownCmd};
}

auto redisCmd = cmd_iter->second;
*cmd = redisCmd->factory();
(*cmd)->SetAttributes(redisCmd);
auto redis_cmd = cmd_iter->second;
*cmd = redis_cmd->factory();
(*cmd)->SetAttributes(redis_cmd);

return Status::OK();
}
Expand Down
12 changes: 6 additions & 6 deletions src/server/worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ void Worker::newTCPConnection(evconnlistener *listener, evutil_socket_t fd, sock
}

event_base *base = evconnlistener_get_base(listener);
auto evThreadSafeFlags =
auto ev_thread_safe_flags =
BEV_OPT_THREADSAFE | BEV_OPT_DEFER_CALLBACKS | BEV_OPT_UNLOCK_CALLBACKS | BEV_OPT_CLOSE_ON_FREE;

bufferevent *bev = nullptr;
Expand All @@ -139,12 +139,12 @@ void Worker::newTCPConnection(evconnlistener *listener, evutil_socket_t fd, sock
evutil_closesocket(fd);
return;
}
bev = bufferevent_openssl_socket_new(base, fd, ssl, BUFFEREVENT_SSL_ACCEPTING, evThreadSafeFlags);
bev = bufferevent_openssl_socket_new(base, fd, ssl, BUFFEREVENT_SSL_ACCEPTING, ev_thread_safe_flags);
} else {
bev = bufferevent_socket_new(base, fd, evThreadSafeFlags);
bev = bufferevent_socket_new(base, fd, ev_thread_safe_flags);
}
#else
bev = bufferevent_socket_new(base, fd, evThreadSafeFlags);
bev = bufferevent_socket_new(base, fd, ev_thread_safe_flags);
#endif
if (!bev) {
auto socket_err = evutil_socket_error_to_string(EVUTIL_SOCKET_ERROR());
Expand Down Expand Up @@ -194,9 +194,9 @@ void Worker::newUnixSocketConnection(evconnlistener *listener, evutil_socket_t f
DLOG(INFO) << "[worker] New connection: fd=" << fd << " from unixsocket: " << worker->svr_->GetConfig()->unixsocket
<< " thread #" << worker->tid_;
event_base *base = evconnlistener_get_base(listener);
auto evThreadSafeFlags =
auto ev_thread_safe_flags =
BEV_OPT_THREADSAFE | BEV_OPT_DEFER_CALLBACKS | BEV_OPT_UNLOCK_CALLBACKS | BEV_OPT_CLOSE_ON_FREE;
bufferevent *bev = bufferevent_socket_new(base, fd, evThreadSafeFlags);
bufferevent *bev = bufferevent_socket_new(base, fd, ev_thread_safe_flags);

auto conn = new Redis::Connection(bev, worker);
bufferevent_setcb(bev, Redis::Connection::OnRead, Redis::Connection::OnWrite, Redis::Connection::OnEvent, conn);
Expand Down
2 changes: 1 addition & 1 deletion src/stats/stats.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

Stats::Stats() {
for (int i = 0; i < STATS_METRIC_COUNT; i++) {
struct inst_metric im;
struct InstMetric im;
im.last_sample_time = 0;
im.last_sample_count = 0;
im.idx = 0;
Expand Down
8 changes: 4 additions & 4 deletions src/stats/stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ enum StatsMetricFlags {

const int STATS_METRIC_SAMPLES = 16; // Number of samples per metric

struct command_stat {
struct CommandStat {
std::atomic<uint64_t> calls;
std::atomic<uint64_t> latency;
};

struct inst_metric {
struct InstMetric {
uint64_t last_sample_time; // Timestamp of the last sample in ms
uint64_t last_sample_count; // Count in the last sample
uint64_t samples[STATS_METRIC_SAMPLES];
Expand All @@ -59,12 +59,12 @@ class Stats {
std::atomic<uint64_t> total_calls = {0};
std::atomic<uint64_t> in_bytes = {0};
std::atomic<uint64_t> out_bytes = {0};
std::vector<struct inst_metric> inst_metrics;
std::vector<struct InstMetric> inst_metrics;

std::atomic<uint64_t> fullsync_counter = {0};
std::atomic<uint64_t> psync_err_counter = {0};
std::atomic<uint64_t> psync_ok_counter = {0};
std::map<std::string, command_stat> commands_stats;
std::map<std::string, CommandStat> commands_stats;

Stats();
void IncrCalls(const std::string &command_name);
Expand Down
10 changes: 5 additions & 5 deletions src/storage/compaction_checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ void CompactionChecker::PickCompactionFiles(const std::string &cf_name) {
// the live files was too few, Hard code to 1 here.
if (props.size() <= 1) return;

size_t maxFilesToCompact = 1;
if (props.size() / 360 > maxFilesToCompact) {
maxFilesToCompact = props.size() / 360;
size_t max_files_to_compact = 1;
if (props.size() / 360 > max_files_to_compact) {
max_files_to_compact = props.size() / 360;
}
int64_t now = Util::GetTimeStamp();

Expand All @@ -66,7 +66,7 @@ void CompactionChecker::PickCompactionFiles(const std::string &cf_name) {
int64_t total_keys = 0, deleted_keys = 0;
rocksdb::Slice start_key, stop_key, best_start_key, best_stop_key;
for (const auto &iter : props) {
if (maxFilesToCompact == 0) return;
if (max_files_to_compact == 0) return;

uint64_t file_creation_time = iter.second->file_creation_time;
if (file_creation_time == 0) {
Expand Down Expand Up @@ -115,7 +115,7 @@ void CompactionChecker::PickCompactionFiles(const std::string &cf_name) {
auto s = storage_->Compact(&start_key, &stop_key);
LOG(INFO) << "[compaction checker] Compact the key in file (force compact policy): " << iter.first
<< " finished, result: " << s.ToString();
maxFilesToCompact--;
max_files_to_compact--;
continue;
}

Expand Down
Loading

0 comments on commit 0f7f0b2

Please sign in to comment.