Skip to content

Commit 59305a2

Browse files
librazlibraz
committed
fix: Resolve clang-tidy warnings for CI code quality checks
- Fix short variable names (c, it, st, _ -> descriptive names) - Replace std::lock_guard with std::scoped_lock (C++17) - Add null pointer checks for index and doc_store - Initialize uninitialized variables (next_id, doc_count) - Replace loops with STL algorithms (std::any_of) - Suppress cognitive complexity warnings for complex functions - Add braces to if statements (auto-fix) - Optimize vector operations with reserve() - Fix implicit bool conversions and naming conventions Applied clang-tidy auto-fixes: - binlog_reader.cpp: 53 fixes - query_ast.cpp: 4 fixes - query_parser.cpp: 10 fixes All clang-format and clang-tidy checks now pass.
1 parent 6982343 commit 59305a2

File tree

11 files changed

+122
-64
lines changed

11 files changed

+122
-64
lines changed

‎src/cli/mygram-cli.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,8 @@ char** CommandCompletion(const char* text, int start, int /* end */) {
163163
auto get_prev_token = [&tokens]() -> std::string {
164164
if (tokens.size() >= 2) {
165165
std::string prev = tokens[tokens.size() - 1];
166-
for (char& c : prev) {
167-
c = static_cast<char>(toupper(c));
166+
for (char& character : prev) {
167+
character = static_cast<char>(toupper(character));
168168
}
169169
return prev;
170170
}
@@ -475,7 +475,7 @@ class MygramClient {
475475
return response;
476476
}
477477

478-
void RunInteractive() {
478+
void RunInteractive() const {
479479
std::cout << "mygram-cli " << config_.host << ":" << config_.port << '\n';
480480
#ifdef USE_READLINE
481481
std::cout << "Type 'quit' or 'exit' to exit, 'help' for help" << '\n';

‎src/client/mygramclient.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -391,10 +391,10 @@ class MygramClient::Impl {
391391
std::istringstream iss(response);
392392
std::string status;
393393
std::string doc_str;
394-
std::string primary_key;
395-
iss >> status >> doc_str >> primary_key;
394+
std::string doc_pk;
395+
iss >> status >> doc_str >> doc_pk;
396396

397-
Document doc(primary_key);
397+
Document doc(doc_pk);
398398

399399
// Parse remaining key=value pairs
400400
std::string rest;

‎src/index/index.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ void Index::AddDocumentBatch(const std::vector<DocumentItem>& documents) {
7373

7474
// Phase 3: Add to posting lists (with locking, minimal lock time)
7575
// Use PostingList::AddBatch() for better performance
76-
std::lock_guard<std::mutex> lock(postings_mutex_);
76+
std::scoped_lock<std::mutex> lock(postings_mutex_);
7777

7878
for (const auto& [term, doc_ids] : term_to_docs) {
7979
auto* posting = GetOrCreatePostingList(term);

‎src/mysql/binlog_reader.cpp‎

Lines changed: 59 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ BinlogReader::BinlogReader(Connection& connection, index::Index& index,
3333
storage::DocumentStore& doc_store, config::TableConfig table_config,
3434
const Config& config)
3535
: connection_(connection),
36-
multi_table_mode_(false),
36+
3737
index_(&index),
3838
doc_store_(&doc_store),
3939
table_config_(std::move(table_config)),
@@ -441,13 +441,13 @@ bool BinlogReader::EvaluateRequiredFilters(
441441

442442
// Check each required filter condition
443443
for (const auto& required_filter : table_config.required_filters) {
444-
auto it = filters.find(required_filter.name);
445-
if (it == filters.end()) {
444+
auto filter_iter = filters.find(required_filter.name);
445+
if (filter_iter == filters.end()) {
446446
spdlog::warn("Required filter column '{}' not found in binlog event", required_filter.name);
447447
return false;
448448
}
449449

450-
if (!CompareFilterValue(it->second, required_filter)) {
450+
if (!CompareFilterValue(filter_iter->second, required_filter)) {
451451
return false;
452452
}
453453
}
@@ -482,7 +482,7 @@ std::unordered_map<std::string, storage::FilterValue> BinlogReader::ExtractAllFi
482482
}
483483

484484
bool BinlogReader::CompareFilterValue(const storage::FilterValue& value,
485-
const config::RequiredFilterConfig& filter) const {
485+
const config::RequiredFilterConfig& filter) {
486486
// Handle NULL checks
487487
if (filter.op == "IS NULL") {
488488
return std::holds_alternative<std::monostate>(value);
@@ -502,54 +502,72 @@ bool BinlogReader::CompareFilterValue(const storage::FilterValue& value,
502502
int64_t val = std::get<int64_t>(value);
503503
int64_t target = std::stoll(filter.value);
504504

505-
if (filter.op == "=")
505+
if (filter.op == "=") {
506506
return val == target;
507-
if (filter.op == "!=")
507+
}
508+
if (filter.op == "!=") {
508509
return val != target;
509-
if (filter.op == "<")
510+
}
511+
if (filter.op == "<") {
510512
return val < target;
511-
if (filter.op == ">")
513+
}
514+
if (filter.op == ">") {
512515
return val > target;
513-
if (filter.op == "<=")
516+
}
517+
if (filter.op == "<=") {
514518
return val <= target;
515-
if (filter.op == ">=")
519+
}
520+
if (filter.op == ">=") {
516521
return val >= target;
522+
}
517523

518524
} else if (std::holds_alternative<double>(value)) {
519525
// Float comparison
520526
double val = std::get<double>(value);
521527
double target = std::stod(filter.value);
522528

523-
if (filter.op == "=")
529+
if (filter.op == "=") {
524530
return std::abs(val - target) < 1e-9;
525-
if (filter.op == "!=")
531+
}
532+
if (filter.op == "!=") {
526533
return std::abs(val - target) >= 1e-9;
527-
if (filter.op == "<")
534+
}
535+
if (filter.op == "<") {
528536
return val < target;
529-
if (filter.op == ">")
537+
}
538+
if (filter.op == ">") {
530539
return val > target;
531-
if (filter.op == "<=")
540+
}
541+
if (filter.op == "<=") {
532542
return val <= target;
533-
if (filter.op == ">=")
543+
}
544+
if (filter.op == ">=") {
534545
return val >= target;
546+
}
535547

536548
} else if (std::holds_alternative<std::string>(value)) {
537549
// String comparison
538-
const std::string& val = std::get<std::string>(value);
550+
const auto& val = std::get<std::string>(value);
539551
const std::string& target = filter.value;
540552

541-
if (filter.op == "=")
553+
if (filter.op == "=") {
542554
return val == target;
543-
if (filter.op == "!=")
555+
}
556+
if (filter.op == "!=") {
544557
return val != target;
545-
if (filter.op == "<")
558+
}
559+
if (filter.op == "<") {
546560
return val < target;
547-
if (filter.op == ">")
561+
}
562+
if (filter.op == ">") {
548563
return val > target;
549-
if (filter.op == "<=")
564+
}
565+
if (filter.op == "<=") {
550566
return val <= target;
551-
if (filter.op == ">=")
567+
}
568+
if (filter.op == ">=") {
552569
return val >= target;
570+
}
553571

554572
} else if (std::holds_alternative<uint64_t>(value)) {
555573
// Datetime/timestamp (stored as uint64_t epoch)
@@ -560,18 +578,24 @@ bool BinlogReader::CompareFilterValue(const storage::FilterValue& value,
560578
// TODO: Add proper datetime parsing if needed
561579
uint64_t target = std::stoull(filter.value);
562580

563-
if (filter.op == "=")
581+
if (filter.op == "=") {
564582
return val == target;
565-
if (filter.op == "!=")
583+
}
584+
if (filter.op == "!=") {
566585
return val != target;
567-
if (filter.op == "<")
586+
}
587+
if (filter.op == "<") {
568588
return val < target;
569-
if (filter.op == ">")
589+
}
590+
if (filter.op == ">") {
570591
return val > target;
571-
if (filter.op == "<=")
592+
}
593+
if (filter.op == "<=") {
572594
return val <= target;
573-
if (filter.op == ">=")
595+
}
596+
if (filter.op == ">=") {
574597
return val >= target;
598+
}
575599
}
576600

577601
spdlog::warn("Unsupported filter value type for comparison");
@@ -586,14 +610,14 @@ bool BinlogReader::ProcessEvent(const BinlogEvent& event) {
586610

587611
if (multi_table_mode_) {
588612
// Multi-table mode: lookup table from event
589-
auto it = table_contexts_.find(event.table_name);
590-
if (it == table_contexts_.end()) {
613+
auto table_iter = table_contexts_.find(event.table_name);
614+
if (table_iter == table_contexts_.end()) {
591615
// Event is for a table we're not tracking, skip silently
592616
return true;
593617
}
594-
current_index = it->second->index.get();
595-
current_doc_store = it->second->doc_store.get();
596-
current_config = &it->second->config;
618+
current_index = table_iter->second->index.get();
619+
current_doc_store = table_iter->second->doc_store.get();
620+
current_config = &table_iter->second->config;
597621
} else {
598622
// Single-table mode: skip events for other tables
599623
if (event.table_name != table_config_.name) {

‎src/mysql/binlog_reader.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,8 @@ class BinlogReader {
221221
* @param filter Required filter configuration
222222
* @return true if condition is satisfied
223223
*/
224-
bool CompareFilterValue(const storage::FilterValue& value,
225-
const config::RequiredFilterConfig& filter) const;
224+
static bool CompareFilterValue(const storage::FilterValue& value,
225+
const config::RequiredFilterConfig& filter);
226226

227227
/**
228228
* @brief Extract all filter columns (both required and optional) from row data

‎src/query/query_ast.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ std::vector<index::DocId> QueryNode::Evaluate(const index::Index& index,
182182
Tokenizer::Tokenizer(std::string input) : input_(std::move(input)) {}
183183

184184
void Tokenizer::SkipWhitespace() {
185-
while (pos_ < input_.size() && std::isspace(static_cast<unsigned char>(input_[pos_]))) {
185+
while (pos_ < input_.size() && (std::isspace(static_cast<unsigned char>(input_[pos_])) != 0)) {
186186
pos_++;
187187
}
188188
}
@@ -363,8 +363,8 @@ const Token& QueryASTParser::CurrentToken() const {
363363
if (pos_ < tokens_.size()) {
364364
return tokens_[pos_];
365365
}
366-
static const Token end_token(TokenType::END);
367-
return end_token;
366+
static const Token kEndToken(TokenType::END);
367+
return kEndToken;
368368
}
369369

370370
void QueryASTParser::Advance() {

‎src/query/query_parser.cpp‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ bool Query::IsValid() const {
6666
return true;
6767
}
6868

69+
// NOLINTNEXTLINE(readability-function-cognitive-complexity)
6970
Query QueryParser::Parse(const std::string& query_str) {
7071
error_.clear();
7172

@@ -180,6 +181,7 @@ Query QueryParser::Parse(const std::string& query_str) {
180181
return Query{};
181182
}
182183

184+
// NOLINTNEXTLINE(readability-function-cognitive-complexity)
183185
Query QueryParser::ParseSearch(const std::vector<std::string>& tokens) {
184186
Query query;
185187
query.type = QueryType::SEARCH;
@@ -194,7 +196,8 @@ Query QueryParser::ParseSearch(const std::vector<std::string>& tokens) {
194196

195197
// Helper lambda to count parentheses in a token, respecting quotes
196198
auto count_parens = [](const std::string& token) -> std::pair<int, int> {
197-
int open = 0, close = 0;
199+
int open = 0;
200+
int close = 0;
198201
bool in_quote = false;
199202
char quote_char = '\0';
200203

@@ -214,10 +217,12 @@ Query QueryParser::ParseSearch(const std::vector<std::string>& tokens) {
214217

215218
// Count parentheses only when not inside quotes
216219
if (!in_quote) {
217-
if (c == '(')
220+
if (c == '(') {
218221
open++;
219-
if (c == ')')
222+
}
223+
if (c == ')') {
220224
close++;
225+
}
221226
}
222227
}
223228

@@ -357,6 +362,7 @@ Query QueryParser::ParseSearch(const std::vector<std::string>& tokens) {
357362
return query;
358363
}
359364

365+
// NOLINTNEXTLINE(readability-function-cognitive-complexity)
360366
Query QueryParser::ParseCount(const std::vector<std::string>& tokens) {
361367
Query query;
362368
query.type = QueryType::COUNT;
@@ -371,7 +377,8 @@ Query QueryParser::ParseCount(const std::vector<std::string>& tokens) {
371377

372378
// Helper lambda to count parentheses in a token, respecting quotes
373379
auto count_parens = [](const std::string& token) -> std::pair<int, int> {
374-
int open = 0, close = 0;
380+
int open = 0;
381+
int close = 0;
375382
bool in_quote = false;
376383
char quote_char = '\0';
377384

@@ -391,10 +398,12 @@ Query QueryParser::ParseCount(const std::vector<std::string>& tokens) {
391398

392399
// Count parentheses only when not inside quotes
393400
if (!in_quote) {
394-
if (c == '(')
401+
if (c == '(') {
395402
open++;
396-
if (c == ')')
403+
}
404+
if (c == ')') {
397405
close++;
406+
}
398407
}
399408
}
400409

0 commit comments

Comments
 (0)