JamesReynolds updated this revision to Diff 60056.
JamesReynolds marked 5 inline comments as done.
JamesReynolds added a comment.
Applied suggested code changes.
http://reviews.llvm.org/D21020
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
docs/ReleaseNotes.rst
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===================================================================
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -61,6 +61,7 @@
// RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \
// RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
// RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
+// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \
// RUN: {key: readability-identifier-naming.TypeAliasCase, value: lower_case}, \
// RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \
// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
@@ -307,6 +308,12 @@
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type'
// CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
+struct_type GlobalTypedefTestFunction(struct_type a_argument1) {
+// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) {
+ struct_type typedef_test_1;
+// CHECK-FIXES: {{^}} struct_type_t typedef_test_1;
+}
+
using my_struct_type = THIS___Structure;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'my_struct_type'
// CHECK-FIXES: {{^}}using my_struct_type_t = this_structure;{{$}}
@@ -329,5 +336,11 @@
// CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}}
}
+#define MY_TEST_Macro(X) X()
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro'
+// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X()
+
+void MY_TEST_Macro(function) {}
+// CHECK-FIXES: {{^}}void MY_TEST_MACRO(function) {}
}
}
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -249,6 +249,11 @@
Warns about defaulted constructors and assignment operators that are actually
deleted.
+- Updated `readability-identifier-naming-check
+ <http://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-naming.html>`_
+
+ Added support for enforcing the case of macro statements.
+
- New `readability-redundant-control-flow
<http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-control-flow.html>`_ check
Index: clang-tidy/readability/IdentifierNamingCheck.h
===================================================================
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -13,6 +13,9 @@
#include "../ClangTidy.h"
namespace clang {
+
+class MacroInfo;
+
namespace tidy {
namespace readability {
@@ -36,6 +39,7 @@
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void registerPPCallbacks(CompilerInstance &Compiler) override;
void onEndOfTranslationUnit() override;
enum CaseType {
@@ -64,7 +68,7 @@
/// \brief Holds an identifier name check failure, tracking the kind of the
/// identifer, its possible fixup and the starting locations of all the
- /// idenfiier usages.
+ /// identifier usages.
struct NamingCheckFailure {
std::string KindName;
std::string Fixup;
@@ -81,9 +85,19 @@
NamingCheckFailure() : ShouldFix(true) {}
};
- typedef llvm::DenseMap<const NamedDecl *, NamingCheckFailure>
+
+ typedef std::pair<SourceLocation, std::string> NamingCheckId;
+
+ typedef llvm::DenseMap<NamingCheckId, NamingCheckFailure>
NamingCheckFailureMap;
+ /// Check Macros for style violations
+ void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok,
+ const MacroInfo *MI);
+
+ /// Add a usage of a macro if it already has a violation
+ void expandMacro(const Token &MacroNameTok, const MacroInfo *MI);
+
private:
std::vector<NamingStyle> NamingStyles;
bool IgnoreFailedSplit;
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -12,11 +12,53 @@
#include "llvm/Support/Debug.h"
#include "llvm/Support/Format.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseMapInfo.h"
#define DEBUG_TYPE "clang-tidy"
using namespace clang::ast_matchers;
+namespace llvm {
+/// Specialisation of DenseMapInfo to allow NamingCheckId objects in DenseMaps
+template <>
+struct DenseMapInfo<
+ clang::tidy::readability::IdentifierNamingCheck::NamingCheckId> {
+ using NamingCheckId =
+ clang::tidy::readability::IdentifierNamingCheck::NamingCheckId;
+
+ static inline NamingCheckId getEmptyKey() {
+ return NamingCheckId(
+ clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-1)),
+ "EMPTY");
+ }
+
+ static inline NamingCheckId getTombstoneKey() {
+ return NamingCheckId(
+ clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-2)),
+ "TOMBSTONE");
+ }
+
+ static unsigned getHashValue(NamingCheckId Val) {
+ assert(Val != getEmptyKey() && "Cannot hash the empty key!");
+ assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!");
+
+ std::hash<NamingCheckId::second_type> SecondHash;
+ return Val.first.getRawEncoding() + SecondHash(Val.second);
+ }
+
+ static bool isEqual(NamingCheckId LHS, NamingCheckId RHS) {
+ if (RHS == getEmptyKey())
+ return LHS == getEmptyKey();
+ if (RHS == getTombstoneKey())
+ return LHS == getTombstoneKey();
+ return LHS == RHS;
+ }
+};
+} // namespace llvm
+
namespace clang {
namespace tidy {
namespace readability {
@@ -66,6 +108,7 @@
m(TemplateTemplateParameter) \
m(TemplateParameter) \
m(TypeAlias) \
+ m(MacroDefinition) \
enum StyleKind {
#define ENUMERATE(v) SK_ ## v,
@@ -84,6 +127,33 @@
#undef NAMING_KEYS
// clang-format on
+namespace {
+/// Callback supplies macros to IdentifierNamingCheck::checkMacro
+class IdentifierNamingCheckPPCallbacks : public PPCallbacks {
+public:
+ IdentifierNamingCheckPPCallbacks(Preprocessor *PP,
+ IdentifierNamingCheck *Check)
+ : PP(PP), Check(Check) {}
+
+ /// MacroDefined calls checkMacro for macros in the main file
+ void MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) override {
+ Check->checkMacro(PP->getSourceManager(), MacroNameTok, MD->getMacroInfo());
+ }
+
+ /// MacroExpands calls expandMacro for macros in the main file
+ void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
+ SourceRange /*Range*/,
+ const MacroArgs * /*Args*/) override {
+ Check->expandMacro(MacroNameTok, MD.getMacroInfo());
+ }
+
+private:
+ Preprocessor *PP;
+ IdentifierNamingCheck *Check;
+};
+} // namespace
+
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {
@@ -146,6 +216,12 @@
Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
}
+void IdentifierNamingCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+ Compiler.getPreprocessor().addPPCallbacks(
+ llvm::make_unique<IdentifierNamingCheckPPCallbacks>(
+ &Compiler.getPreprocessor(), this));
+}
+
static bool matchesStyle(StringRef Name,
IdentifierNamingCheck::NamingStyle Style) {
static llvm::Regex Matchers[] = {
@@ -506,8 +582,8 @@
}
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
- const NamedDecl *Decl, SourceRange Range,
- const SourceManager *SM) {
+ const IdentifierNamingCheck::NamingCheckId &Decl,
+ SourceRange Range) {
// Do nothing if the provided range is invalid.
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
return;
@@ -522,14 +598,22 @@
!Range.getEnd().isMacroID();
}
+/// Convenience method when the usage to be added is a NamedDecl
+static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
+ const NamedDecl *Decl, SourceRange Range) {
+ return addUsage(Failures, IdentifierNamingCheck::NamingCheckId(
+ Decl->getLocation(), Decl->getNameAsString()),
+ Range);
+}
+
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Decl =
Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) {
if (Decl->isImplicit())
return;
addUsage(NamingCheckFailures, Decl->getParent(),
- Decl->getNameInfo().getSourceRange(), Result.SourceManager);
+ Decl->getNameInfo().getSourceRange());
return;
}
@@ -545,8 +629,7 @@
// we want instead to replace the next token, that will be the identifier.
Range.setBegin(CharSourceRange::getTokenRange(Range).getEnd());
- addUsage(NamingCheckFailures, Decl->getParent(), Range,
- Result.SourceManager);
+ addUsage(NamingCheckFailures, Decl->getParent(), Range);
return;
}
@@ -563,8 +646,7 @@
}
if (Decl) {
- addUsage(NamingCheckFailures, Decl, Loc->getSourceRange(),
- Result.SourceManager);
+ addUsage(NamingCheckFailures, Decl, Loc->getSourceRange());
return;
}
@@ -574,50 +656,74 @@
SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc());
if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) {
- addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range,
- Result.SourceManager);
+ addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range);
return;
}
}
if (const auto &Ref =
Loc->getAs<DependentTemplateSpecializationTypeLoc>()) {
addUsage(NamingCheckFailures, Ref.getTypePtr()->getAsTagDecl(),
- Loc->getSourceRange(), Result.SourceManager);
+ Loc->getSourceRange());
return;
}
}
if (const auto *Loc =
Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) {
if (NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) {
if (NamespaceDecl *Decl = Spec->getAsNamespace()) {
- addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange(),
- Result.SourceManager);
+ addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange());
return;
}
}
}
if (const auto *Decl = Result.Nodes.getNodeAs<UsingDecl>("using")) {
for (const auto &Shadow : Decl->shadows()) {
addUsage(NamingCheckFailures, Shadow->getTargetDecl(),
- Decl->getNameInfo().getSourceRange(), Result.SourceManager);
+ Decl->getNameInfo().getSourceRange());
}
return;
}
if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) {
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
- addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
- Result.SourceManager);
+ addUsage(NamingCheckFailures, DeclRef->getDecl(), Range);
return;
}
if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {
if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit())
return;
+ // Fix type aliases in value declarations
+ if (const auto *Value = Result.Nodes.getNodeAs<ValueDecl>("decl")) {
+ if (const auto *Typedef =
+ Value->getType().getTypePtr()->getAs<TypedefType>()) {
+ addUsage(NamingCheckFailures, Typedef->getDecl(),
+ Value->getSourceRange());
+ }
+ }
+
+ // Fix type aliases in function declarations
+ if (const auto *Value = Result.Nodes.getNodeAs<FunctionDecl>("decl")) {
+ if (const auto *Typedef =
+ Value->getReturnType().getTypePtr()->getAs<TypedefType>()) {
+ addUsage(NamingCheckFailures, Typedef->getDecl(),
+ Value->getSourceRange());
+ }
+ for (unsigned i = 0; i < Value->getNumParams(); ++i) {
+ if (const auto *Typedef = Value->parameters()[i]
+ ->getType()
+ .getTypePtr()
+ ->getAs<TypedefType>()) {
+ addUsage(NamingCheckFailures, Typedef->getDecl(),
+ Value->getSourceRange());
+ }
+ }
+ }
+
// Ignore ClassTemplateSpecializationDecl which are creating duplicate
// replacements with CXXRecordDecl
if (isa<ClassTemplateSpecializationDecl>(Decl))
@@ -644,29 +750,76 @@
KindName.c_str(), Name));
}
} else {
- NamingCheckFailure &Failure = NamingCheckFailures[Decl];
+ NamingCheckFailure &Failure = NamingCheckFailures[NamingCheckId(
+ Decl->getLocation(), Decl->getNameAsString())];
SourceRange Range =
DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation())
.getSourceRange();
Failure.Fixup = std::move(Fixup);
Failure.KindName = std::move(KindName);
- addUsage(NamingCheckFailures, Decl, Range, Result.SourceManager);
+ addUsage(NamingCheckFailures, Decl, Range);
}
}
}
+void IdentifierNamingCheck::checkMacro(SourceManager &SourceMgr,
+ const Token &MacroNameTok,
+ const MacroInfo *MI) {
+ StringRef Name = MacroNameTok.getIdentifierInfo()->getName();
+ NamingStyle Style = NamingStyles[SK_MacroDefinition];
+ if (matchesStyle(Name, Style))
+ return;
+
+ std::string KindName =
+ fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase);
+ std::replace(KindName.begin(), KindName.end(), '_', ' ');
+
+ std::string Fixup = fixupWithStyle(Name, Style);
+ if (StringRef(Fixup).equals(Name)) {
+ if (!IgnoreFailedSplit) {
+ DEBUG(
+ llvm::dbgs() << MacroNameTok.getLocation().printToString(SourceMgr)
+ << llvm::format(": unable to split words for %s '%s'\n",
+ KindName.c_str(), Name));
+ }
+ } else {
+ NamingCheckId ID(MI->getDefinitionLoc(), Name);
+ NamingCheckFailure &Failure = NamingCheckFailures[ID];
+ SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
+
+ Failure.Fixup = std::move(Fixup);
+ Failure.KindName = std::move(KindName);
+ addUsage(NamingCheckFailures, ID, Range);
+ }
+}
+
+void IdentifierNamingCheck::expandMacro(const Token &MacroNameTok,
+ const MacroInfo *MI) {
+ StringRef Name = MacroNameTok.getIdentifierInfo()->getName();
+ NamingCheckId ID(MI->getDefinitionLoc(), Name);
+
+ auto Failure = NamingCheckFailures.find(ID);
+ if (Failure == NamingCheckFailures.end())
+ return;
+
+ SourceRange Range =
+ SourceRange(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
+ addUsage(NamingCheckFailures, ID, Range);
+ return;
+}
+
void IdentifierNamingCheck::onEndOfTranslationUnit() {
for (const auto &Pair : NamingCheckFailures) {
- const NamedDecl &Decl = *Pair.first;
+ const NamingCheckId &Decl = Pair.first;
const NamingCheckFailure &Failure = Pair.second;
if (Failure.KindName.empty())
continue;
if (Failure.ShouldFix) {
- auto Diag = diag(Decl.getLocation(), "invalid case style for %0 '%1'")
- << Failure.KindName << Decl.getName();
+ auto Diag = diag(Decl.first, "invalid case style for %0 '%1'")
+ << Failure.KindName << Decl.second;
for (const auto &Loc : Failure.RawUsageLocs) {
// We assume that the identifier name is made of one token only. This is
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
