Repository: eslint/eslint
Pull request #8251 (2017-03-14 20:42:42)

Chore: use precalculated counts in stylish 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:

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (http://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**
Use the precalculated error and warning count in stylish

**Is there anything you'd like reviewers to focus on?**
Is there a reason why we weren't doing this before?



eslintbot: LGTM
mention-bot: @alberto, thanks for your PR! By analyzing the history of the files in this pull request, we identified @mgol, @sindresorhus and @mysticatea to be potential reviewers.
eslintbot: LGTM

Patch:
diff --git a/Makefile.js b/Makefile.js
index 24151b884..60f02b58c 100644
--- a/Makefile.js
+++ b/Makefile.js
@@ -833,7 +833,7 @@ target.checkRuleFiles = function() {
 
         // check for recommended configuration
         if (!isInConfig()) {
-            console.error("Missing eslint:recommended setting for %s in conf/eslint-recommendd.js", basename);
+            console.error("Missing eslint:recommended setting for %s in conf/eslint-recommended.js", basename);
             errors++;
         }
 
diff --git a/docs/rules/arrow-parens.md b/docs/rules/arrow-parens.md
index 2e006c283..371a500ff 100644
--- a/docs/rules/arrow-parens.md
+++ b/docs/rules/arrow-parens.md
@@ -71,7 +71,7 @@ a => a;
 a => {'\n'};
 a.then(foo => {});
 a.then(foo => a);
-a(foo => { if (true) {}; });
+a(foo => { if (true) {} });
 ```
 
 Examples of **correct** code for this rule with the default `"always"` option:
@@ -85,7 +85,7 @@ Examples of **correct** code for this rule with the default `"always"` option:
 (a) => a;
 (a) => {'\n'}
 a.then((foo) => {});
-a.then((foo) => { if (true) {}; });
+a.then((foo) => { if (true) {} });
 ```
 
 #### If Statements
@@ -102,7 +102,7 @@ if (a => b) {
  console.log('bigger');
 } else {
  console.log('smaller');
-};
+}
 // outputs 'bigger', not smaller as expected
 ```
 
@@ -120,7 +120,7 @@ if ((a) => b) {
  console.log('truthy value returned');
 } else {
  console.log('falsey value returned');
-};
+}
 // outputs 'truthy value returned'
 ```
 
@@ -158,7 +158,7 @@ Examples of **incorrect** code for this rule with the `"as-needed"` option:
 (a) => {'\n'};
 a.then((foo) => {});
 a.then((foo) => a);
-a((foo) => { if (true) {}; });
+a((foo) => { if (true) {} });
 ```
 
 Examples of **correct** code for this rule with the `"as-needed"` option:
@@ -172,7 +172,7 @@ a => {};
 a => a;
 a => {'\n'};
 a.then(foo => {});
-a.then(foo => { if (true) {}; });
+a.then(foo => { if (true) {} });
 (a, b, c) => a;
 (a = 10) => a;
 ([a, b]) => a;
@@ -209,8 +209,8 @@ a => ({});
 () => {};
 a => a;
 a.then((foo) => {});
-a.then((foo) => { if (true) {}; });
-a((foo) => { if (true) {}; });
+a.then((foo) => { if (true) {} });
+a((foo) => { if (true) {} });
 (a, b, c) => a;
 (a = 10) => a;
 ([a, b]) => a;
diff --git a/lib/formatters/stylish.js b/lib/formatters/stylish.js
index a176d03ab..c19b95d6e 100644
--- a/lib/formatters/stylish.js
+++ b/lib/formatters/stylish.js
@@ -28,7 +28,6 @@ function pluralize(word, count) {
 module.exports = function(results) {
 
     let output = "\n",
-        total = 0,
         errors = 0,
         warnings = 0,
         summaryColor = "yellow";
@@ -40,7 +39,9 @@ module.exports = function(results) {
             return;
         }
 
-        total += messages.length;
+        errors += result.errorCount;
+        warnings += result.warningCount;
+
         output += `${chalk.underline(result.filePath)}\n`;
 
         output += `${table(
@@ -50,10 +51,8 @@ module.exports = function(results) {
                 if (message.fatal || message.severity === 2) {
                     messageType = chalk.red("error");
                     summaryColor = "red";
-                    errors++;
                 } else {
                     messageType = chalk.yellow("warning");
-                    warnings++;
                 }
 
                 return [
@@ -74,6 +73,8 @@ module.exports = function(results) {
         ).split("\n").map(el => el.replace(/(\d+)\s+(\d+)/, (m, p1, p2) => chalk.dim(`${p1}:${p2}`))).join("\n")}\n\n`;
     });
 
+    const total = errors + warnings;
+
     if (total > 0) {
         output += chalk[summaryColor].bold([
             "\u2716 ", total, pluralize(" problem", total),
diff --git a/lib/rules/func-name-matching.js b/lib/rules/func-name-matching.js
index fc6fa8588..db06d5d46 100644
--- a/lib/rules/func-name-matching.js
+++ b/lib/rules/func-name-matching.js
@@ -132,6 +132,15 @@ module.exports = {
             });
         }
 
+        /**
+         * Determines whether a given node is a string literal
+         * @param {ASTNode} node The node to check
+         * @returns {boolean} `true` if the node is a string literal
+         */
+        function isStringLiteral(node) {
+            return node.type === "Literal" && typeof node.value === "string";
+        }
+
         //--------------------------------------------------------------------------
         // Public
         //--------------------------------------------------------------------------
@@ -139,7 +148,7 @@ module.exports = {
         return {
 
             VariableDeclarator(node) {
-                if (!node.init || node.init.type !== "FunctionExpression") {
+                if (!node.init || node.init.type !== "FunctionExpression" || node.id.type !== "Identifier") {
                     return;
                 }
                 if (node.init.id && shouldWarn(node.id.name, node.init.id.name)) {
@@ -148,10 +157,12 @@ module.exports = {
             },
 
             AssignmentExpression(node) {
-                if (node.right.type !== "FunctionExpression" ||
-                   (node.left.computed && node.left.property.type !== "Literal") ||
-                    (!includeModuleExports && isModuleExports(node.left))
-                   ) {
+                if (
+                    node.right.type !== "FunctionExpression" ||
+                    (node.left.computed && node.left.property.type !== "Literal") ||
+                    (!includeModuleExports && isModuleExports(node.left)) ||
+                    (node.left.type !== "Identifier" && node.left.type !== "MemberExpression")
+                ) {
                     return;
                 }
 
@@ -164,13 +175,13 @@ module.exports = {
             },
 
             Property(node) {
-                if (node.value.type !== "FunctionExpression" || !node.value.id || node.computed && node.key.type !== "Literal") {
+                if (node.value.type !== "FunctionExpression" || !node.value.id || node.computed && !isStringLiteral(node.key)) {
                     return;
                 }
                 if (node.key.type === "Identifier" && shouldWarn(node.key.name, node.value.id.name)) {
                     report(node, node.key.name, node.value.id.name, true);
                 } else if (
-                    node.key.type === "Literal" &&
+                    isStringLiteral(node.key) &&
                     isIdentifier(node.key.value, ecmaVersion) &&
                     shouldWarn(node.key.value, node.value.id.name)
                 ) {
diff --git a/lib/rules/no-useless-escape.js b/lib/rules/no-useless-escape.js
index 91edfe718..ffe119997 100644
--- a/lib/rules/no-useless-escape.js
+++ b/lib/rules/no-useless-escape.js
@@ -147,7 +147,13 @@ module.exports = {
         function check(node) {
             const isTemplateElement = node.type === "TemplateElement";
 
-            if (isTemplateElement && node.parent && node.parent.parent && node.parent.parent.type === "TaggedTemplateExpression") {
+            if (
+                isTemplateElement &&
+                node.parent &&
+                node.parent.parent &&
+                node.parent.parent.type === "TaggedTemplateExpression" &&
+                node.parent === node.parent.parent.quasi
+            ) {
 
                 // Don't report tagged template literals, because the backslash character is accessible to the tag function.
                 return;
diff --git a/lib/rules/sort-vars.js b/lib/rules/sort-vars.js
index 6bee22103..1fdfc9ace 100644
--- a/lib/rules/sort-vars.js
+++ b/lib/rules/sort-vars.js
@@ -37,11 +37,9 @@ module.exports = {
 
         return {
             VariableDeclaration(node) {
-                node.declarations.reduce((memo, decl) => {
-                    if (decl.id.type === "ObjectPattern" || decl.id.type === "ArrayPattern") {
-                        return memo;
-                    }
+                const idDeclarations = node.declarations.filter(decl => decl.id.type === "Identifier");
 
+                idDeclarations.slice(1).reduce((memo, decl) => {
                     let lastVariableName = memo.id.name,
                         currenVariableName = decl.id.name;
 
@@ -56,7 +54,7 @@ module.exports = {
                     }
                     return decl;
 
-                }, node.declarations[0]);
+                }, idDeclarations[0]);
             }
         };
     }
diff --git a/lib/util/traverser.js b/lib/util/traverser.js
index d5710bb8a..fc070186b 100644
--- a/lib/util/traverser.js
+++ b/lib/util/traverser.js
@@ -14,41 +14,32 @@ const estraverse = require("estraverse");
 // Helpers
 //------------------------------------------------------------------------------
 
-const KEY_BLACKLIST = [
+const KEY_BLACKLIST = new Set([
     "parent",
     "leadingComments",
     "trailingComments"
-];
+]);
 
 /**
  * Wrapper around an estraverse controller that ensures the correct keys
  * are visited.
  * @constructor
  */
-function Traverser() {
-
-    const controller = Object.create(new estraverse.Controller()),
-        originalTraverse = controller.traverse;
-
-    // intercept call to traverse() and add the fallback key to the visitor
-    controller.traverse = function(node, visitor) {
+class Traverser extends estraverse.Controller {
+    traverse(node, visitor) {
         visitor.fallback = Traverser.getKeys;
-        return originalTraverse.call(this, node, visitor);
-    };
-
-    return controller;
+        return super.traverse(node, visitor);
+    }
+
+    /**
+     * Calculates the keys to use for traversal.
+     * @param {ASTNode} node The node to read keys from.
+     * @returns {string[]} An array of keys to visit on the node.
+     * @private
+     */
+    static getKeys(node) {
+        return Object.keys(node).filter(key => !KEY_BLACKLIST.has(key));
+    }
 }
 
-/**
- * Calculates the keys to use for traversal.
- * @param {ASTNode} node The node to read keys from.
- * @returns {string[]} An array of keys to visit on the node.
- * @private
- */
-Traverser.getKeys = function(node) {
-    return Object.keys(node).filter(key => KEY_BLACKLIST.indexOf(key) === -1);
-};
-
 module.exports = Traverser;
-
-


Test patch:
diff --git a/tests/lib/formatters/stylish.js b/tests/lib/formatters/stylish.js
index bb63604ec..958693001 100644
--- a/tests/lib/formatters/stylish.js
+++ b/tests/lib/formatters/stylish.js
@@ -59,7 +59,9 @@ describe("formatter:stylish", () => {
     describe("when passed no messages", () => {
         const code = [{
             filePath: "foo.js",
-            messages: []
+            messages: [],
+            errorCount: 0,
+            warningCount: 0
         }];
 
         it("should not return message", () => {
@@ -74,6 +76,8 @@ describe("formatter:stylish", () => {
     describe("when passed a single message", () => {
         const code = [{
             filePath: "foo.js",
+            errorCount: 1,
+            warningCount: 0,
             messages: [{
                 message: "Unexpected foo.",
                 severity: 2,
@@ -93,6 +97,9 @@ describe("formatter:stylish", () => {
 
         it("should return a string in the correct format for warnings", () => {
             code[0].messages[0].severity = 1;
+            code[0].errorCount = 0;
+            code[0].warningCount = 1;
+
             const result = formatter(code);
 
             assert.equal(result, "\nfoo.js\n  5:10  warning  Unexpected foo  foo\n\n\u2716 1 problem (0 errors, 1 warning)\n");
@@ -104,6 +111,8 @@ describe("formatter:stylish", () => {
     describe("when passed a fatal error message", () => {
         const code = [{
             filePath: "foo.js",
+            errorCount: 1,
+            warningCount: 0,
             messages: [{
                 fatal: true,
                 message: "Unexpected foo.",
@@ -125,6 +134,8 @@ describe("formatter:stylish", () => {
     describe("when passed multiple messages", () => {
         const code = [{
             filePath: "foo.js",
+            errorCount: 1,
+            warningCount: 1,
             messages: [{
                 message: "Unexpected foo.",
                 severity: 2,
@@ -152,6 +163,8 @@ describe("formatter:stylish", () => {
     describe("when passed multiple files with 1 message each", () => {
         const code = [{
             filePath: "foo.js",
+            errorCount: 1,
+            warningCount: 0,
             messages: [{
                 message: "Unexpected foo.",
                 severity: 2,
@@ -160,6 +173,8 @@ describe("formatter:stylish", () => {
                 ruleId: "foo"
             }]
         }, {
+            errorCount: 0,
+            warningCount: 1,
             filePath: "bar.js",
             messages: [{
                 message: "Unexpected bar.",
@@ -177,11 +192,39 @@ describe("formatter:stylish", () => {
             assert.equal(chalkStub.yellow.bold.callCount, 0);
             assert.equal(chalkStub.red.bold.callCount, 1);
         });
+
+        it("should add errorCount", () => {
+            code.forEach(c => {
+                c.errorCount = 1;
+                c.warningCount = 0;
+            });
+
+            const result = formatter(code);
+
+            assert.equal(result, "\nfoo.js\n  5:10  error  Unexpected foo  foo\n\nbar.js\n  6:11  warning  Unexpected bar  bar\n\n\u2716 2 problems (2 errors, 0 warnings)\n");
+            assert.equal(chalkStub.yellow.bold.callCount, 0);
+            assert.equal(chalkStub.red.bold.callCount, 1);
+        });
+
+        it("should add warningCount", () => {
+            code.forEach(c => {
+                c.errorCount = 0;
+                c.warningCount = 1;
+            });
+
+            const result = formatter(code);
+
+            assert.equal(result, "\nfoo.js\n  5:10  error  Unexpected foo  foo\n\nbar.js\n  6:11  warning  Unexpected bar  bar\n\n\u2716 2 problems (0 errors, 2 warnings)\n");
+            assert.equal(chalkStub.yellow.bold.callCount, 0);
+            assert.equal(chalkStub.red.bold.callCount, 1);
+        });
     });
 
     describe("when passed one file not found message", () => {
         const code = [{
             filePath: "foo.js",
+            errorCount: 1,
+            warningCount: 0,
             messages: [{
                 fatal: true,
                 message: "Couldn't find foo.js."
diff --git a/tests/lib/rules/func-name-matching.js b/tests/lib/rules/func-name-matching.js
index 5d98cd388..8f0d81425 100644
--- a/tests/lib/rules/func-name-matching.js
+++ b/tests/lib/rules/func-name-matching.js
@@ -117,6 +117,66 @@ ruleTester.run("func-name-matching", rule, {
         {
             code: "({[foo]: function bar() {}})",
             parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[null]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[1]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[true]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[`x`]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[/abc/]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[[1, 2, 3]]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[{x: 1}]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "[] = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({} = function foo() {})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "[a] = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({a} = function foo() {})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var [] = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var {} = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var [a] = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var {a} = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
         }
     ],
     invalid: [
diff --git a/tests/lib/rules/no-useless-escape.js b/tests/lib/rules/no-useless-escape.js
index 8a26683f3..054c16488 100644
--- a/tests/lib/rules/no-useless-escape.js
+++ b/tests/lib/rules/no-useless-escape.js
@@ -269,6 +269,11 @@ ruleTester.run("no-useless-escape", rule, {
             code: "`multiline template\nliteral with useless \\escape`",
             parserOptions: { ecmaVersion: 6 },
             errors: [{ line: 2, column: 22, message: "Unnecessary escape character: \\e.", type: "TemplateElement" }]
+        },
+        {
+            code: "`\\a```",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [{ line: 1, column: 2, message: "Unnecessary escape character: \\a.", type: "TemplateElement" }]
         }
     ]
 });
diff --git a/tests/lib/rules/sort-vars.js b/tests/lib/rules/sort-vars.js
index 3994f52cd..4411c35ef 100644
--- a/tests/lib/rules/sort-vars.js
+++ b/tests/lib/rules/sort-vars.js
@@ -77,7 +77,13 @@ ruleTester.run("sort-vars", rule, {
             "            c;",
             "    }",
             "}"
-        ].join("\n"), env: { es6: true }, parserOptions: { sourceType: "module" } }
+        ].join("\n"), env: { es6: true }, parserOptions: { sourceType: "module" } },
+
+        {
+            code: "var {} = 1, a",
+            options: ignoreCaseArgs,
+            parserOptions: { ecmaVersion: 6 }
+        }
     ],
     invalid: [
         { code: "var b, a", errors: [expectedError] },
@@ -93,6 +99,13 @@ ruleTester.run("sort-vars", rule, {
         { code: "var d, a, [b, c] = {};", options: ignoreCaseArgs,
             parserOptions: { ecmaVersion: 6 }, errors: [expectedError] },
         { code: "var d, a, [b, {x: {c, e}}] = {};", options: ignoreCaseArgs,
-            parserOptions: { ecmaVersion: 6 }, errors: [expectedError] }
+            parserOptions: { ecmaVersion: 6 }, errors: [expectedError] },
+
+        {
+            code: "var {} = 1, b, a",
+            options: ignoreCaseArgs,
+            parserOptions: { ecmaVersion: 6 },
+            errors: [expectedError]
+        }
     ]
 });
diff --git a/tests/lib/util/traverser.js b/tests/lib/util/traverser.js
new file mode 100644
index 000000000..68be3977a
--- /dev/null
+++ b/tests/lib/util/traverser.js
@@ -0,0 +1,43 @@
+"use strict";
+
+const assert = require("chai").assert;
+const Traverser = require("../../../lib/util/traverser");
+
+describe("Traverser", () => {
+    it("traverses all keys except 'parent', 'leadingComments', and 'trailingComments'", () => {
+        const traverser = new Traverser();
+        const fakeAst = {
+            type: "Program",
+            body: [
+                {
+                    type: "ExpressionStatement",
+                    leadingComments: {
+                        type: "Line"
+                    },
+                    trailingComments: {
+                        type: "Block"
+                    }
+                },
+                {
+                    type: "FooStatement",
+                    foo: {
+                        type: "BarStatement"
+                    }
+                }
+            ]
+        };
+
+        fakeAst.body[0].parent = fakeAst;
+
+        const enteredNodes = [];
+        const exitedNodes = [];
+
+        traverser.traverse(fakeAst, {
+            enter: node => enteredNodes.push(node),
+            leave: node => exitedNodes.push(node)
+        });
+
+        assert.deepEqual(enteredNodes, [fakeAst, fakeAst.body[0], fakeAst.body[1], fakeAst.body[1].foo]);
+        assert.deepEqual(exitedNodes, [fakeAst.body[0], fakeAst.body[1].foo, fakeAst.body[1], fakeAst]);
+    });
+});