From 9b3cba79233444ac9af0a34bb7d6e7959a2380b3 Mon Sep 17 00:00:00 2001
From: Eric Liu <ioeric@google.com>
Date: Wed, 30 May 2018 09:03:39 +0000
Subject: [PATCH] [clangd] Avoid inserting new #include when declaration is
 present in the main file.

Summary:
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.

Reviewers: sammccall, ilya-biryukov

Subscribers: klimek, MaskRay, jkorous, cfe-commits

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

llvm-svn: 333519
---
 clang-tools-extra/clangd/CodeComplete.cpp     | 17 ++++++++-
 .../unittests/clangd/CodeCompleteTests.cpp    | 36 +++++++++++++++++--
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 09e1836150624..40d9058040fba 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -235,6 +235,7 @@ struct CompletionCandidate {
                        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,
@@ -242,6 +243,20 @@ struct CompletionCandidate {
       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 =
+              ShouldInsertInclude &&
+              std::none_of(D->redecls_begin(), D->redecls_end(),
+                           [&SM](const Decl *RD) {
+                             return SM.isInMainFile(
+                                 SM.getExpansionLoc(RD->getLocStart()));
+                           });
+        }
+      }
     }
     if (IndexResult) {
       if (I.kind == CompletionItemKind::Missing)
@@ -263,7 +278,7 @@ struct CompletionCandidate {
           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);
diff --git a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
index bb4e87c8ee4bc..2fd8588543aee 100644
--- a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
+++ b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
@@ -159,7 +159,7 @@ Symbol func(StringRef Name) { // Assumes the function has no args.
   return sym(Name, index::SymbolKind::Function, "@F@\\0#"); // no args
 }
 Symbol cls(StringRef Name) {
-  return sym(Name, index::SymbolKind::Class, "@S@\\0@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 @@ TEST(CompletionTest, NoDuplicates) {
   auto Results = completions(
       R"cpp(
           class Adapter {
-            void method();
           };
 
-          void Adapter::method() {
+          void f() {
             Adapter^
           }
       )cpp",
@@ -559,6 +558,37 @@ TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) {
               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;