Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h =================================================================== --- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h +++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h @@ -41,7 +41,7 @@ private: llvm::Optional - GetDeclFailureInfo(const NamedDecl *Decl, + GetDeclFailureInfo(const StringRef &TypeName, const NamedDecl *Decl, const SourceManager &SM) const override; llvm::Optional GetMacroFailureInfo(const Token &MacroNameTok, Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp @@ -152,7 +152,8 @@ } Optional -ReservedIdentifierCheck::GetDeclFailureInfo(const NamedDecl *Decl, +ReservedIdentifierCheck::GetDeclFailureInfo(const StringRef &Type, + const NamedDecl *Decl, const SourceManager &) const { assert(Decl && Decl->getIdentifier() && !Decl->getName().empty() && !Decl->isImplicit() && 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,9 @@ IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context); ~IdentifierNamingCheck(); + static const std::string + getHungarianNotationTypePrefix(const std::string &TypeName, + const NamedDecl *Decl); void storeOptions(ClangTidyOptions::OptionMap &Opts) override; enum CaseType { @@ -45,7 +48,8 @@ CT_UpperCase, CT_CamelCase, CT_CamelSnakeCase, - CT_CamelSnakeBack + CT_CamelSnakeBack, + CT_HungarianNotation }; struct NamingStyle { @@ -62,7 +66,7 @@ private: llvm::Optional - GetDeclFailureInfo(const NamedDecl *Decl, + GetDeclFailureInfo(const StringRef &Type, const NamedDecl *Decl, const SourceManager &SM) const override; llvm::Optional GetMacroFailureInfo(const Token &MacroNameTok, 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,118 @@ 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 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(Decl)) { + auto QT = TD->getType(); + if (QT->isPointerType()) { + // clang-format off + const static llvm::StringMap 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 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; +} + +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 +300,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 +313,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(*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 +422,26 @@ Fixup += Word.substr(1).lower(); } break; - } + case IdentifierNamingCheck::CT_HungarianNotation: { + const NamedDecl *pNamedDecl = dyn_cast(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 +507,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 +528,7 @@ if (isa(D) && NamingStyles[SK_ObjcIvar]) return SK_ObjcIvar; - + if (isa(D) && NamingStyles[SK_Typedef]) return SK_Typedef; @@ -655,21 +799,22 @@ } static llvm::Optional getFailureInfo( - StringRef Name, SourceLocation Location, + const StringRef &Type, const StringRef &Name, const NamedDecl *Decl, + SourceLocation Location, ArrayRef> 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); @@ -684,14 +829,15 @@ } llvm::Optional -IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl, +IdentifierNamingCheck::GetDeclFailureInfo(const StringRef &Type, + const NamedDecl *Decl, const SourceManager &SM) const { SourceLocation Loc = Decl->getLocation(); ArrayRef> NamingStyles = getStyleForFile(SM.getFilename(Loc)); return getFailureInfo( - Decl->getName(), Loc, NamingStyles, + Type, Decl->getName(), Decl, Loc, NamingStyles, findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM, IgnoreFailedSplit); } @@ -701,8 +847,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); } Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h =================================================================== --- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h +++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h @@ -124,7 +124,8 @@ /// Overridden by derived classes, returns information about if and how a Decl /// failed the check. A 'None' result means the Decl did not fail the check. virtual llvm::Optional - GetDeclFailureInfo(const NamedDecl *Decl, const SourceManager &SM) const = 0; + GetDeclFailureInfo(const StringRef &Type, const NamedDecl *Decl, + const SourceManager &SM) const = 0; /// Overridden by derived classes, returns information about if and how a /// macro failed the check. A 'None' result means the macro did not fail the Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp +++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp @@ -389,13 +389,55 @@ return; } - // Fix type aliases in value declarations. + std::string TypeName; if (const auto *Value = Result.Nodes.getNodeAs("decl")) { + // Fix type aliases in value declarations. if (const Type *TypePtr = Value->getType().getTypePtrOrNull()) { if (const auto *Typedef = TypePtr->getAs()) addUsage(Typedef->getDecl(), Value->getSourceRange(), Result.SourceManager); } + + // Get type text of variable declarations. + const auto &SrcMgr = Decl->getASTContext().getSourceManager(); + const char *Begin = SrcMgr.getCharacterData(Decl->getBeginLoc()); + const char *Curr = SrcMgr.getCharacterData(Decl->getLocation()); + const intptr_t StrLen = Curr - Begin; + if (StrLen > 0) { + std::string Type(Begin, StrLen); + + const static std::list 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; + } } // Fix type aliases in function declarations. @@ -418,7 +460,7 @@ return; Optional MaybeFailure = - GetDeclFailureInfo(Decl, *Result.SourceManager); + GetDeclFailureInfo(TypeName, Decl, *Result.SourceManager); if (!MaybeFailure) return; FailureInfo &Info = *MaybeFailure; Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -70,6 +70,9 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Added: Support variables could be checked with Hungarian Notation which the + prefix encodes the actual data type of the variable. + - Improved :doc:`readability-identifier-naming ` check. 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,103 @@ +#include +#include + +// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \ +// RUN: -config="{CheckOptions: [\ +// RUN: {key: readability-identifier-naming.VariableCase, value: szHungarianNotation}, \ +// RUN: ]}" + +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;