mprobst created this revision.
Herald added a subscriber: klimek.

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.


https://reviews.llvm.org/D33714

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestJS.cpp


Index: unittests/Format/FormatTestJS.cpp
===================================================================
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -474,9 +474,8 @@
                "(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");
@@ -1233,6 +1232,10 @@
   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}");
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -368,6 +368,9 @@
         // 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 if (PrevTok && PrevTok->is(tok::r_paren))
+        // `) { ... }` can only occur in function or method declarations.
+        Tok->BlockKind = BK_Block;
       else
         Tok->BlockKind = BK_Unknown;
       LBraceStack.push_back(Tok);
@@ -391,6 +394,8 @@
           // 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).
+          // TODO(martinprobst): 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,


Index: unittests/Format/FormatTestJS.cpp
===================================================================
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -474,9 +474,8 @@
                "(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");
@@ -1233,6 +1232,10 @@
   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}");
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -368,6 +368,9 @@
         // 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 if (PrevTok && PrevTok->is(tok::r_paren))
+        // `) { ... }` can only occur in function or method declarations.
+        Tok->BlockKind = BK_Block;
       else
         Tok->BlockKind = BK_Unknown;
       LBraceStack.push_back(Tok);
@@ -391,6 +394,8 @@
           // 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).
+          // TODO(martinprobst): 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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to