dougpuob updated this revision to Diff 288938.
dougpuob added a comment.
Improved suggestions of code review.
1. Moved release notes to right place. [Eugene.Zelenko]
2. Added new casting types to doc(readability-identifier-naming.rst)
[Eugene.Zelenko]
3. Moved partial code to a new function,
IdentifierNamingCheck::getDeclTypeName(). [njames93]
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86671/new/
https://reviews.llvm.org/D86671
Files:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,120 @@
+#include <stddef.h>
+#include <stdint.h>
+
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN: -config="{CheckOptions: [\
+// RUN: {key: readability-identifier-naming.VariableCase, value: szHungarianNotation}, \
+// RUN: ]}"
+
+class UnlistedClass {};
+UnlistedClass Unlisted1;
+// CHECK-NOT: :[[@LINE-2]]
+
+UnlistedClass cUnlisted2;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for variable 'cUnlisted2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass Unlisted2;
+
+UnlistedClass objUnlistedClass3;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for variable 'objUnlistedClass3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass UnlistedClass3;
+
+typedef int INDEX;
+INDEX iIndex = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'iIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}INDEX Index = 0;
+
+const char *NamePtr = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for variable 'NamePtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char *szNamePtr = "Name";
+
+const char NameArray[] = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for variable 'NameArray' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char szNameArray[] = "Name";
+
+void *BufferPtr1 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'BufferPtr1' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pBufferPtr1 = NULL;
+
+void **BufferPtr2 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'BufferPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr2 = NULL;
+
+void **pBufferPtr3 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'pBufferPtr3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr3 = NULL;
+
+int8_t ValueI8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueI8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int8_t i8ValueI8 = 0;
+
+int16_t ValueI16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int16_t i16ValueI16 = 0;
+
+int32_t ValueI32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int32_t i32ValueI32 = 0;
+
+int64_t ValueI64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int64_t i64ValueI64 = 0;
+
+uint8_t ValueU8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueU8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint8_t u8ValueU8 = 0;
+
+uint16_t ValueU16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint16_t u16ValueU16 = 0;
+
+uint32_t ValueU32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint32_t u32ValueU32 = 0;
+
+uint64_t ValueU64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint64_t u64ValueU64 = 0;
+
+float ValueFloat = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'ValueFloat' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}float fValueFloat = 0;
+
+double ValueDouble = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueDouble' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}double dValueDouble = 0;
+
+char ValueChar = 'c';
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueChar' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}char cValueChar = 'c';
+
+bool ValueBool = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueBool' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}bool bValueBool = true;
+
+int ValueInt = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for variable 'ValueInt' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int iValueInt = 0;
+
+size_t ValueSize = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueSize' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}size_t nValueSize = 0;
+
+wchar_t ValueWchar = 'w';
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueWchar' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}wchar_t wcValueWchar = 'w';
+
+short ValueShort = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'ValueShort' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}short sValueShort = 0;
+
+unsigned ValueUnsigned = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueUnsigned' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned uValueUnsigned = 0;
+
+signed ValueSigned = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueSigned' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}signed iValueSigned = 0;
+
+long ValueLong = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueLong' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}long lValueLong = 0;
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -17,7 +17,8 @@
- ``CamelCase``,
- ``camel_Snake_Back``,
- ``Camel_Snake_Case``,
- - ``aNy_CasE``.
+ - ``aNy_CasE``,
+ - ``szHungarianNotation``.
It also supports a fixed prefix and suffix that will be prepended or appended
to the identifiers, regardless of the casing.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -76,6 +76,13 @@
Added an option `GetConfigPerFile` to support including files which use
different naming styles.
+- Improved :doc:`readability-identifier-naming
+ <clang-tidy/checks/readability-identifier-naming>` check.
+
+ Added a casing types `szHungarianNotation` to support variables could be
+ checked with Hungarian Notation which the prefix encodes the actual data type
+ of the variable.
+
Improvements to include-fixer
-----------------------------
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -36,6 +36,7 @@
IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context);
~IdentifierNamingCheck();
+ std::string getDeclTypeName(const clang::NamedDecl *Decl) const;
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
enum CaseType {
@@ -45,7 +46,8 @@
CT_UpperCase,
CT_CamelCase,
CT_CamelSnakeCase,
- CT_CamelSnakeBack
+ CT_CamelSnakeBack,
+ CT_HungarianNotation
};
struct NamingStyle {
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -44,7 +44,9 @@
{readability::IdentifierNamingCheck::CT_CamelSnakeCase,
"Camel_Snake_Case"},
{readability::IdentifierNamingCheck::CT_CamelSnakeBack,
- "camel_Snake_Back"}};
+ "camel_Snake_Back"},
+ {readability::IdentifierNamingCheck::CT_HungarianNotation,
+ "szHungarianNotation"}};
return llvm::makeArrayRef(Mapping);
}
@@ -178,8 +180,169 @@
Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions);
}
-static bool matchesStyle(StringRef Name,
- IdentifierNamingCheck::NamingStyle Style) {
+static const std::string
+getHungarianNotationTypePrefix(const std::string &TypeName,
+ const NamedDecl *Decl) {
+ if (0 == TypeName.length()) {
+ return TypeName;
+ }
+
+ // clang-format off
+ const static llvm::StringMap<StringRef> HungarianNotationTable = {
+ {"int8_t", "i8"},
+ {"int16_t", "i16"},
+ {"int32_t", "i32"},
+ {"int64_t", "i64"},
+ {"uint8_t", "u8"},
+ {"uint16_t", "u16"},
+ {"uint32_t", "u32"},
+ {"uint64_t", "u64"},
+ {"float", "f"},
+ {"double", "d"},
+ {"char", "c"},
+ {"bool", "b"},
+ {"_Bool", "b"},
+ {"int", "i"},
+ {"size_t", "n"},
+ {"wchar_t", "wc"},
+ {"short", "s"},
+ {"signed", "i"},
+ {"unsigned", "u"},
+ {"long", "l"}};
+ // clang-format on
+
+ std::string ClonedTypeName = TypeName;
+
+ // Handle null string
+ std::string PrefixStr;
+ if (const auto *TD = dyn_cast<ValueDecl>(Decl)) {
+ auto QT = TD->getType();
+ if (QT->isPointerType()) {
+ // clang-format off
+ const static llvm::StringMap<StringRef> NullString = {
+ {"char*", "sz"},
+ {"wchar_t*", "wsz"}};
+ // clang-format on
+ for (const auto &Type : NullString) {
+ const auto &Key = Type.getKey();
+ if (ClonedTypeName.find(Key.str()) == 0) {
+ PrefixStr = Type.getValue().str();
+ ClonedTypeName = ClonedTypeName.substr(
+ Key.size(), ClonedTypeName.size() - Key.size());
+ break;
+ }
+ }
+ } else if (QT->isArrayType()) {
+ // clang-format off
+ const static llvm::StringMap<StringRef> NullString = {
+ {"char", "sz"},
+ {"wchar_t", "wsz"}};
+ // clang-format on
+ for (const auto &Type : NullString) {
+ const auto &Key = Type.getKey();
+ if (ClonedTypeName.find(Key.str()) == 0) {
+ PrefixStr = Type.getValue().str();
+ ClonedTypeName = ClonedTypeName.substr(
+ Key.size(), ClonedTypeName.size() - Key.size());
+ break;
+ }
+ }
+ }
+ }
+
+ // Handle pointers
+ size_t PtrCount = [&](std::string TypeName) -> size_t {
+ size_t Pos = TypeName.find('*');
+ size_t Count = 0;
+ for (; Pos < TypeName.length(); Pos++, Count++) {
+ if ('*' != TypeName[Pos])
+ break;
+ }
+ return Count;
+ }(ClonedTypeName);
+ if (PtrCount > 0) {
+ ClonedTypeName = [&](std::string Str, const std::string &From,
+ const std::string &To) {
+ size_t StartPos = 0;
+ while ((StartPos = Str.find(From, StartPos)) != std::string::npos) {
+ Str.replace(StartPos, From.length(), To);
+ StartPos += To.length();
+ }
+ return Str;
+ }(ClonedTypeName, "*", "");
+ }
+
+ for (const auto &Type : HungarianNotationTable) {
+ const auto &Key = Type.getKey();
+ if (ClonedTypeName == Key) {
+ PrefixStr = Type.getValue().str();
+ break;
+ }
+ }
+
+ if (PtrCount > 0) {
+ for (size_t Idx = 0; Idx < PtrCount; Idx++) {
+ PrefixStr.insert(PrefixStr.begin(), 'p');
+ }
+ }
+
+ return PrefixStr;
+}
+
+std::string
+IdentifierNamingCheck::getDeclTypeName(const clang::NamedDecl *Decl) const {
+ const ValueDecl *ValDecl = dyn_cast<ValueDecl>(Decl);
+ if (!ValDecl) {
+ return "";
+ }
+
+ // Get type text of variable declarations.
+ const auto &SM = ValDecl->getASTContext().getSourceManager();
+ const char *Begin = SM.getCharacterData(ValDecl->getBeginLoc());
+ const char *Curr = SM.getCharacterData(ValDecl->getLocation());
+ const intptr_t StrLen = Curr - Begin;
+ std::string TypeName;
+ if (StrLen > 0) {
+ std::string Type(Begin, StrLen);
+
+ const static std::list<std::string> Keywords = {
+ // Qualifier
+ "const", "volatile",
+ // Storage class specifiers
+ "auto", "register", "static", "extern", "thread_local",
+ // Constexpr specifiers
+ "constexpr", "constinit", "const_cast", "consteval"};
+
+ // Remove keywords
+ for (const auto &Kw : Keywords) {
+ for (size_t Pos = 0; (Pos = Type.find(Kw, Pos)) != std::string::npos;) {
+ Type.replace(Pos, Kw.length(), "");
+ }
+ }
+
+ // Replace spaces with single space
+ for (size_t Pos = 0; (Pos = Type.find(" ", Pos)) != std::string::npos;
+ Pos += strlen(" ")) {
+ Type.replace(Pos, strlen(" "), " ");
+ }
+
+ // Replace " *" with "*"
+ for (size_t Pos = 0; (Pos = Type.find(" *", Pos)) != std::string::npos;
+ Pos += strlen("*")) {
+ Type.replace(Pos, strlen(" *"), "*");
+ }
+
+ Type = Type.erase(Type.find_last_not_of(" ") + 1);
+ Type = Type.erase(0, Type.find_first_not_of(" "));
+ TypeName = Type;
+ }
+
+ return TypeName;
+}
+
+static bool matchesStyle(StringRef Type, StringRef Name,
+ IdentifierNamingCheck::NamingStyle Style,
+ const NamedDecl *Decl) {
static llvm::Regex Matchers[] = {
llvm::Regex("^.*$"),
llvm::Regex("^[a-z][a-z0-9_]*$"),
@@ -188,6 +351,7 @@
llvm::Regex("^[A-Z][a-zA-Z0-9]*$"),
llvm::Regex("^[A-Z]([a-z0-9]*(_[A-Z])?)*"),
llvm::Regex("^[a-z]([a-z0-9]*(_[A-Z])?)*"),
+ llvm::Regex("^[A-Z][a-zA-Z0-9]*$"),
};
if (!Name.consume_front(Style.Prefix))
@@ -200,13 +364,24 @@
if (Name.startswith("_") || Name.endswith("_"))
return false;
+ if (Style.Case == IdentifierNamingCheck::CaseType::CT_HungarianNotation) {
+ const std::string TypePrefix =
+ getHungarianNotationTypePrefix(Type.str(), Decl);
+ if (TypePrefix.length() > 0) {
+ if (!Name.startswith(TypePrefix))
+ return false;
+ Name = Name.drop_front(TypePrefix.size());
+ }
+ }
+
if (Style.Case && !Matchers[static_cast<size_t>(*Style.Case)].match(Name))
return false;
return true;
}
-static std::string fixupWithCase(StringRef Name,
+static std::string fixupWithCase(const StringRef &Type, const StringRef &Name,
+ const Decl *pDecl,
IdentifierNamingCheck::CaseType Case) {
static llvm::Regex Splitter(
"([a-z0-9A-Z]*)(_+)|([A-Z]?[a-z0-9]+)([A-Z]|$)|([A-Z]+)([A-Z]|$)");
@@ -298,8 +473,26 @@
Fixup += Word.substr(1).lower();
}
break;
- }
+ case IdentifierNamingCheck::CT_HungarianNotation: {
+ const NamedDecl *pNamedDecl = dyn_cast<NamedDecl>(pDecl);
+ const std::string TypePrefix =
+ getHungarianNotationTypePrefix(Type.str(), pNamedDecl);
+ Fixup = TypePrefix;
+ for (size_t Idx = 0; Idx < Words.size(); Idx++) {
+ // Skip first part if it's a lowercase string
+ if (Idx == 0) {
+ const bool LowerAlnum =
+ std::all_of(Words[Idx].begin(), Words[Idx].end(),
+ [](const char c) { return isdigit(c) || islower(c); });
+ if (LowerAlnum)
+ continue;
+ }
+ Fixup += Words[Idx];
+ }
+ break;
+ }
+ }
return Fixup.str().str();
}
@@ -365,10 +558,12 @@
}
static std::string
-fixupWithStyle(StringRef Name,
- const IdentifierNamingCheck::NamingStyle &Style) {
+fixupWithStyle(const StringRef &Type, const StringRef &Name,
+ const IdentifierNamingCheck::NamingStyle &Style,
+ const Decl *Decl) {
const std::string Fixed = fixupWithCase(
- Name, Style.Case.getValueOr(IdentifierNamingCheck::CaseType::CT_AnyCase));
+ Type, Name, Decl,
+ Style.Case.getValueOr(IdentifierNamingCheck::CaseType::CT_AnyCase));
StringRef Mid = StringRef(Fixed).trim("_");
if (Mid.empty())
Mid = "_";
@@ -384,7 +579,7 @@
if (isa<ObjCIvarDecl>(D) && NamingStyles[SK_ObjcIvar])
return SK_ObjcIvar;
-
+
if (isa<TypedefDecl>(D) && NamingStyles[SK_Typedef])
return SK_Typedef;
@@ -655,21 +850,22 @@
}
static llvm::Optional<RenamerClangTidyCheck::FailureInfo> getFailureInfo(
- StringRef Name, SourceLocation Location,
+ const StringRef &Type, const StringRef &Name, const NamedDecl *Decl,
+ SourceLocation Location,
ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>> NamingStyles,
StyleKind SK, const SourceManager &SM, bool IgnoreFailedSplit) {
if (SK == SK_Invalid || !NamingStyles[SK])
return None;
const IdentifierNamingCheck::NamingStyle &Style = *NamingStyles[SK];
- if (matchesStyle(Name, Style))
+ if (matchesStyle(Type, Name, Style, Decl))
return None;
- std::string KindName =
- fixupWithCase(StyleNames[SK], IdentifierNamingCheck::CT_LowerCase);
+ std::string KindName = fixupWithCase(Type, StyleNames[SK], Decl,
+ IdentifierNamingCheck::CT_LowerCase);
std::replace(KindName.begin(), KindName.end(), '_', ' ');
- std::string Fixup = fixupWithStyle(Name, Style);
+ std::string Fixup = fixupWithStyle(Type, Name, Style, Decl);
if (StringRef(Fixup).equals(Name)) {
if (!IgnoreFailedSplit) {
LLVM_DEBUG(Location.print(llvm::dbgs(), SM);
@@ -690,8 +886,9 @@
ArrayRef<llvm::Optional<NamingStyle>> NamingStyles =
getStyleForFile(SM.getFilename(Loc));
+ std::string TypeName = getDeclTypeName(Decl);
return getFailureInfo(
- Decl->getName(), Loc, NamingStyles,
+ TypeName, Decl->getName(), Decl, Loc, NamingStyles,
findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM,
IgnoreFailedSplit);
}
@@ -701,8 +898,8 @@
const SourceManager &SM) const {
SourceLocation Loc = MacroNameTok.getLocation();
- return getFailureInfo(MacroNameTok.getIdentifierInfo()->getName(), Loc,
- getStyleForFile(SM.getFilename(Loc)),
+ return getFailureInfo("", MacroNameTok.getIdentifierInfo()->getName(), NULL,
+ Loc, getStyleForFile(SM.getFilename(Loc)),
SK_MacroDefinition, SM, IgnoreFailedSplit);
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
