[clang-tools-extra] r332363 - [clangd] Populate #include insertions as additional edits in completion items.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Tue May 15 08:29:33 PDT 2018


Author: ioeric
Date: Tue May 15 08:29:32 2018
New Revision: 332363

URL: http://llvm.org/viewvc/llvm-project?rev=332363&view=rev
Log:
[clangd] Populate #include insertions as additional edits in completion items.

Summary:
o Remove IncludeInsertion LSP command.
o Populate include insertion edits synchromously in completion items.
o Share the code completion compiler instance and precompiled preamble to get existing inclusions in main file.
o Include insertion logic lives only in CodeComplete now.
o Use tooling::HeaderIncludes for inserting new includes.
o Refactored tests.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits

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

Modified:
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/CodeComplete.cpp
    clang-tools-extra/trunk/clangd/CodeComplete.h
    clang-tools-extra/trunk/clangd/Headers.cpp
    clang-tools-extra/trunk/clangd/Headers.h
    clang-tools-extra/trunk/clangd/Protocol.h
    clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
    clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=332363&r1=332362&r2=332363&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue May 15 08:29:32 2018
@@ -161,6 +161,7 @@ void ClangdServer::codeComplete(PathRef
     // 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->Inclusions : std::vector<Inclusion>(),
         IP->Contents, Pos, FS, PCHs, CodeCompleteOpts);
     CB(std::move(Result));
   };

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=332363&r1=332362&r2=332363&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue May 15 08:29:32 2018
@@ -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,28 @@ std::string sortText(float Score, llvm::
   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};
+}
+
 /// 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 +279,10 @@ struct CompletionCandidate {
   }
 
   // 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,6 +313,30 @@ struct CompletionCandidate {
           I.documentation = D->Documentation;
         if (I.detail.empty())
           I.detail = D->CompletionDetail;
+        if (Includes && !D->IncludeHeader.empty()) {
+          auto Edit = [&]() -> Expected<Optional<TextEdit>> {
+            auto ResolvedDeclaring = toHeaderFile(
+                IndexResult->CanonicalDeclaration.FileURI, FileName);
+            if (!ResolvedDeclaring)
+              return ResolvedDeclaring.takeError();
+            auto ResolvedInserted = toHeaderFile(D->IncludeHeader, FileName);
+            if (!ResolvedInserted)
+              return ResolvedInserted.takeError();
+            return Includes->insert(*ResolvedDeclaring, *ResolvedInserted);
+          }();
+          if (!Edit) {
+            std::string ErrMsg =
+                ("Failed to generate include insertion edits for adding header "
+                 "(FileURI=\"" +
+                 IndexResult->CanonicalDeclaration.FileURI +
+                 "\", IncludeHeader=\"" + D->IncludeHeader + "\") into " +
+                 FileName)
+                    .str();
+            log(ErrMsg + ":" + llvm::toString(Edit.takeError()));
+          } else if (*Edit) {
+            I.additionalTextEdits = {std::move(**Edit)};
+          }
+        }
       }
     }
     I.scoreInfo = Scores;
@@ -649,6 +697,7 @@ struct SemaCompleteInput {
   PathRef FileName;
   const tooling::CompileCommand &Command;
   PrecompiledPreamble const *Preamble;
+  const std::vector<Inclusion> &PreambleInclusions;
   StringRef Contents;
   Position Pos;
   IntrusiveRefCntPtr<vfs::FileSystem> VFS;
@@ -656,9 +705,12 @@ struct SemaCompleteInput {
 };
 
 // Invokes Sema code completion on a file.
+// If \p Includes is set, it will be initialized after a compiler instance has
+// been set up.
 bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
                       const clang::CodeCompleteOptions &Options,
-                      const SemaCompleteInput &Input) {
+                      const SemaCompleteInput &Input,
+                      std::unique_ptr<IncludeInserter> *Includes = nullptr) {
   trace::Span Tracer("Sema completion");
   std::vector<const char *> ArgStrs;
   for (const auto &S : Input.Command.CommandLine)
@@ -729,6 +781,28 @@ bool semaCodeComplete(std::unique_ptr<Co
         Input.FileName);
     return false;
   }
+  if (Includes) {
+    // Initialize Includes if provided.
+
+    // FIXME(ioeric): needs more consistent style support in clangd server.
+    auto Style = format::getStyle("file", Input.FileName, "LLVM",
+                                  Input.Contents, Input.VFS.get());
+    if (!Style) {
+      log("Failed to get FormatStyle for file" + Input.FileName +
+          ". Fall back to use LLVM style. Error: " +
+          llvm::toString(Style.takeError()));
+      Style = format::getLLVMStyle();
+    }
+    *Includes = llvm::make_unique<IncludeInserter>(
+        Input.FileName, Input.Contents, *Style, Input.Command.Directory,
+        Clang->getPreprocessor().getHeaderSearchInfo());
+    for (const auto &Inc : Input.PreambleInclusions)
+      Includes->get()->addExisting(Inc);
+    Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
+        Clang->getSourceManager(), [Includes](Inclusion Inc) {
+          Includes->get()->addExisting(std::move(Inc));
+        }));
+  }
   if (!Action.Execute()) {
     log("Execute() failed when running codeComplete for " + Input.FileName);
     return false;
@@ -863,7 +937,8 @@ class CodeCompleteFlow {
   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.
@@ -872,20 +947,25 @@ public:
 
   CompletionList run(const SemaCompleteInput &SemaCCInput) && {
     trace::Span Tracer("CodeCompleteFlow");
+
     // 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.
       Output = runWithSema();
+      Includes.reset(); // 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, &Includes);
 
     SPAN_ATTACH(Tracer, "sema_results", NSema);
     SPAN_ATTACH(Tracer, "index_results", NIndex);
@@ -1011,19 +1091,21 @@ private:
     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> &PreambleInclusions,
                             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, PreambleInclusions, Contents, Pos, VFS,
+            PCHs});
 }
 
 SignatureHelp signatureHelp(PathRef FileName,
@@ -1040,7 +1122,8 @@ SignatureHelp signatureHelp(PathRef File
   Options.IncludeBriefComments = true;
   semaCodeComplete(llvm::make_unique<SignatureHelpCollector>(Options, Result),
                    Options,
-                   {FileName, Command, Preamble, Contents, Pos, std::move(VFS),
+                   {FileName, Command, Preamble,
+                    /*PreambleInclusions=*/{}, Contents, Pos, std::move(VFS),
                     std::move(PCHs)});
   return Result;
 }

Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=332363&r1=332362&r2=332363&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Tue May 15 08:29:32 2018
@@ -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 @@ struct CodeCompleteOptions {
 CompletionList codeComplete(PathRef FileName,
                             const tooling::CompileCommand &Command,
                             PrecompiledPreamble const *Preamble,
+                            const std::vector<Inclusion> &PreambleInclusions,
                             StringRef Contents, Position Pos,
                             IntrusiveRefCntPtr<vfs::FileSystem> VFS,
                             std::shared_ptr<PCHContainerOperations> PCHs,

Modified: clang-tools-extra/trunk/clangd/Headers.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.cpp?rev=332363&r1=332362&r2=332363&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Headers.cpp (original)
+++ clang-tools-extra/trunk/clangd/Headers.cpp Tue May 15 08:29:32 2018
@@ -15,8 +15,6 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/HeaderSearch.h"
-#include "clang/Lex/PreprocessorOptions.h"
-#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
@@ -74,64 +72,13 @@ collectInclusionsInMainFileCallback(cons
 
 /// 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());
-  std::vector<Inclusion> Inclusions;
-  Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
-      Clang->getSourceManager(),
-      [&Inclusions](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }));
-  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);
@@ -144,14 +91,13 @@ calculateIncludePath(llvm::StringRef Fil
   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
@@ -161,5 +107,35 @@ calculateIncludePath(llvm::StringRef Fil
   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);
+}
+
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/Headers.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.h?rev=332363&r1=332362&r2=332363&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Headers.h (original)
+++ clang-tools-extra/trunk/clangd/Headers.h Tue May 15 08:29:32 2018
@@ -12,10 +12,14 @@
 
 #include "Path.h"
 #include "Protocol.h"
+#include "SourceCode.h"
 #include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Format/Format.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 {
@@ -51,20 +55,51 @@ collectInclusionsInMainFileCallback(cons
 /// '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);
+
+// Calculates insertion edit for including a new header in a file.
+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 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.
+  ///
+  /// \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;
+
+private:
+  StringRef FileName;
+  StringRef Code;
+  StringRef BuildDir;
+  HeaderSearch &HeaderSearchInfo;
+  std::vector<Inclusion> Inclusions;
+  tooling::HeaderIncludes Inserter; // Computers insertion replacement.
+};
 
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=332363&r1=332362&r2=332363&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Tue May 15 08:29:32 2018
@@ -99,6 +99,9 @@ struct Position {
     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);

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=332363&r1=332362&r2=332363&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Tue May 15 08:29:32 2018
@@ -45,6 +45,16 @@ MATCHER_P(Kind, K, "") { return arg.kind
 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 @@ MATCHER(NameContainsFilter, "") {
     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 @@ std::unique_ptr<SymbolIndex> memIndex(st
   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 @@ CompletionList completions(StringRef Tex
     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 @@ CompletionList completions(StringRef Tex
   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);
@@ -505,6 +523,42 @@ TEST(CompletionTest, SemaIndexMergeWithL
   EXPECT_TRUE(Results.isIncomplete);
 }
 
+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());
+  Symbol::Details Scratch;
+  auto BarURI = URI::createFile(BarHeader).toString();
+  Symbol Sym = cls("ns::X");
+  Sym.CanonicalDeclaration.FileURI = BarURI;
+  Scratch.IncludeHeader = BarURI;
+  Sym.Detail = &Scratch;
+  // Shoten include path based on search dirctory and insert.
+  auto Results = completions(Server,
+                             R"cpp(
+          int main() { ns::^ }
+      )cpp",
+                             {Sym});
+  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, so should only match resolved.
+          int main() { ns::^ }
+      )cpp",
+                        {Sym});
+  EXPECT_THAT(Results.items,
+              ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits()))));
+}
+
 TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;

Modified: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp?rev=332363&r1=332362&r2=332363&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp Tue May 15 08:29:32 2018
@@ -8,7 +8,13 @@
 //===----------------------------------------------------------------------===//
 
 #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"
 
@@ -16,30 +22,83 @@ 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(),
+        [&](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }));
+    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 +108,50 @@ protected:
     }
     return std::move(*Path);
   }
+
+  Expected<Optional<TextEdit>>
+  insert(const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader,
+         const std::vector<Inclusion> &ExistingInclusions = {}) {
+    auto Clang = setupClang();
+    PreprocessOnlyAction Action;
+    EXPECT_TRUE(
+        Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
+
+    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);
+    Action.EndSourceFile();
+    return Edit;
+  }
+
   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.
-}
 
-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), "");
+  EXPECT_THAT(collectIncludes(),
+              UnorderedElementsAre(
+                  AllOf(Written("\"sub/bar.h\""), Resolved(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 +161,92 @@ TEST_F(HeadersTest, StillInsertIfTrasiti
   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), "");
+}
+
+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());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
-




More information about the cfe-commits mailing list