[clangd] Rewrite TweakTesting helpers to avoid reparsing the same code. NFC

[sammccall]

Summary

Previously the EXPECT_AVAILABLE macros would rebuild the code at each marked
point, by expanding the cases textually.
There were often lots, and it's nice to have lots!

This reduces total unittest time by ~10% on my machine.
I did have to sacrifice a little apply() coverage in AddUsingTests (was calling
expandCases directly, which was otherwise unused), but we have
EXPECT_AVAILABLE tests covering that, I don't think there's real risk here.

[ilya-biryukov]

LGTM.

clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp

struct {
     llvm::StringRef TestSource;
     llvm::StringRef ExpectedSource;
   } Cases[]{{
                 // Function, no other using, namespace.
                 R"cpp(
 #include "test.hpp"
 namespace {
 void fun() {
-  ^o^n^e^:^:^t^w^o^:^:^f^f();
+  ^one::two::ff();
 }
 })cpp",
                 R"cpp(
 #include "test.hpp"
 namespace {using one::two::ff;
 
 void fun() {
   ff();
 }
 })cpp",
             },
             // Type, no other using, namespace.
             {
                 R"cpp(
 #include "test.hpp"
 namespace {
 void fun() {
-  ::on^e::t^wo::c^c inst;
+  ::one::t^wo::cc inst;
 }
 })cpp",
                 R"cpp(
 #include "test.hpp"
 namespace {using ::one::two::cc;
 
 void fun() {
   cc inst;
 }
 })cpp",
             },
             // Type, no other using, no namespace.
             {
                 R"cpp(
 #include "test.hpp"
 
 void fun() {
-  on^e::t^wo::e^e inst;
+  one::two::e^e inst;
 })cpp",
                 R"cpp(
 #include "test.hpp"
 
 using one::two::ee;
 
 void fun() {
   ee inst;

 
 using one::two::cc;
 
 namespace {
 
 using one::two::ff;
 
 void fun() {
-  o^ne::o^o();
+  o^ne::oo();
 }
 })cpp",
                 R"cpp(
 #include "test.hpp"
 
 using one::two::cc;
 
 namespace {
)cpp",
              R"cpp(
 #include "test.hpp"
 using one::vec;
 
 vec<int> foo;
 )cpp"}};
   llvm::StringMap<std::string> EditedFiles;
   for (const auto &Case : Cases) {
-    for (const auto &SubCase : expandCases(Case.TestSource)) {
-      ExtraFiles["test.hpp"] = R"cpp(
 namespace one {
 void oo() {}
 namespace two {
 enum ee {ee_one};
 void ff() {}
 class cc {
 public:
   struct st { struct nested {}; };
   static void mm() {}
 };
 }
 using uu = two::cc;
 template<typename T> struct vec {};
 })cpp";
-      EXPECT_EQ(apply(SubCase, &EditedFiles), Case.ExpectedSource);
-    }
+    EXPECT_EQ(apply(Case.TestSource, &EditedFiles), Case.ExpectedSource);
   }
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h

 //===--- TweakTesting.h - Test helpers for refactoring actions ---*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
 
+#include "ParsedAST.h"
 #include "index/Index.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <memory>
 #include <string>
 
 namespace clang {
 namespace clangd {
 
protected:
   //    The context added to MarkedCode will be stripped away before returning,
   //    unless the tweak edited it.
   //  - if the tweak produces a message, returns "message:\n<message>"
   //  - if prepare() returns false, returns "unavailable"
   //  - if apply() returns an error, returns "fail: <message>"
   std::string apply(llvm::StringRef MarkedCode,
                     llvm::StringMap<std::string> *EditedFiles = nullptr) const;
 
-  // Accepts a code snippet with many ranges (or points) marked, and returns a
-  // list of snippets with one range marked each.
-  // Primarily used from EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macro.
-  static std::vector<std::string> expandCases(llvm::StringRef MarkedCode);
-
-  // Returns a matcher that accepts marked code snippets where the tweak is
-  // available at the marked range.
-  ::testing::Matcher<llvm::StringRef> isAvailable() const;
+  // Helpers for EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macros.
+  using WrappedAST = std::pair<ParsedAST, /*WrappingOffset*/ unsigned>;
+  WrappedAST build(llvm::StringRef) const;
+  bool isAvailable(WrappedAST &, llvm::Annotations::Range) const;
+  // Return code re-decorated with a single point/range.
+  static std::string decorate(llvm::StringRef, unsigned);
+  static std::string decorate(llvm::StringRef, llvm::Annotations::Range);
 };
 
 MATCHER_P2(FileWithContents, FileName, Contents, "") {
   return arg.first() == FileName && arg.second == Contents;
 }
 
 #define TWEAK_TEST(TweakID)                                                    \
   class TweakID##Test : public ::clang::clangd::TweakTest {                    \
   protected:                                                                   \
     TweakID##Test() : TweakTest(#TweakID) {}                                   \
   }
 
-#define EXPECT_AVAILABLE(MarkedCode)                                           \
+#define EXPECT_AVAILABLE_(MarkedCode, Available)                               \
   do {                                                                         \
-    for (const auto &Case : expandCases(MarkedCode))                           \
-      EXPECT_THAT(Case, ::clang::clangd::TweakTest::isAvailable());            \
-  } while (0)
-
-#define EXPECT_UNAVAILABLE(MarkedCode)                                         \
-  do {                                                                         \
-    for (const auto &Case : expandCases(MarkedCode))                           \
-      EXPECT_THAT(Case,                                                        \
-                  ::testing::Not(::clang::clangd::TweakTest::isAvailable()));  \
+    llvm::Annotations A{llvm::StringRef(MarkedCode)};                          \
+    auto AST = build(A.code());                                                \
+    assert(!A.points().empty() || !A.ranges().empty());                        \
+    for (const auto &P : A.points())                                           \
+      EXPECT_EQ(Available, isAvailable(AST, {P, P})) << decorate(A.code(), P); \
+    for (const auto &R : A.ranges())                                           \
+      EXPECT_EQ(Available, isAvailable(AST, R)) << decorate(A.code(), R);      \
   } while (0)
+#define EXPECT_AVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, true)
+#define EXPECT_UNAVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, false)
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp

 //===-- TweakTesting.cpp ------------------------------------------------*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "TweakTesting.h"
 
-#include "Annotations.h"
 #include "SourceCode.h"
 #include "TestTU.h"
 #include "refactor/Tweak.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <string>
 
llvm::StringRef unwrap(Context Ctx, llvm::StringRef Outer) {
   // Don't allow the begin/end wrapping to overlap!
   if (Outer.startswith(Wrapping.first) && Outer.endswith(Wrapping.second) &&
       Outer.size() >= Wrapping.first.size() + Wrapping.second.size())
     return Outer.drop_front(Wrapping.first.size())
         .drop_back(Wrapping.second.size());
   return Outer;
 }
 
-std::pair<unsigned, unsigned> rangeOrPoint(const Annotations &A) {
-  Range SelectionRng;
+llvm::Annotations::Range rangeOrPoint(const llvm::Annotations &A) {
   if (A.points().size() != 0) {
     assert(A.ranges().size() == 0 &&
            "both a cursor point and a selection range were specified");
-    SelectionRng = Range{A.point(), A.point()};
-  } else {
-    SelectionRng = A.range();
+    return {A.point(), A.point()};
   }
-  return {cantFail(positionToOffset(A.code(), SelectionRng.start)),
-          cantFail(positionToOffset(A.code(), SelectionRng.end))};
+  return A.range();
 }
 
 // Prepare and apply the specified tweak based on the selection in Input.
 // Returns None if and only if prepare() failed.
 llvm::Optional<llvm::Expected<Tweak::Effect>>
-applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID,
+applyTweak(ParsedAST &AST, llvm::Annotations::Range Range, StringRef TweakID,
            const SymbolIndex *Index, llvm::vfs::FileSystem *FS) {
-  auto Range = rangeOrPoint(Input);
   llvm::Optional<llvm::Expected<Tweak::Effect>> Result;
-  SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
-                            Range.second, [&](SelectionTree ST) {
-                              Tweak::Selection S(Index, AST, Range.first,
-                                                 Range.second, std::move(ST),
-                                                 FS);
+  SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.Begin,
+                            Range.End, [&](SelectionTree ST) {
+                              Tweak::Selection S(Index, AST, Range.Begin,
+                                                 Range.End, std::move(ST), FS);
                               if (auto T = prepareTweak(TweakID, S, nullptr)) {
                                 Result = (*T)->apply(S);
                                 return true;
                               } else {
                                 llvm::consumeError(T.takeError());
                                 return false;
                               }
                             });
   return Result;
 }
 
-MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
-           FileName,
-           (TweakID + (negation ? " is unavailable" : " is available")).str()) {
-  std::string WrappedCode = wrap(Ctx, arg);
-  Annotations Input(WrappedCode);
-  TestTU TU;
-  TU.Filename = std::string(FileName);
-  TU.HeaderCode = Header;
-  TU.Code = std::string(Input.code());
-  TU.ExtraArgs = ExtraArgs;
-  TU.AdditionalFiles = std::move(ExtraFiles);
-  ParsedAST AST = TU.build();
-  auto Result = applyTweak(
-      AST, Input, TweakID, Index,
-      &AST.getSourceManager().getFileManager().getVirtualFileSystem());
-  // We only care if prepare() succeeded, but must handle Errors.
-  if (Result && !*Result)
-    consumeError(Result->takeError());
-  return Result.hasValue();
-}
-
 } // namespace
 
 std::string TweakTest::apply(llvm::StringRef MarkedCode,
                              llvm::StringMap<std::string> *EditedFiles) const {
   std::string WrappedCode = wrap(Context, MarkedCode);
-  Annotations Input(WrappedCode);
+  llvm::Annotations Input(WrappedCode);
   TestTU TU;
   TU.Filename = std::string(FileName);
   TU.HeaderCode = Header;
   TU.AdditionalFiles = std::move(ExtraFiles);
   TU.Code = std::string(Input.code());
   TU.ExtraArgs = ExtraArgs;
   ParsedAST AST = TU.build();
 
   auto Result = applyTweak(
-      AST, Input, TweakID, Index.get(),
+      AST, rangeOrPoint(Input), TweakID, Index.get(),
       &AST.getSourceManager().getFileManager().getVirtualFileSystem());
   if (!Result)
     return "unavailable";
   if (!*Result)
     return "fail: " + llvm::toString(Result->takeError());
   const auto &Effect = **Result;
   if ((*Result)->ShowMessage)
     return "message:\n" + *Effect.ShowMessage;
else {
                          "provided a nullptr for EditedFiles.";
       else
         EditedFiles->insert_or_assign(It.first(), Unwrapped.str());
     }
   }
   return EditedMainFile;
 }
 
-::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const {
-  return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, ExtraArgs,
-                          ExtraFiles, Index.get(), FileName);
-}
-
-std::vector<std::string> TweakTest::expandCases(llvm::StringRef MarkedCode) {
-  Annotations Test(MarkedCode);
-  llvm::StringRef Code = Test.code();
-  std::vector<std::string> Cases;
-  for (const auto &Point : Test.points()) {
-    size_t Offset = llvm::cantFail(positionToOffset(Code, Point));
-    Cases.push_back((Code.substr(0, Offset) + "^" + Code.substr(Offset)).str());
-  }
-  for (const auto &Range : Test.ranges()) {
-    size_t Begin = llvm::cantFail(positionToOffset(Code, Range.start));
-    size_t End = llvm::cantFail(positionToOffset(Code, Range.end));
-    Cases.push_back((Code.substr(0, Begin) + "[[" +
-                     Code.substr(Begin, End - Begin) + "]]" + Code.substr(End))
-                        .str());
-  }
-  assert(!Cases.empty() && "No markings in MarkedCode?");
-  return Cases;
+bool TweakTest::isAvailable(WrappedAST &AST,
+                            llvm::Annotations::Range Range) const {
+  // Adjust range for wrapping offset.
+  Range.Begin += AST.second;
+  Range.End += AST.second;
+  auto Result = applyTweak(
+      AST.first, Range, TweakID, Index.get(),
+      &AST.first.getSourceManager().getFileManager().getVirtualFileSystem());
+  // We only care if prepare() succeeded, but must handle Errors.
+  if (Result && !*Result)
+    consumeError(Result->takeError());
+  return Result.hasValue();
+
+TweakTest::WrappedAST TweakTest::build(llvm::StringRef Code) const {
+  TestTU TU;
+  TU.Filename = std::string(FileName);
+  TU.HeaderCode = Header;
+  TU.Code = wrap(Context, Code);
+  TU.ExtraArgs = ExtraArgs;
+  TU.AdditionalFiles = std::move(ExtraFiles);
+  return {TU.build(), wrapping(Context).first.size()};
+}
+
+std::string TweakTest::decorate(llvm::StringRef Code, unsigned Point) {
+  return (Code.substr(0, Point) + "^" + Code.substr(Point)).str();
+}
+
+std::string TweakTest::decorate(llvm::StringRef Code,
+                                llvm::Annotations::Range Range) {
+  return (Code.substr(0, Range.Begin) + "[[" +
+          Code.substr(Range.Begin, Range.End) + "]]" + Code.substr(Range.End))
+      .str();
 }
 
 } // namespace clangd
 } // namespace clang