Repository: eslint/eslint
Pull request #8296 (2017-03-20 18:42:24)

Chore: use precalculated counts in codeframe formatter
**What is the purpose of this pull request? (put an "X" next to item)**

[ ] Documentation update
[ ] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[ ] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:
Chore. See below.

**What changes did you make? (Give an overview)**
Updated the codeframe formatter to use the precalculated error and warning counts, just like it was done in https://github.com/eslint/eslint/pull/8251.

**Is there anything you'd like reviewers to focus on?**
I think not!



eslintbot: LGTM
vitorbal: Hmm, I'm not sure I understand why `appveyor/pr` is currently failing 🤔
```
AppVeyor was unable to build non-mergeable pull request
```
ilyavolodin: I think we should be able to ignore it. Since branch build completed on both appveyor as well as travis.

Patch:
diff --git a/lib/formatters/codeframe.js b/lib/formatters/codeframe.js
index 39ba06de4..1191ccb8e 100644
--- a/lib/formatters/codeframe.js
+++ b/lib/formatters/codeframe.js
@@ -101,15 +101,10 @@ module.exports = function(results) {
     const resultsWithMessages = results.filter(result => result.messages.length > 0);
 
     let output = resultsWithMessages.reduce((resultsOutput, result) => {
-        const messages = result.messages.map(message => {
-            if (message.fatal || message.severity === 2) {
-                errors++;
-            } else {
-                warnings++;
-            }
-
-            return `${formatMessage(message, result)}\n\n`;
-        });
+        const messages = result.messages.map(message => `${formatMessage(message, result)}\n\n`);
+
+        errors += result.errorCount;
+        warnings += result.warningCount;
 
         return resultsOutput.concat(messages);
     }, []).join("\n");
diff --git a/lib/rules/operator-assignment.js b/lib/rules/operator-assignment.js
index 43f21abc8..1c40eed15 100644
--- a/lib/rules/operator-assignment.js
+++ b/lib/rules/operator-assignment.js
@@ -135,7 +135,7 @@ module.exports = {
                                 const equalsToken = getOperatorToken(node);
                                 const operatorToken = getOperatorToken(expr);
                                 const leftText = sourceCode.getText().slice(node.range[0], equalsToken.range[0]);
-                                const rightText = sourceCode.getText().slice(operatorToken.range[1], expr.right.range[1]);
+                                const rightText = sourceCode.getText().slice(operatorToken.range[1], node.right.range[1]);
 
                                 return fixer.replaceText(node, `${leftText}${expr.operator}=${rightText}`);
                             }
diff --git a/package.json b/package.json
index 0b14bf07f..0a7ac385d 100644
--- a/package.json
+++ b/package.json
@@ -71,42 +71,42 @@
     "user-home": "^2.0.0"
   },
   "devDependencies": {
-    "babel-polyfill": "^6.9.1",
-    "babel-preset-es2015": "^6.9.0",
+    "babel-polyfill": "^6.23.0",
+    "babel-preset-es2015": "^6.24.0",
     "babelify": "^7.3.0",
-    "beefy": "^2.0.0",
-    "brfs": "0.0.9",
-    "browserify": "^12.0.1",
+    "beefy": "^2.1.8",
+    "brfs": "1.4.3",
+    "browserify": "^14.1.0",
     "chai": "^3.5.0",
-    "cheerio": "^0.19.0",
-    "coveralls": "^2.11.16",
-    "dateformat": "^1.0.8",
-    "ejs": "^2.3.3",
+    "cheerio": "^0.22.0",
+    "coveralls": "^2.12.0",
+    "dateformat": "^2.0.0",
+    "ejs": "^2.5.6",
     "eslint-plugin-eslint-plugin": "^0.7.1",
-    "eslint-plugin-node": "^4.1.0",
-    "eslint-release": "^0.10.0",
-    "esprima": "^2.4.1",
+    "eslint-plugin-node": "^4.2.1",
+    "eslint-release": "^0.10.1",
+    "esprima": "^3.1.3",
     "esprima-fb": "^15001.1001.0-dev-harmony-fb",
-    "istanbul": "^0.4.0",
-    "jsdoc": "^3.3.0-beta1",
-    "karma": "^0.13.22",
+    "istanbul": "^0.4.5",
+    "jsdoc": "^3.4.3",
+    "karma": "^1.5.0",
     "karma-babel-preprocessor": "^6.0.1",
-    "karma-mocha": "^1.0.1",
-    "karma-mocha-reporter": "^2.0.3",
-    "karma-phantomjs-launcher": "^1.0.0",
-    "leche": "^2.1.1",
+    "karma-mocha": "^1.3.0",
+    "karma-mocha-reporter": "^2.2.2",
+    "karma-phantomjs-launcher": "^1.0.4",
+    "leche": "^2.1.2",
     "load-perf": "^0.2.0",
-    "markdownlint": "^0.3.1",
-    "mocha": "^2.4.5",
+    "markdownlint": "^0.4.0",
+    "mocha": "^3.2.0",
     "mock-fs": "^4.2.0",
-    "npm-license": "^0.3.2",
-    "phantomjs-prebuilt": "^2.1.7",
-    "proxyquire": "^1.7.10",
-    "semver": "^5.0.3",
-    "shelljs-nodecli": "~0.1.0",
-    "sinon": "^1.17.2",
+    "npm-license": "^0.3.3",
+    "phantomjs-prebuilt": "^2.1.14",
+    "proxyquire": "^1.7.11",
+    "semver": "^5.3.0",
+    "shelljs-nodecli": "~0.1.1",
+    "sinon": "^2.0.0",
     "temp": "^0.8.3",
-    "through": "^2.3.6"
+    "through": "^2.3.8"
   },
   "keywords": [
     "ast",


Test patch:
diff --git a/tests/lib/formatters/codeframe.js b/tests/lib/formatters/codeframe.js
index 63e45db4e..fffaea2ee 100644
--- a/tests/lib/formatters/codeframe.js
+++ b/tests/lib/formatters/codeframe.js
@@ -54,7 +54,9 @@ describe("formatter:codeframe", () => {
     describe("when passed no messages", () => {
         const code = [{
             filePath: "foo.js",
-            messages: []
+            messages: [],
+            errorCount: 0,
+            warningCount: 0
         }];
 
         it("should return nothing", () => {
@@ -64,70 +66,84 @@ describe("formatter:codeframe", () => {
         });
     });
 
-    describe("when passed a single message", () => {
+    describe("when passed a single warning message", () => {
         const code = [{
             filePath: path.join(process.cwd(), "lib", "foo.js"),
             source: "var foo = 1;\n var bar = 2;\n",
             messages: [{
                 message: "Unexpected foo.",
-                severity: 2,
+                severity: 1,
                 line: 1,
                 column: 5,
                 ruleId: "foo"
-            }]
+            }],
+            errorCount: 0,
+            warningCount: 1
         }];
 
-        it("should return a string in the correct format for errors", () => {
+        it("should return a string in the correct format for warnings", () => {
             const result = formatter(code);
 
             assert.equal(chalk.stripColor(result), [
-                `error: Unexpected foo (foo) at ${path.join("lib", "foo.js")}:1:5:`,
+                `warning: Unexpected foo (foo) at ${path.join("lib", "foo.js")}:1:5:`,
                 "> 1 | var foo = 1;",
                 "    |     ^",
                 "  2 |  var bar = 2;",
                 "  3 | ",
                 "\n",
-                "1 error found."
+                "1 warning found."
             ].join("\n"));
         });
 
-        it("should return a string in the correct format for warnings", () => {
-            code[0].messages[0].severity = 1;
+        it("should return bold yellow summary when there are only warnings", () => {
+            sandbox.spy(chalkStub.yellow, "bold");
+            sandbox.spy(chalkStub.red, "bold");
+
+            formatter(code);
+
+            assert.equal(chalkStub.yellow.bold.callCount, 1);
+            assert.equal(chalkStub.red.bold.callCount, 0);
+        });
+    });
+
+    describe("when passed a single error message", () => {
+        const code = [{
+            filePath: path.join(process.cwd(), "lib", "foo.js"),
+            source: "var foo = 1;\n var bar = 2;\n",
+            messages: [{
+                message: "Unexpected foo.",
+                severity: 2,
+                line: 1,
+                column: 5,
+                ruleId: "foo"
+            }],
+            errorCount: 1,
+            warningCount: 0
+        }];
 
+        it("should return a string in the correct format for errors", () => {
             const result = formatter(code);
 
             assert.equal(chalk.stripColor(result), [
-                `warning: Unexpected foo (foo) at ${path.join("lib", "foo.js")}:1:5:`,
+                `error: Unexpected foo (foo) at ${path.join("lib", "foo.js")}:1:5:`,
                 "> 1 | var foo = 1;",
                 "    |     ^",
                 "  2 |  var bar = 2;",
                 "  3 | ",
                 "\n",
-                "1 warning found."
+                "1 error found."
             ].join("\n"));
         });
 
         it("should return bold red summary when there are errors", () => {
             sandbox.spy(chalkStub.yellow, "bold");
             sandbox.spy(chalkStub.red, "bold");
-            code[0].messages[0].severity = 2;
 
             formatter(code);
 
             assert.equal(chalkStub.yellow.bold.callCount, 0);
             assert.equal(chalkStub.red.bold.callCount, 1);
         });
-
-        it("should return bold yellow summary when there are only warnings", () => {
-            sandbox.spy(chalkStub.yellow, "bold");
-            sandbox.spy(chalkStub.red, "bold");
-            code[0].messages[0].severity = 1;
-
-            formatter(code);
-
-            assert.equal(chalkStub.yellow.bold.callCount, 1);
-            assert.equal(chalkStub.red.bold.callCount, 0);
-        });
     });
 
     describe("when passed multiple messages", () => {
@@ -146,7 +162,9 @@ describe("formatter:codeframe", () => {
                 line: 1,
                 column: 7,
                 ruleId: "no-unused-vars"
-            }]
+            }],
+            errorCount: 2,
+            warningCount: 0
         }];
 
         it("should return a string with multiple entries", () => {
@@ -171,6 +189,8 @@ describe("formatter:codeframe", () => {
             sandbox.spy(chalkStub.yellow, "bold");
             sandbox.spy(chalkStub.red, "bold");
             code[0].messages[0].severity = 1;
+            code[0].warningCount = 1;
+            code[0].errorCount = 1;
 
             formatter(code);
 
@@ -190,6 +210,8 @@ describe("formatter:codeframe", () => {
                 column: 11,
                 source: "    const foo = 1;"
             }],
+            errorCount: 1,
+            warningCount: 0,
             output: "function foo() {\n\n    // a comment\n    const foo = 1;\n}\n\n"
         }];
 
@@ -221,7 +243,9 @@ describe("formatter:codeframe", () => {
                 line: 1,
                 column: 14,
                 ruleId: "semi"
-            }]
+            }],
+            errorCount: 1,
+            warningCount: 0
         }, {
             filePath: "bar.js",
             source: "const bar = 2\n",
@@ -231,7 +255,9 @@ describe("formatter:codeframe", () => {
                 line: 1,
                 column: 14,
                 ruleId: "semi"
-            }]
+            }],
+            errorCount: 1,
+            warningCount: 0
         }];
 
         it("should return a string with multiple entries", () => {
@@ -265,7 +291,9 @@ describe("formatter:codeframe", () => {
                 message: "Parsing error: Unexpected token {",
                 line: 1,
                 column: 2
-            }]
+            }],
+            errorCount: 1,
+            warningCount: 0
         }];
 
         it("should return a string in the correct format", () => {
@@ -288,7 +316,9 @@ describe("formatter:codeframe", () => {
             messages: [{
                 fatal: true,
                 message: "Couldn't find foo.js."
-            }]
+            }],
+            errorCount: 1,
+            warningCount: 0
         }];
 
         it("should return a string without code preview (codeframe)", () => {
@@ -306,7 +336,9 @@ describe("formatter:codeframe", () => {
                 message: "Unexpected foo.",
                 severity: 2,
                 source: "foo"
-            }]
+            }],
+            errorCount: 1,
+            warningCount: 0
         }];
 
         it("should return a string without code preview (codeframe)", () => {
diff --git a/tests/lib/rules/array-bracket-spacing.js b/tests/lib/rules/array-bracket-spacing.js
index bea84ff76..35f13def7 100644
--- a/tests/lib/rules/array-bracket-spacing.js
+++ b/tests/lib/rules/array-bracket-spacing.js
@@ -695,7 +695,7 @@ ruleTester.run("array-bracket-spacing", rule, {
             code: "([ a, b ]: Array<any>) => {}",
             output: "([a, b]: Array<any>) => {}",
             options: ["never"],
-            ecmaFeatures: {
+            parserOptions: {
                 ecmaVersion: 6
             },
             errors: [
@@ -718,7 +718,7 @@ ruleTester.run("array-bracket-spacing", rule, {
             code: "([a, b]: Array< any >) => {}",
             output: "([ a, b ]: Array< any >) => {}",
             options: ["always"],
-            ecmaFeatures: {
+            parserOptions: {
                 ecmaVersion: 6
             },
             errors: [
diff --git a/tests/lib/rules/no-restricted-modules.js b/tests/lib/rules/no-restricted-modules.js
index ea4d5e1d4..cadf6bba6 100644
--- a/tests/lib/rules/no-restricted-modules.js
+++ b/tests/lib/rules/no-restricted-modules.js
@@ -26,11 +26,11 @@ ruleTester.run("no-restricted-modules", rule, {
         { code: "require(2)", options: ["crypto"] },
         { code: "require(foo)", options: ["crypto"] },
         { code: "var foo = bar('crypto');", options: ["crypto"] },
-        { code: "require(\"foo/bar\");", option: ["foo"] },
-        { code: "var withPaths = require(\"foo/bar\");", option: [{ paths: ["foo", "bar"] }] },
-        { code: "var withPatterns = require(\"foo/bar\");", option: [{ patterns: ["foo/c*"] }] },
-        { code: "var withPatternsAndPaths = require(\"foo/bar\");", option: [{ paths: ["foo"], patterns: ["foo/c*"] }] },
-        { code: "var withGitignores = require(\"foo/bar\");", option: [{ paths: ["foo"], patterns: ["foo/*", "!foo/bar"] }] }
+        { code: "require(\"foo/bar\");", options: ["foo"] },
+        { code: "var withPaths = require(\"foo/bar\");", options: [{ paths: ["foo", "bar"] }] },
+        { code: "var withPatterns = require(\"foo/bar\");", options: [{ patterns: ["foo/c*"] }] },
+        { code: "var withPatternsAndPaths = require(\"foo/bar\");", options: [{ paths: ["foo"], patterns: ["foo/c*"] }] },
+        { code: "var withGitignores = require(\"foo/bar\");", options: [{ paths: ["foo"], patterns: ["foo/*", "!foo/bar"] }] }
     ],
     invalid: [{
         code: "require(\"fs\")",
diff --git a/tests/lib/rules/no-restricted-properties.js b/tests/lib/rules/no-restricted-properties.js
index fca512cbc..e632663ee 100644
--- a/tests/lib/rules/no-restricted-properties.js
+++ b/tests/lib/rules/no-restricted-properties.js
@@ -108,7 +108,7 @@ ruleTester.run("no-restricted-properties", rule, {
             options: [{ object: "foo", property: "bar" }],
             parserOptions: { ecmaVersion: 6 }
         }, {
-            code: "let {baz: {bar: qux}} = foo;", property: "bar",
+            code: "let {baz: {bar: qux}} = foo;",
             options: [{ object: "foo", property: "bar" }],
             parserOptions: { ecmaVersion: 6 }
         }, {
diff --git a/tests/lib/rules/no-unexpected-multiline.js b/tests/lib/rules/no-unexpected-multiline.js
index eb0e9f92b..3a5dee1e2 100644
--- a/tests/lib/rules/no-unexpected-multiline.js
+++ b/tests/lib/rules/no-unexpected-multiline.js
@@ -47,60 +47,79 @@ ruleTester.run("no-unexpected-multiline", rule, {
     invalid: [
         {
             code: "var a = b\n(x || y).doSomething()",
-            line: 2,
-            column: 1,
-            errors: [{ message: "Unexpected newline between function and ( of function call." }]
+            errors: [{
+                message: "Unexpected newline between function and ( of function call.",
+                line: 2,
+                column: 1
+
+            }]
         },
         {
             code: "var a = (a || b)\n(x || y).doSomething()",
-            line: 2,
-            column: 1,
-            errors: [{ message: "Unexpected newline between function and ( of function call." }]
+            errors: [{
+                line: 2,
+                column: 1,
+                message: "Unexpected newline between function and ( of function call."
+            }]
         },
         {
             code: "var a = (a || b)\n(x).doSomething()",
-            line: 2,
-            column: 1,
-            errors: [{ message: "Unexpected newline between function and ( of function call." }]
+            errors: [{
+                line: 2,
+                column: 1,
+                message: "Unexpected newline between function and ( of function call."
+            }]
         },
         {
             code: "var a = b\n[a, b, c].forEach(doSomething)",
-            line: 2,
-            column: 1,
-            errors: [{ message: "Unexpected newline between object and [ of property access." }]
+            errors: [{
+                line: 2,
+                column: 1,
+                message: "Unexpected newline between object and [ of property access."
+            }]
         },
         {
             code: "var a = b\n    (x || y).doSomething()",
-            line: 2,
-            column: 5,
-            errors: [{ message: "Unexpected newline between function and ( of function call." }]
+            errors: [{
+                line: 2,
+                column: 5,
+                message: "Unexpected newline between function and ( of function call."
+            }]
         },
         {
             code: "var a = b\n  [a, b, c].forEach(doSomething)",
-            line: 2,
-            column: 3,
-            errors: [{ message: "Unexpected newline between object and [ of property access." }]
+            errors: [{
+                line: 2,
+                column: 3,
+                message: "Unexpected newline between object and [ of property access."
+            }]
         },
         {
             code: "let x = function() {}\n `hello`",
             parserOptions: { ecmaVersion: 6 },
-            line: 1,
-            column: 9,
-            errors: [{ message: "Unexpected newline between template tag and template literal." }]
+            errors: [{
+                line: 1,
+                column: 9,
+                message: "Unexpected newline between template tag and template literal."
+            }]
         },
         {
             code: "let x = function() {}\nx\n`hello`",
             parserOptions: { ecmaVersion: 6 },
-            line: 2,
-            column: 1,
-            errors: [{ message: "Unexpected newline between template tag and template literal." }]
+            errors: [{
+                line: 2,
+                column: 1,
+                message: "Unexpected newline between template tag and template literal."
+            }]
         },
         {
             code: "x\n.y\nz\n`Invalid Test Case`",
             parserOptions: { ecmaVersion: 6 },
-            line: 3,
-            column: 1,
-            errors: [{ message: "Unexpected newline between template tag and template literal." }]
+            errors: [{
+                line: 3,
+                column: 1,
+                message: "Unexpected newline between template tag and template literal."
+            }]
         }
     ]
 });
diff --git a/tests/lib/rules/operator-assignment.js b/tests/lib/rules/operator-assignment.js
index e0f3e4e13..5e5e65564 100644
--- a/tests/lib/rules/operator-assignment.js
+++ b/tests/lib/rules/operator-assignment.js
@@ -154,6 +154,16 @@ ruleTester.run("operator-assignment", rule, {
         output: "x += y",
         options: ["always"],
         errors: EXPECTED_OPERATOR_ASSIGNMENT
+    }, {
+        code: "x = x + (y)",
+        output: "x += (y)",
+        options: ["always"],
+        errors: EXPECTED_OPERATOR_ASSIGNMENT
+    }, {
+        code: "x += (y)",
+        output: "x = x + (y)",
+        options: ["never"],
+        errors: UNEXPECTED_OPERATOR_ASSIGNMENT
     }, {
         code: "x += y",
         output: "x = x + y",