mprobst created this revision.
mprobst added a reviewer: djasper.
mprobst added subscribers: cfe-commits, sammccall.
Herald added a subscriber: klimek.

Change r291428 introduced ASI detection after closing curly braces. That would 
generally be correct, however this breaks indentation for structural 
statements. What happens is that CompoundStatementIndenter increases 
indentation for the current line, then after reading ASI creates a new line 
(with the increased line level), and only after the structural parser sees e.g. 
the if/then/else branch closed, line level is reduced. That leads to the new 
line started by ASI having a level too high.


https://reviews.llvm.org/D28763

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


Index: unittests/Format/FormatTestJS.cpp
===================================================================
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -858,26 +858,44 @@
                "return 1",
                "a = null\n"
                "  return   1");
+  // Below "class Y {}" should ideally be on its own line.
   verifyFormat(
       "x = {\n"
       "  a: 1\n"
-      "}\n"
-      "class Y {}",
+      "} class Y {}",
       "  x  =  {a  : 1}\n"
       "   class  Y {  }");
+  verifyFormat(
+      "if (x) {\n"
+      "}\n"
+      "return 1",
+      "if (x) {}\n"
+      " return   1");
+  verifyFormat(
+      "if (x) {\n"
+      "}\n"
+      "class X {}",
+      "if (x) {}\n"
+      " class X {}");
 }
 
 TEST_F(FormatTestJS, ImportExportASI) {
   verifyFormat(
       "import {x} from 'y'\n"
       "export function z() {}",
       "import   {x} from 'y'\n"
       "  export function z() {}");
+  // Below "class Y {}" should ideally be on its own line.
   verifyFormat(
-      "export {x}\n"
-      "class Y {}",
+      "export {x} class Y {}",
       "  export {x}\n"
       "  class  Y {\n}");
+  verifyFormat(
+      "if (x) {\n"
+      "}\n"
+      "export class Y {}",
+      "if ( x ) { }\n"
+      " export class Y {}");
 }
 
 TEST_F(FormatTestJS, ClosureStyleCasts) {
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -746,8 +746,7 @@
        Previous->isOneOf(tok::r_square, tok::r_paren, tok::plusplus,
                          tok::minusminus)))
     return addUnwrappedLine();
-  if ((PreviousMustBeValue || Previous->is(tok::r_brace)) &&
-      isJSDeclOrStmt(Keywords, Next))
+  if (PreviousMustBeValue && isJSDeclOrStmt(Keywords, Next))
     return addUnwrappedLine();
 }
 


Index: unittests/Format/FormatTestJS.cpp
===================================================================
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -858,26 +858,44 @@
                "return 1",
                "a = null\n"
                "  return   1");
+  // Below "class Y {}" should ideally be on its own line.
   verifyFormat(
       "x = {\n"
       "  a: 1\n"
-      "}\n"
-      "class Y {}",
+      "} class Y {}",
       "  x  =  {a  : 1}\n"
       "   class  Y {  }");
+  verifyFormat(
+      "if (x) {\n"
+      "}\n"
+      "return 1",
+      "if (x) {}\n"
+      " return   1");
+  verifyFormat(
+      "if (x) {\n"
+      "}\n"
+      "class X {}",
+      "if (x) {}\n"
+      " class X {}");
 }
 
 TEST_F(FormatTestJS, ImportExportASI) {
   verifyFormat(
       "import {x} from 'y'\n"
       "export function z() {}",
       "import   {x} from 'y'\n"
       "  export function z() {}");
+  // Below "class Y {}" should ideally be on its own line.
   verifyFormat(
-      "export {x}\n"
-      "class Y {}",
+      "export {x} class Y {}",
       "  export {x}\n"
       "  class  Y {\n}");
+  verifyFormat(
+      "if (x) {\n"
+      "}\n"
+      "export class Y {}",
+      "if ( x ) { }\n"
+      " export class Y {}");
 }
 
 TEST_F(FormatTestJS, ClosureStyleCasts) {
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -746,8 +746,7 @@
        Previous->isOneOf(tok::r_square, tok::r_paren, tok::plusplus,
                          tok::minusminus)))
     return addUnwrappedLine();
-  if ((PreviousMustBeValue || Previous->is(tok::r_brace)) &&
-      isJSDeclOrStmt(Keywords, Next))
+  if (PreviousMustBeValue && isJSDeclOrStmt(Keywords, Next))
     return addUnwrappedLine();
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D28763: clang-format... Martin Probst via Phabricator via cfe-commits

Reply via email to