[PATCH] D65433: [clangd] DefineInline action availability checks

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:59
+
+void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available,
+                    const llvm::StringMap<std::string> &AdditionalFiles) {
----------------
This function, and all the other helpers in this file, are actually imminently going away in favor of TweakTesting.h.
Sorry it's been stuck in my backlog for a couple of weeks...

We should work out how to do that stuff there.
One approach is to give TweakTest a `StringMap<string> ExtraFiles` and `StringMap<string> EditedFiles` that excludes main, so you'd write
```
ExtraFiles["foo.h"] = "...";
EXPECT_EQ(apply("..."), "...");
EXPECT_THAT(EditedFiles, ElementsAre(Pair(testPath("foo.h"), "...");
```
or something like this.

(Whatever we do, we should bear in mind the fact that out-line will mean we need to supply an index too)


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:631
+
+  // Should it also trigger on NestedNameSpecifier? i.e "Bar::"
+  [[void [[Bar::[[b^a^z]]]]() [[{
----------------
I don't think so


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:653
+
+TEST_F(DefineInlineTest, NoDecl) {
+  checkNotAvailable(TweakID, R"cpp(
----------------
nit: NoForwardDecl


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:683
+                    })cpp",
+                 {{"a.h", "void bar(); void foo(int test);"}});
+
----------------
FWIW, I find these params rather cryptic - it'd be nice to have an API for the helpers that is explicit about this being files/filenames


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65433/new/

https://reviews.llvm.org/D65433