[clang-format] Put '/**' and '*/' on own lines in jsdocs ending in comment pragmas

Summary:
This handles a case where the trailing '*/' of a multiline jsdoc eding in a
comment pragma wouldn't be put on a new line.

Reviewers: mprobst

Reviewed By: mprobst

Subscribers: cfe-commits, klimek

Differential Revision: https://reviews.llvm.org/D36359

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@310458 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Format/BreakableToken.cpp b/lib/Format/BreakableToken.cpp
index 3844c8e7c01073e87f308e30a7ec2142ab30aabd..20e3e5b1dfd665abb79af9e7ba3276ec0f1b971d 100644 (file)
--- a/lib/Format/BreakableToken.cpp
+++ b/lib/Format/BreakableToken.cpp
@@ -681,12 +681,18 @@ void BreakableBlockComment::replaceWhitespaceBefore(
       InPPDirective, /*Newlines=*/1, ContentColumn[LineIndex] - Prefix.size());
 }
 
-BreakableToken::Split BreakableBlockComment::getSplitAfterLastLine(
-    unsigned TailOffset, unsigned ColumnLimit,
-    llvm::Regex &CommentPragmasRegex) const {
-  if (DelimitersOnNewline)
-    return getSplit(Lines.size() - 1, TailOffset, ColumnLimit,
-                    CommentPragmasRegex);
+BreakableToken::Split
+BreakableBlockComment::getSplitAfterLastLine(unsigned TailOffset,
+                                             unsigned ColumnLimit) const {
+  if (DelimitersOnNewline) {
+    // Replace the trailing whitespace of the last line with a newline.
+    // In case the last line is empty, the ending '*/' is already on its own
+    // line.
+    StringRef Line = Content.back().substr(TailOffset);
+    StringRef TrimmedLine = Line.rtrim(Blanks);
+    if (!TrimmedLine.empty())
+      return Split(TrimmedLine.size(), Line.size() - TrimmedLine.size());
+  }
   return Split(StringRef::npos, 0);
 }
 

diff --git a/lib/Format/BreakableToken.h b/lib/Format/BreakableToken.h
index e5cbe8f197e1cb312c488604fd5972af6f838492..630e927e8fc237715ef135bf41bc4f21f8446720 100644 (file)
--- a/lib/Format/BreakableToken.h
+++ b/lib/Format/BreakableToken.h
@@ -161,8 +161,8 @@ public:
   ///
   /// A result having offset == StringRef::npos means that no reformat is
   /// necessary.
-  virtual Split getSplitAfterLastLine(unsigned TailOffset, unsigned ColumnLimit,
-                                      llvm::Regex &CommentPragmasRegex) const {
+  virtual Split getSplitAfterLastLine(unsigned TailOffset,
+                                      unsigned ColumnLimit) const {
     return Split(StringRef::npos, 0);
   }
 
@@ -347,8 +347,8 @@ public:
   void replaceWhitespaceBefore(unsigned LineIndex, unsigned PreviousEndColumn,
                                unsigned ColumnLimit, Split SplitBefore,
                                WhitespaceManager &Whitespaces) override;
-  Split getSplitAfterLastLine(unsigned TailOffset, unsigned ColumnLimit,
-                              llvm::Regex &CommentPragmasRegex) const override;
+  Split getSplitAfterLastLine(unsigned TailOffset,
+                              unsigned ColumnLimit) const override;
 
   bool mayReflow(unsigned LineIndex,
                  llvm::Regex &CommentPragmasRegex) const override;

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index 25a470474b714c744e4f10dc4b7008eff7f8229d..d8ae82c1843e8c86eadb64fb1d1b3883e1b8072a 100644 (file)
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -1383,8 +1383,8 @@ unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
     }
   }
 
-  BreakableToken::Split SplitAfterLastLine = Token->getSplitAfterLastLine(
-      TailOffset, ColumnLimit, CommentPragmasRegex);
+  BreakableToken::Split SplitAfterLastLine =
+      Token->getSplitAfterLastLine(TailOffset, ColumnLimit);
   if (SplitAfterLastLine.first != StringRef::npos) {
     if (!DryRun)
       Token->replaceWhitespaceAfterLastLine(TailOffset, SplitAfterLastLine,

diff --git a/unittests/Format/FormatTestJS.cpp b/unittests/Format/FormatTestJS.cpp
index 033c024e5a06810f5533fef5fae2efb9c6db252c..e6827296e1264ffe3ecd35065ebc226069fb0cd5 100644 (file)
--- a/unittests/Format/FormatTestJS.cpp
+++ b/unittests/Format/FormatTestJS.cpp
@@ -158,6 +158,53 @@ TEST_F(FormatTestJS, JSDocComments) {
                    "var x = 1;\n"
                    "}",
                    getGoogleJSStyleWithColumns(20)));
+
+  // Don't break the first line of a single line short jsdoc comment pragma.
+  EXPECT_EQ("/** @returns j */",
+            format("/** @returns j */",
+                   getGoogleJSStyleWithColumns(20)));
+
+  // Break a single line long jsdoc comment pragma.
+  EXPECT_EQ("/**\n"
+            " * @returns {string} jsdoc line 12\n"
+            " */",
+            format("/** @returns {string} jsdoc line 12 */",
+                   getGoogleJSStyleWithColumns(20)));
+
+  EXPECT_EQ("/**\n"
+            " * @returns {string} jsdoc line 12\n"
+            " */",
+            format("/** @returns {string} jsdoc line 12  */",
+                   getGoogleJSStyleWithColumns(20)));
+
+  EXPECT_EQ("/**\n"
+            " * @returns {string} jsdoc line 12\n"
+            " */",
+            format("/** @returns {string} jsdoc line 12*/",
+                   getGoogleJSStyleWithColumns(20)));
+
+  // Fix a multiline jsdoc comment ending in a comment pragma.
+  EXPECT_EQ("/**\n"
+            " * line 1\n"
+            " * line 2\n"
+            " * @returns {string} jsdoc line 12\n"
+            " */",
+            format("/** line 1\n"
+                   " * line 2\n"
+                   " * @returns {string} jsdoc line 12 */",
+                   getGoogleJSStyleWithColumns(20)));
+
+  EXPECT_EQ("/**\n"
+            " * line 1\n"
+            " * line 2\n"
+            " *\n"
+            " * @returns j\n"
+            " */",
+            format("/** line 1\n"
+                   " * line 2\n"
+                   " *\n"
+                   " * @returns j */",
+                   getGoogleJSStyleWithColumns(20)));
 }
 
 TEST_F(FormatTestJS, UnderstandsJavaScriptOperators) {