Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -183,6 +183,10 @@ // We may have a result from Sema, from the index, or both. const CodeCompletionResult *SemaResult = nullptr; const Symbol *IndexResult = nullptr; + // Whether or not this candidate is in a completion where index is allowed. + // This can affect how items are built (e.g. indent label to align with visual + // indicator in index results). + bool AllowIndexCompletion = false; // Builds an LSP completion item. CompletionItem build(StringRef FileName, const CompletionItemScores &Scores, @@ -191,8 +195,10 @@ const IncludeInserter *Includes, llvm::StringRef SemaDocComment) const { assert(bool(SemaResult) == bool(SemaCCS)); + assert((AllowIndexCompletion || !IndexResult)); CompletionItem I; bool ShouldInsertInclude = true; + bool InsertingInclude = false; // Whether a new #include will be added. if (SemaResult) { I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->Declaration); getLabelAndInsertText(*SemaCCS, &I.label, &I.insertText, @@ -236,7 +242,8 @@ if (I.detail.empty()) I.detail = D->CompletionDetail; if (ShouldInsertInclude && Includes && !D->IncludeHeader.empty()) { - auto Edit = [&]() -> Expected> { + auto HeaderAndEdit = + [&]() -> Expected>> { auto ResolvedDeclaring = toHeaderFile( IndexResult->CanonicalDeclaration.FileURI, FileName); if (!ResolvedDeclaring) @@ -246,7 +253,7 @@ return ResolvedInserted.takeError(); return Includes->insert(*ResolvedDeclaring, *ResolvedInserted); }(); - if (!Edit) { + if (!HeaderAndEdit) { std::string ErrMsg = ("Failed to generate include insertion edits for adding header " "(FileURI=\"" + @@ -254,13 +261,21 @@ "\", IncludeHeader=\"" + D->IncludeHeader + "\") into " + FileName) .str(); - log(ErrMsg + ":" + llvm::toString(Edit.takeError())); - } else if (*Edit) { - I.additionalTextEdits = {std::move(**Edit)}; + log(ErrMsg + ":" + llvm::toString(HeaderAndEdit.takeError())); + } else { + InsertingInclude = static_cast(HeaderAndEdit->second); + if (InsertingInclude) { + I.detail += ((I.detail.empty() ? "" : "\n") + + StringRef(HeaderAndEdit->first).trim('"')) + .str(); + I.additionalTextEdits = {std::move(*(HeaderAndEdit->second))}; + } } } } } + if (AllowIndexCompletion) + I.label = (InsertingInclude ? "+" : " ") + I.label; I.scoreInfo = Scores; I.sortText = sortText(Scores.finalScore, Name); I.insertTextFormat = Opts.EnableSnippets ? InsertTextFormat::Snippet @@ -848,6 +863,7 @@ bool Incomplete = false; // Would more be available with a higher limit? llvm::Optional Filter; // Initialized once Sema runs. std::unique_ptr Includes; // Initialized once compiler runs. + bool AllowIndexCompletion = false; // Initialized once Sema runs. public: // A CodeCompleteFlow object is only useful for calling run() exactly once. @@ -897,12 +913,13 @@ CompletionList runWithSema() { Filter = FuzzyMatcher( Recorder->CCSema->getPreprocessor().getCodeCompletionFilter()); + AllowIndexCompletion = Opts.Index && allowIndex(Recorder->CCContext); // Sema provides the needed context to query the index. // FIXME: in addition to querying for extra/overlapping symbols, we should // explicitly request symbols corresponding to Sema results. // We can use their signals even if the index can't suggest them. // We must copy index results to preserve them, but there are at most Limit. - auto IndexResults = queryIndex(); + auto IndexResults = AllowIndexCompletion ? queryIndex() : SymbolSlab(); // Merge Sema and Index results, score them, and pick the winners. auto Top = mergeResults(Recorder->Results, IndexResults); // Convert the results to the desired LSP structs. @@ -914,8 +931,6 @@ } SymbolSlab queryIndex() { - if (!Opts.Index || !allowIndex(Recorder->CCContext)) - return SymbolSlab(); trace::Span Tracer("Query index"); SPAN_ATTACH(Tracer, "limit", Opts.Limit); @@ -986,6 +1001,7 @@ const CodeCompletionResult *SemaResult, const Symbol *IndexResult) { CompletionCandidate C; + C.AllowIndexCompletion = AllowIndexCompletion; C.SemaResult = SemaResult; C.IndexResult = IndexResult; C.Name = IndexResult ? IndexResult->Name : Recorder->getName(*SemaResult); Index: clangd/Headers.h =================================================================== --- clangd/Headers.h +++ clangd/Headers.h @@ -60,11 +60,12 @@ /// InsertedHeader e.g. the header that declares a symbol. /// \param InsertedHeader The preferred header to be inserted. This could be the /// same as DeclaringHeader but must be provided. -// \return A quoted "path" or . This returns an empty string if: +/// \return A quoted "path" or to be included and a boolean that is true +/// if the include can be added, and false if: /// - Either \p DeclaringHeader or \p InsertedHeader is already (directly) /// in \p Inclusions (including those included via different paths). /// - \p DeclaringHeader or \p InsertedHeader is the same as \p File. -llvm::Expected calculateIncludePath( +llvm::Expected> calculateIncludePath( PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo, const std::vector &Inclusions, const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader); @@ -81,16 +82,18 @@ void addExisting(Inclusion Inc) { Inclusions.push_back(std::move(Inc)); } - /// Returns a TextEdit that inserts a new header; if the header is not - /// inserted e.g. it's an existing header, this returns None. If any header is - /// invalid, this returns error. + /// Returns a pair where Header is verbatim + /// (e.g. ) and Edit, if not None, inserts the entire include + /// directive. Edit is None is the header is not suitable to include e.g. + /// it's already included. If any header is invalid, this returns error. /// /// \param DeclaringHeader is the original header corresponding to \p /// InsertedHeader e.g. the header that declares a symbol. /// \param InsertedHeader The preferred header to be inserted. This could be /// the same as DeclaringHeader but must be provided. - Expected> insert(const HeaderFile &DeclaringHeader, - const HeaderFile &InsertedHeader) const; + Expected>> + insert(const HeaderFile &DeclaringHeader, + const HeaderFile &InsertedHeader) const; private: StringRef FileName; Index: clangd/Headers.cpp =================================================================== --- clangd/Headers.cpp +++ clangd/Headers.cpp @@ -72,13 +72,13 @@ /// FIXME(ioeric): we might not want to insert an absolute include path if the /// path is not shortened. -llvm::Expected calculateIncludePath( +llvm::Expected> calculateIncludePath( PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo, const std::vector &Inclusions, const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader) { assert(DeclaringHeader.valid() && InsertedHeader.valid()); - if (File == DeclaringHeader.File || File == InsertedHeader.File) - return ""; + bool AddInclude = + File != DeclaringHeader.File && File != InsertedHeader.File; llvm::StringSet<> IncludedHeaders; for (const auto &Inc : Inclusions) { IncludedHeaders.insert(Inc.Written); @@ -88,13 +88,13 @@ auto Included = [&](llvm::StringRef Header) { return IncludedHeaders.find(Header) != IncludedHeaders.end(); }; - if (Included(DeclaringHeader.File) || Included(InsertedHeader.File)) - return ""; + AddInclude &= + !Included(DeclaringHeader.File) && !Included(InsertedHeader.File); bool IsSystem = false; if (InsertedHeader.Verbatim) - return InsertedHeader.File; + return std::make_pair(InsertedHeader.File, AddInclude); std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics( InsertedHeader.File, BuildDir, &IsSystem); @@ -102,12 +102,10 @@ Suggested = "<" + Suggested + ">"; else Suggested = "\"" + Suggested + "\""; - - log("Suggested #include for " + InsertedHeader.File + " is: " + Suggested); - return Suggested; + return std::make_pair(Suggested, AddInclude); } -Expected> +Expected>> IncludeInserter::insert(const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader) const { auto Validate = [](const HeaderFile &Header) { @@ -127,14 +125,13 @@ DeclaringHeader, InsertedHeader); if (!Include) return Include.takeError(); - if (Include->empty()) - return llvm::None; - StringRef IncludeRef = *Include; - auto Insertion = - Inserter.insert(IncludeRef.trim("\"<>"), IncludeRef.startswith("<")); - if (!Insertion) - return llvm::None; - return replacementToEdit(Code, *Insertion); + StringRef IncludeRef = Include->first; + Optional Edit = None; + if (Include->second) + if (auto Insertion = Inserter.insert(IncludeRef.trim("\"<>"), + IncludeRef.startswith("<"))) + Edit = replacementToEdit(Code, *Insertion); + return std::make_pair(IncludeRef, std::move(Edit)); } } // namespace clangd Index: test/clangd/completion-snippets.test =================================================================== --- test/clangd/completion-snippets.test +++ test/clangd/completion-snippets.test @@ -32,7 +32,7 @@ # CHECK-NEXT: "insertText": "func_with_args(${1:int a}, ${2:int b})", # CHECK-NEXT: "insertTextFormat": 2, # CHECK-NEXT: "kind": 3, -# CHECK-NEXT: "label": "func_with_args(int a, int b)", +# CHECK-NEXT: "label": " func_with_args(int a, int b)", # CHECK-NEXT: "sortText": "{{.*}}func_with_args" # CHECK-NEXT: } # CHECK-NEXT: ] Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -566,8 +566,8 @@ int main() { ns::^ } )cpp", {Sym}); - EXPECT_THAT(Results.items, - ElementsAre(AllOf(Named("X"), InsertInclude("\"bar.h\"")))); + EXPECT_THAT(Results.items, ElementsAre(AllOf(Named("X"), Labeled("+X"), + InsertInclude("\"bar.h\"")))); // Duplicate based on inclusions in preamble. Results = completions(Server, R"cpp( @@ -575,8 +575,8 @@ int main() { ns::^ } )cpp", {Sym}); - EXPECT_THAT(Results.items, - ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits())))); + EXPECT_THAT(Results.items, ElementsAre(AllOf(Named("X"), Labeled(" X"), + Not(HasAdditionalEdits())))); } TEST(CompletionTest, NoIncludeInsertionWhenDeclFoundInFile) { Index: unittests/clangd/HeadersTests.cpp =================================================================== --- unittests/clangd/HeadersTests.cpp +++ unittests/clangd/HeadersTests.cpp @@ -106,7 +106,7 @@ } else { EXPECT_FALSE(ExpectError); } - return std::move(*Path); + return Path->second ? std::move(Path->first) : ""; } Expected> @@ -123,9 +123,13 @@ for (const auto &Inc : ExistingInclusions) Inserter.addExisting(Inc); - auto Edit = Inserter.insert(DeclaringHeader, InsertedHeader); + auto HeaderAndEdit = Inserter.insert(DeclaringHeader, InsertedHeader); Action.EndSourceFile(); - return Edit; + if (!HeaderAndEdit) + return HeaderAndEdit.takeError(); + if (!HeaderAndEdit->second) + return llvm::None; + return *(HeaderAndEdit->second); } MockFSProvider FS;