Re: [PATCH] D17910: clang-format: [JS] Handle certain cases of ASI.

[Martin Probst via cfe-commits]

mprobst updated this revision to Diff 49883.
mprobst added a comment.

- Handle unary !.


http://reviews.llvm.org/D17910

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


Index: unittests/Format/FormatTestJS.cpp
===================================================================
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -52,6 +52,13 @@
     std::string result = format(test::messUp(Code), Style);
     EXPECT_EQ(Code.str(), result) << "Formatted:\n" << result;
   }
+
+  static void verifyFormatNoMessup(
+      llvm::StringRef Code,
+      const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_JavaScript)) {
+    std::string result = format(Code, Style);
+    EXPECT_EQ(Code.str(), result) << "Formatted:\n" << result;
+  }
 };
 
 TEST_F(FormatTestJS, UnderstandsJavaScriptOperators) {
@@ -608,7 +615,7 @@
                "}");
 }
 
-TEST_F(FormatTestJS, AutomaticSemicolonInsertion) {
+TEST_F(FormatTestJS, WrapRespectsAutomaticSemicolonInsertion) {
   // The following statements must not wrap, as otherwise the program meaning
   // would change due to automatic semicolon insertion.
   // See http://www.ecma-international.org/ecma-262/5.1/#sec-7.9.1.
@@ -624,6 +631,30 @@
                getGoogleJSStyleWithColumns(12));
 }
 
+TEST_F(FormatTestJS, AutomaticSemicolonInsertionHeuristic) {
+  verifyFormatNoMessup("a\n"
+                       "b;");
+  verifyFormatNoMessup("a()\n"
+                       "b;");
+  verifyFormatNoMessup("a[b]\n"
+                       "c;");
+  verifyFormatNoMessup("1\n"
+                       "a;");
+  verifyFormatNoMessup("a\n"
+                       "1;");
+  verifyFormatNoMessup("a\n"
+                       "'x';");
+  verifyFormatNoMessup("a++\n"
+                       "b;");
+  verifyFormatNoMessup("a\n"
+                       "!b && c;");
+  EXPECT_EQ("var a", format("var\n"
+                            "a"));
+  EXPECT_EQ("x instanceof String", format("x\n"
+                                          "instanceof\n"
+                                          "String"));
+}
+
 TEST_F(FormatTestJS, ClosureStyleCasts) {
   verifyFormat("var x = /** @type {foo} */ (bar);");
 }
Index: lib/Format/UnwrappedLineParser.h
===================================================================
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -81,6 +81,8 @@
   void parsePPElse();
   void parsePPEndIf();
   void parsePPUnknown();
+  bool shouldInsertSemiBetween(FormatToken *Previous, FormatToken *Next);
+  bool isJavaScriptIdentifier(FormatToken *FormatTok);
   void parseStructuralElement();
   bool tryToParseBracedList();
   bool parseBracedList(bool ContinueOnSemicolons = false);
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -658,6 +658,48 @@
          Tok.isNot(tok::kw_noexcept);
 }
 
+// shouldInsertSemiBetween returns true if Automatic Semicolon Insertion must
+// happen between |Previous| and |Next|. This method is conservative - it cannot
+// cover all edge cases of JavaScript, but only aims to correctly handle certain
+// well known cases. It *must not* return true in speculative cases.
+bool UnwrappedLineParser::shouldInsertSemiBetween(FormatToken *Previous,
+                                                  FormatToken *Next) {
+  bool IsOnSameLine =
+      CommentsBeforeNextToken.empty()
+          ? Next->NewlinesBefore == 0
+          : CommentsBeforeNextToken.front()->NewlinesBefore == 0;
+  if (IsOnSameLine)
+    return false;
+
+  bool PreviousMustBeValue =
+      isJavaScriptIdentifier(Previous) || Previous->Tok.isLiteral();
+  bool NextMustBeValue = isJavaScriptIdentifier(Next) || Next->Tok.isLiteral();
+  if (NextMustBeValue && PreviousMustBeValue)
+    return true;
+  if ((Previous->is(tok::r_square) || Previous->is(tok::r_paren)) &&
+      NextMustBeValue)
+    return true;
+  if (PreviousMustBeValue && Next->is(tok::exclaim))
+    return true;
+  if (Previous->isOneOf(tok::plusplus, tok::minusminus) &&
+      NextMustBeValue)
+    return true;
+  return false;
+}
+
+bool UnwrappedLineParser::isJavaScriptIdentifier(FormatToken *FormatTok) {
+  return FormatTok->is(tok::identifier) &&
+         (FormatTok->Tok.getIdentifierInfo() == nullptr ||
+          !FormatTok->isOneOf(Keywords.kw_in, Keywords.kw_of,
+                              Keywords.kw_finally, Keywords.kw_function,
+                              Keywords.kw_import, Keywords.kw_is,
+                              Keywords.kw_let, Keywords.kw_var,
+                              Keywords.kw_abstract, Keywords.kw_extends,
+                              Keywords.kw_implements, Keywords.kw_instanceof,
+                              Keywords.kw_interface, Keywords.kw_throws));
+}
+
+
 void UnwrappedLineParser::parseStructuralElement() {
   assert(!FormatTok->is(tok::l_brace));
   if (Style.Language == FormatStyle::LK_TableGen &&
@@ -934,6 +976,7 @@
         return;
       }
 
+      // See if the following token should start a new unwrapped line.
       StringRef Text = FormatTok->TokenText;
       nextToken();
       if (Line->Tokens.size() == 1 &&
@@ -1896,7 +1939,12 @@
     return;
   flushComments(isOnNewLine(*FormatTok));
   pushToken(FormatTok);
+  FormatToken *Previous = FormatTok;
   readToken();
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+      shouldInsertSemiBetween(Previous, FormatTok)) {
+    addUnwrappedLine();
+  }
 }
 
 const FormatToken *UnwrappedLineParser::getPreviousToken() {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits