Bug 1434429 - Use the current offset, not the offset of the start of the current token, when reporting errors for unterminated string/template literals. r=till
☠☠ backed out by 622a8e0b2b83 ☠ ☠
authorJeff Walden <jwalden@mit.edu>
Thu, 18 Jan 2018 11:34:26 -0800
changeset 402347 4b03e86fb95a61670dc54c1b3245ce8faf9dd528
parent 402334 1fec20a62617e7cebe5cda33fcfbf600d2cef987
child 402348 dcde99a8fa6c5bee9bba23525b402f3ce6dd479b
push id33380
push usercsabou@mozilla.com
push dateSat, 03 Feb 2018 21:52:46 +0000
treeherdermozilla-central@44b20b38eb65 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill
bugs1434429
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1434429 - Use the current offset, not the offset of the start of the current token, when reporting errors for unterminated string/template literals. r=till
js/src/frontend/TokenStream.cpp
js/src/frontend/TokenStream.h
js/src/js.msg
js/src/tests/non262/extensions/unterminated-literal-error-location.js
--- a/js/src/frontend/TokenStream.cpp
+++ b/js/src/frontend/TokenStream.cpp
@@ -979,37 +979,39 @@ TokenStreamSpecific<CharT, AnyCharsAcces
 
 template<typename CharT, class AnyCharsAccess>
 void
 TokenStreamSpecific<CharT, AnyCharsAccess>::error(unsigned errorNumber, ...)
 {
     va_list args;
     va_start(args, errorNumber);
 
-    TokenStreamAnyChars& anyChars = anyCharsAccess();
     ErrorMetadata metadata;
-    if (computeErrorMetadata(&metadata, anyChars.currentToken().pos.begin))
+    if (computeErrorMetadata(&metadata, userbuf.offset())) {
+        TokenStreamAnyChars& anyChars = anyCharsAccess();
         ReportCompileError(anyChars.cx, Move(metadata), nullptr, JSREPORT_ERROR, errorNumber,
                            args);
+    }
 
     va_end(args);
 }
 
 template<typename CharT, class AnyCharsAccess>
 void
 TokenStreamSpecific<CharT, AnyCharsAccess>::errorAt(uint32_t offset, unsigned errorNumber, ...)
 {
     va_list args;
     va_start(args, errorNumber);
 
-    TokenStreamAnyChars& anyChars = anyCharsAccess();
     ErrorMetadata metadata;
-    if (computeErrorMetadata(&metadata, offset))
+    if (computeErrorMetadata(&metadata, offset)) {
+        TokenStreamAnyChars& anyChars = anyCharsAccess();
         ReportCompileError(anyChars.cx, Move(metadata), nullptr, JSREPORT_ERROR, errorNumber,
                            args);
+    }
 
     va_end(args);
 }
 
 // We have encountered a '\': check for a Unicode escape sequence after it.
 // Return the length of the escape sequence and the character code point (by
 // value) if we found a Unicode escape sequence.  Otherwise, return 0.  In both
 // cases, do not advance along the buffer.
@@ -1702,17 +1704,17 @@ TokenStreamSpecific<CharT, AnyCharsAcces
         tp->type = TokenKind::Number;
         tp->setNumber(dval, decimalPoint);
         goto out;
     }
 
     // Look for a string or a template string.
     //
     if (c1kind == String) {
-        if (!getStringOrTemplateToken(c, &tp))
+        if (!getStringOrTemplateToken(static_cast<char>(c), &tp))
             goto error;
         goto out;
     }
 
     // Skip over EOL chars, updating line state along the way.
     //
     if (c1kind == EOL) {
         // If it's a \r\n sequence: treat as a single EOL, skip over the \n.
@@ -2126,45 +2128,55 @@ TokenStreamSpecific<CharT, AnyCharsAcces
     userbuf.poison();
 #endif
     MOZ_MAKE_MEM_UNDEFINED(ttp, sizeof(*ttp));
     return false;
 }
 
 template<typename CharT, class AnyCharsAccess>
 bool
-TokenStreamSpecific<CharT, AnyCharsAccess>::getStringOrTemplateToken(int untilChar, Token** tp)
+TokenStreamSpecific<CharT, AnyCharsAccess>::getStringOrTemplateToken(char untilChar, Token** tp)
 {
+    MOZ_ASSERT(untilChar == '\'' || untilChar == '"' || untilChar == '`',
+               "unexpected string/template literal delimiter");
+
     int c;
     int nc = -1;
 
     bool parsingTemplate = (untilChar == '`');
 
     *tp = newToken(-1);
     tokenbuf.clear();
 
     // We need to detect any of these chars:  " or ', \n (or its
     // equivalents), \\, EOF.  Because we detect EOL sequences here and
     // put them back immediately, we can use getCharIgnoreEOL().
     while ((c = getCharIgnoreEOL()) != untilChar) {
         if (c == EOF) {
             ungetCharIgnoreEOL(c);
-            error(JSMSG_UNTERMINATED_STRING);
+            const char delimiters[] = { untilChar, untilChar, '\0' };
+            error(JSMSG_EOF_BEFORE_END_OF_LITERAL, delimiters);
             return false;
         }
 
         if (c == '\\') {
             // When parsing templates, we don't immediately report errors for
             // invalid escapes; these are handled by the parser.
             // In those cases we don't append to tokenbuf, since it won't be
             // read.
             if (!getChar(&c))
                 return false;
 
-            switch (c) {
+            if (c == EOF) {
+                const char delimiters[] = { untilChar, untilChar, '\0' };
+                error(JSMSG_EOF_IN_ESCAPE_IN_LITERAL, delimiters);
+                return false;
+            }
+
+            switch (static_cast<CharT>(c)) {
               case 'b': c = '\b'; break;
               case 'f': c = '\f'; break;
               case 'n': c = '\n'; break;
               case 'r': c = '\r'; break;
               case 't': c = '\t'; break;
               case 'v': c = '\v'; break;
 
               case '\n':
@@ -2340,17 +2352,18 @@ TokenStreamSpecific<CharT, AnyCharsAcces
 
                     c = char16_t(val);
                 }
                 break;
             }
         } else if (TokenBuf::isRawEOLChar(c)) {
             if (!parsingTemplate) {
                 ungetCharIgnoreEOL(c);
-                error(JSMSG_UNTERMINATED_STRING);
+                const char delimiters[] = { untilChar, untilChar, '\0' };
+                error(JSMSG_EOL_BEFORE_END_OF_STRING, delimiters);
                 return false;
             }
             if (c == '\r') {
                 c = '\n';
                 if (userbuf.peekRawChar() == '\n')
                     skipCharsIgnoreEOL(1);
             }
 
--- a/js/src/frontend/TokenStream.h
+++ b/js/src/frontend/TokenStream.h
@@ -1432,17 +1432,17 @@ class MOZ_STACK_CLASS TokenStreamSpecifi
     }
 
     const CharT* rawLimit() const {
         return userbuf.limit();
     }
 
     MOZ_MUST_USE bool getTokenInternal(TokenKind* ttp, Modifier modifier);
 
-    MOZ_MUST_USE bool getStringOrTemplateToken(int untilChar, Token** tp);
+    MOZ_MUST_USE bool getStringOrTemplateToken(char untilChar, Token** tp);
 
     // Try to get the next character, normalizing '\r', '\r\n', and '\n' into
     // '\n'.  Also updates internal line-counter state.  Return true on success
     // and store the character in |*c|.  Return false and leave |*c| undefined
     // on failure.
     MOZ_MUST_USE bool getChar(int32_t* cp);
 
     Token* newToken(ptrdiff_t adjust);
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -333,17 +333,19 @@ MSG_DEF(JSMSG_TOO_MANY_YIELDS,         0
 MSG_DEF(JSMSG_TOUGH_BREAK,             0, JSEXN_SYNTAXERR, "unlabeled break must be inside loop or switch")
 MSG_DEF(JSMSG_UNEXPECTED_TOKEN,        2, JSEXN_SYNTAXERR, "expected {0}, got {1}")
 MSG_DEF(JSMSG_UNEXPECTED_TOKEN_NO_EXPECT,  1, JSEXN_SYNTAXERR, "unexpected token: {0}")
 MSG_DEF(JSMSG_UNEXPECTED_PARAMLIST_END,0, JSEXN_SYNTAXERR, "unexpected end of function parameter list")
 MSG_DEF(JSMSG_UNNAMED_CLASS_STMT,      0, JSEXN_SYNTAXERR, "class statement requires a name")
 MSG_DEF(JSMSG_UNNAMED_FUNCTION_STMT,   0, JSEXN_SYNTAXERR, "function statement requires a name")
 MSG_DEF(JSMSG_UNTERMINATED_COMMENT,    0, JSEXN_SYNTAXERR, "unterminated comment")
 MSG_DEF(JSMSG_UNTERMINATED_REGEXP,     0, JSEXN_SYNTAXERR, "unterminated regular expression literal")
-MSG_DEF(JSMSG_UNTERMINATED_STRING,     0, JSEXN_SYNTAXERR, "unterminated string literal")
+MSG_DEF(JSMSG_EOF_BEFORE_END_OF_LITERAL,1,JSEXN_SYNTAXERR, "{0} literal not terminated before end of script")
+MSG_DEF(JSMSG_EOL_BEFORE_END_OF_STRING,1, JSEXN_SYNTAXERR, "{0} string literal contains an unescaped line break")
+MSG_DEF(JSMSG_EOF_IN_ESCAPE_IN_LITERAL,1, JSEXN_SYNTAXERR, "reached end of script in the middle of an escape sequence in a {0} literal")
 MSG_DEF(JSMSG_USELESS_EXPR,            0, JSEXN_TYPEERR, "useless expression")
 MSG_DEF(JSMSG_USE_ASM_DIRECTIVE_FAIL,  0, JSEXN_SYNTAXERR, "\"use asm\" is only meaningful in the Directive Prologue of a function body")
 MSG_DEF(JSMSG_VAR_HIDES_ARG,           1, JSEXN_TYPEERR, "variable {0} redeclares argument")
 MSG_DEF(JSMSG_WHILE_AFTER_DO,          0, JSEXN_SYNTAXERR, "missing while after do-loop body")
 MSG_DEF(JSMSG_YIELD_IN_DEFAULT,        0, JSEXN_SYNTAXERR, "yield in default expression")
 MSG_DEF(JSMSG_YIELD_OUTSIDE_GENERATOR, 0, JSEXN_SYNTAXERR, "yield expression is only valid in generators")
 MSG_DEF(JSMSG_BAD_COLUMN_NUMBER,       0, JSEXN_RANGEERR, "column number out of range")
 MSG_DEF(JSMSG_COMPUTED_NAME_IN_PATTERN,0, JSEXN_SYNTAXERR, "computed property names aren't supported in this destructuring declaration")
new file mode 100644
--- /dev/null
+++ b/js/src/tests/non262/extensions/unterminated-literal-error-location.js
@@ -0,0 +1,119 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var BUGNUMBER = 9999999;
+var summary =
+  "Report unterminated string/template literal errors with the line/column " +
+  "number of the point of non-termination";
+
+function test(f, quotes, [line, col])
+{
+  var caught = false;
+  try
+  {
+    f();
+  }
+  catch (e)
+  {
+    caught = true;
+    assertEq(e.lineNumber, line, "line number");
+    assertEq(e.columnNumber, col, "column number");
+    assertEq(e.message.includes(quotes), true,
+             "message must contain delimiter");
+  }
+
+  assertEq(caught, true);
+}
+
+test(function() {
+      //0123
+  eval("'hi");
+}, "''", [1, 3]);
+
+test(function() {
+      //0123 4
+  eval("'hi\\");
+}, "''", [1, 4]);
+
+test(function() {
+      //0123456
+  eval("   'hi");
+}, "''", [1, 6]);
+
+test(function() {
+      //0123456 7
+  eval("   'hi\\");
+}, "''", [1, 7]);
+
+test(function() {
+      //01234567 01234567
+  eval('var x =\n    "hi');
+}, '""', [2, 7]);
+
+test(function() {
+      //0123456  01234567 8
+  eval('var x =\n    "hi\\');
+}, '""', [2, 8]);
+
+test(function() {
+      //                                          1
+      //0123456  01234567   012345678   01234567890123
+  eval('var x =\n    "hi\\\n     bye\\\n    no really');
+}, '""', [4, 13]);
+
+test(function() {
+      //                                          1
+      //0123456  01234567   012345678   01234567890123 4
+  eval('var x =\n    "hi\\\n     bye\\\n    no really\\');
+}, '""', [4, 14]);
+
+test(function() {
+      //0123456  01234567   012345678
+  eval('var x =\n    "hi\\\n     bye\n');
+}, '""', [3, 8]);
+
+test(function() {
+      //0123456  01234567   012345678 9
+  eval('var x =\n    "hi\\\n     bye\\');
+}, '""', [3, 9]);
+
+test(function() {
+      //0123456  01234567
+  eval('var x =\n      `');
+}, '``', [2, 7]);
+
+test(function() {
+      //0123456  01234567 8
+  eval('var x =\n      `\\');
+}, '``', [2, 8]);
+
+test(function() {
+      //                   1
+      //0123456  0123456789012345
+  eval('var x =\n    htmlEscape`');
+}, '``', [2, 15]);
+
+test(function() {
+      //                   1
+      //0123456  0123456789012345 6
+  eval('var x =\n    htmlEscape`\\');
+}, '``', [2, 16]);
+
+test(function() {
+      //                   1
+      //0123456  01234567890123  01234
+  eval('var x =\n    htmlEscape\n   `');
+}, '``', [3, 4]);
+
+test(function() {
+      //                   1
+      //0123456  01234567890123  01234 5
+  eval('var x =\n    htmlEscape\n   `\\');
+}, '``', [3, 5]);
+
+if (typeof reportCompare === "function")
+  reportCompare(0, 0, "ok");
+
+print("Tests complete");