From 6ae0f5f3e1d465e6a663a50f2cc077671bc6d097 Mon Sep 17 00:00:00 2001 From: Nathan James Date: Mon, 22 Jun 2020 18:26:17 +0100 Subject: [PATCH] [clang-tidy] RenamerClangTidy wont emit fixes in scratch space Prevent fixes being displayed if usages are found in the scratch buffer. See [[ https://bugs.llvm.org/show_bug.cgi?id=46219 | 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. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D82162 --- .../clang-tidy/utils/RenamerClangTidyCheck.cpp | 39 +++++++++++++--------- .../checkers/readability-identifier-naming.cpp | 16 +++++++++ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp index e90ab17..040378d 100644 --- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp @@ -156,14 +156,17 @@ void RenamerClangTidyCheck::addUsage( // 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, @@ -248,13 +251,15 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Decl = Result.Nodes.getNodeAs("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. } @@ -271,7 +276,7 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { // 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; } @@ -289,7 +294,7 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { // further TypeLocs handled below if (Decl) { - addUsage(Decl, Loc->getSourceRange()); + addUsage(Decl, Loc->getSourceRange(), Result.SourceManager); return; } @@ -300,7 +305,7 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc()); if (const auto *ClassDecl = dyn_cast(Decl)) { if (const NamedDecl *TemplDecl = ClassDecl->getTemplatedDecl()) - addUsage(TemplDecl, Range); + addUsage(TemplDecl, Range, Result.SourceManager); return; } } @@ -308,7 +313,7 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { if (const auto &Ref = Loc->getAs()) { if (const TagDecl *Decl = Ref.getTypePtr()->getAsTagDecl()) - addUsage(Decl, Loc->getSourceRange()); + addUsage(Decl, Loc->getSourceRange(), Result.SourceManager); return; } } @@ -317,7 +322,7 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { Result.Nodes.getNodeAs("nestedNameLoc")) { if (const NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) { if (const NamespaceDecl *Decl = Spec->getAsNamespace()) { - addUsage(Decl, Loc->getLocalSourceRange()); + addUsage(Decl, Loc->getLocalSourceRange(), Result.SourceManager); return; } } @@ -325,7 +330,8 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Decl = Result.Nodes.getNodeAs("using")) { for (const auto *Shadow : Decl->shadows()) - addUsage(Shadow->getTargetDecl(), Decl->getNameInfo().getSourceRange()); + addUsage(Shadow->getTargetDecl(), Decl->getNameInfo().getSourceRange(), + Result.SourceManager); return; } @@ -371,14 +377,14 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { // Fix using namespace declarations. if (const auto *UsingNS = dyn_cast(Decl)) addUsage(UsingNS->getNominatedNamespaceAsWritten(), - UsingNS->getIdentLocation()); + UsingNS->getIdentLocation(), Result.SourceManager); if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit()) return; const auto *Canonical = cast(Decl->getCanonicalDecl()); if (Canonical != Decl) { - addUsage(Canonical, Decl->getLocation()); + addUsage(Canonical, Decl->getLocation(), Result.SourceManager); return; } @@ -386,7 +392,8 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Value = Result.Nodes.getNodeAs("decl")) { if (const Type *TypePtr = Value->getType().getTypePtrOrNull()) { if (const auto *Typedef = TypePtr->getAs()) - addUsage(Typedef->getDecl(), Value->getSourceRange()); + addUsage(Typedef->getDecl(), Value->getSourceRange(), + Result.SourceManager); } } @@ -394,11 +401,13 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Value = Result.Nodes.getNodeAs("decl")) { if (const auto *Typedef = Value->getReturnType().getTypePtr()->getAs()) - 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()) - addUsage(Typedef->getDecl(), Value->getSourceRange()); + addUsage(Typedef->getDecl(), Value->getSourceRange(), + Result.SourceManager); } } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp index 3a3bfec..24c1c42 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp @@ -562,3 +562,19 @@ void ReferenceBadNamedFunction() { } } // 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 -- 2.7.4