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

Wed Dec 8 07:06:48 PST 2021

sammccall marked 4 inline comments as done.
sammccall added inline comments.

================
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 {};
----------------
nridge wrote:
> 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.
Good point. I shuffled around some code so these strings won't go out of sync.

And they were ("Add include X" vs "Include X") so being stubborn I changed all the existing ones to add match this one (since "include" isn't really a noun)

================
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,
----------------
nridge wrote:
> 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?
Oops, the intent was to document the semantics of returned values.

Reworded and also added docs at contributeFixes.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114667