[clangd] UI for completion items that would trigger include insertion.

Summary:
For completion items that would trigger include insertions (i.e. index symbols
that are not #included yet), add a visual indicator "+" before the completion
label. The inserted headers will appear in the completion detail.

Open to suggestions for better visual indicators; "+" was picked because it
seems cleaner than a few other candidates I've tried (*, #, @ ...).

The displayed header would be like a/b/c.h (without quote) or <vector> for system
headers. I didn't add quotation or "#include" because they can take up limited
space and do not provide additional information after users know what the
headers are. I think a header alone should be obvious for users to infer that
this is an include header..

To align indentation, also prepend ' ' to labels of candidates that would not
trigger include insertions (only for completions where index results are
possible).

Vim:
{F6357587}

vscode:
{F6357589}
{F6357591}

Reviewers: sammccall, ilya-biryukov, hokein

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, cfe-commits

Differential Revision: https://reviews.llvm.org/D48163

9 files changed, 133 insertions, 137 deletions

diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 97aa3e4a0da..1a654eb5028 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -240,10 +240,11 @@ struct CompletionCandidate {
   CompletionItem build(StringRef FileName, const CompletionItemScores &Scores,
                        const CodeCompleteOptions &Opts,
                        CodeCompletionString *SemaCCS,
-                       const IncludeInserter *Includes,
+                       const IncludeInserter &Includes,
                        llvm::StringRef SemaDocComment) const {
     assert(bool(SemaResult) == bool(SemaCCS));
     CompletionItem I;
+    bool InsertingInclude = false; // Whether a new #include will be added.
     if (SemaResult) {
       I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->Declaration);
       getLabelAndInsertText(*SemaCCS, &I.label, &I.insertText,
@@ -273,7 +274,7 @@ struct CompletionCandidate {
         if (I.detail.empty())
           I.detail = D->CompletionDetail;
         if (auto Inserted = headerToInsertIfNotPresent()) {
-          auto Edit = [&]() -> Expected<Optional<TextEdit>> {
+          auto IncludePath = [&]() -> Expected<std::string> {
             auto ResolvedDeclaring = toHeaderFile(
                 IndexResult->CanonicalDeclaration.FileURI, FileName);
             if (!ResolvedDeclaring)
@@ -281,9 +282,13 @@ struct CompletionCandidate {
             auto ResolvedInserted = toHeaderFile(*Inserted, FileName);
             if (!ResolvedInserted)
               return ResolvedInserted.takeError();
-            return Includes->insert(*ResolvedDeclaring, *ResolvedInserted);
+            if (!Includes.shouldInsertInclude(*ResolvedDeclaring,
+                                              *ResolvedInserted))
+              return "";
+            return Includes.calculateIncludePath(*ResolvedDeclaring,
+                                                 *ResolvedInserted);
           }();
-          if (!Edit) {
+          if (!IncludePath) {
             std::string ErrMsg =
                 ("Failed to generate include insertion edits for adding header "
                  "(FileURI=\"" +
@@ -291,13 +296,22 @@ struct CompletionCandidate {
                  "\", IncludeHeader=\"" + D->IncludeHeader + "\") into " +
                  FileName)
                     .str();
-            log(ErrMsg + ":" + llvm::toString(Edit.takeError()));
-          } else if (*Edit) {
-            I.additionalTextEdits = {std::move(**Edit)};
+            log(ErrMsg + ":" + llvm::toString(IncludePath.takeError()));
+          } else if (!IncludePath->empty()) {
+            // FIXME: consider what to show when there is no #include insertion,
+            // and for sema results, for consistency.
+            if (auto Edit = Includes.insert(*IncludePath)) {
+              I.detail += ("\n" + StringRef(*IncludePath).trim('"')).str();
+              I.additionalTextEdits = {std::move(*Edit)};
+              InsertingInclude = true;
+            }
           }
         }
       }
     }
+    I.label = (InsertingInclude ? Opts.IncludeIndicator.Insert
+                                : Opts.IncludeIndicator.NoInsert) +
+              I.label;
     I.scoreInfo = Scores;
     I.sortText = sortText(Scores.finalScore, Name);
     I.insertTextFormat = Opts.EnableSnippets ? InsertTextFormat::Snippet
@@ -318,7 +332,13 @@ struct CompletionCandidate {
     llvm::StringRef Name = Bundle.front().Name;
     First.insertText =
         Opts.EnableSnippets ? (Name + "(${0})").str() : Name.str();
-    First.label = (Name + "(…)").str();
+    // Keep the visual indicator of the original label.
+    bool InsertingInclude =
+        StringRef(First.label).startswith(Opts.IncludeIndicator.Insert);
+    First.label = (Twine(InsertingInclude ? Opts.IncludeIndicator.Insert
+                                          : Opts.IncludeIndicator.NoInsert) +
+                   Name + "(…)")
+                      .str();
     First.detail = llvm::formatv("[{0} overloads]", Bundle.size());
     return First;
   }
@@ -964,7 +984,9 @@ private:
     //        explicitly request symbols corresponding to Sema results.
     //        We can use their signals even if the index can't suggest them.
     // We must copy index results to preserve them, but there are at most Limit.
-    auto IndexResults = queryIndex();
+    auto IndexResults = (Opts.Index && allowIndex(Recorder->CCContext))
+                            ? queryIndex()
+                            : SymbolSlab();
     // Merge Sema and Index results, score them, and pick the winners.
     auto Top = mergeResults(Recorder->Results, IndexResults);
     // Convert the results to the desired LSP structs.
@@ -976,8 +998,6 @@ private:
   }
 
   SymbolSlab queryIndex() {
-    if (!Opts.Index || !allowIndex(Recorder->CCContext))
-      return SymbolSlab();
     trace::Span Tracer("Query index");
     SPAN_ATTACH(Tracer, "limit", Opts.Limit);
 
@@ -1127,7 +1147,7 @@ private:
     }
     return CompletionCandidate::build(
         Bundle,
-        Bundle.front().build(FileName, Scores, Opts, SemaCCS, Includes.get(),
+        Bundle.front().build(FileName, Scores, Opts, SemaCCS, *Includes,
                              FrontDocComment),
         Opts);
   }
diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h
index 4fb19cb1073..f89aad76cfe 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -60,6 +60,13 @@ struct CodeCompleteOptions {
   /// If more results are available, we set CompletionList.isIncomplete.
   size_t Limit = 0;
 
+  /// A visual indicator to prepend to the completion label to indicate whether
+  /// completion result would trigger an #include insertion or not.
+  struct IncludeInsertionIndicator {
+    std::string Insert = "•";
+    std::string NoInsert = " ";
+  } IncludeIndicator;
+
   // Populated internally by clangd, do not set.
   /// If `Index` is set, it is used to augment the code completion
   /// results.
diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp
index c6f18fa3933..35ecbf3d5f4 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -72,13 +72,11 @@ collectInclusionsInMainFileCallback(const SourceManager &SM,
 
 /// FIXME(ioeric): we might not want to insert an absolute include path if the
 /// path is not shortened.
-llvm::Expected<std::string> calculateIncludePath(
-    PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo,
-    const std::vector<Inclusion> &Inclusions, const HeaderFile &DeclaringHeader,
-    const HeaderFile &InsertedHeader) {
+bool IncludeInserter::shouldInsertInclude(
+    const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader) const {
   assert(DeclaringHeader.valid() && InsertedHeader.valid());
-  if (File == DeclaringHeader.File || File == InsertedHeader.File)
-    return "";
+  if (FileName == DeclaringHeader.File || FileName == InsertedHeader.File)
+    return false;
   llvm::StringSet<> IncludedHeaders;
   for (const auto &Inc : Inclusions) {
     IncludedHeaders.insert(Inc.Written);
@@ -88,53 +86,31 @@ llvm::Expected<std::string> calculateIncludePath(
   auto Included = [&](llvm::StringRef Header) {
     return IncludedHeaders.find(Header) != IncludedHeaders.end();
   };
-  if (Included(DeclaringHeader.File) || Included(InsertedHeader.File))
-    return "";
-
-  bool IsSystem = false;
+  return !Included(DeclaringHeader.File) && !Included(InsertedHeader.File);
+}
 
+std::string
+IncludeInserter::calculateIncludePath(const HeaderFile &DeclaringHeader,
+                                      const HeaderFile &InsertedHeader) const {
+  assert(DeclaringHeader.valid() && InsertedHeader.valid());
   if (InsertedHeader.Verbatim)
     return InsertedHeader.File;
-
+  bool IsSystem = false;
   std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics(
       InsertedHeader.File, BuildDir, &IsSystem);
   if (IsSystem)
     Suggested = "<" + Suggested + ">";
   else
     Suggested = "\"" + Suggested + "\"";
-
-  log("Suggested #include for " + InsertedHeader.File + " is: " + Suggested);
   return Suggested;
 }
 
-Expected<Optional<TextEdit>>
-IncludeInserter::insert(const HeaderFile &DeclaringHeader,
-                        const HeaderFile &InsertedHeader) const {
-  auto Validate = [](const HeaderFile &Header) {
-    return Header.valid()
-               ? llvm::Error::success()
-               : llvm::make_error<llvm::StringError>(
-                     "Invalid HeaderFile: " + Header.File +
-                         " (verbatim=" + std::to_string(Header.Verbatim) + ").",
-                     llvm::inconvertibleErrorCode());
-  };
-  if (auto Err = Validate(DeclaringHeader))
-    return std::move(Err);
-  if (auto Err = Validate(InsertedHeader))
-    return std::move(Err);
-  auto Include =
-      calculateIncludePath(FileName, BuildDir, HeaderSearchInfo, Inclusions,
-                           DeclaringHeader, InsertedHeader);
-  if (!Include)
-    return Include.takeError();
-  if (Include->empty())
-    return llvm::None;
-  StringRef IncludeRef = *Include;
-  auto Insertion =
-      Inserter.insert(IncludeRef.trim("\"<>"), IncludeRef.startswith("<"));
-  if (!Insertion)
-    return llvm::None;
-  return replacementToEdit(Code, *Insertion);
+Optional<TextEdit> IncludeInserter::insert(StringRef VerbatimHeader) const {
+  Optional<TextEdit> Edit = None;
+  if (auto Insertion = Inserter.insert(VerbatimHeader.trim("\"<>"),
+                                       VerbatimHeader.startswith("<")))
+    Edit = replacementToEdit(Code, *Insertion);
+  return Edit;
 }
 
 } // namespace clangd
diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h
index 987832fc92e..3f92c3a120b 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -50,25 +50,6 @@ std::unique_ptr<PPCallbacks>
 collectInclusionsInMainFileCallback(const SourceManager &SM,
                                     std::function<void(Inclusion)> Callback);
 
-/// Determines the preferred way to #include a file, taking into account the
-/// search path. Usually this will prefer a shorter representation like
-/// 'Foo/Bar.h' over a longer one like 'Baz/include/Foo/Bar.h'.
-///
-/// \param File is an absolute file path.
-/// \param Inclusions Existing inclusions in the main file.
-/// \param DeclaringHeader is the original header corresponding to \p
-/// InsertedHeader e.g. the header that declares a symbol.
-/// \param InsertedHeader The preferred header to be inserted. This could be the
-/// same as DeclaringHeader but must be provided.
-//  \return A quoted "path" or <path>. This returns an empty string if:
-///   - Either \p DeclaringHeader or \p InsertedHeader is already (directly)
-///   in \p Inclusions (including those included via different paths).
-///   - \p DeclaringHeader or \p InsertedHeader is the same as \p File.
-llvm::Expected<std::string> calculateIncludePath(
-    PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo,
-    const std::vector<Inclusion> &Inclusions, const HeaderFile &DeclaringHeader,
-    const HeaderFile &InsertedHeader);
-
 // Calculates insertion edit for including a new header in a file.
 class IncludeInserter {
 public:
@@ -81,16 +62,36 @@ public:
 
   void addExisting(Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }
 
-  /// Returns a TextEdit that inserts a new header; if the header is not
-  /// inserted e.g. it's an existing header, this returns None. If any header is
-  /// invalid, this returns error.
+  /// Checks whether to add an #include of the header into \p File.
+  /// An #include will not be added if:
+  ///   - Either \p DeclaringHeader or \p InsertedHeader is already (directly)
+  ///   in \p Inclusions (including those included via different paths).
+  ///   - \p DeclaringHeader or \p InsertedHeader is the same as \p File.
+  ///
+  /// \param DeclaringHeader is the original header corresponding to \p
+  /// InsertedHeader e.g. the header that declares a symbol.
+  /// \param InsertedHeader The preferred header to be inserted. This could be
+  /// the same as DeclaringHeader but must be provided. \param Inclusions
+  /// Existing includes in the main file.
+  bool shouldInsertInclude(const HeaderFile &DeclaringHeader,
+                           const HeaderFile &InsertedHeader) const;
+
+  /// Determines the preferred way to #include a file, taking into account the
+  /// search path. Usually this will prefer a shorter representation like
+  /// 'Foo/Bar.h' over a longer one like 'Baz/include/Foo/Bar.h'.
   ///
   /// \param DeclaringHeader is the original header corresponding to \p
   /// InsertedHeader e.g. the header that declares a symbol.
   /// \param InsertedHeader The preferred header to be inserted. This could be
   /// the same as DeclaringHeader but must be provided.
-  Expected<Optional<TextEdit>> insert(const HeaderFile &DeclaringHeader,
-                                      const HeaderFile &InsertedHeader) const;
+  ///
+  /// \return A quoted "path" or <path> to be included.
+  std::string calculateIncludePath(const HeaderFile &DeclaringHeader,
+                                   const HeaderFile &InsertedHeader) const;
+
+  /// Calculates an edit that inserts \p VerbatimHeader into code. If the header
+  /// is already included, this returns None.
+  llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader) const;
 
 private:
   StringRef FileName;
diff --git a/clang-tools-extra/test/clangd/completion-snippets.test b/clang-tools-extra/test/clangd/completion-snippets.test
index 1b9a24e0565..06896319e15 100644
--- a/clang-tools-extra/test/clangd/completion-snippets.test
+++ b/clang-tools-extra/test/clangd/completion-snippets.test
@@ -32,7 +32,7 @@
 # CHECK-NEXT:      "insertText": "func_with_args(${1:int a}, ${2:int b})",
 # CHECK-NEXT:      "insertTextFormat": 2,
 # CHECK-NEXT:      "kind": 3,
-# CHECK-NEXT:      "label": "func_with_args(int a, int b)",
+# CHECK-NEXT:      "label": " func_with_args(int a, int b)",
 # CHECK-NEXT:      "sortText": "{{.*}}func_with_args"
 # CHECK-NEXT:    }
 # CHECK-NEXT:    ]
diff --git a/clang-tools-extra/test/clangd/completion.test b/clang-tools-extra/test/clangd/completion.test
index 441cc7df63d..c679e328394 100644
--- a/clang-tools-extra/test/clangd/completion.test
+++ b/clang-tools-extra/test/clangd/completion.test
@@ -16,7 +16,7 @@
 # CHECK-NEXT:      "insertText": "a",
 # CHECK-NEXT:      "insertTextFormat": 1,
 # CHECK-NEXT:      "kind": 5,
-# CHECK-NEXT:      "label": "a",
+# CHECK-NEXT:      "label": " a",
 # CHECK-NEXT:      "sortText": "{{.*}}a"
 # CHECK-NEXT:    }
 # CHECK-NEXT:  ]
@@ -36,7 +36,7 @@
 # CHECK-NEXT:      "insertText": "b",
 # CHECK-NEXT:      "insertTextFormat": 1,
 # CHECK-NEXT:      "kind": 5,
-# CHECK-NEXT:      "label": "b",
+# CHECK-NEXT:      "label": " b",
 # CHECK-NEXT:      "sortText": "{{.*}}b"
 # CHECK-NEXT:    }
 # CHECK-NEXT:  ]
diff --git a/clang-tools-extra/test/clangd/protocol.test b/clang-tools-extra/test/clangd/protocol.test
index fcbba15b4cc..7295ca57980 100644
--- a/clang-tools-extra/test/clangd/protocol.test
+++ b/clang-tools-extra/test/clangd/protocol.test
@@ -38,7 +38,7 @@ Content-Length: 146
 # CHECK-NEXT:        "insertText": "a",

 # CHECK-NEXT:        "insertTextFormat": 1,

 # CHECK-NEXT:        "kind": 5,

-# CHECK-NEXT:        "label": "a",

+# CHECK-NEXT:        "label": " a",

 # CHECK-NEXT:        "sortText": "{{.*}}"

 #      CHECK:    ]

 # CHECK-NEXT:  }

@@ -67,7 +67,7 @@ Content-Length: 146
 # CHECK-NEXT:        "insertText": "a",

 # CHECK-NEXT:        "insertTextFormat": 1,

 # CHECK-NEXT:        "kind": 5,

-# CHECK-NEXT:        "label": "a",

+# CHECK-NEXT:        "label": " a",

 # CHECK-NEXT:        "sortText": "{{.*}}"

 #      CHECK:    ]

 # CHECK-NEXT:  }

@@ -96,7 +96,7 @@ Content-Length: 146
 # CHECK-NEXT:        "insertText": "a",

 # CHECK-NEXT:        "insertTextFormat": 1,

 # CHECK-NEXT:        "kind": 5,

-# CHECK-NEXT:        "label": "a",

+# CHECK-NEXT:        "label": " a",

 # CHECK-NEXT:        "sortText": "{{.*}}"

 #      CHECK:    ]

 # CHECK-NEXT:  }

diff --git a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
index 825504eb00e..7b40900b005 100644
--- a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
+++ b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
@@ -44,7 +44,19 @@ class IgnoreDiagnostics : public DiagnosticsConsumer {
 
 // GMock helpers for matching completion items.
 MATCHER_P(Named, Name, "") { return arg.insertText == Name; }
-MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
+MATCHER_P(Labeled, Label, "") {
+  std::string Indented;
+  if (!StringRef(Label).startswith(
+          CodeCompleteOptions().IncludeIndicator.Insert) &&
+      !StringRef(Label).startswith(
+          CodeCompleteOptions().IncludeIndicator.NoInsert))
+    Indented =
+        (Twine(CodeCompleteOptions().IncludeIndicator.NoInsert) + Label).str();
+  else
+    Indented = Label;
+  return arg.label == Indented;
+}
+MATCHER_P(SigHelpLabeled, Label, "") { return arg.label == Label; }
 MATCHER_P(Kind, K, "") { return arg.kind == K; }
 MATCHER_P(Filter, F, "") { return arg.filterText == F; }
 MATCHER_P(Doc, D, "") { return arg.documentation == D; }
@@ -563,7 +575,10 @@ TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) {
       )cpp",
                              {Sym});
   EXPECT_THAT(Results.items,
-              ElementsAre(AllOf(Named("X"), InsertInclude("\"bar.h\""))));
+              ElementsAre(AllOf(
+                  Named("X"),
+                  Labeled(CodeCompleteOptions().IncludeIndicator.Insert + "X"),
+                  InsertInclude("\"bar.h\""))));
   // Duplicate based on inclusions in preamble.
   Results = completions(Server,
                         R"cpp(
@@ -571,8 +586,8 @@ TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) {
           int main() { ns::^ }
       )cpp",
                         {Sym});
-  EXPECT_THAT(Results.items,
-              ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits()))));
+  EXPECT_THAT(Results.items, ElementsAre(AllOf(Named("X"), Labeled("X"),
+                                               Not(HasAdditionalEdits()))));
 }
 
 TEST(CompletionTest, NoIncludeInsertionWhenDeclFoundInFile) {
@@ -830,7 +845,7 @@ MATCHER_P(ParamsAre, P, "") {
 
 Matcher<SignatureInformation> Sig(std::string Label,
                                   std::vector<std::string> Params) {
-  return AllOf(Labeled(Label), ParamsAre(Params));
+  return AllOf(SigHelpLabeled(Label), ParamsAre(Params));
 }
 
 TEST(SignatureHelpTest, Overloads) {
@@ -1095,12 +1110,16 @@ TEST(CompletionTest, OverloadBundling) {
   NoArgsGFunc.Detail = &Detail;
   EXPECT_THAT(
       completions(Context + "int y = GFunc^", {NoArgsGFunc}, Opts).items,
-      UnorderedElementsAre(AllOf(Named("GFuncC"), InsertInclude("<foo>")),
-                           Labeled("GFuncC(int)"), Labeled("GFuncD(int)")));
+      UnorderedElementsAre(
+          AllOf(
+              Named("GFuncC"),
+              Labeled(CodeCompleteOptions().IncludeIndicator.Insert + "GFuncC"),
+              InsertInclude("<foo>")),
+          Labeled("GFuncC(int)"), Labeled("GFuncD(int)")));
 
   // Examine a bundled completion in detail.
   auto A = completions(Context + "int y = X().a^", {}, Opts).items.front();
-  EXPECT_EQ(A.label, "a(…)");
+  EXPECT_EQ(A.label, " a(…)");
   EXPECT_EQ(A.detail, "[2 overloads]");
   EXPECT_EQ(A.insertText, "a");
   EXPECT_EQ(A.kind, CompletionItemKind::Method);
diff --git a/clang-tools-extra/unittests/clangd/HeadersTests.cpp b/clang-tools-extra/unittests/clangd/HeadersTests.cpp
index 94dcd3170ca..e3a903f11fa 100644
--- a/clang-tools-extra/unittests/clangd/HeadersTests.cpp
+++ b/clang-tools-extra/unittests/clangd/HeadersTests.cpp
@@ -78,10 +78,10 @@ protected:
     return Inclusions;
   }
 
-  // Calculates the include path, or returns "" on error.
+  // Calculates the include path, or returns "" on error or header should not be
+  // inserted.
   std::string calculate(PathRef Original, PathRef Preferred = "",
-                        const std::vector<Inclusion> &Inclusions = {},
-                        bool ExpectError = false) {
+                        const std::vector<Inclusion> &Inclusions = {}) {
     auto Clang = setupClang();
     PreprocessOnlyAction Action;
     EXPECT_TRUE(
@@ -94,24 +94,21 @@ protected:
                         /*Verbatim=*/!llvm::sys::path::is_absolute(Header)};
     };
 
-    auto Path = calculateIncludePath(
-        MainFile, CDB.getCompileCommand(MainFile)->Directory,
-        Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions,
-        ToHeaderFile(Original), ToHeaderFile(Preferred));
+    IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+                             CDB.getCompileCommand(MainFile)->Directory,
+                             Clang->getPreprocessor().getHeaderSearchInfo());
+    for (const auto &Inc : Inclusions)
+      Inserter.addExisting(Inc);
+    auto Declaring = ToHeaderFile(Original);
+    auto Inserted = ToHeaderFile(Preferred);
+    if (!Inserter.shouldInsertInclude(Declaring, Inserted))
+      return "";
+    std::string Path = Inserter.calculateIncludePath(Declaring, Inserted);
     Action.EndSourceFile();
-    if (!Path) {
-      llvm::consumeError(Path.takeError());
-      EXPECT_TRUE(ExpectError);
-      return std::string();
-    } else {
-      EXPECT_FALSE(ExpectError);
-    }
-    return std::move(*Path);
+    return Path;
   }
 
-  Expected<Optional<TextEdit>>
-  insert(const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader,
-         const std::vector<Inclusion> &ExistingInclusions = {}) {
+  Optional<TextEdit> insert(StringRef VerbatimHeader) {
     auto Clang = setupClang();
     PreprocessOnlyAction Action;
     EXPECT_TRUE(
@@ -120,10 +117,7 @@ protected:
     IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
                              CDB.getCompileCommand(MainFile)->Directory,
                              Clang->getPreprocessor().getHeaderSearchInfo());
-    for (const auto &Inc : ExistingInclusions)
-      Inserter.addExisting(Inc);
-
-    auto Edit = Inserter.insert(DeclaringHeader, InsertedHeader);
+    auto Edit = Inserter.insert(VerbatimHeader);
     Action.EndSourceFile();
     return Edit;
   }
@@ -220,31 +214,10 @@ TEST_F(HeadersTest, DontInsertDuplicateResolved) {
   EXPECT_EQ(calculate(BarHeader, "\"BAR.h\"", Inclusions), "");
 }
 
-HeaderFile literal(StringRef Include) {
-  HeaderFile H{Include, /*Verbatim=*/true};
-  assert(H.valid());
-  return H;
-}
-
 TEST_F(HeadersTest, PreferInserted) {
-  auto Edit = insert(literal("<x>"), literal("<y>"));
-  EXPECT_TRUE(static_cast<bool>(Edit));
-  EXPECT_TRUE(llvm::StringRef((*Edit)->newText).contains("<y>"));
-}
-
-TEST_F(HeadersTest, ExistingInclusion) {
-  Inclusion Existing{Range(), /*Written=*/"<c.h>", /*Resolved=*/""};
-  auto Edit = insert(literal("<c.h>"), literal("<c.h>"), {Existing});
-  EXPECT_TRUE(static_cast<bool>(Edit));
-  EXPECT_FALSE(static_cast<bool>(*Edit));
-}
-
-TEST_F(HeadersTest, ValidateHeaders) {
-  HeaderFile InvalidHeader{"a.h", /*Verbatim=*/true};
-  assert(!InvalidHeader.valid());
-  auto Edit = insert(InvalidHeader, literal("\"c.h\""));
-  EXPECT_FALSE(static_cast<bool>(Edit));
-  llvm::consumeError(Edit.takeError());
+  auto Edit = insert("<y>");
+  EXPECT_TRUE(Edit.hasValue());
+  EXPECT_TRUE(llvm::StringRef(Edit->newText).contains("<y>"));
 }
 
 } // namespace