[clang-tools-extra] r249402 - Create interfaces and tests for the overlapping replacements fix in clang-tidy.
Angel Garcia Gomez via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 6 06:52:51 PDT 2015
Author: angelgarcia
Date: Tue Oct 6 08:52:51 2015
New Revision: 249402
URL: http://llvm.org/viewvc/llvm-project?rev=249402&view=rev
Log:
Create interfaces and tests for the overlapping replacements fix in clang-tidy.
Summary: No changes in clang-tidy yet.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D13469
Added:
clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp
Modified:
clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp
Modified: clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt?rev=249402&r1=249401&r2=249402&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt Tue Oct 6 08:52:51 2015
@@ -13,6 +13,7 @@ add_extra_unittest(ClangTidyTests
GoogleModuleTest.cpp
LLVMModuleTest.cpp
MiscModuleTest.cpp
+ OverlappingReplacementsTest.cpp
ReadabilityModuleTest.cpp)
target_link_libraries(ClangTidyTests
Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=249402&r1=249401&r2=249402&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Oct 6 08:52:51 2015
@@ -18,6 +18,7 @@
#include "clang/Tooling/Refactoring.h"
#include "clang/Tooling/Tooling.h"
#include <map>
+#include <memory>
namespace clang {
namespace tidy {
@@ -25,9 +26,10 @@ namespace test {
class TestClangTidyAction : public ASTFrontendAction {
public:
- TestClangTidyAction(ClangTidyCheck &Check, ast_matchers::MatchFinder &Finder,
+ TestClangTidyAction(SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Checks,
+ ast_matchers::MatchFinder &Finder,
ClangTidyContext &Context)
- : Check(Check), Finder(Finder), Context(Context) {}
+ : Checks(Checks), Finder(Finder), Context(Context) {}
private:
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
@@ -36,17 +38,37 @@ private:
Context.setCurrentFile(File);
Context.setASTContext(&Compiler.getASTContext());
- Check.registerMatchers(&Finder);
- Check.registerPPCallbacks(Compiler);
+ for (auto &Check : Checks) {
+ Check->registerMatchers(&Finder);
+ Check->registerPPCallbacks(Compiler);
+ }
return Finder.newASTConsumer();
}
- ClangTidyCheck &Check;
+ SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Checks;
ast_matchers::MatchFinder &Finder;
ClangTidyContext &Context;
};
-template <typename T>
+template <typename Check, typename... Checks> struct CheckFactory {
+ static void
+ createChecks(ClangTidyContext *Context,
+ SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Result) {
+ CheckFactory<Check>::createChecks(Context, Result);
+ CheckFactory<Checks...>::createChecks(Context, Result);
+ }
+};
+
+template <typename Check> struct CheckFactory<Check> {
+ static void
+ createChecks(ClangTidyContext *Context,
+ SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Result) {
+ Result.emplace_back(llvm::make_unique<Check>(
+ "test-check-" + std::to_string(Result.size()), Context));
+ }
+};
+
+template <typename... CheckList>
std::string
runCheckOnCode(StringRef Code, std::vector<ClangTidyError> *Errors = nullptr,
const Twine &Filename = "input.cc",
@@ -59,7 +81,6 @@ runCheckOnCode(StringRef Code, std::vect
ClangTidyContext Context(llvm::make_unique<DefaultOptionsProvider>(
ClangTidyGlobalOptions(), Options));
ClangTidyDiagnosticConsumer DiagConsumer(Context);
- T Check("test-check", &Context);
std::vector<std::string> ArgCXX11(1, "clang-tidy");
ArgCXX11.push_back("-fsyntax-only");
@@ -71,8 +92,11 @@ runCheckOnCode(StringRef Code, std::vect
ast_matchers::MatchFinder Finder;
llvm::IntrusiveRefCntPtr<FileManager> Files(
new FileManager(FileSystemOptions()));
+
+ SmallVector<std::unique_ptr<ClangTidyCheck>, 1> Checks;
+ CheckFactory<CheckList...>::createChecks(&Context, Checks);
tooling::ToolInvocation Invocation(
- ArgCXX11, new TestClangTidyAction(Check, Finder, Context), Files.get());
+ ArgCXX11, new TestClangTidyAction(Checks, Finder, Context), Files.get());
Invocation.mapVirtualFile(Filename.str(), Code);
for (const auto &FileContent : PathsToContent) {
Invocation.mapVirtualFile(Twine("include/" + FileContent.first).str(),
Added: clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp?rev=249402&view=auto
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp (added)
+++ clang-tools-extra/trunk/unittests/clang-tidy/OverlappingReplacementsTest.cpp Tue Oct 6 08:52:51 2015
@@ -0,0 +1,379 @@
+//===---- OverlappingReplacementsTest.cpp - clang-tidy --------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangTidyTest.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+namespace {
+
+const char BoundDecl[] = "decl";
+const char BoundIf[] = "if";
+
+// We define a reduced set of very small checks that allow to test different
+// overlapping situations (no overlapping, replacements partially overlap, etc),
+// as well as different kinds of diagnostics (one check produces several errors,
+// several replacement ranges in an error, etc).
+class UseCharCheck : public ClangTidyCheck {
+public:
+ UseCharCheck(StringRef CheckName, ClangTidyContext *Context)
+ : ClangTidyCheck(CheckName, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+ using namespace ast_matchers;
+ Finder->addMatcher(varDecl(hasType(isInteger())).bind(BoundDecl), this);
+ }
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override {
+ auto *VD = Result.Nodes.getNodeAs<VarDecl>(BoundDecl);
+ diag(VD->getLocStart(), "use char") << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(VD->getLocStart(), VD->getLocStart()),
+ "char");
+ }
+};
+
+class IfFalseCheck : public ClangTidyCheck {
+public:
+ IfFalseCheck(StringRef CheckName, ClangTidyContext *Context)
+ : ClangTidyCheck(CheckName, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+ using namespace ast_matchers;
+ Finder->addMatcher(ifStmt().bind(BoundIf), this);
+ }
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override {
+ auto *If = Result.Nodes.getNodeAs<IfStmt>(BoundIf);
+ auto *Cond = If->getCond();
+ SourceRange Range = Cond->getSourceRange();
+ if (auto *D = If->getConditionVariable()) {
+ Range = SourceRange(D->getLocStart(), D->getLocEnd());
+ }
+ diag(Range.getBegin(), "the cake is a lie") << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(Range), "false");
+ }
+};
+
+class RefactorCheck : public ClangTidyCheck {
+public:
+ RefactorCheck(StringRef CheckName, ClangTidyContext *Context)
+ : ClangTidyCheck(CheckName, Context), NamePattern("::$") {}
+ RefactorCheck(StringRef CheckName, ClangTidyContext *Context,
+ StringRef NamePattern)
+ : ClangTidyCheck(CheckName, Context), NamePattern(NamePattern) {}
+ virtual std::string newName(StringRef OldName) = 0;
+
+ void registerMatchers(ast_matchers::MatchFinder *Finder) final {
+ using namespace ast_matchers;
+ Finder->addMatcher(varDecl(matchesName(NamePattern)).bind(BoundDecl), this);
+ }
+
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) final {
+ auto *VD = Result.Nodes.getNodeAs<VarDecl>(BoundDecl);
+ std::string NewName = newName(VD->getName());
+
+ auto Diag = diag(VD->getLocation(), "refactor")
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(VD->getLocation(),
+ VD->getLocation()),
+ NewName);
+
+ class UsageVisitor : public RecursiveASTVisitor<UsageVisitor> {
+ public:
+ UsageVisitor(const ValueDecl *VD, StringRef NewName,
+ DiagnosticBuilder &Diag)
+ : VD(VD), NewName(NewName), Diag(Diag) {}
+ bool VisitDeclRefExpr(DeclRefExpr *E) {
+ if (const ValueDecl *D = E->getDecl()) {
+ if (VD->getCanonicalDecl() == D->getCanonicalDecl()) {
+ Diag << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(E->getSourceRange()), NewName);
+ }
+ }
+ return RecursiveASTVisitor<UsageVisitor>::VisitDeclRefExpr(E);
+ }
+
+ private:
+ const ValueDecl *VD;
+ StringRef NewName;
+ DiagnosticBuilder &Diag;
+ };
+
+ UsageVisitor(VD, NewName, Diag)
+ .TraverseDecl(Result.Context->getTranslationUnitDecl());
+ }
+
+protected:
+ const std::string NamePattern;
+};
+
+class StartsWithPotaCheck : public RefactorCheck {
+public:
+ StartsWithPotaCheck(StringRef CheckName, ClangTidyContext *Context)
+ : RefactorCheck(CheckName, Context, "::pota") {}
+
+ std::string newName(StringRef OldName) override {
+ return "toma" + OldName.substr(4).str();
+ }
+};
+
+class EndsWithTatoCheck : public RefactorCheck {
+public:
+ EndsWithTatoCheck(StringRef CheckName, ClangTidyContext *Context)
+ : RefactorCheck(CheckName, Context, "tato$") {}
+
+ std::string newName(StringRef OldName) override {
+ return OldName.substr(0, OldName.size() - 4).str() + "melo";
+ }
+};
+
+} // namespace
+
+TEST(OverlappingReplacementsTest, UseCharCheckTest) {
+ const char Code[] =
+ R"(void f() {
+ int a = 0;
+ if (int b = 0) {
+ int c = a;
+ }
+})";
+
+ const char CharFix[] =
+ R"(void f() {
+ char a = 0;
+ if (char b = 0) {
+ char c = a;
+ }
+})";
+ EXPECT_EQ(CharFix, runCheckOnCode<UseCharCheck>(Code));
+}
+
+TEST(OverlappingReplacementsTest, IfFalseCheckTest) {
+ const char Code[] =
+ R"(void f() {
+ int potato = 0;
+ if (int b = 0) {
+ int c = potato;
+ } else if (true) {
+ int d = 0;
+ }
+})";
+
+ const char IfFix[] =
+ R"(void f() {
+ int potato = 0;
+ if (false) {
+ int c = potato;
+ } else if (false) {
+ int d = 0;
+ }
+})";
+ EXPECT_EQ(IfFix, runCheckOnCode<IfFalseCheck>(Code));
+}
+
+TEST(OverlappingReplacementsTest, StartsWithCheckTest) {
+ const char Code[] =
+ R"(void f() {
+ int a = 0;
+ int potato = 0;
+ if (int b = 0) {
+ int c = potato;
+ } else if (true) {
+ int d = 0;
+ }
+})";
+
+ const char StartsFix[] =
+ R"(void f() {
+ int a = 0;
+ int tomato = 0;
+ if (int b = 0) {
+ int c = tomato;
+ } else if (true) {
+ int d = 0;
+ }
+})";
+ EXPECT_EQ(StartsFix, runCheckOnCode<StartsWithPotaCheck>(Code));
+}
+
+TEST(OverlappingReplacementsTest, EndsWithCheckTest) {
+ const char Code[] =
+ R"(void f() {
+ int a = 0;
+ int potato = 0;
+ if (int b = 0) {
+ int c = potato;
+ } else if (true) {
+ int d = 0;
+ }
+})";
+
+ const char EndsFix[] =
+ R"(void f() {
+ int a = 0;
+ int pomelo = 0;
+ if (int b = 0) {
+ int c = pomelo;
+ } else if (true) {
+ int d = 0;
+ }
+})";
+ EXPECT_EQ(EndsFix, runCheckOnCode<EndsWithTatoCheck>(Code));
+}
+
+TEST(OverlappingReplacementTest, ReplacementsDoNotOverlap) {
+ std::string Res;
+ const char Code[] =
+ R"(void f() {
+ int potassium = 0;
+ if (true) {
+ int Potato = potassium;
+ }
+})";
+
+ const char CharIfFix[] =
+ R"(void f() {
+ char potassium = 0;
+ if (false) {
+ char Potato = potassium;
+ }
+})";
+ Res = runCheckOnCode<UseCharCheck, IfFalseCheck>(Code);
+ EXPECT_EQ(CharIfFix, Res);
+
+ const char StartsEndsFix[] =
+ R"(void f() {
+ int tomassium = 0;
+ if (true) {
+ int Pomelo = tomassium;
+ }
+})";
+ Res = runCheckOnCode<StartsWithPotaCheck, EndsWithTatoCheck>(Code);
+ EXPECT_EQ(StartsEndsFix, Res);
+
+ const char CharIfStartsEndsFix[] =
+ R"(void f() {
+ char tomassium = 0;
+ if (false) {
+ char Pomelo = tomassium;
+ }
+})";
+ Res = runCheckOnCode<UseCharCheck, IfFalseCheck, StartsWithPotaCheck,
+ EndsWithTatoCheck>(Code);
+ EXPECT_EQ(CharIfStartsEndsFix, Res);
+}
+
+TEST(OverlappingReplacementsTest, ReplacementInsideOtherReplacement) {
+ std::string Res;
+ const char Code[] =
+ R"(void f() {
+ if (char potato = 0) {
+ } else if (int a = 0) {
+ char potato = 0;
+ if (potato) potato;
+ }
+})";
+
+ // Apply the UseCharCheck together with the IfFalseCheck.
+ //
+ // The 'If' fix is bigger, so that is the one that has to be applied.
+ // } else if (int a = 0) {
+ // ^^^ -> char
+ // ~~~~~~~~~ -> false
+ const char CharIfFix[] =
+ R"(void f() {
+ if (false) {
+ } else if (false) {
+ char potato = 0;
+ if (false) potato;
+ }
+})";
+ Res = runCheckOnCode<UseCharCheck, IfFalseCheck>(Code);
+ // FIXME: EXPECT_EQ(CharIfFix, Res);
+
+ // Apply the IfFalseCheck with the StartsWithPotaCheck.
+ //
+ // The 'If' replacement is bigger here.
+ // if (char potato = 0) {
+ // ^^^^^^ -> tomato
+ // ~~~~~~~~~~~~~~~ -> false
+ //
+ // But the refactoring is bigger here:
+ // char potato = 0;
+ // ^^^^^^ -> tomato
+ // if (potato) potato;
+ // ^^^^^^ ^^^^^^ -> tomato, tomato
+ // ~~~~~~ -> false
+ const char IfStartsFix[] =
+ R"(void f() {
+ if (false) {
+ } else if (false) {
+ char tomato = 0;
+ if (tomato) tomato;
+ }
+})";
+ Res = runCheckOnCode<IfFalseCheck, StartsWithPotaCheck>(Code);
+ // FIXME: EXPECT_EQ(IfStartsFix, Res);
+
+ // Silence warnings.
+ (void)CharIfFix;
+ (void)IfStartsFix;
+}
+
+TEST(OverlappingReplacementsTest, ApplyFullErrorOrNothingWhenOverlapping) {
+ std::string Res;
+ const char Code[] =
+ R"(void f() {
+ int potato = 0;
+ potato += potato * potato;
+ if (char this_name_make_this_if_really_long = potato) potato;
+})";
+
+ // StartsWithPotaCheck will try to refactor 'potato' into 'tomato',
+ // and EndsWithTatoCheck will try to use 'pomelo'. We have to apply
+ // either all conversions from one check, or all from the other.
+ const char StartsFix[] =
+ R"(void f() {
+ int tomato = 0;
+ tomato += tomato * tomato;
+ if (char this_name_make_this_if_really_long = tomato) tomato;
+})";
+ const char EndsFix[] =
+ R"(void f() {
+ int pomelo = 0;
+ pomelo += pomelo * pomelo;
+ if (char this_name_make_this_if_really_long = pomelo) pomelo;
+})";
+ // In case of overlapping, we will prioritize the biggest fix. However, these
+ // two fixes have the same size and position, so we don't know yet which one
+ // will have preference.
+ Res = runCheckOnCode<StartsWithPotaCheck, EndsWithTatoCheck>(Code);
+ // FIXME: EXPECT_TRUE(Res == StartsFix || Res == EndsFix);
+
+ // StartsWithPotaCheck will try to refactor 'potato' into 'tomato', but
+ // replacing the 'if' condition is a bigger change than all the refactoring
+ // changes together (48 vs 36), so this is the one that is going to be
+ // applied.
+ const char IfFix[] =
+ R"(void f() {
+ int potato = 0;
+ potato += potato * potato;
+ if (true) potato;
+})";
+ Res = runCheckOnCode<StartsWithPotaCheck, IfFalseCheck>(Code);
+ // FIXME: EXPECT_EQ(IfFix, Res);
+
+ // Silence warnings.
+ (void)StartsFix;
+ (void)EndsFix;
+ (void)IfFix;
+}
+
+} // namespace test
+} // namespace tidy
+} // namespace clang
Modified: clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp?rev=249402&r1=249401&r2=249402&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp Tue Oct 6 08:52:51 2015
@@ -237,7 +237,7 @@ TEST(BracesAroundStatementsCheck, If) {
TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) {
ClangTidyOptions Options;
- Options.CheckOptions["test-check.ShortStatementLines"] = "1";
+ Options.CheckOptions["test-check-0.ShortStatementLines"] = "1";
EXPECT_EQ("int main() {\n"
" if (true) return 1;\n"
More information about the cfe-commits
mailing list