clang-format: [JS] Better support for fat arrows.

[mprobst]

Summary

Assigns a token type (TT_JsFatArrow) to => tokens, and uses that to
more easily recognize and format fat arrow functions.
Improves function parsing to better recognize formal parameter
lists and return type declarations.
Recognizes arrow functions and parse function bodies as child blocks.

[klimek]

lib/Format/UnwrappedLineParser.cpp

885–887

Seems like they have TokenKind TT_JsFatArrow now.

1042–1043

Most of the time we enter parse* functions with the first token of that projection. In this case, I'd expect the current token still be the OpenKind.

1046

Nit: add '.'

1048

Use pre-increment in C++.

lib/Format/UnwrappedLineParser.h

110–114

a) use doxygen comments for functions
b) we usually don't have nested parens or nested blocks where there can't be anything in we'd want to parse explicitly; I assume you know what you're doing for JS here, but I find that really curious

[mprobst]

PTAL.

lib/Format/UnwrappedLineParser.cpp

885–887

It has TokenType TT_JsFatArrow, but tok::TokenKind tok::equal. Don't blame me, I'm just the messenger :-)

Updated the comment to be explicit about tok::TokenKind.

1042–1043

Done, that's better indeed.

1046

Done.

1048

Done.

lib/Format/UnwrappedLineParser.h

110–114

(a): done.
(b): this matches what the code used to do before. In the positions where I call it, there can never be stuff that belongs on separate lines (it's within a formal parameter list or within inline type declarations). I'm not 100% certain this will always be the best thing to do, but it's definitely not a regression and I cannot come up with a case where it'd be wrong right now.

[klimek]

lg

[mprobst]

This has gone in.

lib/Format/Format.cpp

if (Style.Language == FormatStyle::LK_JavaScript) {
         return;
 
       static const tok::TokenKind JSIdentity[] = {tok::equalequal, tok::equal};
       static const tok::TokenKind JSNotIdentity[] = {tok::exclaimequal,
                                                      tok::equal};
       static const tok::TokenKind JSShiftEqual[] = {tok::greater, tok::greater,
                                                     tok::greaterequal};
       static const tok::TokenKind JSRightArrow[] = {tok::equal, tok::greater};
-      // FIXME: We probably need to change token type to mimic operator with the
-      // correct priority.
-      if (tryMergeTokens(JSIdentity))
+      // FIXME: Investigate what token type gives the correct operator priority.
+      if (tryMergeTokens(JSIdentity, TT_BinaryOperator))
         return;
-      if (tryMergeTokens(JSNotIdentity))
+      if (tryMergeTokens(JSNotIdentity, TT_BinaryOperator))
         return;
-      if (tryMergeTokens(JSShiftEqual))
+      if (tryMergeTokens(JSShiftEqual, TT_BinaryOperator))
         return;
-      if (tryMergeTokens(JSRightArrow))
+      if (tryMergeTokens(JSRightArrow, TT_JsFatArrow))
         return;
     }
   }
 
   bool tryMergeLessLess() {
     // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.
     if (Tokens.size() < 3)
       return false;
bool tryMergeLessLess() {
 
     First[0]->Tok.setKind(tok::lessless);
     First[0]->TokenText = "<<";
     First[0]->ColumnWidth += 1;
     Tokens.erase(Tokens.end() - 2);
     return true;
   }
 
-  bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds) {
+  bool tryMergeTokens(ArrayRef<tok::TokenKind> Kinds, TokenType NewType) {
     if (Tokens.size() < Kinds.size())
       return false;
 
     SmallVectorImpl<FormatToken *>::const_iterator First =
         Tokens.end() - Kinds.size();
     if (!First[0]->is(Kinds[0]))
       return false;
     unsigned AddLength = 0;
     for (unsigned i = 1; i < Kinds.size(); ++i) {
       if (!First[i]->is(Kinds[i]) ||
           First[i]->WhitespaceRange.getBegin() !=
               First[i]->WhitespaceRange.getEnd())
         return false;
       AddLength += First[i]->TokenText.size();
     }
     Tokens.resize(Tokens.size() - Kinds.size() + 1);
     First[0]->TokenText = StringRef(First[0]->TokenText.data(),
                                     First[0]->TokenText.size() + AddLength);
     First[0]->ColumnWidth += AddLength;
+    First[0]->Type = NewType;
     return true;
   }
 
   // Tries to merge an escape sequence, i.e. a "\\" and the following
   // character. Use e.g. inside JavaScript regex literals.
   bool tryMergeEscapeSequence() {
     if (Tokens.size() < 2)
       return false;

lib/Format/FormatToken.h

enum TokenType {
   TT_FunctionDeclarationName,
   TT_FunctionLBrace,
   TT_FunctionTypeLParen,
   TT_ImplicitStringLiteral,
   TT_InheritanceColon,
   TT_InlineASMBrace,
   TT_InlineASMColon,
   TT_JavaAnnotation,
+  TT_JsFatArrow,
   TT_JsTypeColon,
   TT_JsTypeOptionalQuestion,
   TT_LambdaArrow,
   TT_LambdaLSquare,
   TT_LeadingJavaAnnotation,
   TT_LineComment,
   TT_ObjCBlockLBrace,
   TT_ObjCBlockLParen,

lib/Format/UnwrappedLineParser.h

private:
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
   void parseObjCInterfaceOrImplementation();
   void parseObjCProtocol();
   void parseJavaScriptEs6ImportExport();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
+  /// \brief Parses tokens until encountering the CloseKind token, but balances
+  /// tokens when encountering more OpenKind tokens. Useful for e.g. parsing a
+  /// curly brace delimited block that can contain nested blocks.
+  /// The parser must be positioned on a token of OpenKind.
+  void parseBalanced(tok::TokenKind OpenKind, tok::TokenKind CloseKind);
   void addUnwrappedLine();
   bool eof() const;
   void nextToken();
   void readToken();
   void flushComments(bool NewlineBeforeNext);
   void pushToken(FormatToken *Tok);
   void calculateBraceTypes(bool ExpectClassBody = false);
 

lib/Format/UnwrappedLineParser.cpp

case tok::identifier: {
             tokenCanStartNewLine(FormatTok->Tok) && Text == Text.upper()) {
           addUnwrappedLine();
           return;
         }
       }
       break;
     }
     case tok::equal:
+      // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType
+      // TT_JsFatArrow. The always start an expression or a child block if
+      // followed by a curly.
+      if (FormatTok->is(TT_JsFatArrow)) {
+        nextToken();
+        if (FormatTok->is(tok::l_brace)) {
+          parseChildBlock();
+        }
+        break;
+      }
+
       nextToken();
       if (FormatTok->Tok.is(tok::l_brace)) {
         parseBracedList();
       }
       break;
     case tok::l_square:
       parseSquare();
       break;
void UnwrappedLineParser::tryToParseJSFunction() {
   nextToken();
 
   // Consume function name.
   if (FormatTok->is(tok::identifier))
     nextToken();
 
   if (FormatTok->isNot(tok::l_paren))
     return;
+
+  // Parse formal parameter list.
+  parseBalanced(tok::l_paren, tok::r_paren);
+
+  if (FormatTok->is(tok::colon)) {
+    // Parse a type definition.
     nextToken();
-  while (FormatTok->isNot(tok::l_brace)) {
-    // Err on the side of caution in order to avoid consuming the full file in
-    // case of incomplete code.
-    if (!FormatTok->isOneOf(tok::identifier, tok::comma, tok::r_paren,
-                            tok::comment))
-      return;
+
+    // Eat the type declaration. For braced inline object types, balance braces,
+    // otherwise just parse until finding an l_brace for the function body.
+    if (FormatTok->is(tok::l_brace)) {
+      parseBalanced(tok::l_brace, tok::r_brace);
+    } else {
+      while(FormatTok->isNot(tok::l_brace) && !eof()) {
         nextToken();
       }
+    }
+  }
+
   parseChildBlock();
 }
 
+void UnwrappedLineParser::parseBalanced(tok::TokenKind OpenKind,
+                                        tok::TokenKind CloseKind) {
+  assert(FormatTok->is(OpenKind));
+  nextToken();
+  int Depth = 1;
+  while (Depth > 0 && !eof()) {
+    // Parse the formal parameter list.
+    if (FormatTok->is(OpenKind)) {
+      ++Depth;
+    } else if (FormatTok->is(CloseKind)) {
+      --Depth;
+    }
+    nextToken();
+  }
+}
+
 bool UnwrappedLineParser::tryToParseBracedList() {
   if (FormatTok->BlockKind == BK_Unknown)
     calculateBraceTypes();
   assert(FormatTok->BlockKind != BK_Unknown);
   if (FormatTok->BlockKind == BK_Block)
     return false;
   parseBracedList();
   return true;
 }
 
 bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons) {
   bool HasError = false;
   nextToken();
 
   // FIXME: Once we have an expression parser in the UnwrappedLineParser,
   // replace this by using parseAssigmentExpression() inside.
   do {
-    if (Style.Language == FormatStyle::LK_JavaScript &&
-        FormatTok->is(Keywords.kw_function)) {
+    if (Style.Language == FormatStyle::LK_JavaScript) {
+      if (FormatTok->is(Keywords.kw_function)) {
         tryToParseJSFunction();
         continue;
+      } else if (FormatTok->is(TT_JsFatArrow)) {
+        nextToken();
+        // Fat arrows can be followed by simple expressions or by child blocks
+        // in curly braces.
+        if (FormatTok->is(tok::l_brace)){
+          parseChildBlock();
+          continue;
+        }
+      }
     }
     switch (FormatTok->Tok.getKind()) {
     case tok::caret:
       nextToken();
       if (FormatTok->is(tok::l_brace)) {
         parseChildBlock();
       }
       break;

unittests/Format/FormatTestJS.cpp

 }
 
 TEST_F(FormatTestJS, ES6DestructuringAssignment) {
   verifyFormat("var [a, b, c] = [1, 2, 3];");
   verifyFormat("var {a, b} = {a: 1, b: 2};");
 }
 
 TEST_F(FormatTestJS, ContainerLiterals) {
+  verifyFormat("var x = {y: function(a) { return a; }};");
   verifyFormat("return {\n"
                "  link: function() {\n"
                "    f();  //\n"
                "  }\n"
                "};");
   verifyFormat("return {\n"
                "  a: a,\n"
                "  link: function() {\n"
verifyFormat("var obj = {\n"
                "  }\n"
                "};");
   // Simple object literal, as opposed to enum style below.
   verifyFormat("var obj = {a: 123};");
   // Enum style top level assignment.
   verifyFormat("X = {\n  a: 123\n};");
   verifyFormat("X.Y = {\n  a: 123\n};");
   verifyFormat("x = foo && {a: 123};");
+
+  // Arrow functions in object literals.
+  verifyFormat("var x = {y: (a) => { return a; }};");
+  verifyFormat("var x = {y: (a) => a};");
 }
 
 TEST_F(FormatTestJS, MethodsInObjectLiterals) {
   verifyFormat("var o = {\n"
                "  value: 'test',\n"
                "  get value() {  // getter\n"
                "    return this.value;\n"
                "  }\n"
verifyFormat("getSomeLongPromise()\n"
                "      body();\n"
                "    });");
 
   verifyFormat("getSomeLongPromise()\n"
                "    .then(function(value) { body(); })\n"
                "    .thenCatch(function(error) { body(); });");
 }
 
+TEST_F(FormatTestJS, ArrowFunctions) {
+  verifyFormat("var x = (a) => {\n"
+               "  return a;\n"
+               "};");
+  verifyFormat("var x = (a) => {\n"
+               "  function y() { return 42; }\n"
+               "  return a;\n"
+               "};");
+  verifyFormat("var x = (a: type): {some: type} => {\n"
+               "  return a;\n"
+               "};");
+  verifyFormat("var x = (a) => a;");
+  verifyFormat("var x = (a) => a;");
+}
+
 TEST_F(FormatTestJS, ReturnStatements) {
   verifyFormat("function() {\n"
                "  return [hello, world];\n"
                "}");
 }
 
-TEST_F(FormatTestJS, ClosureStyleComments) {
+TEST_F(FormatTestJS, ClosureStyleCasts) {
   verifyFormat("var x = /** @type {foo} */ (bar);");
 }
 
 TEST_F(FormatTestJS, TryCatch) {
   verifyFormat("try {\n"
                "  f();\n"
                "} catch (e) {\n"
                "  g();\n"

 
 TEST_F(FormatTestJS, RegexLiteralExamples) {
   verifyFormat("var regex = search.match(/(?:\?|&)times=([^?&]+)/i);");
 }
 
 TEST_F(FormatTestJS, TypeAnnotations) {
   verifyFormat("var x: string;");
   verifyFormat("function x(): string {\n  return 'x';\n}");
+  verifyFormat("function x(): {x: string} {\n  return {x: 'x'};\n}");
   verifyFormat("function x(y: string): string {\n  return 'x';\n}");
   verifyFormat("for (var y: string in x) {\n  x();\n}");
   verifyFormat("((a: string, b: number): string => a + b);");
   verifyFormat("var x: (y: number) => string;");
   verifyFormat("var x: P<string, (a: number) => string>;");
+  verifyFormat("var x = {y: function(): z { return 1; }};");
+  verifyFormat("var x = {y: function(): {a: number} { return 1; }};");
 }
 
 TEST_F(FormatTestJS, ClassDeclarations) {
   verifyFormat("class C {\n  x: string = 12;\n}");
   verifyFormat("class C {\n  x(): string => 12;\n}");
   verifyFormat("class C {\n  ['x' + 2]: string = 12;\n}");
   verifyFormat("class C {\n  private x: string = 12;\n}");
   verifyFormat("class C {\n  private static x: string = 12;\n}");