[clang-tidy] Correct the include-cleaner-check diagnostic message for missing-includes.

[hokein]

Summary

We should print the symbol name rather than the header name in the
message.

[VitaNuo]

Thanks!

clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp

49

Consider making the whole include_cleaner::SymbolReference a field instead, since you're already using two out of its three fields here.

[hokein]

address comments

clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp

 #include <vector>
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::misc {
 
 namespace {
 struct MissingIncludeInfo {
-  SourceLocation SymRefLocation;
+  include_cleaner::SymbolReference SymRef;
   include_cleaner::Header Missing;
 };
 } // namespace
 
 IncludeCleanerCheck::IncludeCleanerCheck(StringRef Name,
                                          ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       IgnoreHeaders(utils::options::parseStringList(
           Options.getLocalOrGlobal("IgnoreHeaders", ""))) {
walkUsed(MainFileDecls, RecordedPreprocessor.MacroReferences, &RecordedPI,
                     RecordedPreprocessor.Includes.match(H)) {
                  Used.insert(I);
                  Satisfied = true;
                }
              }
              if (!Satisfied && !Providers.empty() &&
                  Ref.RT == include_cleaner::RefType::Explicit &&
                  !shouldIgnore(Providers.front()))
-               Missing.push_back({Ref.RefLocation, Providers.front()});
+               Missing.push_back({Ref, Providers.front()});
            });
 
   std::vector<const include_cleaner::Include *> Unused;
   for (const include_cleaner::Include &I :
        RecordedPreprocessor.Includes.all()) {
     if (Used.contains(&I) || !I.Resolved)
       continue;
     if (RecordedPI.shouldKeep(I.Line))
for (const auto &Inc : Missing) {
     bool Angled = llvm::StringRef{Spelling}.starts_with("<");
     // We might suggest insertion of an existing include in edge cases, e.g.,
     // include is present in a PP-disabled region, or spelling of the header
     // turns out to be the same as one of the unresolved includes in the
     // main file.
     if (auto Replacement =
             HeaderIncludes.insert(llvm::StringRef{Spelling}.trim("\"<>"),
                                   Angled, tooling::IncludeDirective::Include))
-      diag(SM->getSpellingLoc(Inc.SymRefLocation),
-           "no header providing %0 is directly included")
-          << Spelling
+      diag(SM->getSpellingLoc(Inc.SymRef.RefLocation),
+           "no header providing \"%0\" is directly included")
+          << Inc.SymRef.Target.name()
           << FixItHint::CreateInsertion(
                  SM->getComposedLoc(SM->getMainFileID(),
                                     Replacement->getOffset()),
                  Replacement->getReplacementText());
   }
 }
 
 } // namespace clang::tidy::misc

clang-tools-extra/clangd/IncludeCleaner.cpp

llvm::StringRef getResolvedPath(const include_cleaner::Header &SymProvider) {
   case include_cleaner::Header::Standard:
     return SymProvider.standard().name().trim("<>\"");
   case include_cleaner::Header::Verbatim:
     return SymProvider.verbatim().trim("<>\"");
   }
   llvm_unreachable("Unknown header kind");
 }
 
-std::string getSymbolName(const include_cleaner::Symbol &Sym) {
-  switch (Sym.kind()) {
-  case include_cleaner::Symbol::Macro:
-    return Sym.macro().Name->getName().str();
-  case include_cleaner::Symbol::Declaration:
-    return llvm::dyn_cast<NamedDecl>(&Sym.declaration())
-        ->getQualifiedNameAsString();
-  }
-  llvm_unreachable("Unknown symbol kind");
-}
-
 std::vector<Diag> generateMissingIncludeDiagnostics(
     ParsedAST &AST, llvm::ArrayRef<MissingIncludeDiagInfo> MissingIncludes,
     llvm::StringRef Code, HeaderFilter IgnoreHeaders) {
   std::vector<Diag> Result;
   const SourceManager &SM = AST.getSourceManager();
   const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
 
   auto FileStyle = format::getStyle(
for (const auto &SymbolWithMissingInclude : MissingIncludes) {
     std::optional<tooling::Replacement> Replacement = HeaderIncludes.insert(
         HeaderRef.trim("\"<>"), Angled, tooling::IncludeDirective::Include);
     if (!Replacement.has_value())
       continue;
 
     Diag &D = Result.emplace_back();
     D.Message =
         llvm::formatv("No header providing \"{0}\" is directly included",
-                      getSymbolName(SymbolWithMissingInclude.Symbol));
+                      SymbolWithMissingInclude.Symbol.name());
     D.Name = "missing-includes";
     D.Source = Diag::DiagSource::Clangd;
     D.File = AST.tuPath();
     D.InsideMainFile = true;
     // We avoid the "warning" severity here in favor of LSP's "information".
     //
     // Users treat most warnings on code being edited as high-priority.
     // They don't think of include cleanups the same way: they want to edit

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h

struct Symbol {
   Symbol(const Decl &D) : Storage(&D) {}
   Symbol(struct Macro M) : Storage(M) {}
 
   Kind kind() const { return static_cast<Kind>(Storage.index()); }
   bool operator==(const Symbol &RHS) const { return Storage == RHS.Storage; }
 
   const Decl &declaration() const { return *std::get<Declaration>(Storage); }
   struct Macro macro() const { return std::get<Macro>(Storage); }
+  std::string name() const;
 
 private:
   // Order must match Kind enum!
   std::variant<const Decl *, struct Macro> Storage;
 
   // Disambiguation tag to make sure we can call the right constructor from
   // DenseMapInfo methods.
   struct SentinelTag {};

clang-tools-extra/include-cleaner/lib/Types.cpp

 #include "TypesInternal.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/FileEntry.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang::include_cleaner {
 
+std::string Symbol::name() const {
+  switch (kind()) {
+  case include_cleaner::Symbol::Macro:
+    return macro().Name->getName().str();
+  case include_cleaner::Symbol::Declaration:
+    return llvm::dyn_cast<NamedDecl>(&declaration())
+        ->getQualifiedNameAsString();
+  }
+  llvm_unreachable("Unknown symbol kind");
+}
+
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S) {
   switch (S.kind()) {
   case Symbol::Declaration:
     if (const auto *ND = llvm::dyn_cast<NamedDecl>(&S.declaration()))
       return OS << ND->getQualifiedNameAsString();
     return OS << S.declaration().getDeclKindName();
   case Symbol::Macro:
     return OS << S.macro().Name->getName();

clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp

 // RUN: %check_clang_tidy %s misc-include-cleaner %t -- -- -I%S/Inputs -isystem%S/Inputs/system
 #include "bar.h"
 // CHECK-FIXES: {{^}}#include "baz.h"{{$}}
 #include "foo.h"
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header "foo.h" is not used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 // CHECK-FIXES: {{^}}#include <string>{{$}}
 #include <vector.h>
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header <vector.h> is not used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 int BarResult = bar();
 int BazResult = baz();
-// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: no header providing "baz.h" is directly included [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: no header providing "baz" is directly included [misc-include-cleaner]
 std::string HelloString;
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: no header providing <string> is directly included [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: no header providing "std::string" is directly included [misc-include-cleaner]
 int FooBarResult = foobar();
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: no header providing "public.h" is directly included [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: no header providing "foobar" is directly included [misc-include-cleaner]