berenm updated this revision to Diff 35672.
berenm added a comment.
- Do not check for identifier names from system headers
- Check for SourceLocation validity before modifying them
http://reviews.llvm.org/D13081
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
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
@@ -64,12 +64,11 @@
// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing
-// FIXME: There should be more test cases for checking that references to class
-// FIXME: name, declaration contexts, forward declarations, etc, are correctly
-// FIXME: checked and renamed
-
// clang-format off
+#include <iostream>
+// Expect NO warnings or errors from system headers, it shouldn't even be checked
+
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
@@ -104,6 +103,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}} CMyClass();{{$}}
+ ~my_class();
+// CHECK-FIXES: {{^}} ~CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -137,15 +139,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+// CHECK-FIXES: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+
+class my_derived_class : public virtual my_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_derived_class'
+// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
+
+class CMyWellNamedClass {};
+// No warning expected as this class is well named.
+
+template<typename T>
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template<typename t_t>{{$}}
+class my_templated_class : CMyWellNamedClass {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_templated_class'
+// CHECK-FIXES: {{^}}class CMyTemplatedClass : CMyWellNamedClass {};{{$}}
+
+template<typename T>
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template<typename t_t>{{$}}
+class my_other_templated_class : my_templated_class<my_class>, private my_derived_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_other_templated_class'
+// CHECK-FIXES: {{^}}class CMyOtherTemplatedClass : CMyTemplatedClass<CMyClass>, private CMyDerivedClass {};{{$}}
+
+template<typename t_t>
+using MYSUPER_TPL = my_other_templated_class<::FOO_NS::my_class>;
+// CHECK-FIXES: {{^}}using MYSUPER_TPL = CMyOtherTemplatedClass<::foo_ns::CMyClass>;{{$}}
const int global_Constant = 6;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global constant 'global_Constant'
@@ -186,7 +209,7 @@
void Global_Fun(TYPE_parameters... PARAMETER_PACK) {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global function 'Global_Fun'
// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: invalid case style for parameter pack 'PARAMETER_PACK'
-// CHECK-FIXES: {{^}}void GlobalFun(TYPE_parameters... parameterPack) {{{$}}
+// CHECK-FIXES: {{^}}void GlobalFun(typeParameters_t... parameterPack) {{{$}}
global_function(1, 2);
// CHECK-FIXES: {{^}} GlobalFunction(1, 2);{{$}}
FOO_bar = Global_variable;
@@ -214,6 +237,8 @@
void non_Virtual_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for private method 'non_Virtual_METHOD'
// CHECK-FIXES: {{^}} void __non_Virtual_METHOD() {}{{$}}
+
+public:
static void CLASS_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for class method 'CLASS_METHOD'
// CHECK-FIXES: {{^}} static void classMethod() {}{{$}}
@@ -244,23 +269,28 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for struct 'THIS___Structure'
// CHECK-FIXES: {{^}}struct this_structure {{{$}}
THIS___Structure();
-// FIXME: There should be a fixup for the constructor as well
-// FIXME: {{^}} this_structure();{{$}}
+// CHECK-FIXES: {{^}} this_structure();{{$}}
union __MyUnion_is_wonderful__ {};
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for union '__MyUnion_is_wonderful__'
// CHECK-FIXES: {{^}} union UMyUnionIsWonderful {};{{$}}
};
typedef THIS___Structure struct_type;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for typedef 'struct_type'
-// CHECK-FIXES: {{^}}typedef THIS___Structure struct_type_t;{{$}}
-// FIXME: The fixup should reflect structure name fixups as well:
-// FIXME: {{^}}typedef this_structure struct_type_t;{{$}}
+// CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
static void static_Function() {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for function 'static_Function'
// CHECK-FIXES: {{^}}static void staticFunction() {{{$}}
+
+ ::FOO_NS::InlineNamespace::abstract_class::CLASS_METHOD();
+// CHECK-FIXES: {{^}} ::foo_ns::inline_namespace::AAbstractClass::classMethod();{{$}}
+ ::FOO_NS::InlineNamespace::static_Function();
+// CHECK-FIXES: {{^}} ::foo_ns::inline_namespace::staticFunction();{{$}}
+
+ using ::FOO_NS::InlineNamespace::CE_function;
+// CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}}
}
}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -136,13 +136,24 @@
}
void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
- // FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking
- // and replacement. There is a lot of missing cases, such as references to a
- // class name (as in 'const int CMyClass::kClassConstant = 4;'), to an
- // enclosing context (namespace, class, etc).
-
- Finder->addMatcher(namedDecl().bind("decl"), this);
- Finder->addMatcher(declRefExpr().bind("declref"), this);
+ Finder->addMatcher(
+ namedDecl(unless(isExpansionInSystemHeader())).bind("decl"), this);
+ Finder->addMatcher(
+ usingDecl(unless(isExpansionInSystemHeader())).bind("using"), this);
+ Finder->addMatcher(
+ declRefExpr(unless(isExpansionInSystemHeader())).bind("declRef"), this);
+ Finder->addMatcher(
+ cxxConstructorDecl(unless(isExpansionInSystemHeader())).bind("classRef"),
+ this);
+ Finder->addMatcher(
+ cxxDestructorDecl(unless(isExpansionInSystemHeader())).bind("classRef"),
+ this);
+ Finder->addMatcher(
+ typeLoc(unless(isExpansionInSystemHeader())).bind("typeLoc"), this);
+ Finder->addMatcher(loc(nestedNameSpecifier(specifiesNamespace(
+ unless(isExpansionInSystemHeader()))))
+ .bind("nestedNameLoc"),
+ this);
}
static bool matchesStyle(StringRef Name,
@@ -508,14 +519,122 @@
if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
return;
+ if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
+ return;
+
Failure.ShouldFix = Failure.ShouldFix && SM->isInMainFile(Range.getBegin()) &&
SM->isInMainFile(Range.getEnd()) &&
!Range.getBegin().isMacroID() &&
!Range.getEnd().isMacroID();
}
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
- if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declref")) {
+ if (const auto *Decl =
+ Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) {
+ if (Decl->isImplicit())
+ return;
+
+ return addUsage(NamingCheckFailures, Decl->getParent(),
+ Decl->getNameInfo().getSourceRange(), Result.SourceManager);
+ }
+
+ if (const auto *Decl =
+ Result.Nodes.getNodeAs<CXXDestructorDecl>("classRef")) {
+ if (Decl->isImplicit())
+ return;
+
+ SourceRange Range = Decl->getNameInfo().getSourceRange();
+
+ if (Range.getBegin().isInvalid())
+ return;
+ Range.setBegin(Range.getBegin().getLocWithOffset(1));
+
+ return addUsage(NamingCheckFailures, Decl->getParent(), Range,
+ Result.SourceManager);
+ }
+
+ if (const auto *Loc = Result.Nodes.getNodeAs<TypeLoc>("typeLoc")) {
+ if (const auto &Ref = Loc->getAs<TagTypeLoc>()) {
+ return addUsage(NamingCheckFailures, Ref.getDecl(), Loc->getSourceRange(),
+ Result.SourceManager);
+ }
+
+ if (const auto &Ref = Loc->getAs<InjectedClassNameTypeLoc>()) {
+ return addUsage(NamingCheckFailures, Ref.getDecl(), Loc->getSourceRange(),
+ Result.SourceManager);
+ }
+
+ if (const auto &Ref = Loc->getAs<UnresolvedUsingTypeLoc>()) {
+ return addUsage(NamingCheckFailures, Ref.getDecl(), Loc->getSourceRange(),
+ Result.SourceManager);
+ }
+
+ if (const auto &Ref = Loc->getAs<TemplateTypeParmTypeLoc>()) {
+ return addUsage(NamingCheckFailures, Ref.getDecl(), Loc->getSourceRange(),
+ Result.SourceManager);
+ }
+
+ if (const auto &Ref = Loc->getAs<TemplateSpecializationTypeLoc>()) {
+ const auto *Decl =
+ Ref.getTypePtr()->getTemplateName().getAsTemplateDecl();
+
+ if (Ref.getLAngleLoc().isInvalid())
+ return;
+ SourceRange Range = SourceRange(Ref.getTemplateNameLoc(),
+ Ref.getLAngleLoc().getLocWithOffset(-1));
+
+ if (const auto *ClassDecl = dyn_cast<ClassTemplateDecl>(Decl)) {
+ return addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(),
+ Range, Result.SourceManager);
+ }
+
+ if (const auto *ClassDecl = dyn_cast<FunctionTemplateDecl>(Decl)) {
+ return addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(),
+ Range, Result.SourceManager);
+ }
+
+ if (const auto *ClassDecl = dyn_cast<TypeAliasTemplateDecl>(Decl)) {
+ return addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(),
+ Range, Result.SourceManager);
+ }
+
+ if (const auto *ClassDecl = dyn_cast<VarTemplateDecl>(Decl)) {
+ return addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(),
+ Range, Result.SourceManager);
+ }
+ }
+
+ if (const auto &Ref =
+ Loc->getAs<DependentTemplateSpecializationTypeLoc>()) {
+ return addUsage(NamingCheckFailures, Ref.getTypePtr()->getAsTagDecl(),
+ Loc->getSourceRange(), Result.SourceManager);
+ }
+ }
+
+ if (const auto *Loc =
+ Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) {
+ if (NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) {
+ if (NamespaceDecl *Decl = Spec->getAsNamespace()) {
+ SourceRange Range = Loc->getLocalSourceRange();
+
+ if (Range.getEnd().isInvalid())
+ return;
+ Range.setEnd(Range.getEnd().getLocWithOffset(-1));
+
+ return addUsage(NamingCheckFailures, Decl, Range, Result.SourceManager);
+ }
+ }
+ }
+
+ if (const auto *Decl = Result.Nodes.getNodeAs<UsingDecl>("using")) {
+ for (const auto &Shadow : Decl->shadows()) {
+ addUsage(NamingCheckFailures, Shadow->getTargetDecl(),
+ Decl->getNameInfo().getSourceRange(), Result.SourceManager);
+ }
+ return;
+ }
+
+ if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) {
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
return addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
Result.SourceManager);
@@ -525,6 +644,11 @@
if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit())
return;
+ // Ignore ClassTemplateSpecializationDecl which are creating duplicate
+ // replacements with CXXRecordDecl
+ if (isa<ClassTemplateSpecializationDecl>(Decl))
+ return;
+
StyleKind SK = findStyleKind(Decl, NamingStyles);
if (SK == SK_Invalid)
return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
