ioeric updated this revision to Diff 146045.
ioeric marked 15 inline comments as done.
ioeric added a comment.

- Merged with origin/master
- Address review comments.
- Revert unintended change.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46497

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/XRefs.cpp
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/insert-include.test
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/HeadersTests.cpp

Index: unittests/clangd/HeadersTests.cpp
===================================================================
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -8,38 +8,96 @@
 //===----------------------------------------------------------------------===//
 
 #include "Headers.h"
+
+#include "Compiler.h"
 #include "TestFS.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
+using ::testing::AllOf;
+using ::testing::UnorderedElementsAre;
+
 class HeadersTest : public ::testing::Test {
 public:
   HeadersTest() {
     CDB.ExtraClangFlags = {SearchDirArg.c_str()};
     FS.Files[MainFile] = "";
+    // Make sure directory sub/ exists.
+    FS.Files[testPath("sub/EMPTY")] = "";
+  }
+
+private:
+  std::unique_ptr<CompilerInstance> setupClang() {
+    auto Cmd = CDB.getCompileCommand(MainFile);
+    assert(static_cast<bool>(Cmd));
+    auto VFS = FS.getFileSystem();
+    VFS->setCurrentWorkingDirectory(Cmd->Directory);
+
+    std::vector<const char *> Argv;
+    for (const auto &S : Cmd->CommandLine)
+      Argv.push_back(S.c_str());
+    auto CI = clang::createInvocationFromCommandLine(
+        Argv,
+        CompilerInstance::createDiagnostics(new DiagnosticOptions(),
+                                            &IgnoreDiags, false),
+        VFS);
+    EXPECT_TRUE(static_cast<bool>(CI));
+    CI->getFrontendOpts().DisableFree = false;
+
+    // The diagnostic options must be set before creating a CompilerInstance.
+    CI->getDiagnosticOpts().IgnoreWarnings = true;
+    auto Clang = prepareCompilerInstance(
+        std::move(CI), /*Preamble=*/nullptr,
+        llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
+        std::make_shared<PCHContainerOperations>(), VFS, IgnoreDiags);
+
+    EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
+    return Clang;
   }
 
 protected:
+  std::vector<Inclusion> collectIncludes() {
+    auto Clang = setupClang();
+    PreprocessOnlyAction Action;
+    EXPECT_TRUE(
+        Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+    std::vector<Inclusion> Inclusions;
+    Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
+        Clang->getSourceManager(), Inclusions));
+    EXPECT_TRUE(Action.Execute());
+    Action.EndSourceFile();
+    return Inclusions;
+  }
+
   // Calculates the include path, or returns "" on error.
   std::string calculate(PathRef Original, PathRef Preferred = "",
+                        const std::vector<Inclusion> &Inclusions = {},
                         bool ExpectError = false) {
+    auto Clang = setupClang();
+    PreprocessOnlyAction Action;
+    EXPECT_TRUE(
+        Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
     if (Preferred.empty())
       Preferred = Original;
-    auto VFS = FS.getFileSystem();
-    auto Cmd = CDB.getCompileCommand(MainFile);
-    assert(static_cast<bool>(Cmd));
-    VFS->setCurrentWorkingDirectory(Cmd->Directory);
     auto ToHeaderFile = [](llvm::StringRef Header) {
       return HeaderFile{Header,
                         /*Verbatim=*/!llvm::sys::path::is_absolute(Header)};
     };
-    auto Path = calculateIncludePath(MainFile, FS.Files[MainFile],
-                                     ToHeaderFile(Original),
-                                     ToHeaderFile(Preferred), *Cmd, VFS);
+
+    auto Path = calculateIncludePath(
+        MainFile, CDB.getCompileCommand(MainFile)->Directory,
+        Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions,
+        ToHeaderFile(Original), ToHeaderFile(Preferred));
+    Action.EndSourceFile();
     if (!Path) {
       llvm::consumeError(Path.takeError());
       EXPECT_TRUE(ExpectError);
@@ -49,52 +107,31 @@
     }
     return std::move(*Path);
   }
+
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   std::string MainFile = testPath("main.cpp");
   std::string Subdir = testPath("sub");
   std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str();
+  IgnoringDiagConsumer IgnoreDiags;
 };
 
-TEST_F(HeadersTest, InsertInclude) {
-  std::string Path = testPath("sub/bar.h");
-  FS.Files[Path] = "";
-  EXPECT_EQ(calculate(Path), "\"bar.h\"");
-}
+MATCHER_P(Written, Name, "") { return arg.Written == Name; }
+MATCHER_P(Resolved, Name, "") { return arg.Resolved == Name; }
 
-TEST_F(HeadersTest, DontInsertDuplicateSameName) {
+TEST_F(HeadersTest, CollectRewrittenAndResolved) {
   FS.Files[MainFile] = R"cpp(
-#include "bar.h"
+#include "sub/bar.h" // not shortest
 )cpp";
   std::string BarHeader = testPath("sub/bar.h");
   FS.Files[BarHeader] = "";
-  EXPECT_EQ(calculate(BarHeader), "");
-}
 
-TEST_F(HeadersTest, DontInsertDuplicateDifferentName) {
-  std::string BarHeader = testPath("sub/bar.h");
-  FS.Files[BarHeader] = "";
-  FS.Files[MainFile] = R"cpp(
-#include "sub/bar.h"  // not shortest
-)cpp";
-  EXPECT_EQ(calculate("\"sub/bar.h\""), ""); // Duplicate rewritten.
-  EXPECT_EQ(calculate(BarHeader), "");       // Duplicate resolved.
-  EXPECT_EQ(calculate(BarHeader, "\"BAR.h\""), ""); // Do not insert preferred.
+  EXPECT_THAT(collectIncludes(),
+              UnorderedElementsAre(
+                  AllOf(Written("\"sub/bar.h\""), Resolved(BarHeader))));
 }
 
-TEST_F(HeadersTest, DontInsertDuplicatePreferred) {
-  std::string BarHeader = testPath("sub/bar.h");
-  FS.Files[BarHeader] = "";
-  FS.Files[MainFile] = R"cpp(
-#include "sub/bar.h"  // not shortest
-)cpp";
-  // Duplicate written.
-  EXPECT_EQ(calculate("\"original.h\"", "\"sub/bar.h\""), "");
-  // Duplicate resolved.
-  EXPECT_EQ(calculate("\"original.h\"", BarHeader), "");
-}
-
-TEST_F(HeadersTest, StillInsertIfTrasitivelyIncluded) {
+TEST_F(HeadersTest, OnlyCollectInclusionsInMain) {
   std::string BazHeader = testPath("sub/baz.h");
   FS.Files[BazHeader] = "";
   std::string BarHeader = testPath("sub/bar.h");
@@ -104,27 +141,65 @@
   FS.Files[MainFile] = R"cpp(
 #include "bar.h"
 )cpp";
-  EXPECT_EQ(calculate(BazHeader), "\"baz.h\"");
+  EXPECT_THAT(
+      collectIncludes(),
+      UnorderedElementsAre(AllOf(Written("\"bar.h\""), Resolved(BarHeader))));
+}
+
+TEST_F(HeadersTest, UnResolvedInclusion) {
+  FS.Files[MainFile] = R"cpp(
+#include "foo.h"
+)cpp";
+
+  EXPECT_THAT(collectIncludes(),
+              UnorderedElementsAre(AllOf(Written("\"foo.h\""), Resolved(""))));
+}
+
+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");
-  FS.Files[MainFile] = "";
   EXPECT_EQ(calculate(MainFile), "");
 }
 
+TEST_F(HeadersTest, ShortenedInclude) {
+  std::string BarHeader = testPath("sub/bar.h");
+  EXPECT_EQ(calculate(BarHeader), "\"bar.h\"");
+}
+
+TEST_F(HeadersTest, NotShortenedInclude) {
+  std::string BarHeader = testPath("sub-2/bar.h");
+  EXPECT_EQ(calculate(BarHeader, ""), "\"" + BarHeader + "\"");
+}
+
 TEST_F(HeadersTest, PreferredHeader) {
-  FS.Files[MainFile] = "";
   std::string BarHeader = testPath("sub/bar.h");
-  FS.Files[BarHeader] = "";
   EXPECT_EQ(calculate(BarHeader, "<bar>"), "<bar>");
 
   std::string BazHeader = testPath("sub/baz.h");
-  FS.Files[BazHeader] = "";
   EXPECT_EQ(calculate(BarHeader, BazHeader), "\"baz.h\"");
 }
 
+TEST_F(HeadersTest, DontInsertDuplicatePreferred) {
+  std::vector<Inclusion> Inclusions = {
+      {Range(), /*Written*/ "\"bar.h\"", /*Resolved*/ ""}};
+  EXPECT_EQ(calculate(testPath("sub/bar.h"), "\"bar.h\"", Inclusions), "");
+  EXPECT_EQ(calculate("\"x.h\"", "\"bar.h\"", Inclusions), "");
+}
+
+TEST_F(HeadersTest, DontInsertDuplicateResolved) {
+  std::string BarHeader = testPath("sub/bar.h");
+  std::vector<Inclusion> Inclusions = {
+      {Range(), /*Written*/ "fake-bar.h", /*Resolved*/ BarHeader}};
+  EXPECT_EQ(calculate(BarHeader, "", Inclusions), "");
+  // Do not insert preferred.
+  EXPECT_EQ(calculate(BarHeader, "\"BAR.h\"", Inclusions), "");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
-
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -45,6 +45,16 @@
 MATCHER_P(Filter, F, "") { return arg.filterText == F; }
 MATCHER_P(Doc, D, "") { return arg.documentation == D; }
 MATCHER_P(Detail, D, "") { return arg.detail == D; }
+MATCHER_P(InsertInclude, IncludeHeader, "") {
+  if (arg.additionalTextEdits.size() != 1)
+    return false;
+  const auto &Edit = arg.additionalTextEdits[0];
+  if (Edit.range.start != Edit.range.end)
+    return false;
+  SmallVector<StringRef, 2> Matches;
+  llvm::Regex RE(R"(#include[ ]*(["<][^">]*[">]))");
+  return RE.match(Edit.newText, &Matches) && Matches[1] == IncludeHeader;
+}
 MATCHER_P(PlainText, Text, "") {
   return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
          arg.insertText == Text;
@@ -58,6 +68,8 @@
     return true;
   return llvm::StringRef(arg.insertText).contains(arg.filterText);
 }
+MATCHER(HasAdditionalEdits, "") { return !arg.additionalTextEdits.empty(); }
+
 // Shorthand for Contains(Named(Name)).
 Matcher<const std::vector<CompletionItem> &> Has(std::string Name) {
   return Contains(Named(std::move(Name)));
@@ -75,9 +87,7 @@
   return MemIndex::build(std::move(Slab).build());
 }
 
-// Builds a server and runs code completion.
-// If IndexSymbols is non-empty, an index will be built and passed to opts.
-CompletionList completions(StringRef Text,
+CompletionList completions(ClangdServer &Server, StringRef Text,
                            std::vector<Symbol> IndexSymbols = {},
                            clangd::CodeCompleteOptions Opts = {}) {
   std::unique_ptr<SymbolIndex> OverrideIndex;
@@ -87,10 +97,6 @@
     Opts.Index = OverrideIndex.get();
   }
 
-  MockFSProvider FS;
-  MockCompilationDatabase CDB;
-  IgnoreDiagnostics DiagConsumer;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
   auto File = testPath("foo.cpp");
   Annotations Test(Text);
   runAddDocument(Server, File, Test.code());
@@ -101,6 +107,18 @@
   return CompletionList;
 }
 
+// Builds a server and runs code completion.
+// If IndexSymbols is non-empty, an index will be built and passed to opts.
+CompletionList completions(StringRef Text,
+                           std::vector<Symbol> IndexSymbols = {},
+                           clangd::CodeCompleteOptions Opts = {}) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  return completions(Server, Text, std::move(IndexSymbols), std::move(Opts));
+}
+
 std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) {
   std::string Result;
   raw_string_ostream OS(Result);
@@ -144,6 +162,20 @@
 Symbol var(StringRef Name) {
   return sym(Name, index::SymbolKind::Variable, "@\\0");
 }
+
+Symbol::Details detailWithInclude(StringRef IncludeHeader) {
+  Symbol::Details D;
+  D.IncludeHeader = IncludeHeader;
+  return D;
+}
+Symbol &&addDeclaringHeaders(Symbol &&Sym, StringRef DeclaringHeader = "") {
+  Sym.CanonicalDeclaration.FileURI = DeclaringHeader;
+  return std::move(Sym);
+}
+Symbol &&addDetails(Symbol &&Sym, const Symbol::Details &D) {
+  Sym.Detail = &D;
+  return std::move(Sym);
+}
 Symbol ns(StringRef Name) {
   return sym(Name, index::SymbolKind::Namespace, "@N@\\0");
 }
@@ -505,6 +537,73 @@
   EXPECT_TRUE(Results.isIncomplete);
 }
 
+TEST(CompletionTest, InsertNewInclude) {
+  auto Results = completions(
+      R"cpp(
+          int main() { ns::^ }
+      )cpp",
+      {addDetails(cls("ns::X"), detailWithInclude("<x>"))});
+  EXPECT_THAT(Results.items,
+              ElementsAre(AllOf(Named("X"), InsertInclude("<x>"))));
+}
+
+TEST(CompletionTest, PreferIncludeHeaderToDeclaringHeader) {
+  auto Results = completions(
+      R"cpp(
+          int main() { ns::^ }
+      )cpp",
+      {addDetails(addDeclaringHeaders(cls("ns::X"), "<y>"),
+                  detailWithInclude("<x>"))});
+  EXPECT_THAT(Results.items,
+              ElementsAre(AllOf(Named("X"), InsertInclude("<x>"))));
+}
+
+TEST(CompletionTest, HeaderAlreadyIncludedRegex) {
+  auto Results = completions(
+      R"cpp(
+          #include <x>
+          int main() { ns::^ }
+      )cpp",
+      {addDetails(cls("ns::X"), detailWithInclude("<x>"))});
+  EXPECT_THAT(Results.items,
+              ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits()))));
+}
+
+TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  std::string Subdir = testPath("sub");
+  std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str();
+  CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+  std::string BarHeader = testPath("sub/bar.h");
+  FS.Files[BarHeader] = "";
+
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  // Shoten include path based on search dirctory.
+  auto Results = completions(
+      Server,
+      R"cpp(
+          int main() { ns::^ }
+      )cpp",
+      {addDetails(cls("ns::X"),
+                  detailWithInclude(URI::createFile(BarHeader).toString()))});
+  EXPECT_THAT(Results.items,
+              ElementsAre(AllOf(Named("X"), InsertInclude("\"bar.h\""))));
+  // Duplicate based on inclusions in preamble.
+  Results = completions(
+      Server,
+      R"cpp(
+          #include "sub/bar.h"  // not shortest
+          int main() { ns::^ }
+      )cpp",
+      {addDetails(cls("ns::X"),
+                  detailWithInclude(URI::createFile(BarHeader).toString()))});
+  EXPECT_THAT(Results.items,
+              ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits()))));
+}
+
 TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -938,67 +938,6 @@
   ASSERT_EQ(DiagConsumer.Count, 2); // Sanity check - we actually ran both?
 }
 
-TEST_F(ClangdVFSTest, InsertIncludes) {
-  MockFSProvider FS;
-  ErrorCheckingDiagConsumer DiagConsumer;
-  MockCompilationDatabase CDB;
-  std::string SearchDirArg = (llvm::Twine("-I") + testRoot()).str();
-  CDB.ExtraClangFlags.insert(CDB.ExtraClangFlags.end(), {SearchDirArg.c_str()});
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
-
-  auto FooCpp = testPath("foo.cpp");
-  const auto Code = R"cpp(
-#include "x.h"
-#include "z.h"
-
-void f() {}
-)cpp";
-  FS.Files[FooCpp] = Code;
-  runAddDocument(Server, FooCpp, Code);
-
-  auto ChangedCode = [&](llvm::StringRef Original, llvm::StringRef Preferred) {
-    auto Replaces = Server.insertInclude(
-        FooCpp, Code, Original, Preferred.empty() ? Original : Preferred);
-    EXPECT_TRUE(static_cast<bool>(Replaces));
-    auto Changed = tooling::applyAllReplacements(Code, *Replaces);
-    EXPECT_TRUE(static_cast<bool>(Changed));
-    return *Changed;
-  };
-  auto Inserted = [&](llvm::StringRef Original, llvm::StringRef Preferred,
-                      llvm::StringRef Expected) {
-    return llvm::StringRef(ChangedCode(Original, Preferred))
-        .contains((llvm::Twine("#include ") + Expected + "").str());
-  };
-
-  EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"", "\"y.h\""));
-  EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"\"Y.h\"", "\"Y.h\""));
-  EXPECT_TRUE(Inserted("<string>", /*Preferred=*/"", "<string>"));
-  EXPECT_TRUE(Inserted("<string>", /*Preferred=*/"", "<string>"));
-
-  std::string OriginalHeader = URI::createFile(testPath("y.h")).toString();
-  std::string PreferredHeader = URI::createFile(testPath("Y.h")).toString();
-  EXPECT_TRUE(Inserted(OriginalHeader,
-                       /*Preferred=*/"", "\"y.h\""));
-  EXPECT_TRUE(Inserted(OriginalHeader,
-                       /*Preferred=*/"<Y.h>", "<Y.h>"));
-  EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\""));
-  EXPECT_TRUE(Inserted("<y.h>", PreferredHeader, "\"Y.h\""));
-  auto TestURIHeader =
-      URI::parse(llvm::formatv("{0}:///x/y/z.h", ClangdTestScheme).str());
-  EXPECT_TRUE(static_cast<bool>(TestURIHeader));
-  EXPECT_TRUE(Inserted(TestURIHeader->toString(), "", "\"x/y/z.h\""));
-
-  // Check that includes are sorted.
-  const auto Expected = R"cpp(
-#include "x.h"
-#include "y.h"
-#include "z.h"
-
-void f() {}
-)cpp";
-  EXPECT_EQ(Expected, ChangedCode("\"y.h\"", /*Preferred=*/""));
-}
-
 TEST_F(ClangdVFSTest, FormatCode) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
Index: test/clangd/insert-include.test
===================================================================
--- test/clangd/insert-include.test
+++ /dev/null
@@ -1,36 +0,0 @@
-# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
----
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void f() {}"}}}
----
-{"jsonrpc":"2.0","id":3,"method":"workspace/executeCommand","params":{"arguments":[{"declaringHeader":"\"/usr/include/bits/vector\"", "preferredHeader":"<vector>","textDocument":{"uri":"test:///main.cpp"}}],"command":"clangd.insertInclude"}}
-#      CHECK:    "id": 3,
-# CHECK-NEXT:    "jsonrpc": "2.0",
-# CHECK-NEXT:    "result": "Inserted header \"/usr/include/bits/vector\" (<vector>)"
-# CHECK-NEXT:  }
-#      CHECK:    "method": "workspace/applyEdit",
-# CHECK-NEXT:    "params": {
-# CHECK-NEXT:      "edit": {
-# CHECK-NEXT:        "changes": {
-# CHECK-NEXT:          "file://{{.*}}/main.cpp": [
-# CHECK-NEXT:            {
-# CHECK-NEXT:              "newText": "#include <vector>\n",
-# CHECK-NEXT:              "range": {
-# CHECK-NEXT:                "end": {
-# CHECK-NEXT:                  "character": 0,
-# CHECK-NEXT:                  "line": 0
-# CHECK-NEXT:                },
-# CHECK-NEXT:                "start": {
-# CHECK-NEXT:                  "character": 0,
-# CHECK-NEXT:                  "line": 0
-# CHECK-NEXT:                }
-# CHECK-NEXT:              }
-# CHECK-NEXT:            }
-# CHECK-NEXT:          ]
-# CHECK-NEXT:        }
-# CHECK-NEXT:      }
-# CHECK-NEXT:    }
-# CHECK-NEXT:  }
----
-{"jsonrpc":"2.0","id":4,"method":"shutdown"}
Index: test/clangd/initialize-params.test
===================================================================
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -24,8 +24,7 @@
 # CHECK-NEXT:      "documentRangeFormattingProvider": true,
 # CHECK-NEXT:      "executeCommandProvider": {
 # CHECK-NEXT:        "commands": [
-# CHECK-NEXT:          "clangd.applyFix",
-# CHECK-NEXT:          "clangd.insertInclude"
+# CHECK-NEXT:          "clangd.applyFix"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "hoverProvider": true,
Index: test/clangd/initialize-params-invalid.test
===================================================================
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -24,8 +24,7 @@
 # CHECK-NEXT:      "documentRangeFormattingProvider": true,
 # CHECK-NEXT:      "executeCommandProvider": {
 # CHECK-NEXT:        "commands": [
-# CHECK-NEXT:          "clangd.applyFix",
-# CHECK-NEXT:          "clangd.insertInclude"
+# CHECK-NEXT:          "clangd.applyFix"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "hoverProvider": true,
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -235,11 +235,12 @@
   std::vector<Location> Result;
   // Handle goto definition for #include.
   for (auto &IncludeLoc : AST.getInclusionLocations()) {
-    Range R = IncludeLoc.first;
+    if (IncludeLoc.Resolved.empty())
+      continue;
     Position Pos = sourceLocToPosition(SourceMgr, SourceLocationBeg);
 
-    if (R.contains(Pos))
-      Result.push_back(Location{URIForFile{IncludeLoc.second}, {}});
+    if (IncludeLoc.R.contains(Pos))
+      Result.push_back(Location{URIForFile{IncludeLoc.Resolved}, {}});
   }
   if (!Result.empty())
     return Result;
Index: clangd/SourceCode.h
===================================================================
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -15,6 +15,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #include "Protocol.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Core/Replacement.h"
 
 namespace clang {
 class SourceManager;
@@ -55,6 +56,15 @@
 std::pair<llvm::StringRef, llvm::StringRef>
 splitQualifiedName(llvm::StringRef QName);
 
+TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R);
+
+std::vector<TextEdit>
+replacementsToEdits(StringRef Code,
+                    const std::vector<tooling::Replacement> &Replacements);
+
+std::vector<TextEdit> replacementsToEdits(StringRef Code,
+                                          const tooling::Replacements &Repls);
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: clangd/SourceCode.cpp
===================================================================
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -166,5 +166,31 @@
   return {QName.substr(0, Pos + 2), QName.substr(Pos + 2)};
 }
 
+TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R) {
+  Range ReplacementRange = {
+      offsetToPosition(Code, R.getOffset()),
+      offsetToPosition(Code, R.getOffset() + R.getLength())};
+  return {ReplacementRange, R.getReplacementText()};
+}
+
+std::vector<TextEdit>
+replacementsToEdits(StringRef Code,
+                    const std::vector<tooling::Replacement> &Replacements) {
+  // Turn the replacements into the format specified by the Language Server
+  // Protocol. Fuse them into one big JSON array.
+  std::vector<TextEdit> Edits;
+  for (const auto &R : Replacements)
+    Edits.push_back(replacementToEdit(Code, R));
+  return Edits;
+}
+
+std::vector<TextEdit> replacementsToEdits(StringRef Code,
+                                          const tooling::Replacements &Repls) {
+  std::vector<TextEdit> Edits;
+  for (const auto &R : Repls)
+    Edits.push_back(replacementToEdit(Code, R));
+  return Edits;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -99,6 +99,9 @@
     return std::tie(LHS.line, LHS.character) ==
            std::tie(RHS.line, RHS.character);
   }
+  friend bool operator!=(const Position &LHS, const Position &RHS) {
+    return !(LHS == RHS);
+  }
   friend bool operator<(const Position &LHS, const Position &RHS) {
     return std::tie(LHS.line, LHS.character) <
            std::tie(RHS.line, RHS.character);
@@ -538,26 +541,6 @@
 bool fromJSON(const json::Expr &, WorkspaceEdit &);
 json::Expr toJSON(const WorkspaceEdit &WE);
 
-struct IncludeInsertion {
-  /// The document in which the command was invoked.
-  /// If either originalHeader or preferredHeader has been (directly) included
-  /// in the current file, no new include will be inserted.
-  TextDocumentIdentifier textDocument;
-
-  /// The declaring header corresponding to this insertion e.g. the header that
-  /// declares a symbol. This could be either a URI or a literal string quoted
-  /// with <> or "" that can be #included directly.
-  std::string declaringHeader;
-  /// The preferred header to be inserted. This may be different from
-  /// originalHeader as a header file can have a different canonical include.
-  /// This could be either a URI or a literal string quoted with <> or "" that
-  /// can be #included directly. If empty, declaringHeader is used to calculate
-  /// the #include path.
-  std::string preferredHeader;
-};
-bool fromJSON(const json::Expr &, IncludeInsertion &);
-json::Expr toJSON(const IncludeInsertion &II);
-
 /// Exact commands are not specified in the protocol so we define the
 /// ones supported by Clangd here. The protocol specifies the command arguments
 /// to be "any[]" but to make this safer and more manageable, each command we
@@ -569,16 +552,12 @@
 struct ExecuteCommandParams {
   // Command to apply fix-its. Uses WorkspaceEdit as argument.
   const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND;
-  // Command to insert an #include into code.
-  const static llvm::StringLiteral CLANGD_INSERT_HEADER_INCLUDE;
 
   /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
   std::string command;
 
   // Arguments
   llvm::Optional<WorkspaceEdit> workspaceEdit;
-
-  llvm::Optional<IncludeInsertion> includeInsertion;
 };
 bool fromJSON(const json::Expr &, ExecuteCommandParams &);
 
@@ -750,7 +729,6 @@
   /// themselves.
   std::vector<TextEdit> additionalTextEdits;
 
-  llvm::Optional<Command> command;
   // TODO(krasimir): The following optional fields defined by the language
   // server protocol are unsupported:
   //
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -390,9 +390,6 @@
 
 const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND =
     "clangd.applyFix";
-const llvm::StringLiteral ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE =
-    "clangd.insertInclude";
-
 bool fromJSON(const json::Expr &Params, ExecuteCommandParams &R) {
   json::ObjectMapper O(Params);
   if (!O || !O.map("command", R.command))
@@ -402,9 +399,6 @@
   if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) {
     return Args && Args->size() == 1 &&
            fromJSON(Args->front(), R.workspaceEdit);
-  } else if (R.command == ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) {
-    return Args && Args->size() == 1 &&
-           fromJSON(Args->front(), R.includeInsertion);
   }
   return false; // Unrecognized command.
 }
@@ -433,8 +427,6 @@
   auto Cmd = json::obj{{"title", C.title}, {"command", C.command}};
   if (C.workspaceEdit)
     Cmd["arguments"] = {*C.workspaceEdit};
-  else if (C.includeInsertion)
-    Cmd["arguments"] = {*C.includeInsertion};
   return std::move(Cmd);
 }
 
@@ -447,18 +439,6 @@
   return json::obj{{"changes", std::move(FileChanges)}};
 }
 
-bool fromJSON(const json::Expr &II, IncludeInsertion &R) {
-  json::ObjectMapper O(II);
-  return O && O.map("textDocument", R.textDocument) &&
-         O.map("declaringHeader", R.declaringHeader) &&
-         O.map("preferredHeader", R.preferredHeader);
-}
-json::Expr toJSON(const IncludeInsertion &II) {
-  return json::obj{{"textDocument", II.textDocument},
-                   {"declaringHeader", II.declaringHeader},
-                   {"preferredHeader", II.preferredHeader}};
-}
-
 json::Expr toJSON(const ApplyWorkspaceEditParams &Params) {
   return json::obj{{"edit", Params.edit}};
 }
@@ -519,8 +499,6 @@
     Result["textEdit"] = *CI.textEdit;
   if (!CI.additionalTextEdits.empty())
     Result["additionalTextEdits"] = json::ary(CI.additionalTextEdits);
-  if (CI.command)
-    Result["command"] = *CI.command;
   return std::move(Result);
 }
 
Index: clangd/Headers.h
===================================================================
--- clangd/Headers.h
+++ clangd/Headers.h
@@ -11,9 +11,16 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H
 
 #include "Path.h"
+#include "Protocol.h"
 #include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Format/Format.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/PPCallbacks.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Core/HeaderIncludes.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
@@ -32,25 +39,37 @@
   bool valid() const;
 };
 
+// A header inclusion in the main file.
+struct Inclusion {
+  Range R;             // Inclusion range.
+  std::string Written; // Inclusion name as written e.g. <vector>.
+  Path Resolved;       // Resolved path of included file. Empty if not resolved.
+};
+
+/// Returns a PPCallback that collects all inclusions in the main file.
+/// Inclusions are added to \p Includes.
+std::unique_ptr<PPCallbacks>
+collectInclusionsInMainFileCallback(const SourceManager &SM,
+                                    std::vector<Inclusion> &Inclusions);
+
 /// 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)
-///   included in the file (including those included via different paths).
+///   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, llvm::StringRef Code,
-                     const HeaderFile &DeclaringHeader,
-                     const HeaderFile &InsertedHeader,
-                     const tooling::CompileCommand &CompileCommand,
-                     IntrusiveRefCntPtr<vfs::FileSystem> FS);
+llvm::Expected<std::string> calculateIncludePath(
+    PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo,
+    const std::vector<Inclusion> &Inclusions, const HeaderFile &DeclaringHeader,
+    const HeaderFile &InsertedHeader);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/Headers.cpp
===================================================================
--- clangd/Headers.cpp
+++ clangd/Headers.cpp
@@ -10,40 +10,43 @@
 #include "Headers.h"
 #include "Compiler.h"
 #include "Logger.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/FrontendActions.h"
+#include "SourceCode.h"
+#include "Trace.h"
 #include "clang/Lex/HeaderSearch.h"
-#include "clang/Lex/PreprocessorOptions.h"
-#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 class RecordHeaders : public PPCallbacks {
 public:
-  RecordHeaders(llvm::StringSet<> &WrittenHeaders,
-                llvm::StringSet<> &ResolvedHeaders)
-      : WrittenHeaders(WrittenHeaders), ResolvedHeaders(ResolvedHeaders) {}
+  RecordHeaders(const SourceManager &SM, std::vector<Inclusion> &Includes)
+      : SM(SM), Includes(Includes) {}
 
-  void InclusionDirective(SourceLocation /*HashLoc*/,
-                          const Token & /*IncludeTok*/,
+  // Record existing #includes - both written and resolved paths. Only #includes
+  // in the main file are collected.
+  void InclusionDirective(SourceLocation HashLoc, const Token & /*IncludeTok*/,
                           llvm::StringRef FileName, bool IsAngled,
-                          CharSourceRange /*FilenameRange*/,
-                          const FileEntry *File, llvm::StringRef /*SearchPath*/,
+                          CharSourceRange FilenameRange, const FileEntry *File,
+                          llvm::StringRef /*SearchPath*/,
                           llvm::StringRef /*RelativePath*/,
                           const Module * /*Imported*/) override {
-    WrittenHeaders.insert(
-        (IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").str());
-    if (File != nullptr && !File->tryGetRealPathName().empty())
-      ResolvedHeaders.insert(File->tryGetRealPathName());
+    // Only inclusion directives in the main file make sense. The user cannot
+    // select directives not in the main file.
+    if (HashLoc.isInvalid() || !SM.isInMainFile(HashLoc))
+      return;
+    std::string Written =
+        (IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").str();
+    std::string Resolved = (!File || File->tryGetRealPathName().empty())
+                               ? ""
+                               : File->tryGetRealPathName();
+    Includes.push_back({halfOpenToRange(SM, FilenameRange), Written, Resolved});
   }
 
 private:
-  llvm::StringSet<> &WrittenHeaders;
-  llvm::StringSet<> &ResolvedHeaders;
+  const SourceManager &SM;
+  std::vector<Inclusion> &Includes;
 };
 
 } // namespace
@@ -57,81 +60,42 @@
          (!Verbatim && llvm::sys::path::is_absolute(File));
 }
 
+
+std::unique_ptr<PPCallbacks>
+collectInclusionsInMainFileCallback(const SourceManager &SM,
+                                    std::vector<Inclusion> &Includes) {
+  return llvm::make_unique<RecordHeaders>(SM, Includes);
+}
+
 /// FIXME(ioeric): we might not want to insert an absolute include path if the
 /// path is not shortened.
-llvm::Expected<std::string>
-calculateIncludePath(llvm::StringRef File, llvm::StringRef Code,
-                     const HeaderFile &DeclaringHeader,
-                     const HeaderFile &InsertedHeader,
-                     const tooling::CompileCommand &CompileCommand,
-                     IntrusiveRefCntPtr<vfs::FileSystem> FS) {
-  assert(llvm::sys::path::is_absolute(File));
+llvm::Expected<std::string> calculateIncludePath(
+    PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo,
+    const std::vector<Inclusion> &Inclusions, const HeaderFile &DeclaringHeader,
+    const HeaderFile &InsertedHeader) {
   assert(DeclaringHeader.valid() && InsertedHeader.valid());
   if (File == DeclaringHeader.File || File == InsertedHeader.File)
     return "";
-  FS->setCurrentWorkingDirectory(CompileCommand.Directory);
-
-  // Set up a CompilerInstance and create a preprocessor to collect existing
-  // #include headers in \p Code. Preprocesor also provides HeaderSearch with
-  // which we can calculate the shortest include path for \p Header.
-  std::vector<const char *> Argv;
-  for (const auto &S : CompileCommand.CommandLine)
-    Argv.push_back(S.c_str());
-  IgnoringDiagConsumer IgnoreDiags;
-  auto CI = clang::createInvocationFromCommandLine(
-      Argv,
-      CompilerInstance::createDiagnostics(new DiagnosticOptions(), &IgnoreDiags,
-                                          false),
-      FS);
-  if (!CI)
-    return llvm::make_error<llvm::StringError>(
-        "Failed to create a compiler instance for " + File,
-        llvm::inconvertibleErrorCode());
-  CI->getFrontendOpts().DisableFree = false;
-  // Parse the main file to get all existing #includes in the file, and then we
-  // can make sure the same header (even with different include path) is not
-  // added more than once.
-  CI->getPreprocessorOpts().SingleFileParseMode = true;
-
-  // The diagnostic options must be set before creating a CompilerInstance.
-  CI->getDiagnosticOpts().IgnoreWarnings = true;
-  auto Clang = prepareCompilerInstance(
-      std::move(CI), /*Preamble=*/nullptr,
-      llvm::MemoryBuffer::getMemBuffer(Code, File),
-      std::make_shared<PCHContainerOperations>(), FS, IgnoreDiags);
-
-  if (Clang->getFrontendOpts().Inputs.empty())
-    return llvm::make_error<llvm::StringError>(
-        "Empty frontend action inputs empty for file " + File,
-        llvm::inconvertibleErrorCode());
-  PreprocessOnlyAction Action;
-  if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]))
-    return llvm::make_error<llvm::StringError>(
-        "Failed to begin preprocessor only action for file " + File,
-        llvm::inconvertibleErrorCode());
-  llvm::StringSet<> WrittenHeaders;
-  llvm::StringSet<> ResolvedHeaders;
-  Clang->getPreprocessor().addPPCallbacks(
-      llvm::make_unique<RecordHeaders>(WrittenHeaders, ResolvedHeaders));
-  if (!Action.Execute())
-    return llvm::make_error<llvm::StringError>(
-        "Failed to execute preprocessor only action for file " + File,
-        llvm::inconvertibleErrorCode());
+
+  llvm::StringSet<> IncludedHeaders;
+  for (const auto &Inc : Inclusions) {
+    IncludedHeaders.insert(Inc.Written);
+    if (!Inc.Resolved.empty())
+      IncludedHeaders.insert(Inc.Resolved);
+  }
   auto Included = [&](llvm::StringRef Header) {
-    return WrittenHeaders.find(Header) != WrittenHeaders.end() ||
-           ResolvedHeaders.find(Header) != ResolvedHeaders.end();
+    return IncludedHeaders.find(Header) != IncludedHeaders.end();
   };
   if (Included(DeclaringHeader.File) || Included(InsertedHeader.File))
     return "";
 
-  auto &HeaderSearchInfo = Clang->getPreprocessor().getHeaderSearchInfo();
   bool IsSystem = false;
 
   if (InsertedHeader.Verbatim)
     return InsertedHeader.File;
 
   std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics(
-      InsertedHeader.File, CompileCommand.Directory, &IsSystem);
+      InsertedHeader.File, BuildDir, &IsSystem);
   if (IsSystem)
     Suggested = "<" + Suggested + ">";
   else
Index: clangd/CodeComplete.h
===================================================================
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -15,6 +15,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H
 
+#include "Headers.h"
 #include "Logger.h"
 #include "Path.h"
 #include "Protocol.h"
@@ -69,6 +70,7 @@
 CompletionList codeComplete(PathRef FileName,
                             const tooling::CompileCommand &Command,
                             PrecompiledPreamble const *Preamble,
+                            const std::vector<Inclusion> &Inclusions,
                             StringRef Contents, Position Pos,
                             IntrusiveRefCntPtr<vfs::FileSystem> VFS,
                             std::shared_ptr<PCHContainerOperations> PCHs,
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -18,9 +18,11 @@
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
 #include "FuzzyMatch.h"
+#include "Headers.h"
 #include "Logger.h"
 #include "SourceCode.h"
 #include "Trace.h"
+#include "URI.h"
 #include "index/Index.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -219,6 +221,76 @@
   return S;
 }
 
+/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal
+/// include.
+static llvm::Expected<HeaderFile> toHeaderFile(StringRef Header,
+                                               llvm::StringRef HintPath) {
+  if (isLiteralInclude(Header))
+    return HeaderFile{Header.str(), /*Verbatim=*/true};
+  auto U = URI::parse(Header);
+  if (!U)
+    return U.takeError();
+
+  auto IncludePath = URI::includeSpelling(*U);
+  if (!IncludePath)
+    return IncludePath.takeError();
+  if (!IncludePath->empty())
+    return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
+
+  auto Resolved = URI::resolve(*U, HintPath);
+  if (!Resolved)
+    return Resolved.takeError();
+  return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
+}
+
+// Calculates include path and insertion edit for a new header.
+class IncludeInserter {
+public:
+  IncludeInserter(StringRef FileName, StringRef Code,
+                   const format::FormatStyle &Style, StringRef BuildDir,
+                   HeaderSearch &HeaderSearchInfo)
+      : FileName(FileName), Code(Code), BuildDir(BuildDir),
+        HeaderSearchInfo(HeaderSearchInfo),
+        Inserter(FileName, Code, Style.IncludeStyle) {}
+
+  void setExistingInclusions(std::vector<Inclusion> &&Inclusions) {
+    this->Inclusions = std::move(Inclusions);
+  }
+
+  // The header name is either a URI or a literal include.
+  Expected<Optional<TextEdit>> insert(StringRef FileName,
+                                      StringRef DeclaringHeader,
+                                      StringRef InsertedHeader) const {
+    auto ResolvedDeclaring = toHeaderFile(DeclaringHeader, FileName);
+    if (!ResolvedDeclaring)
+      return ResolvedDeclaring.takeError();
+    auto ResolvedInserted = toHeaderFile(InsertedHeader, FileName);
+    if (!ResolvedInserted)
+      return ResolvedInserted.takeError();
+    auto Include =
+        calculateIncludePath(FileName, BuildDir, HeaderSearchInfo, Inclusions,
+                             *ResolvedDeclaring, *ResolvedInserted);
+    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);
+  }
+
+private:
+  StringRef FileName;
+  StringRef Code;
+  StringRef BuildDir;
+  HeaderSearch &HeaderSearchInfo;
+  std::vector<Inclusion> Inclusions;
+  tooling::HeaderIncludes Inserter; // Computers insertion replacement.
+};
+
 /// A code completion result, in clang-native form.
 /// It may be promoted to a CompletionItem if it's among the top-ranked results.
 struct CompletionCandidate {
@@ -255,10 +327,10 @@
   }
 
   // Builds an LSP completion item.
-  CompletionItem build(llvm::StringRef FileName,
-                       const CompletionItemScores &Scores,
+  CompletionItem build(StringRef FileName, const CompletionItemScores &Scores,
                        const CodeCompleteOptions &Opts,
-                       CodeCompletionString *SemaCCS) const {
+                       CodeCompletionString *SemaCCS,
+                       const IncludeInserter *Includes) const {
     assert(bool(SemaResult) == bool(SemaCCS));
     CompletionItem I;
     if (SemaResult) {
@@ -289,26 +361,25 @@
           I.documentation = D->Documentation;
         if (I.detail.empty())
           I.detail = D->CompletionDetail;
-        // FIXME: delay creating include insertion command to
-        // "completionItem/resolve", when it is supported
-        if (!D->IncludeHeader.empty()) {
-          // LSP favors additionalTextEdits over command. But we are still using
-          // command here because it would be expensive to calculate #include
-          // insertion edits for all candidates, and the include insertion edit
-          // is unlikely to conflict with the code completion edits.
-          Command Cmd;
-          // Command title is not added since this is not a user-facing command.
-          Cmd.command = ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE;
-          IncludeInsertion Insertion;
+        if (Includes && !D->IncludeHeader.empty()) {
           // Fallback to canonical header if declaration location is invalid.
-          Insertion.declaringHeader =
+          llvm::StringRef DeclaringHeader =
               IndexResult->CanonicalDeclaration.FileURI.empty()
                   ? D->IncludeHeader
                   : IndexResult->CanonicalDeclaration.FileURI;
-          Insertion.preferredHeader = D->IncludeHeader;
-          Insertion.textDocument.uri = URIForFile(FileName);
-          Cmd.includeInsertion = std::move(Insertion);
-          I.command = std::move(Cmd);
+          llvm::StringRef InsertedHeader = D->IncludeHeader;
+          assert(!DeclaringHeader.empty() && !InsertedHeader.empty());
+          Expected<Optional<TextEdit>> Edit =
+              Includes->insert(FileName, DeclaringHeader, InsertedHeader);
+          if (!Edit) {
+            std::string ErrMsg =
+                ("Failed to generate include insertion edits for adding " +
+                 DeclaringHeader + " (" + InsertedHeader + ") into " + FileName)
+                    .str();
+            log(ErrMsg + ":" + llvm::toString(Edit.takeError()));
+          } else if (*Edit) {
+            I.additionalTextEdits = {std::move(**Edit)};
+          }
         }
       }
     }
@@ -670,16 +741,22 @@
   PathRef FileName;
   const tooling::CompileCommand &Command;
   PrecompiledPreamble const *Preamble;
+  const std::vector<Inclusion> &Inclusions;
   StringRef Contents;
   Position Pos;
   IntrusiveRefCntPtr<vfs::FileSystem> VFS;
   std::shared_ptr<PCHContainerOperations> PCHs;
 };
 
 // Invokes Sema code completion on a file.
+// If \p CallbackBeforeAction is set, it will be run after a compiler instance
+// has been set up and before the frontend action is executed. For example, this
+// can be used to register preprocessor callbacks.
 bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
                       const clang::CodeCompleteOptions &Options,
-                      const SemaCompleteInput &Input) {
+                      const SemaCompleteInput &Input,
+                      llvm::function_ref<void(CompilerInstance &)>
+                          CallbackBeforeAction = nullptr) {
   trace::Span Tracer("Sema completion");
   std::vector<const char *> ArgStrs;
   for (const auto &S : Input.Command.CommandLine)
@@ -750,6 +827,8 @@
         Input.FileName);
     return false;
   }
+  if (CallbackBeforeAction)
+    CallbackBeforeAction(*Clang);
   if (!Action.Execute()) {
     log("Execute() failed when running codeComplete for " + Input.FileName);
     return false;
@@ -856,29 +935,54 @@
   CompletionRecorder *Recorder = nullptr;
   int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
   bool Incomplete = false; // Would more be available with a higher limit?
-  llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
+  llvm::Optional<FuzzyMatcher> Filter;       // Initialized once Sema runs.
+  std::unique_ptr<IncludeInserter> Includes; // Initialized once compiler runs.
 
 public:
   // A CodeCompleteFlow object is only useful for calling run() exactly once.
   CodeCompleteFlow(PathRef FileName, const CodeCompleteOptions &Opts)
       : FileName(FileName), Opts(Opts) {}
 
   CompletionList run(const SemaCompleteInput &SemaCCInput) && {
     trace::Span Tracer("CodeCompleteFlow");
+    auto Style = format::getStyle("file", FileName, "LLVM",
+                                  SemaCCInput.Contents, SemaCCInput.VFS.get());
+    if (!Style) {
+      log("Failed to get FormaStyle for file" + FileName +
+          ". Fall back to use LLVM style. Error: " +
+          llvm::toString(Style.takeError()));
+      Style = format::getLLVMStyle();
+    }
+    // Inclusions from preamble or the current preprocesor run in sema.
+    std::vector<Inclusion> Inclusions = SemaCCInput.Inclusions;
     // We run Sema code completion first. It builds an AST and calculates:
     //   - completion results based on the AST.
     //   - partial identifier and context. We need these for the index query.
     CompletionList Output;
     auto RecorderOwner = llvm::make_unique<CompletionRecorder>(Opts, [&]() {
       assert(Recorder && "Recorder is not set");
+      assert(Includes && "Includes is not set");
+      // If preprocessor was run, inclusions from preprocessor callback should
+      // already be added to Inclusions.
+      Includes->setExistingInclusions(std::move(Inclusions));
       Output = runWithSema();
+      Includes.release(); // Make sure this doesn't out-live Clang.
       SPAN_ATTACH(Tracer, "sema_completion_kind",
                   getCompletionKindString(Recorder->CCContext.getKind()));
     });
 
     Recorder = RecorderOwner.get();
     semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
-                     SemaCCInput);
+                     SemaCCInput,
+                     /*CallbackBeforeAction*/ [&](CompilerInstance &Clang) {
+                       Clang.getPreprocessor().addPPCallbacks(
+                           collectInclusionsInMainFileCallback(
+                               Clang.getSourceManager(), Inclusions));
+                       Includes = llvm::make_unique<IncludeInserter>(
+                           FileName, SemaCCInput.Contents, *Style,
+                           SemaCCInput.Command.Directory,
+                           Clang.getPreprocessor().getHeaderSearchInfo());
+                     });
 
     SPAN_ATTACH(Tracer, "sema_results", NSema);
     SPAN_ATTACH(Tracer, "index_results", NIndex);
@@ -1004,19 +1108,20 @@
     CodeCompletionString *SemaCCS = nullptr;
     if (auto *SR = Candidate.SemaResult)
       SemaCCS = Recorder->codeCompletionString(*SR, Opts.IncludeBriefComments);
-    return Candidate.build(FileName, Scores, Opts, SemaCCS);
+    return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get());
   }
 };
 
 CompletionList codeComplete(PathRef FileName,
                             const tooling::CompileCommand &Command,
                             PrecompiledPreamble const *Preamble,
+                            const std::vector<Inclusion> &Inclusions,
                             StringRef Contents, Position Pos,
                             IntrusiveRefCntPtr<vfs::FileSystem> VFS,
                             std::shared_ptr<PCHContainerOperations> PCHs,
                             CodeCompleteOptions Opts) {
   return CodeCompleteFlow(FileName, Opts)
-      .run({FileName, Command, Preamble, Contents, Pos, VFS, PCHs});
+      .run({FileName, Command, Preamble, Inclusions, Contents, Pos, VFS, PCHs});
 }
 
 SignatureHelp signatureHelp(PathRef FileName,
@@ -1031,10 +1136,10 @@
   Options.IncludeMacros = false;
   Options.IncludeCodePatterns = false;
   Options.IncludeBriefComments = true;
-  semaCodeComplete(llvm::make_unique<SignatureHelpCollector>(Options, Result),
-                   Options,
-                   {FileName, Command, Preamble, Contents, Pos, std::move(VFS),
-                    std::move(PCHs)});
+  semaCodeComplete(
+      llvm::make_unique<SignatureHelpCollector>(Options, Result), Options,
+      {FileName, Command, Preamble,
+       /*Inclusions*/ {}, Contents, Pos, std::move(VFS), std::move(PCHs)});
   return Result;
 }
 
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -12,6 +12,7 @@
 
 #include "Diagnostics.h"
 #include "Function.h"
+#include "Headers.h"
 #include "Path.h"
 #include "Protocol.h"
 #include "clang/Frontend/FrontendAction.h"
@@ -40,18 +41,16 @@
 
 namespace clangd {
 
-using InclusionLocations = std::vector<std::pair<Range, Path>>;
-
 // Stores Preamble and associated data.
 struct PreambleData {
   PreambleData(PrecompiledPreamble Preamble,
                std::vector<serialization::DeclID> TopLevelDeclIDs,
-               std::vector<Diag> Diags, InclusionLocations IncLocations);
+               std::vector<Diag> Diags, std::vector<Inclusion> IncLocations);
 
   PrecompiledPreamble Preamble;
   std::vector<serialization::DeclID> TopLevelDeclIDs;
   std::vector<Diag> Diags;
-  InclusionLocations IncLocations;
+  std::vector<Inclusion> IncLocations;
 };
 
 /// Information required to run clang, e.g. to parse AST or do code completion.
@@ -95,14 +94,14 @@
   /// Returns the esitmated size of the AST and the accessory structures, in
   /// bytes. Does not include the size of the preamble.
   std::size_t getUsedBytes() const;
-  const InclusionLocations &getInclusionLocations() const;
+  const std::vector<Inclusion> &getInclusionLocations() const;
 
 private:
   ParsedAST(std::shared_ptr<const PreambleData> Preamble,
             std::unique_ptr<CompilerInstance> Clang,
             std::unique_ptr<FrontendAction> Action,
             std::vector<const Decl *> TopLevelDecls, std::vector<Diag> Diags,
-            InclusionLocations IncLocations);
+            std::vector<Inclusion> IncLocations);
 
 private:
   void ensurePreambleDeclsDeserialized();
@@ -122,7 +121,7 @@
   std::vector<Diag> Diags;
   std::vector<const Decl *> TopLevelDecls;
   bool PreambleDeclsDeserialized;
-  InclusionLocations IncLocations;
+  std::vector<Inclusion> IncLocations;
 };
 
 using ASTParsedCallback = std::function<void(PathRef Path, ParsedAST *)>;
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -83,41 +83,13 @@
   std::vector<const Decl *> TopLevelDecls;
 };
 
-class InclusionLocationsCollector : public PPCallbacks {
-public:
-  InclusionLocationsCollector(SourceManager &SourceMgr,
-                              InclusionLocations &IncLocations)
-      : SourceMgr(SourceMgr), IncLocations(IncLocations) {}
-
-  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
-                          StringRef FileName, bool IsAngled,
-                          CharSourceRange FilenameRange, const FileEntry *File,
-                          StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override {
-    auto SR = FilenameRange.getAsRange();
-    if (SR.isInvalid() || !File || File->tryGetRealPathName().empty())
-      return;
-
-    if (SourceMgr.isInMainFile(SR.getBegin())) {
-      // Only inclusion directives in the main file make sense. The user cannot
-      // select directives not in the main file.
-      IncLocations.emplace_back(halfOpenToRange(SourceMgr, FilenameRange),
-                                File->tryGetRealPathName());
-    }
-  }
-
-private:
-  SourceManager &SourceMgr;
-  InclusionLocations &IncLocations;
-};
-
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   std::vector<serialization::DeclID> takeTopLevelDeclIDs() {
     return std::move(TopLevelDeclIDs);
   }
 
-  InclusionLocations takeInclusionLocations() {
+  std::vector<Inclusion> takeInclusionLocations() {
     return std::move(IncLocations);
   }
 
@@ -145,14 +117,13 @@
 
   std::unique_ptr<PPCallbacks> createPPCallbacks() override {
     assert(SourceMgr && "SourceMgr must be set at this point");
-    return llvm::make_unique<InclusionLocationsCollector>(*SourceMgr,
-                                                          IncLocations);
+    return collectInclusionsInMainFileCallback(*SourceMgr, IncLocations);
   }
 
 private:
   std::vector<Decl *> TopLevelDecls;
   std::vector<serialization::DeclID> TopLevelDeclIDs;
-  InclusionLocations IncLocations;
+  std::vector<Inclusion> IncLocations;
   SourceManager *SourceMgr = nullptr;
 };
 
@@ -190,15 +161,14 @@
     return llvm::None;
   }
 
-  InclusionLocations IncLocations;
+  std::vector<Inclusion> IncLocations;
   // Copy over the includes from the preamble, then combine with the
   // non-preamble includes below.
   if (Preamble)
     IncLocations = Preamble->IncLocations;
 
-  Clang->getPreprocessor().addPPCallbacks(
-      llvm::make_unique<InclusionLocationsCollector>(Clang->getSourceManager(),
-                                                     IncLocations));
+  Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
+      Clang->getSourceManager(), IncLocations));
 
   if (!Action->Execute())
     log("Execute() failed when building AST for " + MainInput.getFile());
@@ -278,23 +248,23 @@
          ::getUsedBytes(TopLevelDecls) + ::getUsedBytes(Diags);
 }
 
-const InclusionLocations &ParsedAST::getInclusionLocations() const {
+const std::vector<Inclusion> &ParsedAST::getInclusionLocations() const {
   return IncLocations;
 }
 
 PreambleData::PreambleData(PrecompiledPreamble Preamble,
                            std::vector<serialization::DeclID> TopLevelDeclIDs,
                            std::vector<Diag> Diags,
-                           InclusionLocations IncLocations)
+                           std::vector<Inclusion> IncLocations)
     : Preamble(std::move(Preamble)),
       TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)),
       IncLocations(std::move(IncLocations)) {}
 
 ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble,
                      std::unique_ptr<CompilerInstance> Clang,
                      std::unique_ptr<FrontendAction> Action,
                      std::vector<const Decl *> TopLevelDecls,
-                     std::vector<Diag> Diags, InclusionLocations IncLocations)
+                     std::vector<Diag> Diags, std::vector<Inclusion> IncLocations)
     : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
       Action(std::move(Action)), Diags(std::move(Diags)),
       TopLevelDecls(std::move(TopLevelDecls)), PreambleDeclsDeserialized(false),
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -179,22 +179,6 @@
   void rename(PathRef File, Position Pos, llvm::StringRef NewName,
               Callback<std::vector<tooling::Replacement>> CB);
 
-  /// Inserts a new #include into \p File, if it's not present in \p Code.
-  ///
-  /// \p DeclaringHeader The original header corresponding to this insertion
-  /// e.g. the header that declared a symbol. This can be either a URI or a
-  /// literal string quoted with <> or "" that can be #included directly.
-  /// \p InsertedHeader The preferred header to be inserted. This may be
-  /// different from \p DeclaringHeader as a header file can have a different
-  /// canonical include. This can be either a URI or a literal string quoted
-  /// with <> or "" that can be #included directly.
-  ///
-  /// Both OriginalHeader and InsertedHeader will be considered to determine
-  /// whether an include needs to be added.
-  Expected<tooling::Replacements> insertInclude(PathRef File, StringRef Code,
-                                                StringRef DeclaringHeader,
-                                                StringRef InsertedHeader);
-
   /// Only for testing purposes.
   /// Waits until all requests to worker thread are finished and dumps AST for
   /// \p File. \p File must be in the list of added documents.
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -161,6 +161,7 @@
     // both the old and the new version in case only one of them matches.
     CompletionList Result = clangd::codeComplete(
         File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr,
+        PreambleData ? PreambleData->IncLocations : std::vector<Inclusion>(),
         IP->Contents, Pos, FS, PCHs, CodeCompleteOpts);
     CB(std::move(Result));
   };
@@ -272,66 +273,6 @@
       "Rename", File, Bind(Action, File.str(), NewName.str(), std::move(CB)));
 }
 
-/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal
-/// include.
-static llvm::Expected<HeaderFile> toHeaderFile(StringRef Header,
-                                               llvm::StringRef HintPath) {
-  if (isLiteralInclude(Header))
-    return HeaderFile{Header.str(), /*Verbatim=*/true};
-  auto U = URI::parse(Header);
-  if (!U)
-    return U.takeError();
-
-  auto IncludePath = URI::includeSpelling(*U);
-  if (!IncludePath)
-    return IncludePath.takeError();
-  if (!IncludePath->empty())
-    return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
-
-  auto Resolved = URI::resolve(*U, HintPath);
-  if (!Resolved)
-    return Resolved.takeError();
-  return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
-}
-
-Expected<tooling::Replacements>
-ClangdServer::insertInclude(PathRef File, StringRef Code,
-                            StringRef DeclaringHeader,
-                            StringRef InsertedHeader) {
-  assert(!DeclaringHeader.empty() && !InsertedHeader.empty());
-  std::string ToInclude;
-  auto ResolvedOrginal = toHeaderFile(DeclaringHeader, File);
-  if (!ResolvedOrginal)
-    return ResolvedOrginal.takeError();
-  auto ResolvedPreferred = toHeaderFile(InsertedHeader, File);
-  if (!ResolvedPreferred)
-    return ResolvedPreferred.takeError();
-  tooling::CompileCommand CompileCommand = CompileArgs.getCompileCommand(File);
-  auto Include =
-      calculateIncludePath(File, Code, *ResolvedOrginal, *ResolvedPreferred,
-                           CompileCommand, FSProvider.getFileSystem());
-  if (!Include)
-    return Include.takeError();
-  if (Include->empty())
-    return tooling::Replacements();
-  ToInclude = std::move(*Include);
-
-  auto Style = format::getStyle("file", File, "llvm");
-  if (!Style) {
-    llvm::consumeError(Style.takeError());
-    // FIXME(ioeric): needs more consistent style support in clangd server.
-    Style = format::getLLVMStyle();
-  }
-  // Replacement with offset UINT_MAX and length 0 will be treated as include
-  // insertion.
-  tooling::Replacement R(File, /*Offset=*/UINT_MAX, 0, "#include " + ToInclude);
-  auto Replaces =
-      format::cleanupAroundReplacements(Code, tooling::Replacements(R), *Style);
-  if (!Replaces)
-    return Replaces;
-  return formatReplacements(Code, *Replaces, *Style);
-}
-
 void ClangdServer::dumpAST(PathRef File,
                            UniqueFunction<void(std::string)> Callback) {
   auto Action = [](decltype(Callback) Callback,
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -60,32 +60,6 @@
 static URISchemeRegistry::Add<TestScheme>
     X("test", "Test scheme for clangd lit tests.");
 
-TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R) {
-  Range ReplacementRange = {
-      offsetToPosition(Code, R.getOffset()),
-      offsetToPosition(Code, R.getOffset() + R.getLength())};
-  return {ReplacementRange, R.getReplacementText()};
-}
-
-std::vector<TextEdit>
-replacementsToEdits(StringRef Code,
-                    const std::vector<tooling::Replacement> &Replacements) {
-  // Turn the replacements into the format specified by the Language Server
-  // Protocol. Fuse them into one big JSON array.
-  std::vector<TextEdit> Edits;
-  for (const auto &R : Replacements)
-    Edits.push_back(replacementToEdit(Code, R));
-  return Edits;
-}
-
-std::vector<TextEdit> replacementsToEdits(StringRef Code,
-                                          const tooling::Replacements &Repls) {
-  std::vector<TextEdit> Edits;
-  for (const auto &R : Repls)
-    Edits.push_back(replacementToEdit(Code, R));
-  return Edits;
-}
-
 SymbolKindBitset defaultSymbolKinds() {
   SymbolKindBitset Defaults;
   for (size_t I = SymbolKindMin; I <= static_cast<size_t>(SymbolKind::Array);
@@ -141,9 +115,7 @@
             {"workspaceSymbolProvider", true},
             {"executeCommandProvider",
              json::obj{
-                 {"commands",
-                  {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
-                   ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE}},
+                 {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}},
              }},
         }}}});
 }
@@ -216,42 +188,6 @@
 
     reply("Fix applied.");
     ApplyEdit(*Params.workspaceEdit);
-  } else if (Params.command ==
-             ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) {
-    auto &FileURI = Params.includeInsertion->textDocument.uri;
-    auto Code = DraftMgr.getDraft(FileURI.file());
-    if (!Code)
-      return replyError(ErrorCode::InvalidParams,
-                        ("command " +
-                         ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE +
-                         " called on non-added file " + FileURI.file())
-                            .str());
-    llvm::StringRef DeclaringHeader = Params.includeInsertion->declaringHeader;
-    if (DeclaringHeader.empty())
-      return replyError(
-          ErrorCode::InvalidParams,
-          "declaringHeader must be provided for include insertion.");
-    llvm::StringRef PreferredHeader = Params.includeInsertion->preferredHeader;
-    auto Replaces = Server.insertInclude(
-        FileURI.file(), *Code, DeclaringHeader,
-        PreferredHeader.empty() ? DeclaringHeader : PreferredHeader);
-    if (!Replaces) {
-      std::string ErrMsg =
-          ("Failed to generate include insertion edits for adding " +
-           DeclaringHeader + " (" + PreferredHeader + ") into " +
-           FileURI.file())
-              .str();
-      log(ErrMsg + ":" + llvm::toString(Replaces.takeError()));
-      replyError(ErrorCode::InternalError, ErrMsg);
-      return;
-    }
-    auto Edits = replacementsToEdits(*Code, *Replaces);
-    WorkspaceEdit WE;
-    WE.changes = {{FileURI.uri(), Edits}};
-
-    reply(("Inserted header " + DeclaringHeader + " (" + PreferredHeader + ")")
-              .str());
-    ApplyEdit(std::move(WE));
   } else {
     // We should not get here because ExecuteCommandParams would not have
     // parsed in the first place and this handler should not be called. But if
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to