[clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros

[berenm]

Summary

This fixes https://llvm.org/bugs/show_bug.cgi?id=24835.

The patch might be slightly different after http://reviews.llvm.org/D13079 and http://reviews.llvm.org/D13081, but the idea is the same.

[berenm]

Even better with the corresponding unit test fix.

[berenm]

Update the diff with more context, thanks to arcanist.

[berenm]

This will also disable all warnings for declaration / usages outside of the main file.

It might be better to disable the warnings and fixes whenever a macro is involved (in the declaration or any usage), but at least keep the warning across files, even if we don't offer fixes. I think the most common use-case will be to have some declaration in a header file and usages in multiple source files, and in this case warnings are interesting.

I'm assuming here that "main file" in clang tooling framework means the .cpp file from which the ast is built.

[alexfh]

Looks good. Should I submit this?

[berenm]

Actually I believe it's better fixed by D13081.

clang-tidy/readability/IdentifierNamingCheck.cpp

 void IdentifierNamingCheck::onEndOfTranslationUnit() {
   for (const auto &Pair : NamingCheckFailures) {
     const NamedDecl &Decl = *Pair.first;
     const NamingCheckFailure &Failure = Pair.second;
 
     SourceRange DeclRange =
         DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
             .getSourceRange();
+
+    if (Failure.ShouldFix) {
       auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
                   << Failure.KindName << Decl.getName();
 
-    if (Failure.ShouldFix) {
       Diag << FixItHint::CreateReplacement(
           CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
 
       for (const auto &Range : Failure.Usages) {
         Diag << FixItHint::CreateReplacement(
             CharSourceRange::getTokenRange(Range), Failure.Fixup);
       }
     }
   }
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang

test/clang-tidy/readability-identifier-naming.cpp

 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
 inline namespace InlineNamespace {
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for inline namespace 'InlineNamespace'
 // CHECK-FIXES: {{^}}inline namespace inline_namespace {{{$}}
 
 #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'
 // CHECK-FIXES: {{^}}enum EMyEnumeration {{{$}}
     MyConstant = 1,
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for enum constant 'MyConstant'
 // CHECK-FIXES: {{^}}    MY_CONSTANT = 1,{{$}}
     your_CONST = 1,