clang-format: [JS] handle single lines comments ending in `\\`.

[mprobst]

Summary

Previously, clang-format would consider the following code line to be part of
the comment and incorrectly format the rest of the file.

[mprobst]

Friendly ping.

[klimek]

lg

lib/Format/FormatTokenLexer.cpp

544–545 ↗

(On Diff #109145)

Just wondering whether the \n can be in the next token. Probably not, though, the way we set up the lexer.

[mprobst]

lib/Format/FormatTokenLexer.cpp

544–545 ↗

(On Diff #109145)

AFAIU (and experimentation confirms) LLVM always lexes the \n as separate whitespace from the // ... line comment, but for backslash escaped line endings after the \\ the \n is inside of it - after all it's part of the one comment token that continues on the next line.

cfe/trunk/lib/Format/FormatTokenLexer.cpp

while (FormatTok->Tok.is(tok::unknown)) {
 
     if (FormatTok->is(TT_ImplicitStringLiteral))
       break;
     WhitespaceLength += FormatTok->Tok.getLength();
 
     readRawToken(*FormatTok);
   }
 
+  // JavaScript and Java do not allow to escape the end of the line with a
+  // backslash. Backslashes are syntax errors in plain source, but can occur in
+  // comments. When a single line comment ends with a \, it'll cause the next
+  // line of code to be lexed as a comment, breaking formatting. The code below
+  // finds comments that contain a backslash followed by a line break, truncates
+  // the comment token at the backslash, and resets the lexer to restart behind
+  // the backslash.
+  if ((Style.Language == FormatStyle::LK_JavaScript ||
+       Style.Language == FormatStyle::LK_Java) &&
+      FormatTok->is(tok::comment) && FormatTok->TokenText.startswith("//")) {
+    size_t BackslashPos = FormatTok->TokenText.find('\\');
+    while (BackslashPos != StringRef::npos) {
+      if (BackslashPos + 1 < FormatTok->TokenText.size() &&
+          FormatTok->TokenText[BackslashPos + 1] == '\n') {
+        const char *Offset = Lex->getBufferLocation();
+        Offset -= FormatTok->TokenText.size();
+        Offset += BackslashPos + 1;
+        resetLexer(SourceMgr.getFileOffset(Lex->getSourceLocation(Offset)));
+        FormatTok->TokenText = FormatTok->TokenText.substr(0, BackslashPos + 1);
+        FormatTok->ColumnWidth = encoding::columnWidthWithTabs(
+            FormatTok->TokenText, FormatTok->OriginalColumn, Style.TabWidth,
+            Encoding);
+        break;
+      }
+      BackslashPos = FormatTok->TokenText.find('\\', BackslashPos + 1);
+    }
+  }
+
   // In case the token starts with escaped newlines, we want to
   // take them into account as whitespace - this pattern is quite frequent
   // in macro definitions.
   // FIXME: Add a more explicit test.
   while (FormatTok->TokenText.size() > 1 && FormatTok->TokenText[0] == '\\' &&
          FormatTok->TokenText[1] == '\n') {
     ++FormatTok->NewlinesBefore;
     WhitespaceLength += 2;

cfe/trunk/unittests/Format/FormatTestJS.cpp

TEST_F(FormatTestJS, NestedLiterals) {
   verifyFormat("var o = {\n"
                "    1: 1,\n"
                "    2: [\n"
                "        3,\n"
                "    ],\n"
                "};", FourSpaces);
 }
 
+TEST_F(FormatTestJS, BackslashesInComments) {
+  verifyFormat("// hello \\\n"
+               "if (x) foo();\n",
+               "// hello \\\n"
+               "     if ( x) \n"
+               "   foo();\n");
+  verifyFormat("/* ignore \\\n"
+               " */\n"
+               "if (x) foo();\n",
+               "/* ignore \\\n"
+               " */\n"
+               " if (  x) foo();\n");
+  verifyFormat("// st \\ art\\\n"
+               "// comment"
+               "// continue \\\n"
+               "formatMe();\n",
+               "// st \\ art\\\n"
+               "// comment"
+               "// continue \\\n"
+               "formatMe( );\n");
+}
+
 } // end namespace tooling
 } // end namespace clang