clang-format: [JS] improve calculateBraceType heuristic

[mprobst]

Summary

calculateBraceTypes decides for braced init for empty brace pairs ({}). In context of a function declaration, this incorrectly classifies empty function or method bodies as braced inits, leading to missing wraps:

class C {
  foo() {}[bar]() {}
}

Where could should have wrapped after "}". This change adds another piece of contextual information in that braces following closing parentheses must always be the opening braces of function blocks. This fixes brace detection for methods immediately followed by brackets (computed property declarations), but also curlies.

[klimek]

Apart from fixme LG.

lib/Format/UnwrappedLineParser.cpp

397 ↗

(On Diff #100837)

Here we call them FIXME and put them without a name:
// FIXME: ...

[mprobst]

TODO => FIXME

[mprobst]

• restrict to JS, C++ actually wants this.

cfe/trunk/lib/Format/UnwrappedLineParser.cpp

do {
     unsigned ReadTokens = 0;
     do {
       NextTok = Tokens->getNextToken();
       ++ReadTokens;
     } while (NextTok->is(tok::comment));
 
     switch (Tok->Tok.getKind()) {
     case tok::l_brace:
-      if (Style.Language == FormatStyle::LK_JavaScript && PrevTok &&
-          PrevTok->is(tok::colon))
-        // A colon indicates this code is in a type, or a braced list following
-        // a label in an object literal ({a: {b: 1}}).
-        // The code below could be confused by semicolons between the individual
-        // members in a type member list, which would normally trigger BK_Block.
-        // In both cases, this must be parsed as an inline braced init.
+      if (Style.Language == FormatStyle::LK_JavaScript && PrevTok) {
+        if (PrevTok->is(tok::colon))
+          // A colon indicates this code is in a type, or a braced list
+          // following a label in an object literal ({a: {b: 1}}). The code
+          // below could be confused by semicolons between the individual
+          // members in a type member list, which would normally trigger
+          // BK_Block. In both cases, this must be parsed as an inline braced
+          // init.
           Tok->BlockKind = BK_BracedInit;
-      else
+        else if (PrevTok->is(tok::r_paren))
+          // `) { }` can only occur in function or method declarations in JS.
+          Tok->BlockKind = BK_Block;
+      } else {
         Tok->BlockKind = BK_Unknown;
+      }
       LBraceStack.push_back(Tok);
       break;
     case tok::r_brace:
       if (LBraceStack.empty())
         break;
       if (LBraceStack.back()->BlockKind == BK_Unknown) {
         bool ProbablyBracedList = false;
         if (Style.Language == FormatStyle::LK_Proto) {
           ProbablyBracedList = NextTok->isOneOf(tok::comma, tok::r_square);
         } else {
           // Using OriginalColumn to distinguish between ObjC methods and
           // binary operators is a bit hacky.
           bool NextIsObjCMethod = NextTok->isOneOf(tok::plus, tok::minus) &&
                                   NextTok->OriginalColumn == 0;
 
           // If there is a comma, semicolon or right paren after the closing
           // brace, we assume this is a braced initializer list.  Note that
           // regardless how we mark inner braces here, we will overwrite the
           // BlockKind later if we parse a braced list (where all blocks
           // inside are by default braced lists), or when we explicitly detect
           // blocks (for example while parsing lambdas).
+          // FIXME: Some of these do not apply to JS, e.g. "} {" can never be a
+          // braced list in JS.
           ProbablyBracedList =
               (Style.Language == FormatStyle::LK_JavaScript &&
                NextTok->isOneOf(Keywords.kw_of, Keywords.kw_in,
                                 Keywords.kw_as)) ||
               (Style.isCpp() && NextTok->is(tok::l_paren)) ||
               NextTok->isOneOf(tok::comma, tok::period, tok::colon,
                                tok::r_paren, tok::r_square, tok::l_brace,
                                tok::l_square, tok::ellipsis) ||

cfe/trunk/unittests/Format/FormatTestJS.cpp

verifyFormat("function outer1(a, b) {\n"
                "  }\n"
                "  inner2(a, b);\n"
                "}");
   verifyFormat("function f() {}");
   verifyFormat("function aFunction() {}\n"
                "(function f() {\n"
                "  var x = 1;\n"
                "}());\n");
-  // Known issue: this should wrap after {}, but calculateBraceTypes
-  // misclassifies the first braces as a BK_BracedInit.
-  verifyFormat("function aFunction(){} {\n"
+  verifyFormat("function aFunction() {}\n"
+               "{\n"
                "  let x = 1;\n"
                "  console.log(x);\n"
                "}\n");
 }
 
 TEST_F(FormatTestJS, GeneratorFunctions) {
   verifyFormat("function* f() {\n"
                "  let x = 1;\n"
TEST_F(FormatTestJS, UnionIntersectionTypes) {
   verifyFormat("let x: Bar<X>|Baz;");
   verifyFormat("let x: (Foo|Bar)[];");
 }
 
 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"
+               "  foo() {}\n"
+               "  [bar]() {}\n"
+               "}\n");
   verifyFormat("class C {\n  private x: string = 12;\n}");
   verifyFormat("class C {\n  private static x: string = 12;\n}");
   verifyFormat("class C {\n  static x(): string {\n    return 'asd';\n  }\n}");
   verifyFormat("class C extends P implements I {}");
   verifyFormat("class C extends p.P implements i.I {}");
   verifyFormat(
       "x(class {\n"
       "  a(): A {}\n"