[clangd] Loose include-cleaner matching for verbatim headers

[sammccall]

Summary

This updates clangd to take advantage of the APIs added in D155819.
The main difficulties here are around path normalization.

For layering and performance reasons Includes compares paths lexically, and so
we should have consistent paths that can be compared across addSearchPath() and
add(): symlinks resolved or not, relative or absolute.
This patch spells out that requirement, for most tools consistent use of
FileManager/HeaderSearch is enough.

For clangd this does not work: IncludeStructure doesn't hold FileEntrys due to
the preamble/main-file split. It records paths, but canonicalizes them first.
We choose to use this canonical form as our common representation, so we have
to canonicalize the directory entries too. This is done in preamble-build and
recorded in IncludeStructure, as canonicalization is quite expensive.

Left as future work: while include-cleaner correctly uses FileEntry for Header,
as it cares about inode identity, actual Includes have a single path.
They can be modeled with FileEntryRef, and probably should be.

[sammccall]

extract isPreferredProvider helper

[kadircet]

clang-tools-extra/clangd/Headers.cpp

185

nit: early exit

clang-tools-extra/clangd/Headers.h

179

s/SearchPathCanonical/SearchPathsCanonical/

clang-tools-extra/clangd/Hover.cpp

1250–1253

oops, looks like we were getting away with some dangling references.
the reference here is wrong, convertIncludes returns a value type. can you fix that while here?

clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

483–488

i don't think making this test rely on walkUsed is a good idea. what about just populating the providers directly, like:

EXPECT_TRUE(isPreferredProvider(Decl, Includes, {include_cleaner::Header{"decl.h"}, include_cleaner::Header{"def.h"}}));

and verifying we're recognizing preferred providers purely on that ordering ?

[sammccall]

clang-tools-extra/clangd/Headers.cpp

185

I don't think it makes sense here, this isn't "the remaining case" for this function, it's just one of many things this function does

clang-tools-extra/clangd/Hover.cpp

1250–1253

this isn't dangling, it's lifetime extension.

Can change it for style if you like, though?

clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

483–488

That would be a strict unit test, but I think it's too easy to lose track of how this idea of "preferred" relates to the code.

For example, when writing this test, I expected Y to match the includes for both Decl and Def, because they're intuitively both "best matches" and we do allow multiple includes to match.
But of course we force the *providers* to be ranked, so really ~only multiple includes of the *same* header will be accepted as all-preferred.

This wrinkle seems worth recording, and totally disappears if we make the test abstract by describing a provider list directly.

(Can do it though if that's what you do prefer)

[kadircet]

thanks!

clang-tools-extra/clangd/Hover.cpp

1250–1253

you're right. i think it would be better to have it as value though.

clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

483–488

i completely agree with what you said, but I don't see why ranking should matter here. i do feel like all of those concerns you raised can still be inferred from the fact that we're providing a set of ordered providers, but maybe it's just me. i just feel like the downside is we should update this test if ranking for providers changes for whatever reason, and i don't think we should. but that is not a big deal, so feel free to keep it as-is.

[sammccall]

clang-tools-extra/clangd/Headers.h

179

Huh, I'd always thought of "search path" singular, like each entry is edges along a path, which makes no sense.
Maybe it's path like a reference to $PATH, which has a singular name for whatever reason.

Anyway, done.

clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

483–488

i completely agree with what you said, but I don't see why ranking should matter here.

I guess I'm basically saying that this function does something that seems reasonable, and the ranking function does something that seems reasonable, but the result when you plug them together is surprising.

That makes me want to write a test that has them plugged together, but I don't feel strongly. Changed it back.

clang-tools-extra/clangd/Headers.h

public:
   // Maps HeaderID to the ids of the files included from it.
   llvm::DenseMap<HeaderID, SmallVector<HeaderID>> IncludeChildren;
 
   llvm::DenseMap<tooling::stdlib::Header, llvm::SmallVector<HeaderID>>
       StdlibHeaders;
 
   std::vector<Inclusion> MainFileIncludes;
 
+  // The entries of the header search path. (HeaderSearch::search_dir_range())
+  // Only includes the plain-directory entries (not header maps or frameworks).
+  // All paths are canonical (FileManager::getCanonicalPath()).
+  std::vector<std::string> SearchPathCanonical;
+
   // We reserve HeaderID(0) for the main file and will manually check for that
   // in getID and getOrCreateID because the UniqueID is not stable when the
   // content of the main file changes.
   static const HeaderID MainFileID = HeaderID(0u);
 
   class RecordHeaders;
 
 private:

clang-tools-extra/clangd/Headers.cpp

 //===--- Headers.cpp - Include headers ---------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Headers.h"
 #include "Preamble.h"
 #include "SourceCode.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/DirectoryLookup.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Inclusions/HeaderAnalysis.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
 #include <cstring>
llvm::SmallVector<SymbolInclude, 1> getRankedIncludes(const Symbol &Sym) {
   return Headers;
 }
 
 void IncludeStructure::collect(const CompilerInstance &CI) {
   auto &SM = CI.getSourceManager();
   MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
   auto Collector = std::make_unique<RecordHeaders>(CI, this);
   CI.getPreprocessor().addPPCallbacks(std::move(Collector));
+
+  // If we're reusing a preamble, don't repopulate SearchPathCanonical.
+  // The entries will be the same, but canonicalizing to find out is expensive!
+  if (SearchPathCanonical.empty()) {
+    for (const auto &Dir :
+         CI.getPreprocessor().getHeaderSearchInfo().search_dir_range()) {
+      if (Dir.getLookupType() == DirectoryLookup::LT_NormalDir)
+        SearchPathCanonical.emplace_back(
+          SM.getFileManager().getCanonicalName(*Dir.getDirRef()));
+    }
+  }
 }
 
 std::optional<IncludeStructure::HeaderID>
 IncludeStructure::getID(const FileEntry *Entry) const {
   // HeaderID of the main file is always 0;
   if (Entry == MainFileEntry) {
     return static_cast<IncludeStructure::HeaderID>(0u);
   }

clang-tools-extra/clangd/Hover.cpp

void maybeAddSymbolProviders(ParsedAST &AST, HoverInfo &HI,
 
   const SourceManager &SM = AST.getSourceManager();
   llvm::SmallVector<include_cleaner::Header> RankedProviders =
       include_cleaner::headersForSymbol(Sym, SM, AST.getPragmaIncludes());
   if (RankedProviders.empty())
     return;
 
   std::string Result;
-  include_cleaner::Includes ConvertedIncludes =
-      convertIncludes(SM, AST.getIncludeStructure().MainFileIncludes);
+  include_cleaner::Includes ConvertedIncludes = convertIncludes(AST);
   for (const auto &P : RankedProviders) {
     if (P.kind() == include_cleaner::Header::Physical &&
         P.physical() == SM.getFileEntryForID(SM.getMainFileID()))
       // Main file ranked higher than any #include'd file
       break;
 
     // Pick the best-ranked #include'd provider
     auto Matches = ConvertedIncludes.match(P);
case include_cleaner::Symbol::Macro:
     Name = Sym.macro().Name->getName();
     break;
   }
   return Name;
 }
 
 void maybeAddUsedSymbols(ParsedAST &AST, HoverInfo &HI, const Inclusion &Inc) {
   const SourceManager &SM = AST.getSourceManager();
-  const auto &ConvertedMainFileIncludes =
-      convertIncludes(SM, AST.getIncludeStructure().MainFileIncludes);
-  const auto &HoveredInclude = convertIncludes(SM, llvm::ArrayRef{Inc});
+  const auto &Converted = convertIncludes(AST);
+  const auto *Target = Converted.atLine(Inc.HashLine + 1);
+  if (Target == nullptr) // conversion can skip some broken inclueds
+    return;
   llvm::DenseSet<include_cleaner::Symbol> UsedSymbols;
   include_cleaner::walkUsed(
       AST.getLocalTopLevelDecls(), collectMacroReferences(AST),
       AST.getPragmaIncludes(), SM,
       [&](const include_cleaner::SymbolReference &Ref,
           llvm::ArrayRef<include_cleaner::Header> Providers) {
         if (Ref.RT != include_cleaner::RefType::Explicit ||
             UsedSymbols.contains(Ref.Target))
           return;
 
-        auto Provider =
-            firstMatchedProvider(ConvertedMainFileIncludes, Providers);
-        if (!Provider || HoveredInclude.match(*Provider).empty())
-          return;
-
+        for (const auto &H : Providers) {
+          auto Matches = Converted.match(H);
+          if (llvm::is_contained(Matches, Target))
             UsedSymbols.insert(Ref.Target);
+          if (!Matches.empty())
+            break;
+        }
       });
 
   for (const auto &UsedSymbolDecl : UsedSymbols)
     HI.UsedSymbolNames.push_back(getSymbolName(UsedSymbolDecl));
   llvm::sort(HI.UsedSymbolNames);
   HI.UsedSymbolNames.erase(
       std::unique(HI.UsedSymbolNames.begin(), HI.UsedSymbolNames.end()),
       HI.UsedSymbolNames.end());

clang-tools-extra/clangd/IncludeCleaner.h

 /// - symbol names without a unique associated header are not tracked
 /// - references to std-namespaced C types are not properly tracked:
 ///   instead of std::size_t -> <cstddef> we see ::size_t -> <stddef.h>
 /// FIXME: remove this hack once the implementation is good enough.
 void setIncludeCleanerAnalyzesStdlib(bool B);
 
 /// Converts the clangd include representation to include-cleaner
 /// include representation.
-include_cleaner::Includes
-convertIncludes(const SourceManager &SM,
-                const llvm::ArrayRef<Inclusion> Includes);
+include_cleaner::Includes convertIncludes(const ParsedAST &);
 
 std::vector<include_cleaner::SymbolReference>
 collectMacroReferences(ParsedAST &AST);
 
-/// Find the first provider in the list that is matched by the includes.
-std::optional<include_cleaner::Header>
-firstMatchedProvider(const include_cleaner::Includes &Includes,
-                     llvm::ArrayRef<include_cleaner::Header> Providers);
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDECLEANER_H

clang-tools-extra/clangd/IncludeCleaner.cpp

 #include "support/Path.h"
 #include "support/Trace.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
+#include "clang/Lex/DirectoryLookup.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Inclusions/HeaderIncludes.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
for (const auto &Ref : Refs) {
            Ref.InConditionalDirective ? include_cleaner::RefType::Ambiguous
                                       : include_cleaner::RefType::Explicit});
     }
   }
 
   return Macros;
 }
 
-include_cleaner::Includes
-convertIncludes(const SourceManager &SM,
-                const llvm::ArrayRef<Inclusion> Includes) {
+include_cleaner::Includes convertIncludes(const ParsedAST &AST) {
+  auto &SM = AST.getSourceManager();
+
   include_cleaner::Includes ConvertedIncludes;
-  for (const Inclusion &Inc : Includes) {
+  // We satisfy Includes's contract that search dirs and included files have
+  // matching path styles: both ultimately use FileManager::getCanonicalName().
+  for (const auto &Dir : AST.getIncludeStructure().SearchPathCanonical)
+    ConvertedIncludes.addSearchDirectory(Dir);
+
+  for (const Inclusion &Inc : AST.getIncludeStructure().MainFileIncludes) {
     include_cleaner::Include TransformedInc;
     llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written);
     TransformedInc.Spelled = WrittenRef.trim("\"<>");
     TransformedInc.HashLocation =
         SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
     TransformedInc.Line = Inc.HashLine + 1;
     TransformedInc.Angled = WrittenRef.starts_with("<");
+    // Inc.Resolved is canonicalized with clangd::getCanonicalPath(),
+    // which is based on FileManager::getCanonicalName(ParentDir).
+    // Calling getFile() updates FileEntry::getName() to match this path.
+    // FIXME: make this explicit by using FileEntryRef instead.
     auto FE = SM.getFileManager().getFile(Inc.Resolved);
     if (!FE) {
       elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
            Inc.Resolved, FE.getError().message());
       continue;
     }
     TransformedInc.Resolved = *FE;
     ConvertedIncludes.add(std::move(TransformedInc));
   }
   return ConvertedIncludes;
 }
 
 IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
   // Interaction is only polished for C/CPP.
   if (AST.getLangOpts().ObjC)
     return {};
   const auto &SM = AST.getSourceManager();
-  const auto &Includes = AST.getIncludeStructure();
-  include_cleaner::Includes ConvertedIncludes =
-      convertIncludes(SM, Includes.MainFileIncludes);
+  include_cleaner::Includes ConvertedIncludes = convertIncludes(AST);
   const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
   auto *PreamblePatch = PreamblePatch::getPatchEntry(AST.tuPath(), SM);
 
   std::vector<include_cleaner::SymbolReference> Macros =
       collectMacroReferences(AST);
   std::vector<MissingIncludeDiagInfo> MissingIncludes;
   llvm::DenseSet<IncludeStructure::HeaderID> Used;
   trace::Span Tracer("include_cleaner::walkUsed");
   include_cleaner::walkUsed(
       AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
       AST.getPragmaIncludes(), SM,
       [&](const include_cleaner::SymbolReference &Ref,
           llvm::ArrayRef<include_cleaner::Header> Providers) {
         bool Satisfied = false;
         for (const auto &H : Providers) {
           if (H.kind() == include_cleaner::Header::Physical &&
               (H.physical() == MainFile || H.physical() == PreamblePatch)) {
             Satisfied = true;
             continue;
           }
           for (auto *Inc : ConvertedIncludes.match(H)) {
             Satisfied = true;
-            auto HeaderID = Includes.getID(Inc->Resolved);
+            auto HeaderID = AST.getIncludeStructure().getID(Inc->Resolved);
             assert(HeaderID.has_value() &&
                    "ConvertedIncludes only contains resolved includes.");
             Used.insert(*HeaderID);
           }
         }
 
         if (Satisfied || Providers.empty() ||
             Ref.RT != include_cleaner::RefType::Explicit)
for (auto &Diag : UnusedIncludes) {
     AddBatchFix(FixAll, &Diag);
   }
 
   auto Result = std::move(MissingIncludeDiags);
   llvm::move(UnusedIncludes, std::back_inserter(Result));
   return Result;
 }
 
-std::optional<include_cleaner::Header>
-firstMatchedProvider(const include_cleaner::Includes &Includes,
-                     llvm::ArrayRef<include_cleaner::Header> Providers) {
-  for (const auto &H : Providers) {
-    if (!Includes.match(H).empty())
-      return H;
-  }
-  // No match for this provider in the includes list.
-  return std::nullopt;
-}
 } // namespace clang::clangd

clang-tools-extra/clangd/XRefs.cpp

maybeFindIncludeReferences(ParsedAST &AST, Position Pos,
                            URIForFile URIMainFile) {
   const auto &Includes = AST.getIncludeStructure().MainFileIncludes;
   auto IncludeOnLine = llvm::find_if(Includes, [&Pos](const Inclusion &Inc) {
     return Inc.HashLine == Pos.line;
   });
   if (IncludeOnLine == Includes.end())
     return std::nullopt;
 
-  const auto &Inc = *IncludeOnLine;
   const SourceManager &SM = AST.getSourceManager();
   ReferencesResult Results;
-  auto ConvertedMainFileIncludes = convertIncludes(SM, Includes);
-  auto ReferencedInclude = convertIncludes(SM, Inc);
-  include_cleaner::walkUsed(
-      AST.getLocalTopLevelDecls(), collectMacroReferences(AST),
-      AST.getPragmaIncludes(), SM,
-      [&](const include_cleaner::SymbolReference &Ref,
-          llvm::ArrayRef<include_cleaner::Header> Providers) {
-        if (Ref.RT != include_cleaner::RefType::Explicit)
-          return;
-
-        auto Provider =
-            firstMatchedProvider(ConvertedMainFileIncludes, Providers);
-        if (!Provider || ReferencedInclude.match(*Provider).empty())
-          return;
+  auto Converted = convertIncludes(AST);
+  auto *Target = Converted.atLine(IncludeOnLine->HashLine + 1);
+  if (!Target)
+    return std::nullopt;
 
+  auto AddResult =
+      [&](const include_cleaner::SymbolReference &Ref) {
         auto Loc = SM.getFileLoc(Ref.RefLocation);
         // File locations can be outside of the main file if macro is
         // expanded through an #include.
         while (SM.getFileID(Loc) != SM.getMainFileID())
           Loc = SM.getIncludeLoc(SM.getFileID(Loc));
 
         ReferencesResult::Reference Result;
         const auto *Token = AST.getTokens().spelledTokenAt(Loc);
         Result.Loc.range = Range{sourceLocToPosition(SM, Token->location()),
                                  sourceLocToPosition(SM, Token->endLocation())};
         Result.Loc.uri = URIMainFile;
         Results.References.push_back(std::move(Result));
+      };
+  include_cleaner::walkUsed(
+      AST.getLocalTopLevelDecls(), collectMacroReferences(AST),
+      AST.getPragmaIncludes(), SM,
+      [&](const include_cleaner::SymbolReference &Ref,
+          llvm::ArrayRef<include_cleaner::Header> Providers) {
+        if (Ref.RT != include_cleaner::RefType::Explicit)
+          return;
+        for (const auto &H : Providers) {
+          auto Matches = Converted.match(H);
+          if (llvm::is_contained(Matches, Target))
+            AddResult(Ref);
+          if (!Matches.empty())
+            break;
+        }
       });
   if (Results.References.empty())
     return std::nullopt;
 
   // Add the #include line to the references list.
   ReferencesResult::Reference Result;
-  Result.Loc.range =
-      rangeTillEOL(SM.getBufferData(SM.getMainFileID()), Inc.HashOffset);
+  Result.Loc.range = rangeTillEOL(SM.getBufferData(SM.getMainFileID()),
+                                  IncludeOnLine->HashOffset);
   Result.Loc.uri = URIMainFile;
   Results.References.push_back(std::move(Result));
 
   if (Results.References.empty())
     return std::nullopt;
   return Results;
 }
 } // namespace

clang-tools-extra/clangd/unittests/HeadersTests.cpp

)cpp";
   // ms-compatibility changes meaning of #import, make sure it is turned off.
   CDB.ExtraClangFlags.push_back("-fno-ms-compatibility");
   EXPECT_THAT(collectIncludes().MainFileIncludes,
               UnorderedElementsAre(directive(tok::pp_include),
                                    directive(tok::pp_import),
                                    directive(tok::pp_include_next)));
 }
 
+TEST_F(HeadersTest, SearchPath) {
+  FS.Files["foo/bar.h"] = "x";
+  FS.Files["foo/bar/baz.h"] = "y";
+  CDB.ExtraClangFlags.push_back("-Ifoo/bar");
+  CDB.ExtraClangFlags.push_back("-Ifoo/bar/..");
+  EXPECT_THAT(collectIncludes().SearchPathCanonical,
+              ElementsAre(Subdir, testPath("foo/bar"), testPath("foo")));
+}
+
 TEST_F(HeadersTest, InsertInclude) {
   std::string Path = testPath("sub/bar.h");
   FS.Files[Path] = "";
   EXPECT_EQ(calculate(Path), "\"bar.h\"");
 }
 
 TEST_F(HeadersTest, DoNotInsertIfInSameFile) {
   MainFile = testPath("main.h");

clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

const auto &MissingIncludes =
       computeIncludeCleanerFindings(AST).MissingIncludes;
   EXPECT_THAT(MissingIncludes, testing::SizeIs(1));
   auto &SM = AST.getSourceManager();
   EXPECT_EQ(
       halfOpenToRange(SM, MissingIncludes.front().SymRefRange.toCharRange(SM)),
       MainCode.range());
 }
 
-TEST(IncludeCleaner, FirstMatchedProvider) {
-  struct {
-    const char *Code;
-    const std::vector<include_cleaner::Header> Providers;
-    const std::optional<include_cleaner::Header> ExpectedProvider;
-  } Cases[] = {
-      {R"cpp(
-        #include "bar.h"
-        #include "foo.h"
-      )cpp",
-       {include_cleaner::Header{"bar.h"}, include_cleaner::Header{"foo.h"}},
-       include_cleaner::Header{"bar.h"}},
-      {R"cpp(
-        #include "bar.h"
-        #include "foo.h"
-      )cpp",
-       {include_cleaner::Header{"foo.h"}, include_cleaner::Header{"bar.h"}},
-       include_cleaner::Header{"foo.h"}},
-      {"#include \"bar.h\"",
-       {include_cleaner::Header{"bar.h"}},
-       include_cleaner::Header{"bar.h"}},
-      {"#include \"bar.h\"", {include_cleaner::Header{"foo.h"}}, std::nullopt},
-      {"#include \"bar.h\"", {}, std::nullopt}};
-  for (const auto &Case : Cases) {
-    Annotations Code{Case.Code};
-    SCOPED_TRACE(Code.code());
-
-    TestTU TU;
-    TU.Code = Code.code();
-    TU.AdditionalFiles["bar.h"] = "";
-    TU.AdditionalFiles["foo.h"] = "";
-
-    auto AST = TU.build();
-    std::optional<include_cleaner::Header> MatchedProvider =
-        firstMatchedProvider(
-            convertIncludes(AST.getSourceManager(),
-                            AST.getIncludeStructure().MainFileIncludes),
-            Case.Providers);
-    EXPECT_EQ(MatchedProvider, Case.ExpectedProvider);
-  }
-}
-
 TEST(IncludeCleaner, BatchFix) {
   TestTU TU;
   TU.Filename = "main.cpp";
   TU.AdditionalFiles["foo.h"] = guard("class Foo;");
   TU.AdditionalFiles["bar.h"] = guard("class Bar;");
   TU.AdditionalFiles["all.h"] = guard(R"cpp(
     #include "foo.h"
     #include "bar.h"
   )cpp");
 
   TU.Code = R"cpp(
   #include "all.h"
 
   Foo* foo;
   )cpp";
   auto AST = TU.build();
   EXPECT_THAT(
       issueIncludeCleanerDiagnostics(AST, TU.Code,
                                      computeIncludeCleanerFindings(AST)),
       UnorderedElementsAre(withFix({FixMessage("#include \"foo.h\""),
                                     FixMessage("fix all includes")}),
                            withFix({FixMessage("remove #include directive"),
                                     FixMessage("fix all includes")})));
 
   TU.Code = R"cpp(
   #include "all.h"
   #include "bar.h"
 
   Foo* foo;
   )cpp";
   AST = TU.build();
   EXPECT_THAT(
       issueIncludeCleanerDiagnostics(AST, TU.Code,
                                      computeIncludeCleanerFindings(AST)),
       UnorderedElementsAre(withFix({FixMessage("#include \"foo.h\""),
EXPECT_THAT(
                                     FixMessage("fix all includes")}),
                            withFix({FixMessage("#include \"bar.h\""),
                                     FixMessage("add all missing includes"),
                                     FixMessage("fix all includes")}),
                            withFix({FixMessage("remove #include directive"),
                                     FixMessage("fix all includes")})));
 }
 
+// In the presence of IWYU pragma private, we should accept spellings other
+// than the recommended one if they appear to name the same public header.
+TEST(IncludeCleaner, VerbatimEquivalence) {
+  auto TU = TestTU::withCode(R"cpp(
+    #include "lib/rel/public.h"
+    int x = Public;
+  )cpp");
+  TU.AdditionalFiles["repo/lib/rel/private.h"] = R"cpp(
+    #pragma once
+    // IWYU pragma: private, include "rel/public.h"
+    int Public;
+  )cpp";
+  TU.AdditionalFiles["repo/lib/rel/public.h"] = R"cpp(
+    #pragma once
+    #include "rel/private.h"
+  )cpp";
+
+  TU.ExtraArgs.push_back("-Irepo");
+  TU.ExtraArgs.push_back("-Irepo/lib");
+
+  auto AST = TU.build();
+  auto Findings = computeIncludeCleanerFindings(AST);
+  EXPECT_THAT(Findings.MissingIncludes, IsEmpty());
+  EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

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

 
 /// A container for all includes present in a file.
 /// Supports efficiently hit-testing Headers against Includes.
 class Includes {
 public:
   /// Registers a directory on the include path (-I etc) from HeaderSearch.
   /// This allows reasoning about equivalence of e.g. "path/a/b.h" and "a/b.h".
   /// This must be called before calling add() in order to take effect.
+  ///
+  /// The paths may be relative or absolute, but the paths passed to
+  /// addSearchDirectory() and add() (that is: Include.Resolved->getName())
+  /// should be consistent, as they are compared lexically.
+  /// Generally, this is satisfied if you obtain paths through HeaderSearch
+  /// and FileEntries through PPCallbacks::IncludeDirective().
+  ///
+  /// FIXME: change Include to use FileEntryRef so callers don't have to worry
+  ///        about the behavior of FileEntry::getName() to follow this contract.
   void addSearchDirectory(llvm::StringRef);
 
+  /// Registers an include directive seen in the main file.
+  ///
+  /// This should only be called after all search directories are added.
   void add(const Include &);
 
   /// All #includes seen, in the order they appear.
   llvm::ArrayRef<Include> all() const { return All; }
 
   /// Determine #includes that match a header (that provides a used symbol).
   ///
   /// Matching is based on the type of Header specified: