[clang-tidy] RenamerClangTidy wont emit fixes in scratch space

[njames93]

Summary

Prevent fixes being displayed if usages are found in the scratch buffer.
See Fix-It hints are being generated in the ScratchBuffer.
It may be wise down the line to put in a general fix in clang-tidy to prevent ScratchBuffer replacements being applied, but for now this will help.

[aaron.ballman]

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

570

I may be misunderstanding the comment here. I can understand not wanting to generate fixits into the scratch buffer (that would not do good things), but I think we still want a warning were it not for the macro, right? Just verifying that this is not changing warning behaviors, only fix-it behaviors.

[njames93]

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

570

You're absolutely right, however right now this fix just marks it as being InsideMacro. The reason diagnostics aren't emitted is becuase somewhere in this checks life someone decided to not emit warnings for declarations with usages inside macros. If that behaviour is to be changed a separate patch would be required.

[aaron.ballman]

LGTM!

clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp

if (SourceMgr)
     FixLocation = SourceMgr->getSpellingLoc(FixLocation);
   if (FixLocation.isInvalid())
     return;
 
   // Try to insert the identifier location in the Usages map, and bail out if it
   // is already in there
   RenamerClangTidyCheck::NamingCheckFailure &Failure =
       NamingCheckFailures[Decl];
-  if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
-    return;
 
   if (!Failure.ShouldFix())
     return;
 
+  if (SourceMgr && SourceMgr->isWrittenInScratchSpace(FixLocation))
+    Failure.FixStatus = RenamerClangTidyCheck::ShouldFixStatus::InsideMacro;
+
   if (!utils::rangeCanBeFixed(Range, SourceMgr))
     Failure.FixStatus = RenamerClangTidyCheck::ShouldFixStatus::InsideMacro;
+
+  Failure.RawUsageLocs.insert(FixLocation.getRawEncoding());
 }
 
 void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range,
                                      SourceManager *SourceMgr) {
   Decl = cast<NamedDecl>(Decl->getCanonicalDecl());
   return addUsage(RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),
                                                        Decl->getNameAsString()),
                   Range, SourceMgr);
NameLookup findDeclInBases(const CXXRecordDecl &Parent, StringRef DeclName,
   }
   return NameLookup(Found); // If nullptr, decl wasnt found.
 }
 
 void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Decl =
           Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) {
 
-    addUsage(Decl->getParent(), Decl->getNameInfo().getSourceRange());
+    addUsage(Decl->getParent(), Decl->getNameInfo().getSourceRange(),
+             Result.SourceManager);
 
     for (const auto *Init : Decl->inits()) {
       if (!Init->isWritten() || Init->isInClassMemberInitializer())
         continue;
       if (const FieldDecl *FD = Init->getAnyMember())
-        addUsage(FD, SourceRange(Init->getMemberLocation()));
+        addUsage(FD, SourceRange(Init->getMemberLocation()),
+                 Result.SourceManager);
       // Note: delegating constructors and base class initializers are handled
       // via the "typeLoc" matcher.
     }
     return;
   }
 
   if (const auto *Decl =
           Result.Nodes.getNodeAs<CXXDestructorDecl>("classRef")) {
 
     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(Decl->getParent(), Range);
+    addUsage(Decl->getParent(), Range, Result.SourceManager);
     return;
   }
 
   if (const auto *Loc = Result.Nodes.getNodeAs<TypeLoc>("typeLoc")) {
     UnqualTypeLoc Unqual = Loc->getUnqualifiedLoc();
     NamedDecl *Decl = nullptr;
     if (const auto &Ref = Unqual.getAs<TagTypeLoc>())
       Decl = Ref.getDecl();
     else if (const auto &Ref = Unqual.getAs<InjectedClassNameTypeLoc>())
       Decl = Ref.getDecl();
     else if (const auto &Ref = Unqual.getAs<UnresolvedUsingTypeLoc>())
       Decl = Ref.getDecl();
     else if (const auto &Ref = Unqual.getAs<TemplateTypeParmTypeLoc>())
       Decl = Ref.getDecl();
     // further TypeLocs handled below
 
     if (Decl) {
-      addUsage(Decl, Loc->getSourceRange());
+      addUsage(Decl, Loc->getSourceRange(), Result.SourceManager);
       return;
     }
 
     if (const auto &Ref = Loc->getAs<TemplateSpecializationTypeLoc>()) {
       const TemplateDecl *Decl =
           Ref.getTypePtr()->getTemplateName().getAsTemplateDecl();
 
       SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc());
       if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) {
         if (const NamedDecl *TemplDecl = ClassDecl->getTemplatedDecl())
-          addUsage(TemplDecl, Range);
+          addUsage(TemplDecl, Range, Result.SourceManager);
         return;
       }
     }
 
     if (const auto &Ref =
             Loc->getAs<DependentTemplateSpecializationTypeLoc>()) {
       if (const TagDecl *Decl = Ref.getTypePtr()->getAsTagDecl())
-        addUsage(Decl, Loc->getSourceRange());
+        addUsage(Decl, Loc->getSourceRange(), Result.SourceManager);
       return;
     }
   }
 
   if (const auto *Loc =
           Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) {
     if (const NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) {
       if (const NamespaceDecl *Decl = Spec->getAsNamespace()) {
-        addUsage(Decl, Loc->getLocalSourceRange());
+        addUsage(Decl, Loc->getLocalSourceRange(), Result.SourceManager);
         return;
       }
     }
   }
 
   if (const auto *Decl = Result.Nodes.getNodeAs<UsingDecl>("using")) {
     for (const auto *Shadow : Decl->shadows())
-      addUsage(Shadow->getTargetDecl(), Decl->getNameInfo().getSourceRange());
+      addUsage(Shadow->getTargetDecl(), Decl->getNameInfo().getSourceRange(),
+               Result.SourceManager);
     return;
   }
 
   if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) {
     SourceRange Range = DeclRef->getNameInfo().getSourceRange();
     addUsage(DeclRef->getDecl(), Range, Result.SourceManager);
     return;
   }
if (const auto *DepMemberRef =
     }
     return;
   }
 
   if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {
     // Fix using namespace declarations.
     if (const auto *UsingNS = dyn_cast<UsingDirectiveDecl>(Decl))
       addUsage(UsingNS->getNominatedNamespaceAsWritten(),
-               UsingNS->getIdentLocation());
+               UsingNS->getIdentLocation(), Result.SourceManager);
 
     if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit())
       return;
 
     const auto *Canonical = cast<NamedDecl>(Decl->getCanonicalDecl());
     if (Canonical != Decl) {
-      addUsage(Canonical, Decl->getLocation());
+      addUsage(Canonical, Decl->getLocation(), Result.SourceManager);
       return;
     }
 
     // Fix type aliases in value declarations.
     if (const auto *Value = Result.Nodes.getNodeAs<ValueDecl>("decl")) {
       if (const Type *TypePtr = Value->getType().getTypePtrOrNull()) {
         if (const auto *Typedef = TypePtr->getAs<TypedefType>())
-          addUsage(Typedef->getDecl(), Value->getSourceRange());
+          addUsage(Typedef->getDecl(), Value->getSourceRange(),
+                   Result.SourceManager);
       }
     }
 
     // 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(Typedef->getDecl(), Value->getSourceRange());
+        addUsage(Typedef->getDecl(), Value->getSourceRange(),
+                 Result.SourceManager);
       for (const ParmVarDecl *Param : Value->parameters()) {
         if (const TypedefType *Typedef =
                 Param->getType().getTypePtr()->getAs<TypedefType>())
-          addUsage(Typedef->getDecl(), Value->getSourceRange());
+          addUsage(Typedef->getDecl(), Value->getSourceRange(),
+                   Result.SourceManager);
       }
     }
 
     // Ignore ClassTemplateSpecializationDecl which are creating duplicate
     // replacements with CXXRecordDecl.
     if (isa<ClassTemplateSpecializationDecl>(Decl))
       return;
 

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

void ReferenceBadNamedFunction() {
   auto l_Ptr = badNamedFunction;
   // CHECK-FIXES: {{^}}  auto l_Ptr = BadNamedFunction;
   l_Ptr();
   badNamedFunction();
   // CHECK-FIXES: {{^}}  BadNamedFunction();
 }
 
 } // namespace redecls
+
+namespace scratchspace {
+#define DUP(Tok) Tok
+#define M1(Tok) DUP(badName##Tok())
+
+// We don't want a warning here as the call to this in Foo is in a scratch
+// buffer so its fix-it wouldn't be applied, resulting in invalid code.
+void badNameWarn();
+
+void Foo() {
+  M1(Warn);
+}
+
+#undef M1
+#undef DUP
+} // namespace scratchspace