[clang-tools-extra] r332459 - [clangd] Retrieve minimally formatted comment text in completion.
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Wed May 16 05:32:44 PDT 2018
Author: ibiryukov
Date: Wed May 16 05:32:44 2018
New Revision: 332459
URL: http://llvm.org/viewvc/llvm-project?rev=332459&view=rev
Log:
[clangd] Retrieve minimally formatted comment text in completion.
Summary:
Previous implementation used to extract brief text from doxygen comments.
Brief text parsing slows down completion and is not suited for
non-doxygen comments.
This commit switches to providing comments that mimic the ones
originally written in the source code, doing minimal reindenting and
removing the comments markers to make the output more user-friendly.
It means we lose support for doxygen-specific features, e.g. extracting
brief text, but provide useful results for non-doxygen comments.
Switching the doxygen support back is an option, but I suggest to see
whether the current approach gives more useful results.
Reviewers: sammccall, hokein, ioeric
Reviewed By: sammccall
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D45999
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompletionStringsTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=332459&r1=332458&r2=332459&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed May 16 05:32:44 2018
@@ -230,7 +230,8 @@ struct CompletionCandidate {
CompletionItem build(StringRef FileName, const CompletionItemScores &Scores,
const CodeCompleteOptions &Opts,
CodeCompletionString *SemaCCS,
- const IncludeInserter *Includes) const {
+ const IncludeInserter *Includes,
+ llvm::StringRef SemaDocComment) const {
assert(bool(SemaResult) == bool(SemaCCS));
CompletionItem I;
if (SemaResult) {
@@ -238,7 +239,7 @@ struct CompletionCandidate {
getLabelAndInsertText(*SemaCCS, &I.label, &I.insertText,
Opts.EnableSnippets);
I.filterText = getFilterText(*SemaCCS);
- I.documentation = getDocumentation(*SemaCCS);
+ I.documentation = formatDocumentation(*SemaCCS, SemaDocComment);
I.detail = getDetail(*SemaCCS);
}
if (IndexResult) {
@@ -481,17 +482,17 @@ struct CompletionRecorder : public CodeC
case CodeCompletionResult::RK_Pattern:
return Result.Pattern->getTypedText();
}
- auto *CCS = codeCompletionString(Result, /*IncludeBriefComments=*/false);
+ auto *CCS = codeCompletionString(Result);
return CCS->getTypedText();
}
// Build a CodeCompletion string for R, which must be from Results.
// The CCS will be owned by this recorder.
- CodeCompletionString *codeCompletionString(const CodeCompletionResult &R,
- bool IncludeBriefComments) {
+ CodeCompletionString *codeCompletionString(const CodeCompletionResult &R) {
// CodeCompletionResult doesn't seem to be const-correct. We own it, anyway.
return const_cast<CodeCompletionResult &>(R).CreateCodeCompletionString(
- *CCSema, CCContext, *CCAllocator, CCTUInfo, IncludeBriefComments);
+ *CCSema, CCContext, *CCAllocator, CCTUInfo,
+ /*IncludeBriefComments=*/false);
}
private:
@@ -535,7 +536,9 @@ public:
const auto *CCS = Candidate.CreateSignatureString(
CurrentArg, S, *Allocator, CCTUInfo, true);
assert(CCS && "Expected the CodeCompletionString to be non-null");
- SigHelp.signatures.push_back(ProcessOverloadCandidate(Candidate, *CCS));
+ SigHelp.signatures.push_back(ProcessOverloadCandidate(
+ Candidate, *CCS,
+ getParameterDocComment(S.getASTContext(), Candidate, CurrentArg)));
}
}
@@ -548,11 +551,12 @@ private:
// CompletionString.h.
SignatureInformation
ProcessOverloadCandidate(const OverloadCandidate &Candidate,
- const CodeCompletionString &CCS) const {
+ const CodeCompletionString &CCS,
+ llvm::StringRef DocComment) const {
SignatureInformation Result;
const char *ReturnType = nullptr;
- Result.documentation = getDocumentation(CCS);
+ Result.documentation = formatDocumentation(CCS, DocComment);
for (const auto &Chunk : CCS) {
switch (Chunk.Kind) {
@@ -802,7 +806,11 @@ clang::CodeCompleteOptions CodeCompleteO
Result.IncludeCodePatterns = EnableSnippets && IncludeCodePatterns;
Result.IncludeMacros = IncludeMacros;
Result.IncludeGlobals = true;
- Result.IncludeBriefComments = IncludeBriefComments;
+ // We choose to include full comments and not do doxygen parsing in
+ // completion.
+ // FIXME: ideally, we should support doxygen in some form, e.g. do markdown
+ // formatting of the comments.
+ Result.IncludeBriefComments = false;
// When an is used, Sema is responsible for completing the main file,
// the index can provide results from the preamble.
@@ -1020,9 +1028,15 @@ private:
CompletionItem toCompletionItem(const CompletionCandidate &Candidate,
const CompletionItemScores &Scores) {
CodeCompletionString *SemaCCS = nullptr;
- if (auto *SR = Candidate.SemaResult)
- SemaCCS = Recorder->codeCompletionString(*SR, Opts.IncludeBriefComments);
- return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get());
+ std::string DocComment;
+ if (auto *SR = Candidate.SemaResult) {
+ SemaCCS = Recorder->codeCompletionString(*SR);
+ if (Opts.IncludeComments) {
+ assert(Recorder->CCSema);
+ DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR);
+ }
+ }
+ return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get(), DocComment);
}
};
@@ -1050,7 +1064,7 @@ SignatureHelp signatureHelp(PathRef File
Options.IncludeGlobals = false;
Options.IncludeMacros = false;
Options.IncludeCodePatterns = false;
- Options.IncludeBriefComments = true;
+ Options.IncludeBriefComments = false;
semaCodeComplete(llvm::make_unique<SignatureHelpCollector>(Options, Result),
Options,
{FileName, Command, Preamble,
Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=332459&r1=332458&r2=332459&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Wed May 16 05:32:44 2018
@@ -45,10 +45,8 @@ struct CodeCompleteOptions {
/// Add macros to code completion results.
bool IncludeMacros = true;
- /// Add brief comments to completion items, if available.
- /// FIXME(ibiryukov): it looks like turning this option on significantly slows
- /// down completion, investigate if it can be made faster.
- bool IncludeBriefComments = true;
+ /// Add comments to code completion results, if available.
+ bool IncludeComments = true;
/// Include results that are not legal completions in the current context.
/// For example, private members are usually inaccessible.
Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=332459&r1=332458&r2=332459&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Wed May 16 05:32:44 2018
@@ -8,6 +8,8 @@
//===---------------------------------------------------------------------===//
#include "CodeCompletionStrings.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RawCommentList.h"
#include <utility>
namespace clang {
@@ -122,13 +124,43 @@ void processSnippetChunks(const CodeComp
} // namespace
+std::string getDocComment(const ASTContext &Ctx,
+ const CodeCompletionResult &Result) {
+ // FIXME: clang's completion also returns documentation for RK_Pattern if they
+ // contain a pattern for ObjC properties. Unfortunately, there is no API to
+ // get this declaration, so we don't show documentation in that case.
+ if (Result.Kind != CodeCompletionResult::RK_Declaration)
+ return "";
+ auto Decl = Result.getDeclaration();
+ if (!Decl)
+ return "";
+ const RawComment *RC = getCompletionComment(Ctx, Decl);
+ if (!RC)
+ return "";
+ return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+}
+
+std::string
+getParameterDocComment(const ASTContext &Ctx,
+ const CodeCompleteConsumer::OverloadCandidate &Result,
+ unsigned ArgIndex) {
+ auto Func = Result.getFunction();
+ if (!Func)
+ return "";
+ const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
+ if (!RC)
+ return "";
+ return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+}
+
void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,
std::string *InsertText, bool EnableSnippets) {
return EnableSnippets ? processSnippetChunks(CCS, Label, InsertText)
: processPlainTextChunks(CCS, Label, InsertText);
}
-std::string getDocumentation(const CodeCompletionString &CCS) {
+std::string formatDocumentation(const CodeCompletionString &CCS,
+ llvm::StringRef DocComment) {
// Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
// information in the documentation field.
std::string Result;
@@ -146,13 +178,13 @@ std::string getDocumentation(const CodeC
}
}
// Add brief documentation (if there is any).
- if (CCS.getBriefComment() != nullptr) {
+ if (!DocComment.empty()) {
if (!Result.empty()) {
// This means we previously added annotations. Add an extra newline
// character to make the annotations stand out.
Result.push_back('\n');
}
- Result += CCS.getBriefComment();
+ Result += DocComment;
}
return Result;
}
Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.h?rev=332459&r1=332458&r2=332459&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.h (original)
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.h Wed May 16 05:32:44 2018
@@ -17,8 +17,27 @@
#include "clang/Sema/CodeCompleteConsumer.h"
namespace clang {
+class ASTContext;
+
namespace clangd {
+/// Gets a minimally formatted documentation comment of \p Result, with comment
+/// markers stripped. See clang::RawComment::getFormattedText() for the detailed
+/// explanation of how the comment text is transformed.
+/// Returns empty string when no comment is available.
+std::string getDocComment(const ASTContext &Ctx,
+ const CodeCompletionResult &Result);
+
+/// Gets a minimally formatted documentation for parameter of \p Result,
+/// corresponding to argument number \p ArgIndex.
+/// This currently looks for comments attached to the parameter itself, and
+/// doesn't extract them from function documentation.
+/// Returns empty string when no comment is available.
+std::string
+getParameterDocComment(const ASTContext &Ctx,
+ const CodeCompleteConsumer::OverloadCandidate &Result,
+ unsigned ArgIndex);
+
/// Gets label and insert text for a completion item. For example, for function
/// `Foo`, this returns <"Foo(int x, int y)", "Foo"> without snippts enabled.
///
@@ -27,9 +46,13 @@ namespace clangd {
void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,
std::string *InsertText, bool EnableSnippets);
-/// Gets the documentation for a completion item. For example, comment for the
-/// a class declaration.
-std::string getDocumentation(const CodeCompletionString &CCS);
+/// Assembles formatted documentation for a completion result. This includes
+/// documentation comments and other relevant information like annotations.
+///
+/// \param DocComment is a documentation comment for the original declaration,
+/// it should be obtained via getDocComment or getParameterDocComment.
+std::string formatDocumentation(const CodeCompletionString &CCS,
+ llvm::StringRef DocComment);
/// Gets detail to be used as the detail field in an LSP completion item. This
/// is usually the return type of a function.
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=332459&r1=332458&r2=332459&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed May 16 05:32:44 2018
@@ -338,7 +338,8 @@ void SymbolCollector::finish() {
const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
SymbolID ID) {
- auto &SM = ND.getASTContext().getSourceManager();
+ auto &Ctx = ND.getASTContext();
+ auto &SM = Ctx.getSourceManager();
std::string QName;
llvm::raw_string_ostream OS(QName);
@@ -369,7 +370,7 @@ const Symbol *SymbolCollector::addDeclar
const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
*ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator,
*CompletionTUInfo,
- /*IncludeBriefComments*/ true);
+ /*IncludeBriefComments*/ false);
std::string Label;
std::string SnippetInsertText;
std::string IgnoredLabel;
@@ -379,7 +380,8 @@ const Symbol *SymbolCollector::addDeclar
getLabelAndInsertText(*CCS, &IgnoredLabel, &PlainInsertText,
/*EnableSnippets=*/false);
std::string FilterText = getFilterText(*CCS);
- std::string Documentation = getDocumentation(*CCS);
+ std::string Documentation =
+ formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion));
std::string CompletionDetail = getDetail(*CCS);
std::string Include;
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=332459&r1=332458&r2=332459&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed May 16 05:32:44 2018
@@ -261,8 +261,7 @@ void TestAfterDotCompletion(clangd::Code
// completion. At least there aren't any we're aware of.
EXPECT_THAT(Results.items, Not(Contains(Kind(CompletionItemKind::Snippet))));
// Check documentation.
- EXPECT_IFF(Opts.IncludeBriefComments, Results.items,
- Contains(IsDocumented()));
+ EXPECT_IFF(Opts.IncludeComments, Results.items, Contains(IsDocumented()));
}
void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
@@ -307,8 +306,7 @@ void TestGlobalScopeCompletion(clangd::C
AllOf(Has("local_var"), Has("LocalClass"),
Contains(Kind(CompletionItemKind::Snippet))));
// Check documentation.
- EXPECT_IFF(Opts.IncludeBriefComments, Results.items,
- Contains(IsDocumented()));
+ EXPECT_IFF(Opts.IncludeComments, Results.items, Contains(IsDocumented()));
}
TEST(CompletionTest, CompletionOptions) {
@@ -319,7 +317,7 @@ TEST(CompletionTest, CompletionOptions)
// We used to test every combination of options, but that got too slow (2^N).
auto Flags = {
&clangd::CodeCompleteOptions::IncludeMacros,
- &clangd::CodeCompleteOptions::IncludeBriefComments,
+ &clangd::CodeCompleteOptions::IncludeComments,
&clangd::CodeCompleteOptions::EnableSnippets,
&clangd::CodeCompleteOptions::IncludeCodePatterns,
&clangd::CodeCompleteOptions::IncludeIneligibleResults,
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompletionStringsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompletionStringsTests.cpp?rev=332459&r1=332458&r2=332459&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompletionStringsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompletionStringsTests.cpp Wed May 16 05:32:44 2018
@@ -51,14 +51,15 @@ TEST_F(CompletionStringTest, FilterText)
}
TEST_F(CompletionStringTest, Documentation) {
- Builder.addBriefComment("Is this brief?");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()), "Is this brief?");
+ Builder.addBriefComment("This is ignored");
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"),
+ "Is this brief?");
}
TEST_F(CompletionStringTest, DocumentationWithAnnotation) {
- Builder.addBriefComment("Is this brief?");
+ Builder.addBriefComment("This is ignored");
Builder.AddAnnotation("Ano");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()),
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"),
"Annotation: Ano\n\nIs this brief?");
}
@@ -67,7 +68,7 @@ TEST_F(CompletionStringTest, MultipleAnn
Builder.AddAnnotation("Ano2");
Builder.AddAnnotation("Ano3");
- EXPECT_EQ(getDocumentation(*Builder.TakeString()),
+ EXPECT_EQ(formatDocumentation(*Builder.TakeString(), ""),
"Annotations: Ano1 Ano2 Ano3\n");
}
@@ -97,7 +98,7 @@ TEST_F(CompletionStringTest, FunctionPla
TEST_F(CompletionStringTest, FunctionSnippet) {
Builder.AddResultTypeChunk("result no no");
- Builder.addBriefComment("Foo's comment");
+ Builder.addBriefComment("This comment is ignored");
Builder.AddTypedTextChunk("Foo");
Builder.AddChunk(CodeCompletionString::CK_LeftParen);
Builder.AddPlaceholderChunk("p1");
@@ -113,7 +114,7 @@ TEST_F(CompletionStringTest, FunctionSni
labelAndInsertText(*CCS, /*EnableSnippets=*/true);
EXPECT_EQ(Label, "Foo(p1, p2)");
EXPECT_EQ(InsertText, "Foo(${1:p1}, ${2:p2})");
- EXPECT_EQ(getDocumentation(*CCS), "Foo's comment");
+ EXPECT_EQ(formatDocumentation(*CCS, "Foo's comment"), "Foo's comment");
EXPECT_EQ(getFilterText(*CCS), "Foo");
}
Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=332459&r1=332458&r2=332459&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Wed May 16 05:32:44 2018
@@ -490,11 +490,11 @@ TEST_F(SymbolCollectorTest, SymbolWithDo
}
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAre(QName("nx"),
- AllOf(QName("nx::ff"),
- Labeled("ff(int x, double y)"),
- Detail("int"), Doc("Foo comment."))));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"), AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment."))));
}
TEST_F(SymbolCollectorTest, PlainAndSnippet) {
More information about the cfe-commits
mailing list