Skip to content

Commit ac7864e

Browse files
committed
[clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC
Summary: TweakTests.cpp has some pretty good helpers added for the first few tweaks, but they have some limitations: - many assertion failures point at the wrong line - need lots of input/output tests, setup code is duplicated across both - local helpers make it hard to split the file as it grows The new helpers in TweakTests.h are based on old ones (same operations) but try to address these issues and generally make tests more terse while improving error messages. This patch converts everything except ExtractVariable (which is complex and has changes in flight, so will be converted later). It's LOC-neutral, despite not being able to get rid of the old helpers until ExtractVariable is done. Reviewers: ilya-biryukov Subscribers: mgorny, MaskRay, jkorous, arphaman, kadircet, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D65525 llvm-svn: 367667
1 parent b874b3d commit ac7864e

File tree

4 files changed

+366
-365
lines changed

4 files changed

+366
-365
lines changed

clang-tools-extra/clangd/unittests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ add_unittest(ClangdUnitTests ClangdTests
6868
TraceTests.cpp
6969
TypeHierarchyTests.cpp
7070
TweakTests.cpp
71+
TweakTesting.cpp
7172
URITests.cpp
7273
XRefsTests.cpp
7374

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
//===-- TweakTesting.cpp ------------------------------------------------*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "TweakTesting.h"
10+
11+
#include "Annotations.h"
12+
#include "refactor/Tweak.h"
13+
#include "SourceCode.h"
14+
#include "clang/Tooling/Core/Replacement.h"
15+
#include "llvm/Support/Error.h"
16+
17+
namespace clang {
18+
namespace clangd {
19+
namespace {
20+
using Context = TweakTest::CodeContext;
21+
22+
std::pair<llvm::StringRef, llvm::StringRef> wrapping(Context Ctx) {
23+
switch (Ctx) {
24+
case TweakTest::File:
25+
return {"",""};
26+
case TweakTest::Function:
27+
return {"void wrapperFunction(){\n", "\n}"};
28+
case TweakTest::Expression:
29+
return {"auto expressionWrapper(){return\n", "\n;}"};
30+
}
31+
}
32+
33+
std::string wrap(Context Ctx, llvm::StringRef Inner) {
34+
auto Wrapping = wrapping(Ctx);
35+
return (Wrapping.first + Inner + Wrapping.second).str();
36+
}
37+
38+
llvm::StringRef unwrap(Context Ctx, llvm::StringRef Outer) {
39+
auto Wrapping = wrapping(Ctx);
40+
// Unwrap only if the code matches the expected wrapping.
41+
// Don't allow the begin/end wrapping to overlap!
42+
if (Outer.startswith(Wrapping.first) && Outer.endswith(Wrapping.second) &&
43+
Outer.size() >= Wrapping.first.size() + Wrapping.second.size())
44+
return Outer.drop_front(Wrapping.first.size()).drop_back(Wrapping.second.size());
45+
return Outer;
46+
}
47+
48+
std::pair<unsigned, unsigned> rangeOrPoint(const Annotations &A) {
49+
Range SelectionRng;
50+
if (A.points().size() != 0) {
51+
assert(A.ranges().size() == 0 &&
52+
"both a cursor point and a selection range were specified");
53+
SelectionRng = Range{A.point(), A.point()};
54+
} else {
55+
SelectionRng = A.range();
56+
}
57+
return {cantFail(positionToOffset(A.code(), SelectionRng.start)),
58+
cantFail(positionToOffset(A.code(), SelectionRng.end))};
59+
}
60+
61+
MATCHER_P3(TweakIsAvailable, TweakID, Ctx, Header,
62+
(TweakID + (negation ? " is unavailable" : " is available")).str()) {
63+
std::string WrappedCode = wrap(Ctx, arg);
64+
Annotations Input(WrappedCode);
65+
auto Selection = rangeOrPoint(Input);
66+
TestTU TU;
67+
TU.HeaderCode = Header;
68+
TU.Code = Input.code();
69+
ParsedAST AST = TU.build();
70+
Tweak::Selection S(AST, Selection.first, Selection.second);
71+
auto PrepareResult = prepareTweak(TweakID, S);
72+
if (PrepareResult)
73+
return true;
74+
llvm::consumeError(PrepareResult.takeError());
75+
return false;
76+
}
77+
78+
} // namespace
79+
80+
std::string TweakTest::apply(llvm::StringRef MarkedCode) const {
81+
std::string WrappedCode = wrap(Context, MarkedCode);
82+
Annotations Input(WrappedCode);
83+
auto Selection = rangeOrPoint(Input);
84+
TestTU TU;
85+
TU.HeaderCode = Header;
86+
TU.Code = Input.code();
87+
ParsedAST AST = TU.build();
88+
Tweak::Selection S(AST, Selection.first, Selection.second);
89+
90+
auto T = prepareTweak(TweakID, S);
91+
if (!T)
92+
return "unavailable";
93+
llvm::Expected<Tweak::Effect> Result = (*T)->apply(S);
94+
if (!Result)
95+
return "fail: " + llvm::toString(Result.takeError());
96+
if (Result->ShowMessage)
97+
return "message:\n" + *Result->ShowMessage;
98+
if (Result->ApplyEdit) {
99+
if (auto NewText =
100+
tooling::applyAllReplacements(Input.code(), *Result->ApplyEdit))
101+
return unwrap(Context, *NewText);
102+
else
103+
return "bad edits: " + llvm::toString(NewText.takeError());
104+
}
105+
return "no effect";
106+
}
107+
108+
::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const {
109+
return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header);
110+
}
111+
112+
std::vector<std::string> TweakTest::expandCases(llvm::StringRef MarkedCode) {
113+
Annotations Test(MarkedCode);
114+
llvm::StringRef Code = Test.code();
115+
std::vector<std::string> Cases;
116+
for (const auto& Point : Test.points()) {
117+
size_t Offset = llvm::cantFail(positionToOffset(Code, Point));
118+
Cases.push_back((Code.substr(0, Offset) + "^" + Code.substr(Offset)).str());
119+
}
120+
for (const auto& Range : Test.ranges()) {
121+
size_t Begin = llvm::cantFail(positionToOffset(Code, Range.start));
122+
size_t End = llvm::cantFail(positionToOffset(Code, Range.end));
123+
Cases.push_back((Code.substr(0, Begin) + "[[" +
124+
Code.substr(Begin, End - Begin) + "]]" + Code.substr(End))
125+
.str());
126+
}
127+
assert(!Cases.empty() && "No markings in MarkedCode?");
128+
return Cases;
129+
}
130+
131+
} // namespace clangd
132+
} // namespace clang
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
//===--- TweakTesting.h - Test helpers for refactoring actions ---*- C++-*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
11+
12+
#include "TestTU.h"
13+
#include "gtest/gtest.h"
14+
#include "gmock/gmock.h"
15+
16+
namespace clang {
17+
namespace clangd {
18+
19+
// Fixture base for testing tweaks. Intended to be subclassed for each tweak.
20+
//
21+
// Usage:
22+
// TWEAK_TEST(ExpandAutoType);
23+
//
24+
// TEST_F(ExpandAutoTypeTest, ShortensTypes) {
25+
// Header = R"cpp(
26+
// namespace foo { template<typename> class X{}; }
27+
// using namespace foo;
28+
// cpp)";
29+
// Context = Block;
30+
// EXPECT_THAT(apply("[[auto]] X = foo<int>();"),
31+
// "foo<int> X = foo<int();");
32+
// EXPECT_AVAILABLE("^a^u^t^o^ X = foo<int>();");
33+
// EXPECT_UNAVAILABLE("auto ^X^ = ^foo<int>();");
34+
// }
35+
class TweakTest : public ::testing::Test {
36+
const char *TweakID;
37+
38+
public:
39+
// Inputs are wrapped in file boilerplate before attempting to apply a tweak.
40+
// Context describes the type of boilerplate.
41+
enum CodeContext {
42+
// Code snippet is placed directly into the source file. e.g. a declaration.
43+
File,
44+
// Snippet will appear within a function body. e.g. a statement.
45+
Function,
46+
// Snippet is an expression.
47+
Expression,
48+
};
49+
50+
protected:
51+
TweakTest(const char *TweakID) : TweakID(TweakID) {}
52+
53+
// Contents of a header file to be implicitly included.
54+
// This typically contains declarations that will be used for a set of related
55+
// testcases.
56+
std::string Header;
57+
58+
// Context in which snippets of code should be placed to run tweaks.
59+
CodeContext Context = File;
60+
61+
// Apply the current tweak to the range (or point) in MarkedCode.
62+
// MarkedCode will be wrapped according to the Context.
63+
// - if the tweak produces edits, returns the edited code (without markings).
64+
// The context added to MarkedCode will be stripped away before returning,
65+
// unless the tweak edited it.
66+
// - if the tweak produces a message, returns "message:\n<message>"
67+
// - if prepare() returns false, returns "unavailable"
68+
// - if apply() returns an error, returns "fail: <message>"
69+
std::string apply(llvm::StringRef MarkedCode) const;
70+
71+
// Accepts a code snippet with many ranges (or points) marked, and returns a
72+
// list of snippets with one range marked each.
73+
// Primarily used from EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macro.
74+
static std::vector<std::string> expandCases(llvm::StringRef MarkedCode);
75+
76+
// Returns a matcher that accepts marked code snippets where the tweak is
77+
// available at the marked range.
78+
::testing::Matcher<llvm::StringRef> isAvailable() const;
79+
};
80+
81+
#define TWEAK_TEST(TweakID) \
82+
class TweakID##Test : public ::clang::clangd::TweakTest { \
83+
protected: \
84+
TweakID##Test() : TweakTest(#TweakID) {} \
85+
}
86+
87+
#define EXPECT_AVAILABLE(MarkedCode) \
88+
do { \
89+
for (const auto &Case : expandCases(MarkedCode)) \
90+
EXPECT_THAT(Case, ::clang::clangd::TweakTest::isAvailable()); \
91+
} while (0)
92+
93+
#define EXPECT_UNAVAILABLE(MarkedCode) \
94+
do { \
95+
for (const auto &Case : expandCases(MarkedCode)) \
96+
EXPECT_THAT(Case, \
97+
::testing::Not(::clang::clangd::TweakTest::isAvailable())); \
98+
} while (0)
99+
100+
} // namespace clangd
101+
} // namespace clang
102+
103+
#endif

0 commit comments

Comments
 (0)