[clangd] Do not show namespace comments.

[ilya-biryukov]

Summary

Comments from namespaces that clangd produces are too noisy and often
not useful.

Namespaces have too many redecls and we don't have a good way of
determining which of the comments are relevant and which should be
ignored (e.g. because they come from code generators like the protobuf
compiler).

[sammccall]

clangd/CodeCompletionStrings.cpp

172 ↗

(On Diff #151481)

while here, should this be if (Result.Declaration)? I make this mistake a lot...

177 ↗

(On Diff #151481)

nit: canRequestComment looks a little expensive, hoist above that?

[ilya-biryukov]

• Move the namespace check to happen before canRequestComment call

[ilya-biryukov]

clangd/CodeCompletionStrings.cpp

172 ↗

(On Diff #151481)

Looking at the FIXME above, it seems we might show a different comment from the one returned by clang's code completion.
I'll double-check if we do the right thing there and will try to do if (!Result.Declaration) in a follow-up patch.

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

std::string getDocComment(const ASTContext &Ctx,
                           const CodeCompletionResult &Result,
                           bool CommentsFromHeaders) {
   // 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 || !canRequestComment(Ctx, *Decl, CommentsFromHeaders))
+  if (!Decl || llvm::isa<NamespaceDecl>(Decl)) {
+    // Namespaces often have too many redecls for any particular redecl comment
+    // to be useful. Moreover, we often confuse file headers or generated
+    // comments with namespace comments. Therefore we choose to just ignore
+    // the comments for namespaces.
     return "";
+  }
+  if (!canRequestComment(Ctx, *Decl, CommentsFromHeaders))
+    return "";
+
   const RawComment *RC = getCompletionComment(Ctx, Decl);
   if (!RC)
     return "";
   std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
   if (!LooksLikeDocComment(Doc))
     return "";
   return Doc;
 }

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

TEST(CompletionTest, NonDocComments) {
   auto FooCpp = testPath("foo.cpp");
   FS.Files[FooCpp] = "";
 
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   Annotations Source(R"cpp(
+    // We ignore namespace comments, for rationale see CodeCompletionStrings.h.
+    namespace comments_ns {
+    }
+
     // ------------------
     int comments_foo();
 
     // A comment and a decl are separated by newlines.
     // Therefore, the comment shouldn't show up as doc comment.
 
     int comments_bar();
 
CompletionList Completions = cantFail(runCodeComplete(
       Server, FooCpp, Source.point(), clangd::CodeCompleteOptions()));
 
   // We should not get any of those comments in completion.
   EXPECT_THAT(
       Completions.items,
       UnorderedElementsAre(AllOf(Not(IsDocumented()), Named("comments_foo")),
                            AllOf(IsDocumented(), Named("comments_baz")),
                            AllOf(IsDocumented(), Named("comments_quux")),
+                           AllOf(Not(IsDocumented()), Named("comments_ns")),
                            // FIXME(ibiryukov): the following items should have
                            // empty documentation, since they are separated from
                            // a comment with an empty line. Unfortunately, I
                            // couldn't make Sema tests pass if we ignore those.
                            AllOf(IsDocumented(), Named("comments_bar")),
                            AllOf(IsDocumented(), Named("comments_qux"))));
 }