forked from OSchip/llvm-project
[clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
This diff requires http://reviews.llvm.org/D13079 to be applied first. I wasn't sure about how to make patch series in Phabricator, and I wanted to keep the two separate for clarity. It looks like that most cases can be supported with this patch. I'm not totally sure about the actual coverage though. I think that the matchers are very generic, but I'm still not totally fluent with the AST. Patch by Beren Minor! Differential revision: http://reviews.llvm.org/D13081 llvm-svn: 248996
This commit is contained in:
Alexander Kornienko
parent
746ffd6980
commit
30c423b1e3
|
|
@ -136,13 +136,13 @@ void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
|
|||
}
|
||||
|
||||
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(usingDecl().bind("using"), this);
|
||||
Finder->addMatcher(declRefExpr().bind("declRef"), this);
|
||||
Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
|
||||
Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);
|
||||
Finder->addMatcher(typeLoc().bind("typeLoc"), this);
|
||||
Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
|
||||
}
|
||||
|
||||
static bool matchesStyle(StringRef Name,
|
||||
|
|
@ -504,27 +504,121 @@ static StyleKind findStyleKind(
|
|||
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
|
||||
const NamedDecl *Decl, SourceRange Range,
|
||||
const SourceManager *SM) {
|
||||
// Do nothin if the provided range is invalid
|
||||
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
|
||||
return;
|
||||
|
||||
// Try to insert the identifier location in the Usages map, and bail out if it
|
||||
// is already in there
|
||||
auto &Failure = Failures[Decl];
|
||||
if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
|
||||
return;
|
||||
|
||||
Failure.ShouldFix = Failure.ShouldFix && SM->isInMainFile(Range.getBegin()) &&
|
||||
SM->isInMainFile(Range.getEnd()) &&
|
||||
!Range.getBegin().isMacroID() &&
|
||||
Failure.ShouldFix = Failure.ShouldFix && !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;
|
||||
|
||||
addUsage(NamingCheckFailures, Decl->getParent(),
|
||||
Decl->getNameInfo().getSourceRange(), Result.SourceManager);
|
||||
return;
|
||||
}
|
||||
|
||||
if (const auto *Decl =
|
||||
Result.Nodes.getNodeAs<CXXDestructorDecl>("classRef")) {
|
||||
if (Decl->isImplicit())
|
||||
return;
|
||||
|
||||
SourceRange Range = Decl->getNameInfo().getSourceRange();
|
||||
if (Range.getBegin().isInvalid())
|
||||
return;
|
||||
// The first token that will be found is the ~ (or the equivalent trigraph),
|
||||
// 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);
|
||||
return;
|
||||
}
|
||||
|
||||
if (const auto *Loc = Result.Nodes.getNodeAs<TypeLoc>("typeLoc")) {
|
||||
NamedDecl *Decl = nullptr;
|
||||
if (const auto &Ref = Loc->getAs<TagTypeLoc>()) {
|
||||
Decl = Ref.getDecl();
|
||||
} else if (const auto &Ref = Loc->getAs<InjectedClassNameTypeLoc>()) {
|
||||
Decl = Ref.getDecl();
|
||||
} else if (const auto &Ref = Loc->getAs<UnresolvedUsingTypeLoc>()) {
|
||||
Decl = Ref.getDecl();
|
||||
} else if (const auto &Ref = Loc->getAs<TemplateTypeParmTypeLoc>()) {
|
||||
Decl = Ref.getDecl();
|
||||
}
|
||||
|
||||
if (Decl) {
|
||||
addUsage(NamingCheckFailures, Decl, Loc->getSourceRange(),
|
||||
Result.SourceManager);
|
||||
return;
|
||||
}
|
||||
|
||||
if (const auto &Ref = Loc->getAs<TemplateSpecializationTypeLoc>()) {
|
||||
const auto *Decl =
|
||||
Ref.getTypePtr()->getTemplateName().getAsTemplateDecl();
|
||||
|
||||
SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc());
|
||||
if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) {
|
||||
addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range,
|
||||
Result.SourceManager);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (const auto &Ref =
|
||||
Loc->getAs<DependentTemplateSpecializationTypeLoc>()) {
|
||||
addUsage(NamingCheckFailures, Ref.getTypePtr()->getAsTagDecl(),
|
||||
Loc->getSourceRange(), Result.SourceManager);
|
||||
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);
|
||||
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);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) {
|
||||
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
|
||||
return addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
|
||||
Result.SourceManager);
|
||||
addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
|
||||
Result.SourceManager);
|
||||
return;
|
||||
}
|
||||
|
||||
if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {
|
||||
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;
|
||||
|
|
@ -566,13 +660,21 @@ void IdentifierNamingCheck::onEndOfTranslationUnit() {
|
|||
if (Failure.KindName.empty())
|
||||
continue;
|
||||
|
||||
auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
|
||||
<< Failure.KindName << Decl.getName();
|
||||
if (Failure.ShouldFix) {
|
||||
auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
|
||||
<< Failure.KindName << Decl.getName();
|
||||
|
||||
for (const auto &Loc : Failure.RawUsageLocs) {
|
||||
// We assume that the identifier name is made of one token only. This is
|
||||
// always the case as we ignore usages in macros that could build
|
||||
// identifier names by combining multiple tokens.
|
||||
//
|
||||
// For destructors, we alread take care of it by remembering the
|
||||
// location of the start of the identifier and not the start of the
|
||||
// tilde.
|
||||
//
|
||||
// Other multi-token identifiers, such as operators are not checked at
|
||||
// all.
|
||||
Diag << FixItHint::CreateReplacement(
|
||||
SourceRange(SourceLocation::getFromRawEncoding(Loc)),
|
||||
Failure.Fixup);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,17 @@
|
|||
#ifndef SYSTEM_HEADER_H
|
||||
#define SYSTEM_HEADER_H
|
||||
|
||||
#define SYSTEM_MACRO(m) int m = 0
|
||||
|
||||
namespace SYSTEM_NS {
|
||||
class structure {
|
||||
int member;
|
||||
};
|
||||
|
||||
float global;
|
||||
|
||||
void function() {
|
||||
}
|
||||
}
|
||||
|
||||
#endif // SYSTEM_HEADER_H
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
#ifndef USER_HEADER_H
|
||||
#define USER_HEADER_H
|
||||
|
||||
#define USER_MACRO(m) int m = 0
|
||||
|
||||
namespace USER_NS {
|
||||
class object {
|
||||
int member;
|
||||
};
|
||||
|
||||
float global;
|
||||
|
||||
void function() {
|
||||
}
|
||||
}
|
||||
|
||||
#endif // USER_HEADER_H
|
||||
|
|
@ -62,14 +62,17 @@
|
|||
// RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
|
||||
// RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
|
||||
// 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
|
||||
// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \
|
||||
// RUN: -I%S/Inputs/readability-identifier-naming \
|
||||
// RUN: -isystem %S/Inputs/readability-identifier-naming/system
|
||||
|
||||
// clang-format off
|
||||
|
||||
#include <system-header.h>
|
||||
#include "user-header.h"
|
||||
// NO warnings or fixes expected from declarations within header files without
|
||||
// the -header-filter= option
|
||||
|
||||
namespace FOO_NS {
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
|
||||
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
|
||||
|
|
@ -77,10 +80,26 @@ inline namespace InlineNamespace {
|
|||
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for inline namespace 'InlineNamespace'
|
||||
// CHECK-FIXES: {{^}}inline namespace inline_namespace {{{$}}
|
||||
|
||||
SYSTEM_NS::structure g_s1;
|
||||
// NO warnings or fixes expected as SYSTEM_NS and structure are declared in a header file
|
||||
|
||||
USER_NS::object g_s2;
|
||||
// NO warnings or fixes expected as USER_NS and object are declared in a header file
|
||||
|
||||
SYSTEM_MACRO(var1);
|
||||
// NO warnings or fixes expected as var1 is from macro expansion
|
||||
|
||||
USER_MACRO(var2);
|
||||
// NO warnings or fixes expected as var2 is declared in a macro expansion
|
||||
|
||||
int global;
|
||||
#define USE_IN_MACRO(m) auto use_##m = m
|
||||
USE_IN_MACRO(global);
|
||||
// NO warnings or fixes expected as global is used in a macro expansion
|
||||
|
||||
#define BLA int FOO_bar
|
||||
BLA;
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global variable 'FOO_bar'
|
||||
// NO fix expected as FOO_bar is from macro expansion
|
||||
// NO warnings or fixes expected as FOO_bar is from macro expansion
|
||||
|
||||
enum my_enumeration {
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum 'my_enumeration'
|
||||
|
|
@ -104,6 +123,13 @@ class my_class {
|
|||
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
|
||||
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
|
||||
my_class();
|
||||
// CHECK-FIXES: {{^}} CMyClass();{{$}}
|
||||
|
||||
~
|
||||
my_class();
|
||||
// (space in destructor token test, we could check trigraph but they will be deprecated)
|
||||
// CHECK-FIXES: {{^}} ~{{$}}
|
||||
// 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 +163,36 @@ public:
|
|||
|
||||
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 +233,7 @@ template<typename ... TYPE_parameters>
|
|||
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;
|
||||
|
|
@ -208,12 +255,15 @@ class abstract_class {
|
|||
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for abstract class 'abstract_class'
|
||||
// CHECK-FIXES: {{^}}class AAbstractClass {{{$}}
|
||||
virtual ~abstract_class() = 0;
|
||||
// CHECK-FIXES: {{^}} virtual ~AAbstractClass() = 0;{{$}}
|
||||
virtual void VIRTUAL_METHOD();
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for virtual method 'VIRTUAL_METHOD'
|
||||
// CHECK-FIXES: {{^}} virtual void v_VIRTUAL_METHOD();{{$}}
|
||||
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,8 +294,7 @@ struct THIS___Structure {
|
|||
// 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__'
|
||||
|
|
@ -254,13 +303,19 @@ struct THIS___Structure {
|
|||
|
||||
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;{{$}}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue