[PATCH] D47466: [clangd] Avoid inserting new #include when declaration is present in the main file.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, jkorous, MaskRay, klimek.

Also fix USR generation for classes in unit tests. The previous USR
only works for class members, which happens to work when completing class name
inside the class, where constructors are suggested by sema.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47466

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp


Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -159,7 +159,7 @@
   return sym(Name, index::SymbolKind::Function, "@F@\\0#"); // no args
 }
 Symbol cls(StringRef Name) {
-  return sym(Name, index::SymbolKind::Class, "@S@\\0 at S@\\0");
+  return sym(Name, index::SymbolKind::Class, "@S@\\0");
 }
 Symbol var(StringRef Name) {
   return sym(Name, index::SymbolKind::Variable, "@\\0");
@@ -425,10 +425,9 @@
   auto Results = completions(
       R"cpp(
           class Adapter {
-            void method();
           };
 
-          void Adapter::method() {
+          void f() {
             Adapter^
           }
       )cpp",
@@ -559,6 +558,37 @@
               ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits()))));
 }
 
+TEST(CompletionTest, NoIncludeInsertionWhenDeclFoundInFile) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  Symbol::Details Scratch;
+  Symbol SymX = cls("ns::X");
+  Symbol SymY = cls("ns::Y");
+  std::string BarHeader = testPath("bar.h");
+  auto BarURI = URI::createFile(BarHeader).toString();
+  SymX.CanonicalDeclaration.FileURI = BarURI;
+  SymY.CanonicalDeclaration.FileURI = BarURI;
+  Scratch.IncludeHeader = "<bar>";
+  SymX.Detail = &Scratch;
+  SymY.Detail = &Scratch;
+  // Shoten include path based on search dirctory and insert.
+  auto Results = completions(Server,
+                             R"cpp(
+          namespace ns {
+            class X;
+            class Y {}
+          }
+          int main() { ns::^ }
+      )cpp",
+                             {SymX, SymY});
+  EXPECT_THAT(Results.items,
+              ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits())),
+                          AllOf(Named("Y"), Not(HasAdditionalEdits()))));
+}
+
 TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -235,13 +235,23 @@
                        llvm::StringRef SemaDocComment) const {
     assert(bool(SemaResult) == bool(SemaCCS));
     CompletionItem I;
+    bool ShouldInsertInclude = true;
     if (SemaResult) {
       I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->CursorKind);
       getLabelAndInsertText(*SemaCCS, &I.label, &I.insertText,
                             Opts.EnableSnippets);
       I.filterText = getFilterText(*SemaCCS);
       I.documentation = formatDocumentation(*SemaCCS, SemaDocComment);
       I.detail = getDetail(*SemaCCS);
+      // Avoid inserting new #include if the declaration is found in the current
+      // file e.g. the symbol is forward declared.
+      if (SemaResult->Kind == CodeCompletionResult::RK_Declaration) {
+        if (const auto *D = SemaResult->getDeclaration()) {
+          const auto &SM = D->getASTContext().getSourceManager();
+          ShouldInsertInclude =
+              !SM.isInMainFile(SM.getExpansionLoc(D->getLocStart()));
+        }
+      }
     }
     if (IndexResult) {
       if (I.kind == CompletionItemKind::Missing)
@@ -263,7 +273,7 @@
           I.documentation = D->Documentation;
         if (I.detail.empty())
           I.detail = D->CompletionDetail;
-        if (Includes && !D->IncludeHeader.empty()) {
+        if (ShouldInsertInclude && Includes && !D->IncludeHeader.empty()) {
           auto Edit = [&]() -> Expected<Optional<TextEdit>> {
             auto ResolvedDeclaring = toHeaderFile(
                 IndexResult->CanonicalDeclaration.FileURI, FileName);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47466.148870.patch
Type: text/x-patch
Size: 3820 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180529/0d40a159/attachment.bin>