[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

1249

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

482–487

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

1249

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

Can change it for style if you like, though?

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

482–487

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

1249

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

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

482–487

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

482–487

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> SearchPathsCanonical;
+
   // 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 SearchPathsCanonical.
+  // The entries will be the same, but canonicalizing to find out is expensive!
+  if (SearchPathsCanonical.empty()) {
+    for (const auto &Dir :
+         CI.getPreprocessor().getHeaderSearchInfo().search_dir_range()) {
+      if (Dir.getLookupType() == DirectoryLookup::LT_NormalDir)
+        SearchPathsCanonical.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().get());
   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});
+  auto Converted = convertIncludes(AST);
   llvm::DenseSet<include_cleaner::Symbol> UsedSymbols;
   include_cleaner::walkUsed(
       AST.getLocalTopLevelDecls(), collectMacroReferences(AST),
       AST.getPragmaIncludes().get(), 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;
-
+        if (isPreferredProvider(Inc, Converted, Providers))
           UsedSymbols.insert(Ref.Target);
       });
 
   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,
+/// Whether this #include is considered to provide a particular symbol.
+///
+/// This means it satisfies the reference, and no other #include does better.
+/// `Providers` is the symbol's candidate headers according to walkUsed().
+bool isPreferredProvider(const Inclusion &, const include_cleaner::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().SearchPathsCanonical)
+    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).
     auto FE = SM.getFileManager().getFileRef(Inc.Resolved);
     if (!FE) {
       elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
            Inc.Resolved, FE.takeError());
       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().get(), 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->getFileEntry());
+            auto HeaderID =
+                AST.getIncludeStructure().getID(&Inc->Resolved->getFileEntry());
             assert(HeaderID.has_value() &&
                    "ConvertedIncludes only contains resolved includes.");
             Used.insert(*HeaderID);
           }
         }
 
         if (Satisfied || Providers.empty() ||
             Ref.RT != include_cleaner::RefType::Explicit)
llvm::stable_sort(MissingIncludes, [](const MissingIncludeDiagInfo &LHS,
     return MapInfo::getHashValue(LHS.Symbol) <
            MapInfo::getHashValue(RHS.Symbol);
   });
   MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector<const Inclusion *> UnusedIncludes = getUnused(AST, Used);
   return {std::move(UnusedIncludes), std::move(MissingIncludes)};
 }
 
+bool isPreferredProvider(const Inclusion &Inc,
+                         const include_cleaner::Includes &Includes,
+                         llvm::ArrayRef<include_cleaner::Header> Providers) {
+  for (const auto &H : Providers) {
+    auto Matches = Includes.match(H);
+    for (const include_cleaner::Include *Match : Matches)
+      if (Match->Line == unsigned(Inc.HashLine + 1))
+        return true; // this header is (equal) best
+    if (!Matches.empty())
+      return false; // another header is better
+  }
+  return false; // no header provides the symbol
+}
+
 std::vector<Diag>
 issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
                                const IncludeCleanerFindings &Findings,
                                HeaderFilter IgnoreHeaders) {
   trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
   std::vector<Diag> UnusedIncludes = generateUnusedIncludeDiagnostics(
       AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders);
   std::optional<Fix> RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
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);
+  auto Converted = convertIncludes(AST);
   include_cleaner::walkUsed(
       AST.getLocalTopLevelDecls(), collectMacroReferences(AST),
       AST.getPragmaIncludes().get(), 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())
+        if (Ref.RT != include_cleaner::RefType::Explicit ||
+            !isPreferredProvider(*IncludeOnLine, Converted, Providers))
           return;
 
         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));
       });
   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));
   return Results;
 }
 } // namespace
 
 ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
                                 const SymbolIndex *Index, bool AddContext) {

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().SearchPathsCanonical,
+              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

 #include "TestFS.h"
 #include "TestTU.h"
 #include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Types.h"
 #include "support/Context.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cstddef>
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"] = "";
+TEST(IncludeCleaner, IsPreferredProvider) {
+  auto TU = TestTU::withCode(R"cpp(
+     #include "decl.h"
+     #include "def.h"
+     #include "def.h"
+  )cpp");
+  TU.AdditionalFiles["decl.h"] = "";
+  TU.AdditionalFiles["def.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);
-  }
+  auto &IncludeDecl = AST.getIncludeStructure().MainFileIncludes[0];
+  auto &IncludeDef1 = AST.getIncludeStructure().MainFileIncludes[1];
+  auto &IncludeDef2 = AST.getIncludeStructure().MainFileIncludes[2];
+
+  auto &FM = AST.getSourceManager().getFileManager();
+  auto *DeclH = &FM.getOptionalFileRef("decl.h")->getFileEntry();
+  auto *DefH = &FM.getOptionalFileRef("def.h")->getFileEntry();
+
+  auto Includes = convertIncludes(AST);
+  std::vector<include_cleaner::Header> Providers = {
+      include_cleaner::Header(DefH), include_cleaner::Header(DeclH)};
+  EXPECT_FALSE(isPreferredProvider(IncludeDecl, Includes, Providers));
+  EXPECT_TRUE(isPreferredProvider(IncludeDef1, Includes, Providers));
+  EXPECT_TRUE(isPreferredProvider(IncludeDef2, Includes, Providers));
 }
 
 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");
 
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