Update: handle uncommon linebreaks consistently in rules (fixes #7949) (

#8049)

* Update: handle uncommon linebreaks consistently in rules (fixes #7949)

Currently, rules that deal with line-splitting need to include a list of JS linebreaks in some form (e.g. in a regex character class). As a result, splitting a string into JS lines is implemented in 22 different places in the codebase. As described in #7949, not all of these implementations are correct (many of them forget to account for \r, \u2028, and \u2029.

This commit updates ast-utils.js with the following properties:

* `astUtils.LINEBREAKS`: a Set containing all JS linebreaks
* `astUtils.LINEBREAK_MATCHER`: a regular expression to match JS linebreaks
* `astUtils.createGlobalLinebreakMatcher`: a function that returns a new global regex to match JS linebreaks

Additionally, it updates the other line-splitting code to use these constants.

Observable changes:

* The error message for `newline-per-chained-call` will handle property names with \u2028 and \u2029 the same way it handles names with \n.
* `no-multi-str` will now report an error for strings that contain \r, \u2028, or \u2029. (Previously, it only reported strings containing \n.)
* `no-unused-vars` will now report the correct location if a `/* global foo */` comment contains a linebreak other than \n. (Previously, it would report an invalid location.)
* `spaced-comment` will no longer report an error if a comment contains tokens in the `exceptions` option followed by \u2028 or \u2029.

* Fix build on Node 4

* Avoid changing whitespace logic in no-multi-spaces

lib/ast-utils.js

@@ -24,6 +24,9 @@ const bindOrCallOrApplyPattern = /^(?:bind|call|apply)$/;
 const breakableTypePattern = /^(?:(?:Do)?While|For(?:In|Of)?|Switch)Statement$/;
 const thisTagPattern = /^[\s*]*@this/m;
 
+const LINEBREAKS = new Set(["\r\n", "\r", "\n", "\u2028", "\u2029"]);
+const LINEBREAK_MATCHER = /\r\n|[\r\n\u2028\u2029]/;
+
 // A set of node types that can contain a list of statements
 const STATEMENT_LIST_PARENTS = new Set(["Program", "BlockStatement", "SwitchCase"]);
 
@@ -391,6 +394,15 @@ function getOpeningParenOfParams(node, sourceCode) {
         : sourceCode.getFirstToken(node, isOpeningParenToken);
 }
 
+/**
+ * Creates a version of the LINEBREAK_MATCHER regex with the global flag.
+ * Global regexes are mutable, so this needs to be a function instead of a constant.
+ * @returns {RegExp} A global regular expression that matches line terminators
+ */
+function createGlobalLinebreakMatcher() {
+    return new RegExp(LINEBREAK_MATCHER.source, "g");
+}
+
 const lineIndexCache = new WeakMap();
 
 /**
@@ -402,7 +414,7 @@ function getLineIndices(sourceCode) {
 
     if (!lineIndexCache.has(sourceCode)) {
         const lineIndices = [0];
-        const lineEndingPattern = /\r\n|[\r\n\u2028\u2029]/g;
+        const lineEndingPattern = createGlobalLinebreakMatcher();
         let match;
 
         /*
@@ -430,6 +442,8 @@ function getLineIndices(sourceCode) {
 //------------------------------------------------------------------------------
 
 module.exports = {
+    LINEBREAKS,
+    LINEBREAK_MATCHER,
     STATEMENT_LIST_PARENTS,
 
     /**
@@ -452,6 +466,7 @@ module.exports = {
     isInLoop,
     isArrayFromMethod,
     isParenthesised,
+    createGlobalLinebreakMatcher,
 
     isArrowToken,
     isClosingBraceToken,

lib/rules/brace-style.js

@@ -62,7 +62,7 @@ module.exports = {
         function removeNewlineBetween(firstToken, secondToken) {
             const textRange = [firstToken.range[1], secondToken.range[0]];
             const textBetween = sourceCode.text.slice(textRange[0], textRange[1]);
-            const NEWLINE_REGEX = /\r\n|\r|\n|\u2028|\u2029/g;
+            const NEWLINE_REGEX = astUtils.createGlobalLinebreakMatcher();
 
             // Don't do a fix if there is a comment between the tokens
             return fixer => fixer.replaceTextRange(textRange, textBetween.trim() ? null : textBetween.replace(NEWLINE_REGEX, ""));

lib/rules/func-call-spacing.js

@@ -85,7 +85,7 @@ module.exports = {
 
             const textBetweenTokens = text.slice(prevToken.range[1], parenToken.range[0]).replace(/\/\*.*?\*\//g, "");
             const hasWhitespace = /\s/.test(textBetweenTokens);
-            const hasNewline = hasWhitespace && /[\n\r\u2028\u2029]/.test(textBetweenTokens);
+            const hasNewline = hasWhitespace && astUtils.LINEBREAK_MATCHER.test(textBetweenTokens);
 
             /*
              * never allowNewlines hasWhitespace hasNewline message

lib/rules/key-spacing.js

@@ -21,7 +21,7 @@ const astUtils = require("../ast-utils");
  * @returns {boolean} True if str contains a line terminator.
  */
 function containsLineTerminator(str) {
-    return /[\n\r\u2028\u2029]/.test(str);
+    return astUtils.LINEBREAK_MATCHER.test(str);
 }
 
 /**

lib/rules/linebreak-style.js

@@ -5,6 +5,12 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("../ast-utils");
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -60,7 +66,7 @@ module.exports = {
                     expectedLF = linebreakStyle === "unix",
                     expectedLFChars = expectedLF ? "\n" : "\r\n",
                     source = sourceCode.getText(),
-                    pattern = /\r\n|\r|\n|\u2028|\u2029/g;
+                    pattern = astUtils.createGlobalLinebreakMatcher();
                 let match;
 
                 let i = 0;

lib/rules/newline-after-var.js

@@ -5,6 +5,12 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("../ast-utils");
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -201,8 +207,7 @@ module.exports = {
                     message: NEVER_MESSAGE,
                     data: { identifier: node.name },
                     fix(fixer) {
-                        const NEWLINE_REGEX = /\r\n|\r|\n|\u2028|\u2029/;
-                        const linesBetween = sourceCode.getText().slice(lastToken.range[1], nextToken.range[0]).split(NEWLINE_REGEX);
+                        const linesBetween = sourceCode.getText().slice(lastToken.range[1], nextToken.range[0]).split(astUtils.LINEBREAK_MATCHER);
 
                         return fixer.replaceTextRange([lastToken.range[1], nextToken.range[0]], `${linesBetween.slice(0, -1).join("")}\n${linesBetween[linesBetween.length - 1]}`);
                     }

lib/rules/newline-per-chained-call.js

@@ -6,6 +6,8 @@
 
 "use strict";
 
+const astUtils = require("../ast-utils");
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -47,7 +49,7 @@ module.exports = {
          */
         function getPropertyText(node) {
             const prefix = node.computed ? "[" : ".";
-            const lines = sourceCode.getText(node.property).split(/\r\n|\r|\n/g);
+            const lines = sourceCode.getText(node.property).split(astUtils.LINEBREAK_MATCHER);
             const suffix = node.computed && lines.length === 1 ? "]" : "";
 
             return prefix + lines[0] + suffix;

lib/rules/no-irregular-whitespace.js

@@ -6,14 +6,20 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("../ast-utils");
+
 //------------------------------------------------------------------------------
 // Constants
 //------------------------------------------------------------------------------
 
 const ALL_IRREGULARS = /[\f\v\u0085\u00A0\ufeff\u00a0\u1680\u180e\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u200b\u202f\u205f\u3000\u2028\u2029]/;
 const IRREGULAR_WHITESPACE = /[\f\v\u0085\u00A0\ufeff\u00a0\u1680\u180e\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u200b\u202f\u205f\u3000]+/mg;
 const IRREGULAR_LINE_TERMINATORS = /[\u2028\u2029]/mg;
-const LINE_BREAK = /\r\n|\r|\n|\u2028|\u2029/g;
+const LINE_BREAK = astUtils.createGlobalLinebreakMatcher();
 
 //------------------------------------------------------------------------------
 // Rule Definition

lib/rules/no-multi-spaces.js

@@ -5,6 +5,8 @@
 
 "use strict";
 
+const astUtils = require("../ast-utils");
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -93,7 +95,8 @@ module.exports = {
                 const sourceCode = context.getSourceCode(),
                     source = sourceCode.getText(),
                     allComments = sourceCode.getAllComments(),
-                    pattern = /[^\n\r\u2028\u2029\t ].? {2,}/g;  // note: repeating space
+                    JOINED_LINEBEAKS = Array.from(astUtils.LINEBREAKS).join(""),
+                    pattern = new RegExp(String.raw`[^ \t${JOINED_LINEBEAKS}].? {2,}`, "g");  // note: repeating space
                 let parent;
 
 

lib/rules/no-multi-str.js

@@ -5,6 +5,12 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("../ast-utils");
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -39,9 +45,7 @@ module.exports = {
         return {
 
             Literal(node) {
-                const lineBreak = /\n/;
-
-                if (lineBreak.test(node.raw) && !isJSXElement(node.parent)) {
+                if (astUtils.LINEBREAK_MATCHER.test(node.raw) && !isJSXElement(node.parent)) {
                     context.report({ node, message: "Multiline support is limited to browsers supporting ES5 only." });
                 }
             }

lib/rules/no-trailing-spaces.js

@@ -4,6 +4,12 @@
  */
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("../ast-utils");
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -34,7 +40,7 @@ module.exports = {
     create(context) {
         const sourceCode = context.getSourceCode();
 
-        const BLANK_CLASS = "[ \t\u00a0\u2000-\u200b\u2028\u2029\u3000]",
+        const BLANK_CLASS = "[ \t\u00a0\u2000-\u200b\u3000]",
             SKIP_BLANK = `^${BLANK_CLASS}*$`,
             NONBLANK = `${BLANK_CLASS}+$`;
 
@@ -81,7 +87,7 @@ module.exports = {
                 const re = new RegExp(NONBLANK),
                     skipMatch = new RegExp(SKIP_BLANK),
                     lines = sourceCode.lines,
-                    linebreaks = sourceCode.getText().match(/\r\n|\r|\n|\u2028|\u2029/g);
+                    linebreaks = sourceCode.getText().match(astUtils.createGlobalLinebreakMatcher());
                 let totalLength = 0,
                     fixRange = [];
 

lib/rules/no-unused-vars.js

@@ -62,6 +62,7 @@ module.exports = {
     },
 
     create(context) {
+        const sourceCode = context.getSourceCode();
 
         const DEFINED_MESSAGE = "'{{name}}' is defined but never used.";
         const ASSIGNED_MESSAGE = "'{{name}}' is assigned a value but never used.";
@@ -566,23 +567,8 @@ module.exports = {
          */
         function getLocation(variable) {
             const comment = variable.eslintExplicitGlobalComment;
-            const baseLoc = comment.loc.start;
-            let column = getColumnInComment(variable, comment);
-            const prefix = comment.value.slice(0, column);
-            const lineInComment = (prefix.match(/\n/g) || []).length;
 
-            if (lineInComment > 0) {
-                column -= 1 + prefix.lastIndexOf("\n");
-            } else {
-
-                // 2 is for `/*`
-                column += baseLoc.column + 2;
-            }
-
-            return {
-                line: baseLoc.line + lineInComment,
-                column
-            };
+            return astUtils.getLocationFromRangeIndex(sourceCode, comment.range[0] + 2 + getColumnInComment(variable, comment));
         }
 
         //--------------------------------------------------------------------------

lib/rules/no-useless-escape.js

@@ -24,7 +24,7 @@ function union(setA, setB) {
     }());
 }
 
-const VALID_STRING_ESCAPES = new Set("\\nrvtbfux\n\r\u2028\u2029");
+const VALID_STRING_ESCAPES = union(new Set("\\nrvtbfux"), astUtils.LINEBREAKS);
 const REGEX_GENERAL_ESCAPES = new Set("\\bcdDfnrsStvwWxu0123456789]");
 const REGEX_NON_CHARCLASS_ESCAPES = union(REGEX_GENERAL_ESCAPES, new Set("^/.$*+?[{}|()B"));
 

lib/rules/operator-linebreak.js

@@ -15,8 +15,6 @@ const astUtils = require("../ast-utils");
 // Rule Definition
 //------------------------------------------------------------------------------
 
-const LINEBREAK_REGEX = /\r\n|\r|\n|\u2028|\u2029/g;
-
 module.exports = {
     meta: {
         docs: {
@@ -104,6 +102,7 @@ module.exports = {
                     newTextBefore = textAfter;
                     newTextAfter = textBefore;
                 } else {
+                    const LINEBREAK_REGEX = astUtils.createGlobalLinebreakMatcher();
 
                     // Otherwise, if no linebreak is desired and no comments interfere, replace the linebreaks with empty strings.
                     newTextBefore = desiredStyle === "before" || textBefore.trim() ? textBefore : textBefore.replace(LINEBREAK_REGEX, "");

lib/rules/quotes.js

@@ -33,6 +33,9 @@ const QUOTE_SETTINGS = {
     }
 };
 
+// An unescaped newline is a newline preceded by an even number of backslashes.
+const UNESCAPED_LINEBREAK_PATTERN = new RegExp(String.raw`(^|[^\\])(\\\\)*[${Array.from(astUtils.LINEBREAKS).join("")}]`);
+
 /**
  * Switches quoting of javascript string between ' " and `
  * escaping and unescaping as necessary.
@@ -258,11 +261,8 @@ module.exports = {
                     return;
                 }
 
-                /*
-                 * A warning should be produced if the template literal only has one TemplateElement, and has no unescaped newlines.
-                 * An unescaped newline is a newline preceded by an even number of backslashes.
-                 */
-                const shouldWarn = node.quasis.length === 1 && !/(^|[^\\])(\\\\)*[\r\n\u2028\u2029]/.test(node.quasis[0].value.raw);
+                // A warning should be produced if the template literal only has one TemplateElement, and has no unescaped newlines.
+                const shouldWarn = node.quasis.length === 1 && !UNESCAPED_LINEBREAK_PATTERN.test(node.quasis[0].value.raw);
 
                 if (shouldWarn) {
                     context.report({

lib/rules/spaced-comment.js

@@ -5,6 +5,7 @@
 "use strict";
 
 const lodash = require("lodash");
+const astUtils = require("../ast-utils");
 
 //------------------------------------------------------------------------------
 // Helpers
@@ -88,8 +89,7 @@ function createExceptionsPattern(exceptions) {
             pattern += exceptions.map(escapeAndRepeat).join("|");
             pattern += ")";
         }
-
-        pattern += "(?:$|[\n\r]))";
+        pattern += `(?:$|[${Array.from(astUtils.LINEBREAKS).join("")}]))`;
     }
 
     return pattern;

lib/util/source-code.js

@@ -9,7 +9,8 @@
 //------------------------------------------------------------------------------
 
 const TokenStore = require("../token-store"),
-    Traverser = require("./traverser");
+    Traverser = require("./traverser"),
+    astUtils = require("../ast-utils");
 
 //------------------------------------------------------------------------------
 // Private
@@ -160,7 +161,7 @@ function SourceCode(text, ast) {
  * @public
  */
 SourceCode.splitLines = function(text) {
-    return text.split(/\r\n|\r|\n|\u2028|\u2029/g);
+    return text.split(astUtils.createGlobalLinebreakMatcher());
 };
 
 SourceCode.prototype = {

tests/lib/ast-utils.js

@@ -1267,4 +1267,34 @@ describe("ast-utils", () => {
             });
         });
     });
+
+    describe("createGlobalLinebreakMatcher", () => {
+        it("returns a regular expression with the g flag", () => {
+            assert.instanceOf(astUtils.createGlobalLinebreakMatcher(), RegExp);
+            assert(astUtils.createGlobalLinebreakMatcher().toString().endsWith("/g"));
+        });
+        it("returns unique objects on each call", () => {
+            const firstObject = astUtils.createGlobalLinebreakMatcher();
+            const secondObject = astUtils.createGlobalLinebreakMatcher();
+
+            assert.notStrictEqual(firstObject, secondObject);
+        });
+        describe("correctly matches linebreaks", () => {
+            const LINE_COUNTS = {
+                foo: 1,
+                "foo\rbar": 2,
+                "foo\n": 2,
+                "foo\nbar": 2,
+                "foo\r\nbar": 2,
+                "foo\r\u2028bar": 3,
+                "foo\u2029bar": 2
+            };
+
+            Object.keys(LINE_COUNTS).forEach(text => {
+                it(text, () => {
+                    assert.strictEqual(text.split(astUtils.createGlobalLinebreakMatcher()).length, LINE_COUNTS[text]);
+                });
+            });
+        });
+    });
 });