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@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);
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@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);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits