[PATCH] D38639: [clangd] #include statements support for Open definition

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Last drop of NITs. After they're fixed, this change is ready to land!



================
Comment at: unittests/clangd/XRefsTests.cpp:282
+  const char *HeaderContents = R"cpp([[]]int a;)cpp";
+  Annotations HeaderAnnoations(HeaderContents);
+  FS.Files[FooH] = HeaderAnnoations.code();
----------------
There's a typo, should be `HeaderAnnotations`


================
Comment at: unittests/clangd/XRefsTests.cpp:302
+  Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(!Locations.empty());
+  ASSERT_EQ(Locations[0].uri.file, FooH);
----------------
NIT: one assertion using `EXPECT_THAT` will read better and produce more helpful messages in case of failures:
```
  // Create FooHUri to avoid typing URIForFile everywhere.
  auto FooHUri = URIForFile{FooH.str()};

  EXPECT_THAT(Locations, ElementsAre(Location{FooHUri, HeaderAnnoations.range()}));
```



================
Comment at: unittests/clangd/XRefsTests.cpp:328
+  Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(Locations.empty());
+
----------------
NIT: Use `EXPECT_THAT` here too:
`EXPECT_THAT(Locations, IsEmpty());`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38639