Skip to content
Browse files

Make our parser to be smarter about /= and regexs.

The ridiculously long regular expression that produces tokens was
confused about differences between '/=' and regular expressions that
start with '='. Basically it didn't know that the latter exist.

This patch fixes the problem using the simplest (but still dumb)
solution. It makes sure that the token for /= is not followed by
non-whitespace characters and '/[gim]'. I hope this will catch
most regular expressions out there.

Closes GH-536.
  • Loading branch information...
1 parent bc66125 commit 8fe6a09fc61df7199a741baf87d839e2ead96727 @valueof valueof committed
Showing with 24 additions and 4 deletions.
  1. +9 −2 jshint.js
  2. +15 −2 tests/unit/parser.js
View
11 jshint.js
@@ -794,7 +794,7 @@ var JSHINT = (function () {
cx = /[\u0000-\u001f\u007f-\u009f\u00ad\u0600-\u0604\u070f\u17b4\u17b5\u200c-\u200f\u2028-\u202f\u2060-\u206f\ufeff\ufff0-\uffff]/;
// token
- tx = /^\s*([(){}\[.,:;'"~\?\]#@]|==?=?|\/(\*(jshint|jslint|members?|global)?|=|\/)?|\*[\/=]?|\+(?:=|\++)?|-(?:=|-+)?|%=?|&[&=]?|\|[|=]?|>>?>?=?|<([\/=!]|\!(\[|--)?|<=?)?|\^=?|\!=?=?|[a-zA-Z_$][a-zA-Z0-9_$]*|[0-9]+([xX][0-9a-fA-F]+|\.[0-9]*)?([eE][+\-]?[0-9]+)?)/;
+ tx = /^\s*([(){}\[.,:;'"~\?\]#@]|==?=?|\/=(?!(\S*\/[gim]?))|\/(\*(jshint|jslint|members?|global)?|\/)?|\*[\/=]?|\+(?:=|\++)?|-(?:=|-+)?|%=?|&[&=]?|\|[|=]?|>>?>?=?|<([\/=!]|\!(\[|--)?|<=?)?|\^=?|\!=?=?|[a-zA-Z_$][a-zA-Z0-9_$]*|[0-9]+([xX][0-9a-fA-F]+|\.[0-9]*)?([eE][+\-]?[0-9]+)?)/;
// characters in strings that need escapement
nx = /[\u0000-\u001f&<"\/\\\u007f-\u009f\u00ad\u0600-\u0604\u070f\u17b4\u17b5\u200c-\u200f\u2028-\u202f\u2060-\u206f\ufeff\ufff0-\uffff]/;
@@ -1267,6 +1267,7 @@ var JSHINT = (function () {
function match(x) {
var r = x.exec(s), r1;
+
if (r) {
l = r[0].length;
r1 = r[1];
@@ -1312,6 +1313,7 @@ var JSHINT = (function () {
character += n;
c = String.fromCharCode(i);
}
+
j = 0;
unclosedString: for (;;) {
while (j >= s.length) {
@@ -1329,12 +1331,14 @@ unclosedString: for (;;) {
warningAt("Unclosed string.", cl, cf);
}
}
+
c = s.charAt(j);
if (c === x) {
character += 1;
s = s.substr(j + 1);
return it("(string)", r, x);
}
+
if (c < " ") {
if (c === "\n" || c === "\r") {
break;
@@ -1431,7 +1435,9 @@ unclosedString: for (;;) {
if (!s) {
return it(nextLine() ? "(endline)" : "(end)", "");
}
+
t = match(tx);
+
if (!t) {
t = "";
c = "";
@@ -1530,10 +1536,11 @@ unclosedString: for (;;) {
break;
// /
case "/":
- if (token.id === "/=") {
+ if (s.charAt(0) === "=") {
errorAt("A regular expression literal can be confused with '/='.",
line, from);
}
+
if (prereg) {
depth = 0;
captures = 0;
View
17 tests/unit/parser.js
@@ -221,8 +221,7 @@ exports.regexp = function () {
, 'var n = /a??b+?c*?d{3,4}? a?b+c*d{3,4}/;'
, 'var o = /a\\/* [a-^-22-]/;'
, 'var p = /(?:(?=a|(?!b)))/;'
- , 'var q = /=;//;' // hack to not get other errors than "... confused with '/='".
- // The current parser recognizes this as /= and not as regex
+ , 'var q = /=;/;'
, 'var r = /(/;'
, 'var s = /(((/;'
, 'var t = /x/* 2;'
@@ -268,6 +267,20 @@ exports.regexp = function () {
.test(code);
};
+exports.testRegexRegressions = function () {
+ // GH-536
+ TestRun()
+ .test("str /= 5;", {}, { str: true });
+
+ TestRun()
+ .addError(1, "A regular expression literal can be confused with '/='.")
+ .test("str = str.replace(/=/g, '');", {}, { str: true });
+
+ TestRun()
+ .addError(1, "A regular expression literal can be confused with '/='.")
+ .test("str = str.replace(/=abc/g, '');", {}, { str: true });
+};
+
exports.strings = function () {
var code = [
'var a = "\u0012\\r";'

0 comments on commit 8fe6a09

Please sign in to comment.
Something went wrong with that request. Please try again.