[PATCH] D51747: [clangd] Show deprecation diagnostics

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

Awesome, thank you.
Just a couple of last nits.



================
Comment at: unittests/clangd/ClangdTests.cpp:984
+                            std::vector<Diag> Diagnostics) override {
+      std::lock_guard<std::mutex> Lock(Mutex);
+      for(const Diag& D : Diagnostics) {
----------------
Locking is unneccesary. (Diagnostics will be delivered once, and the SyncAPI calls block until diagnostics are delivered)


================
Comment at: unittests/clangd/ClangdTests.cpp:1014
+  runAddDocument(Server, SourcePath, Test.code());
+  EXPECT_TRUE(DiagConsumer.workedAsExpected());
+  DiagConsumer.reset();
----------------
nit: the logic is right, but the message could be better.
If we change something and it fails it will print `Expected true: DiagConsumer.worksAsExpected(), but was false` or so. There are a number of things that could be wrong.
Prefer just to capture a bit more data (all diagnostics) and use a matcher expression:
```
MATCHER(DeprecationWarning, "") {
  return arg.Category == "Deprecations" && arg.Severity == DiagnosticsEngine::Warning;
}
...
EXPECT_THAT(Diags, ElementsAre(DeprecationWarning()));
```
That way if there isn't exactly one diagnostic that's a deprecation warning, it'll print the expectation, the full list of diagnostics, and the reason it didn't match.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747