JamesReynolds created this revision.
JamesReynolds added a reviewer: alexfh.
JamesReynolds added a subscriber: cfe-commits.
Added support for macro definitions.
--
1. Added a pre-processor callback to catch macro definitions
2. Changed the type of the failure map so that macros and declarations can
share the same map
3. Added extra tests to ensure fix-ups work using the new map
4. Added fix-ups for type aliases in variable and function declarations as part
of adding the new tests
http://reviews.llvm.org/D21020
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
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.IgnoreFailedSplit, value: 0} \
// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \
// RUN: -I%S/Inputs/readability-identifier-naming \
@@ -305,6 +306,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;
+}
+
static void static_Function() {
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for function 'static_Function'
// CHECK-FIXES: {{^}}static void staticFunction() {{{$}}
@@ -318,5 +325,9 @@
// 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()
+
}
}
Index: clang-tidy/readability/IdentifierNamingCheck.h
===================================================================
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -36,6 +36,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 +65,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 +82,17 @@
NamingCheckFailure() : ShouldFix(true) {}
};
- typedef llvm::DenseMap<const NamedDecl *, NamingCheckFailure>
+
+ struct NamingCheckId : std::pair<SourceLocation, std::string> {
+ typedef std::pair<SourceLocation, std::string> Parent;
+ using Parent::Parent;
+ };
+
+ typedef llvm::DenseMap<NamingCheckId, NamingCheckFailure>
NamingCheckFailureMap;
+ void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok);
+
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 {
@@ -65,6 +107,7 @@
m(ValueTemplateParameter) \
m(TemplateTemplateParameter) \
m(TemplateParameter) \
+ m(MacroDefinition)
enum StyleKind {
#define ENUMERATE(v) SK_ ## v,
@@ -83,6 +126,32 @@
#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 {
+ (void)MD;
+ SourceManager &SM(PP->getSourceManager());
+ if (!SM.isInMainFile(MacroNameTok.getLocation()))
+ return;
+ if (SM.isInSystemHeader(MacroNameTok.getLocation()))
+ return;
+ Check->checkMacro(SM, MacroNameTok);
+ }
+
+private:
+ Preprocessor *PP;
+ IdentifierNamingCheck *Check;
+};
+} // namespace
+
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {
@@ -145,6 +214,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[] = {
@@ -502,8 +577,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;
@@ -518,14 +593,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;
}
@@ -541,8 +624,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;
}
@@ -559,8 +641,7 @@
}
if (Decl) {
- addUsage(NamingCheckFailures, Decl, Loc->getSourceRange(),
- Result.SourceManager);
+ addUsage(NamingCheckFailures, Decl, Loc->getSourceRange());
return;
}
@@ -570,50 +651,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))
@@ -640,29 +745,61 @@
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) {
+ 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(MacroNameTok.getLocation(), Name);
+ NamingCheckFailure &Failure = NamingCheckFailures[ID];
+ SourceRange Range =
+ SourceRange(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
+
+ Failure.Fixup = std::move(Fixup);
+ Failure.KindName = std::move(KindName);
+ addUsage(NamingCheckFailures, ID, Range);
+ }
+}
+
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
