Skip to content

Commit

Permalink
[clangd] Avoid inserting new #include when declaration is present in …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
Eric Liu committed May 30, 2018
1 parent c4b6d0e commit 9b3cba7
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 4 deletions.
17 changes: 16 additions & 1 deletion clang-tools-extra/clangd/CodeComplete.cpp
Expand Up @@ -235,13 +235,28 @@ 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,
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)
Expand All @@ -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);
Expand Down
36 changes: 33 additions & 3 deletions clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
Expand Up @@ -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");
Expand Down Expand Up @@ -425,10 +425,9 @@ TEST(CompletionTest, NoDuplicates) {
auto Results = completions(
R"cpp(
class Adapter {
void method();
};
void Adapter::method() {
void f() {
Adapter^
}
)cpp",
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 9b3cba7

Please sign in to comment.