clang-format: [JS] remove trailing lines in arrow functions.

[mprobst]

Summary

clang-format already removes lines trailing at the end of blocks:

function x() {
  foo();  // next line will be removed.

}

However because JS arrow functions are parsed as expressions, the existing logic
to remove empty lines in UnwrappedLineFormatter doesn't handle them. This code
special cases arrow functions in ContinuationIndenter to remove any trailing
spaces:

x = () => {
  foo();  // next line will (now) be removed.

};

[djasper]

Is this different for C++ lambdas? I would think that we never should add an empty line before the "}" of a child block.

[mprobst]

• handle C++ lambdas
• handle preceding empty lines

[mprobst]

In D40178#928645, @djasper wrote:

Is this different for C++ lambdas? I would think that we never should add an empty line before the "}" of a child block.

Actually indeed turns out to be identical for C++ lambdas. Fixed as we discussed offline.

[mprobst]

• reduce test cases a bit, no need to separate before/after cases

[djasper]

Looks good. There is a chance that some people do not want this in their coding style. But if so, we can add an option later.

[mprobst]

• document the meaning if Children.size() == 0 in this context.

[mprobst]

• prefer !x.empty() over .size() > 0.

lib/Format/ContinuationIndenter.cpp

if (Previous.isOneOf(TT_TemplateCloser, TT_JavaAnnotation) &&
     State.Stack.back().BreakBeforeParameter = false;
   if (NextNonComment->is(tok::question) ||
       (PreviousNonComment && PreviousNonComment->is(tok::question)))
     State.Stack.back().BreakBeforeParameter = true;
   if (Current.is(TT_BinaryOperator) && Current.CanBreakBefore)
     State.Stack.back().BreakBeforeParameter = false;
 
   if (!DryRun) {
+    unsigned MaxEmptyLinesToKeep = Style.MaxEmptyLinesToKeep + 1;
+    if (Current.is(tok::r_brace) && Current.MatchingParen &&
+        Current.MatchingParen->Children.size() > 0) {
+      // lambdas and arrow functions are expressions, thus their r_brace is not
+      // on its own line, and thus not covered by UnwrappedLineFormatter's logic
+      // about removing empty lines on closing blocks. Special case them here.
+      MaxEmptyLinesToKeep = 1;
+    }
     unsigned Newlines = std::max(
-        1u, std::min(Current.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1));
+        1u, std::min(Current.NewlinesBefore, MaxEmptyLinesToKeep));
     bool ContinuePPDirective =
         State.Line->InPPDirective && State.Line->Type != LT_ImportStatement;
     Whitespaces.replaceWhitespace(Current, Newlines, State.Column, State.Column,
                                   ContinuePPDirective);
   }
 
   if (!Current.isTrailingComment())
     State.Stack.back().LastSpace = State.Column;

lib/Format/UnwrappedLineFormatter.cpp

void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line,
   }
   unsigned Newlines =
       std::min(RootToken.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1);
   // Remove empty lines before "}" where applicable.
   if (RootToken.is(tok::r_brace) &&
       (!RootToken.Next ||
        (RootToken.Next->is(tok::semi) && !RootToken.Next->Next)))
     Newlines = std::min(Newlines, 1u);
+  // Remove empty lines at the start of nested blocks (lambdas/arrow functions)
+  if (PreviousLine == nullptr && Line.Level > 0)
+    Newlines = std::min(Newlines, 1u);
   if (Newlines == 0 && !RootToken.IsFirst)
     Newlines = 1;
   if (RootToken.IsFirst && !RootToken.HasUnescapedNewline)
     Newlines = 0;
 
   // Remove empty lines after "{".
   if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
       PreviousLine->Last->is(tok::l_brace) &&

unittests/Format/FormatTest.cpp

protected:
   FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
     return getStyleWithColumns(getLLVMStyle(), ColumnLimit);
   }
 
   FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) {
     return getStyleWithColumns(getGoogleStyle(), ColumnLimit);
   }
 
-  void verifyFormat(llvm::StringRef Code,
+  void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
                     const FormatStyle &Style = getLLVMStyle()) {
-    EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+    EXPECT_EQ(Expected.str(), format(Code, Style));
     if (Style.Language == FormatStyle::LK_Cpp) {
       // Objective-C++ is a superset of C++, so everything checked for C++
       // needs to be checked for Objective-C++ as well.
       FormatStyle ObjCStyle = Style;
       ObjCStyle.Language = FormatStyle::LK_ObjC;
-      EXPECT_EQ(Code.str(), format(test::messUp(Code), ObjCStyle));
-    }
+      EXPECT_EQ(Expected.str(), format(test::messUp(Code), ObjCStyle));
+    }
+
+  void verifyFormat(llvm::StringRef Code,
+                    const FormatStyle &Style = getLLVMStyle()) {
+    verifyFormat(Code, test::messUp(Code), Style);
   }
 
   void verifyIncompleteFormat(llvm::StringRef Code,
                               const FormatStyle &Style = getLLVMStyle()) {
     EXPECT_EQ(Code.str(),
               format(test::messUp(Code), Style, SC_ExpectIncomplete));
   }
 
verifyFormat(
       "aaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
       "    [aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]()\n"
       "        -> ::std::unordered_set<\n"
       "            aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa> {\n"
       "      //\n"
       "    });");
 }
 
+TEST_F(FormatTest, EmptyLinesInLambdas) {
+  verifyFormat("auto lambda = []() {\n"
+               "  x(); //\n"
+               "};",
+               "auto lambda = []() {\n"
+               "\n"
+               "  x(); //\n"
+               "};");
+  verifyFormat("auto lambda = []() {\n"
+               "  x(); //\n"
+               "};",
+               "auto lambda = []() {\n"
+               "  x(); //\n"
+               "\n"
+               "};");
+}
+
 TEST_F(FormatTest, FormatsBlocks) {
   FormatStyle ShortBlocks = getLLVMStyle();
   ShortBlocks.AllowShortBlocksOnASingleLine = true;
   verifyFormat("int (^Block)(int, int);", ShortBlocks);
   verifyFormat("int (^Block1)(int, int) = ^(int i, int j)", ShortBlocks);
   verifyFormat("void (^block)(int) = ^(id test) { int i; };", ShortBlocks);
   verifyFormat("void (^block)(int) = ^(int test) { int i; };", ShortBlocks);
   verifyFormat("void (^block)(int) = ^id(int test) { int i; };", ShortBlocks);

unittests/Format/FormatTestJS.cpp

TEST_F(FormatTestJS, TypeInterfaceLineWrapping) {
   verifyFormat(
       "interface AbstractStrategyFactoryProvider {\n"
       "  a: number\n"
       "}\n",
       "interface AbstractStrategyFactoryProvider { a: number }\n",
       Style);
 }
 
+TEST_F(FormatTestJS, RemoveEmptyLinesInArrowFunctions) {
+  verifyFormat("x = () => {\n"
+               "  foo();\n"
+               "};\n",
+               "x = () => {\n"
+               "  foo();\n"
+               "\n"
+               "};\n");
+  verifyFormat("x = () => {\n"
+               "  foo();\n"
+               "};\n",
+               "x = () => {\n"
+               "\n"
+               "  foo();\n"
+               "};\n");
+}
+
 TEST_F(FormatTestJS, Modules) {
   verifyFormat("import SomeThing from 'some/module.js';");
   verifyFormat("import {X, Y} from 'some/module.js';");
   verifyFormat("import a, {X, Y} from 'some/module.js';");
   verifyFormat("import {X, Y,} from 'some/module.js';");
   verifyFormat("import {X as myLocalX, Y as myLocalY} from 'some/module.js';");
   // Ensure Automatic Semicolon Insertion does not break on "as\n".
   verifyFormat("import {X as myX} from 'm';", "import {X as\n"