chore: fix issues raised by clang-tidy (part 2)

CMakeLists.txt

@@ -17,14 +17,30 @@ include(ECMUninstallTarget)
 option(ENABLE_TEST "Build Test" On)
 
 # clang-tidy
-option(ENABLE_CLANG_TIDY "Enable clang-tidy" Off)
+option(ENABLE_CLANG_TIDY "Enable clang-tidy" On)
 if (ENABLE_CLANG_TIDY)
-    MESSAGE(STATUS "Enabled clang-tidy")
-    find_program(CLANG_TIDY NAMES "clang-tidy" REQUIRED)
-    MESSAGE(STATUS "Found clang-tidy: ${CLANG_TIDY}")
-    set(CLANG_TIDY_COMMAND "${CLANG_TIDY}" "-checks=-*,clang-analyzer-*,bugprone-*,-bugprone-easily-swappable-parameters,cert-*,readability-*")
-    # one may wants to use the following instead to focus on a specific category
-    # set(CLANG_TIDY_COMMAND "${CLANG_TIDY}" "-checks=-*,clang-analyzer-*")
+    MESSAGE(STATUS "Checking clang-tidy...")
+    find_program(CLANG_TIDY NAMES "clang-tidy")
+    if (CLANG_TIDY)
+        MESSAGE(STATUS "Found clang-tidy: ${CLANG_TIDY}")
+        string(CONCAT CLANG_TIDY_CHECKS "-checks=-*,"
+            "clang-analyzer-*,bugprone-*,cert-*,readability-*,"
+            "-bugprone-easily-swappable-parameters,"
+            "-readability-const-return-type,"
+            "-readability-convert-member-functions-to-static,"
+            "-readability-function-cognitive-complexity,"
+            "-readability-identifier-length,"
+            "-readability-implicit-bool-conversion,"
+            "-readability-isolate-declaration,"
+            "-readability-suspicious-call-argument"
+        )
+        set(CLANG_TIDY_COMMAND "${CLANG_TIDY}" "${CLANG_TIDY_CHECKS}")
+        # one may wants to use the following instead to focus on a specific category
+        # set(CLANG_TIDY_COMMAND "${CLANG_TIDY}" "-checks=-*,clang-analyzer-*")
+    else ()
+        MESSAGE(STATUS "clang-tidy not found, disable clang-tidy checks")
+        set(ENABLE_CLANG_TIDY Off)
+    endif ()
 endif ()
 
 # Setup some compiler option that is generally useful and compatible with Fcitx 5 (C++17)

src/Engine/AssociatedPhrases.cpp

@@ -53,10 +53,7 @@ AssociatedPhrases::~AssociatedPhrases()
 
 bool AssociatedPhrases::isLoaded()
 {
-    if (data) {
-        return true;
-    }
-    return false;
+    return data != nullptr;
 }
 
 bool AssociatedPhrases::open(const char* path)

src/Engine/Mandarin/Mandarin.cpp

@@ -58,7 +58,7 @@ class BopomofoCharacterMap {
 };
 
 const BPMF BPMF::FromHanyuPinyin(const std::string& str) {
-  if (!str.length()) {
+  if (str.length() == 0) {
     return BPMF();
   }
 
@@ -77,8 +77,7 @@ const BPMF BPMF::FromHanyuPinyin(const std::string& str) {
   // NOLINTBEGIN(bugprone-branch-clone)
 
   // the y exceptions fist
-  if (0) {
-  } else if (PinyinParseHelper::ConsumePrefix(pinyin, "yuan")) {
+  if (PinyinParseHelper::ConsumePrefix(pinyin, "yuan")) {
     secondComponent = BPMF::UE;
     thirdComponent = BPMF::AN;
   } else if (PinyinParseHelper::ConsumePrefix(pinyin, "ying")) {
@@ -104,7 +103,7 @@ const BPMF BPMF::FromHanyuPinyin(const std::string& str) {
   }
 
   // try the first character
-  char c = pinyin.length() ? pinyin[0] : '\0';
+  char c = pinyin.length() != 0 ? pinyin[0] : '\0';
   switch (c) {
     case 'b':
       firstComponent = BPMF::B;
@@ -177,8 +176,7 @@ const BPMF BPMF::FromHanyuPinyin(const std::string& str) {
   }
 
   // then we try ZH, CH, SH, R, Z, C, S (in that order)
-  if (0) {
-  } else if (PinyinParseHelper::ConsumePrefix(pinyin, "zh")) {
+  if (PinyinParseHelper::ConsumePrefix(pinyin, "zh")) {
     firstComponent = BPMF::ZH;
     independentConsonant = true;
   } else if (PinyinParseHelper::ConsumePrefix(pinyin, "ch")) {
@@ -203,8 +201,7 @@ const BPMF BPMF::FromHanyuPinyin(const std::string& str) {
 
   // consume exceptions first: (ien, in), (iou, iu), (uen, un), (veng, iong),
   // (ven, vn), (uei, ui), ung but longer sequence takes precedence
-  if (0) {
-  } else if (PinyinParseHelper::ConsumePrefix(pinyin, "veng")) {
+  if (PinyinParseHelper::ConsumePrefix(pinyin, "veng")) {
     secondComponent = BPMF::UE;
     thirdComponent = BPMF::ENG;
   } else if (PinyinParseHelper::ConsumePrefix(pinyin, "iong")) {
@@ -272,8 +269,7 @@ const BPMF BPMF::FromHanyuPinyin(const std::string& str) {
   }
 
   // then consume the middle component...
-  if (0) {
-  } else if (PinyinParseHelper::ConsumePrefix(pinyin, "i")) {
+  if (PinyinParseHelper::ConsumePrefix(pinyin, "i")) {
     secondComponent = independentConsonant ? 0 : BPMF::I;
   } else if (PinyinParseHelper::ConsumePrefix(pinyin, "u")) {
     if (firstComponent == BPMF::J || firstComponent == BPMF::Q ||
@@ -287,8 +283,7 @@ const BPMF BPMF::FromHanyuPinyin(const std::string& str) {
   }
 
   // the vowels, longer sequence takes precedence
-  if (0) {
-  } else if (PinyinParseHelper::ConsumePrefix(pinyin, "ang")) {
+  if (PinyinParseHelper::ConsumePrefix(pinyin, "ang")) {
     thirdComponent = BPMF::ANG;
   } else if (PinyinParseHelper::ConsumePrefix(pinyin, "eng")) {
     thirdComponent = BPMF::ENG;
@@ -322,8 +317,7 @@ const BPMF BPMF::FromHanyuPinyin(const std::string& str) {
   // NOLINTEND(bugprone-branch-clone)
 
   // at last!
-  if (0) {
-  } else if (PinyinParseHelper::ConsumePrefix(pinyin, "1")) {
+  if (PinyinParseHelper::ConsumePrefix(pinyin, "1")) {
     toneComponent = BPMF::Tone1;
   } else if (PinyinParseHelper::ConsumePrefix(pinyin, "2")) {
     toneComponent = BPMF::Tone2;
@@ -383,43 +377,63 @@ const std::string BPMF::HanyuPinyinString(bool includesTone,
       break;
     case J:
       consonant = "j";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case Q:
       consonant = "q";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case X:
       consonant = "x";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case ZH:
       consonant = "zh";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case CH:
       consonant = "ch";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case SH:
       consonant = "sh";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case R:
       consonant = "r";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case Z:
       consonant = "z";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case C:
       consonant = "c";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
     case S:
       consonant = "s";
-      if (hasNoMVCOrVC) middle = "i";
+      if (hasNoMVCOrVC) {
+        middle = "i";
+      }
       break;
   }
 
@@ -567,6 +581,9 @@ const BPMF BPMF::FromComposedString(const std::string& str) {
     // the Bopomofo character map or to split the input by codepoints. This
     // suffices for now.
 
+    // disable for check for code points below
+    // NOLINTBEGIN(readability-magic-numbers)
+
     // Illegal.
     if (!(*iter & 0x80)) {
       break;
@@ -583,6 +600,7 @@ const BPMF BPMF::FromComposedString(const std::string& str) {
       // Illegal.
       break;
     }
+    // NOLINTEND(readability-magic-numbers)
 
     if (iter + (utf8_length - 1) == str.end()) {
       break;
@@ -595,9 +613,8 @@ const BPMF BPMF::FromComposedString(const std::string& str) {
         charToComp.find(component);
     if (result == charToComp.end()) {
       break;
-    } else {
-      syllable += BPMF((*result).second);
     }
+    syllable += BPMF((*result).second);
     iter += utf8_length;
   }
   return syllable;
@@ -669,8 +686,9 @@ BopomofoCharacterMap::BopomofoCharacterMap() {
 
   for (std::map<std::string, BPMF::Component>::iterator iter =
            characterToComponent.begin();
-       iter != characterToComponent.end(); ++iter)
+       iter != characterToComponent.end(); ++iter) {
     componentToCharacter[(*iter).second] = (*iter).first;
+  }
 }
 
 // we don't need parentheses for these macros

src/Engine/McBopomofoLM.cpp

@@ -132,15 +132,15 @@ bool McBopomofoLM::hasUnigrams(const std::string& key)
         return m_userPhrases.hasUnigrams(key) || m_languageModel.hasUnigrams(key);
     }
 
-    return getUnigrams(key).size() > 0;
+    return !getUnigrams(key).empty();
 }
 
 void McBopomofoLM::setPhraseReplacementEnabled(bool enabled)
 {
     m_phraseReplacementEnabled = enabled;
 }
 
-bool McBopomofoLM::phraseReplacementEnabled()
+bool McBopomofoLM::phraseReplacementEnabled() const
 {
     return m_phraseReplacementEnabled;
 }
@@ -150,7 +150,7 @@ void McBopomofoLM::setExternalConverterEnabled(bool enabled)
     m_externalConverterEnabled = enabled;
 }
 
-bool McBopomofoLM::externalConverterEnabled()
+bool McBopomofoLM::externalConverterEnabled() const
 {
     return m_externalConverterEnabled;
 }
@@ -175,7 +175,7 @@ std::vector<Formosa::Gramambular2::LanguageModel::Unigram> McBopomofoLM::filterA
         std::string value = originalValue;
         if (m_phraseReplacementEnabled) {
             std::string replacement = m_phraseReplacement.valueForKey(value);
-            if (replacement != "") {
+            if (!replacement.empty()) {
                 value = replacement;
             }
         }

src/Engine/McBopomofoLM.h

@@ -64,7 +64,7 @@ class McBopomofoLM : public Formosa::Gramambular2::LanguageModel {
 
     /// Asks to load the primary language model at the given path.
     /// @param languageModelPath The path of the language model.
-    void loadLanguageModel(const char* languageModelPath);
+    void loadLanguageModel(const char* languageModelDataPath);
     /// If the data model is already loaded.
     bool isDataModelLoaded();
 
@@ -77,7 +77,7 @@ class McBopomofoLM : public Formosa::Gramambular2::LanguageModel {
     /// Asks to load the user phrases and excluded phrases at the given path.
     /// @param userPhrasesPath The path of user phrases.
     /// @param excludedPhrasesPath The path of excluded phrases.
-    void loadUserPhrases(const char* userPhrasesPath, const char* excludedPhrasesPath);
+    void loadUserPhrases(const char* userPhrasesDataPath, const char* excludedPhrasesDataPath);
     /// Asks to load th phrase replacement table at the given path.
     /// @param phraseReplacementPath The path of the phrase replacement table.
     void loadPhraseReplacementMap(const char* phraseReplacementPath);
@@ -93,12 +93,12 @@ class McBopomofoLM : public Formosa::Gramambular2::LanguageModel {
     /// Enables or disables phrase replacement.
     void setPhraseReplacementEnabled(bool enabled);
     /// If phrase replacement is enabled or not.
-    bool phraseReplacementEnabled();
+    bool phraseReplacementEnabled() const;
 
     /// Enables or disables the external converter.
     void setExternalConverterEnabled(bool enabled);
     /// If the external converted is enabled or not.
-    bool externalConverterEnabled();
+    bool externalConverterEnabled() const;
     /// Sets a lambda to let the values of unigrams could be converted by it.
     void setExternalConverter(std::function<std::string(std::string)> externalConverter);
 

src/Engine/ParselessLM.cpp

@@ -35,10 +35,7 @@ McBopomofo::ParselessLM::~ParselessLM() { close(); }
 
 bool McBopomofo::ParselessLM::isLoaded()
 {
-    if (data_) {
-        return true;
-    }
-    return false;
+    return data_ != nullptr;
 }
 
 bool McBopomofo::ParselessLM::open(const std::string_view& path)
@@ -99,7 +96,7 @@ McBopomofo::ParselessLM::getUnigrams(const std::string& key)
         double score = 0;
 
         // Move ahead until we encounter the first space. This is the key.
-        auto it = row.begin();
+        const auto* it = row.begin();
         while (it != row.end() && *it != ' ') {
             ++it;
         }
@@ -113,7 +110,7 @@ McBopomofo::ParselessLM::getUnigrams(const std::string& key)
 
         if (it != row.end()) {
             // Now it is the start of the value portion.
-            auto value_begin = it;
+            const auto* value_begin = it;
 
             // Move ahead until we encounter the second space. This is the
             // value.

src/Engine/ParselessPhraseDB.cpp

@@ -42,13 +42,6 @@ ParselessPhraseDB::ParselessPhraseDB(
         std::string_view header(buf, SORTED_PRAGMA_HEADER.length());
         assert(header == SORTED_PRAGMA_HEADER);
 
-        uint32_t x = 5381;
-        for (const auto& i : header) {
-            x = x * 33 + i;
-        }
-
-        assert(x == uint32_t { 3012373384 });
-
         begin_ += header.length();
     }
 }

src/Engine/UserOverrideModel.cpp

@@ -54,6 +54,7 @@ UserOverrideModel::UserOverrideModel(size_t capacity, double decayConstant)
     : m_capacity(capacity)
 {
     assert(m_capacity > 0);
+    // NOLINTNEXTLINE(readability-magic-numbers)
     m_decayExponent = log(0.5) / decayConstant;
 }
 

src/Engine/UserPhrasesLM.cpp

@@ -49,10 +49,7 @@ UserPhrasesLM::~UserPhrasesLM()
 
 bool UserPhrasesLM::isLoaded()
 {
-    if (data) {
-        return true;
-    }
-    return false;
+    return data != nullptr;
 }
 
 bool UserPhrasesLM::open(const char* path)