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

[ioeric]

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.

[sammccall]

clangd/CodeComplete.cpp

249 ↗

(On Diff #148870)

do you need to iterate over all redecls?

251 ↗

(On Diff #148870)

ShouldInsertInclude = ShouldInsertInclude && ... would be a little more future-proof

[ioeric]

Addressed review comments.

clang-tools-extra/trunk/clangd/CodeComplete.cpp

struct CompletionCandidate {
   // Builds an LSP completion item.
   CompletionItem build(StringRef FileName, const CompletionItemScores &Scores,
                        const CodeCompleteOptions &Opts,
                        CodeCompletionString *SemaCCS,
                        const IncludeInserter *Includes,
                        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 =
+              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)
         I.kind = toCompletionItemKind(IndexResult->SymInfo.Kind);
       // FIXME: reintroduce a way to show the index source for debugging.
       if (I.label.empty())
         I.label = IndexResult->CompletionLabel;
       if (I.filterText.empty())
         I.filterText = IndexResult->Name;
 
       // FIXME(ioeric): support inserting/replacing scope qualifiers.
       if (I.insertText.empty())
         I.insertText = Opts.EnableSnippets
                            ? IndexResult->CompletionSnippetInsertText
                            : IndexResult->CompletionPlainInsertText;
 
       if (auto *D = IndexResult->Detail) {
         if (I.documentation.empty())
           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);
             if (!ResolvedDeclaring)
               return ResolvedDeclaring.takeError();
             auto ResolvedInserted = toHeaderFile(D->IncludeHeader, FileName);
             if (!ResolvedInserted)
               return ResolvedInserted.takeError();

clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Symbol sym(StringRef QName, index::SymbolKind Kind, StringRef USRFormat) {
   Sym.CompletionLabel = Sym.Name;
   Sym.SymInfo.Kind = Kind;
   return Sym;
 }
 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");
 }
 Symbol ns(StringRef Name) {
   return sym(Name, index::SymbolKind::Namespace, "@N@\\0");
 }
 Symbol withReferences(int N, Symbol S) {
TEST(CompletionTest, Kinds) {
   Results = completions("nam^");
   EXPECT_THAT(Results.items, Has("namespace", CompletionItemKind::Snippet));
 }
 
 TEST(CompletionTest, NoDuplicates) {
   auto Results = completions(
       R"cpp(
           class Adapter {
-            void method();
           };
 
-          void Adapter::method() {
+          void f() {
             Adapter^
           }
       )cpp",
       {cls("Adapter")});
 
   // Make sure there are no duplicate entries of 'Adapter'.
   EXPECT_THAT(Results.items, ElementsAre(Named("Adapter")));
 }
Results = completions(Server,
           #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, 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;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   FS.Files[testPath("bar.h")] =
       R"cpp(namespace ns { struct preamble { int member; }; })cpp";