[clang-tools-extra] r334807 - [clangd] Do not report comments that only have special chars.

[Reid Kleckner via cfe-commits]

This test has been failing on Windows since it has been added:
http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/12549

I took a look at the unittest code, but I'm not familiar enough with this
code to debug it. Can you take a look at this?

On Fri, Jun 15, 2018 at 1:35 AM Ilya Biryukov via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: ibiryukov
> Date: Fri Jun 15 01:31:17 2018
> New Revision: 334807
>
> URL: http://llvm.org/viewvc/llvm-project?rev=334807&view=rev
> Log:
> [clangd] Do not report comments that only have special chars.
>
> Summary:
> Like the following:
>   // -------
>   // =======
>   // *******
>
> It does not cover all the cases, but those are definitely not very
> useful.
>
> Reviewers: sammccall, ioeric, hokein
>
> Reviewed By: sammccall
>
> Subscribers: MaskRay, jkorous, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D48171
>
> Modified:
>     clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
>     clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=334807&r1=334806&r2=334807&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)
> +++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Fri Jun 15
> 01:31:17 2018
> @@ -151,6 +151,16 @@ bool canRequestComment(const ASTContext
>    const ObjCPropertyDecl *PDecl = M ? M->findPropertyDecl() : nullptr;
>    return !PDecl || canRequestForDecl(*PDecl);
>  }
> +
> +bool LooksLikeDocComment(llvm::StringRef CommentText) {
> +  // We don't report comments that only contain "special" chars.
> +  // This avoids reporting various delimiters, like:
> +  //   =================
> +  //   -----------------
> +  //   *****************
> +  return CommentText.find_first_not_of("/*-= \t\r\n") !=
> llvm::StringRef::npos;
> +}
> +
>  } // namespace
>
>  std::string getDocComment(const ASTContext &Ctx,
> @@ -167,7 +177,10 @@ std::string getDocComment(const ASTConte
>    const RawComment *RC = getCompletionComment(Ctx, Decl);
>    if (!RC)
>      return "";
> -  return RC->getFormattedText(Ctx.getSourceManager(),
> Ctx.getDiagnostics());
> +  std::string Doc = RC->getFormattedText(Ctx.getSourceManager(),
> Ctx.getDiagnostics());
> +  if (!LooksLikeDocComment(Doc))
> +    return "";
> +  return Doc;
>  }
>
>  std::string
> @@ -180,7 +193,10 @@ getParameterDocComment(const ASTContext
>    const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
>    if (!RC)
>      return "";
> -  return RC->getFormattedText(Ctx.getSourceManager(),
> Ctx.getDiagnostics());
> +  std::string Doc = RC->getFormattedText(Ctx.getSourceManager(),
> Ctx.getDiagnostics());
> +  if (!LooksLikeDocComment(Doc))
> +    return "";
> +  return Doc;
>  }
>
>  void getLabelAndInsertText(const CodeCompletionString &CCS, std::string
> *Label,
>
> 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=334807&r1=334806&r2=334807&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
> (original)
> +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Jun
> 15 01:31:17 2018
> @@ -1100,6 +1100,65 @@ TEST(CompletionTest, DocumentationFromCh
>                Contains(AllOf(Not(IsDocumented()), Named("func"))));
>  }
>
> +TEST(CompletionTest, NonDocComments) {
> +  MockFSProvider FS;
> +  auto FooCpp = testPath("foo.cpp");
> +  FS.Files[FooCpp] = "";
> +
> +  MockCompilationDatabase CDB;
> +  IgnoreDiagnostics DiagConsumer;
> +  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
> +
> +  Annotations Source(R"cpp(
> +    // ------------------
> +    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();
> +
> +    // this comment should be in the results.
> +    int comments_baz();
> +
> +
> +    template <class T>
> +    struct Struct {
> +      int comments_qux();
> +      int comments_quux();
> +    };
> +
> +
> +    // This comment should not be there.
> +
> +    template <class T>
> +    int Struct<T>::comments_qux() {
> +    }
> +
> +    // This comment **should** be in results.
> +    template <class T>
> +    int Struct<T>::comments_quux() {
> +      int a = comments^;
> +    }
> +  )cpp");
> +  Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
> +  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")),
> +                           // 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"))));
> +}
> +
>  TEST(CompletionTest, CompleteOnInvalidLine) {
>    auto FooCpp = testPath("foo.cpp");
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180618/5cc92606/attachment-0001.html>