[PATCH] D114667: [clangd] Add fixes for clang "include <foo.h>" diagnostics

[Nathan Ridge via Phabricator via cfe-commits]

nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.

Thanks, this is nice low-hanging fruit.



================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:830
+      if (!ReplacementFixes.empty()) {
+        LastDiag->Fixes.insert(LastDiag->Fixes.end(), ReplacementFixes.begin(),
+                               ReplacementFixes.end());
----------------
Do we maybe want to assert `Info.getFixItHints().empty()` here, so that if clang later introduces its own fix-it it gets on our radar and we can decide which one to prefer?


================
Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:52
+llvm::Optional<llvm::StringRef> getArgStr(const clang::Diagnostic &Info,
+                                          unsigned I) {
+  switch (Info.getArgKind(I)) {
----------------
nit: Index or Idx?


================
Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:495
+  if (auto Edit = Inserter->insert(Name))
+    return {Fix{llvm::formatv("Include {0}", Name).str(), {std::move(*Edit)}}};
+  return {};
----------------
In the `note_include_header_or_declare` case, we could pull the second arg from the diagnostic (the name of the symbol) and use it to provide the slightly more detailed hint description that [fixUnresolvedName() does](https://searchfox.org/llvm/rev/168bc7ce7e2ebe6527bf3fdd9262ef5c0deab4fc/clang-tools-extra/clangd/IncludeFixer.cpp#168). Up to you if you think that's worth doing.


================
Comment at: clang-tools-extra/clangd/IncludeFixer.h:43
   /// Returns include insertions that can potentially recover the diagnostic.
+  /// If Info describes a note, it will be replaced by any returned fixes.
   std::vector<Fix> fix(DiagnosticsEngine::Level DiagLevel,
----------------
The added comment describes the behaviour of calling code, not the behaviour of this method. Perhaps it would make more sense in the calling code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114667