clang-format: [JS] ignore comments when wrapping returns.

[mprobst]

Summary

When code contains a comment between return and the value:

return /* lengthy comment here */ (
    lengthyValueComesHere);

Do not wrap before the comment, as that'd break the code through JS' automatic
semicolon insertion.

[mprobst]

lib/Format/TokenAnnotator.cpp

2384

Should the entire method generally consider the previous non-comment, instead of just the previous token? Not sure if that makes sense in all cases, and I don't think we always have a previous non-comment token either.

[djasper]

Looks good.

lib/Format/TokenAnnotator.cpp

2384

Lets leave this as is for now. Yes, some of these should likely look past comments, but probably not all of them.

lib/Format/TokenAnnotator.cpp

bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
   if (Style.Language == FormatStyle::LK_Java) {
     if (Left.isOneOf(Keywords.kw_throws, Keywords.kw_extends,
                      Keywords.kw_implements))
       return false;
     if (Right.isOneOf(Keywords.kw_throws, Keywords.kw_extends,
                       Keywords.kw_implements))
       return true;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
-    if (Left.is(tok::kw_return))
+    const FormatToken *NonComment = Right.getPreviousNonComment();
+    if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+                     tok::kw_throw) ||
+        (NonComment &&
+         NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+                             tok::kw_throw)))
       return false; // Otherwise a semicolon is inserted.
     if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
       return false;
     if (Left.is(TT_JsTypeColon))
       return true;
     if (Right.NestingLevel == 0 && Right.is(Keywords.kw_is))
       return false;
     if (Left.is(Keywords.kw_in))

unittests/Format/FormatTestJS.cpp

verifyFormat("for (let {a, b} in x) {\n"
                "}");
 }
 
 TEST_F(FormatTestJS, WrapRespectsAutomaticSemicolonInsertion) {
   // The following statements must not wrap, as otherwise the program meaning
   // would change due to automatic semicolon insertion.
   // See http://www.ecma-international.org/ecma-262/5.1/#sec-7.9.1.
   verifyFormat("return aaaaa;", getGoogleJSStyleWithColumns(10));
+  verifyFormat("return /* hello! */ aaaaa;", getGoogleJSStyleWithColumns(10));
   verifyFormat("continue aaaaa;", getGoogleJSStyleWithColumns(10));
+  verifyFormat("continue /* hello! */ aaaaa;", getGoogleJSStyleWithColumns(10));
   verifyFormat("break aaaaa;", getGoogleJSStyleWithColumns(10));
   verifyFormat("throw aaaaa;", getGoogleJSStyleWithColumns(10));
   verifyFormat("aaaaaaaaa++;", getGoogleJSStyleWithColumns(10));
   verifyFormat("aaaaaaaaa--;", getGoogleJSStyleWithColumns(10));
   verifyFormat("return [\n"
                "  aaa\n"
                "];",
                getGoogleJSStyleWithColumns(12));