From 2657846b2fae3413a1b57d910d51c77c84d6f76c Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 8 Sep 2016 02:30:53 +0900 Subject: [PATCH] Fix: `no-console` ignores user-defined console (fixes #7010) (#7058) --- lib/rules/no-console.js | 97 +++++++++++++++++++++++++++++------ tests/lib/rules/no-console.js | 10 +++- 2 files changed, 89 insertions(+), 18 deletions(-) diff --git a/lib/rules/no-console.js b/lib/rules/no-console.js index 2fe0056637e..9131a49a01a 100644 --- a/lib/rules/no-console.js +++ b/lib/rules/no-console.js @@ -5,6 +5,12 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const astUtils = require("../ast-utils"); + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -36,30 +42,89 @@ module.exports = { }, create(context) { + const options = context.options[0] || {}; + const allowed = options.allow || []; - return { + /** + * Checks whether the given reference is 'console' or not. + * + * @param {escope.Reference} reference - The reference to check. + * @returns {boolean} `true` if the reference is 'console'. + */ + function isConsole(reference) { + const id = reference.identifier; - MemberExpression(node) { + return id && id.name === "console"; + } - if (node.object.name === "console") { - let blockConsole = true; + /** + * Checks whether the property name of the given MemberExpression node + * is allowed by options or not. + * + * @param {ASTNode} node - The MemberExpression node to check. + * @returns {boolean} `true` if the property name of the node is allowed. + */ + function isAllowed(node) { + const propertyName = astUtils.getStaticPropertyName(node); - if (context.options.length > 0) { - const allowedProperties = context.options[0].allow; - const passedProperty = node.property.name; - const propertyIsAllowed = (allowedProperties.indexOf(passedProperty) > -1); + return propertyName && allowed.indexOf(propertyName) !== -1; + } - if (propertyIsAllowed) { - blockConsole = false; - } - } + /** + * Checks whether the given reference is a member access which is not + * allowed by options or not. + * + * @param {escope.Reference} reference - The reference to check. + * @returns {boolean} `true` if the reference is a member access which + * is not allowed by options. + */ + function isMemberAccessExceptAllowed(reference) { + const node = reference.identifier; + const parent = node.parent; - if (blockConsole) { - context.report(node, "Unexpected console statement."); - } + return ( + parent.type === "MemberExpression" && + parent.object === node && + !isAllowed(parent) + ); + } + + /** + * Reports the given reference as a violation. + * + * @param {escope.Reference} reference - The reference to report. + * @returns {void} + */ + function report(reference) { + const node = reference.identifier.parent; + + context.report({ + node, + loc: node.loc, + message: "Unexpected console statement." + }); + } + + return { + "Program:exit"() { + const scope = context.getScope(); + const consoleVar = astUtils.getVariableByName(scope, "console"); + const shadowed = consoleVar && consoleVar.defs.length > 0; + + /* 'scope.through' includes all references to undefined + * variables. If the variable 'console' is not defined, it uses + * 'scope.through'. + */ + const references = consoleVar + ? consoleVar.references + : scope.through.filter(isConsole); + + if (!shadowed) { + references + .filter(isMemberAccessExceptAllowed) + .forEach(report); } } }; - } }; diff --git a/tests/lib/rules/no-console.js b/tests/lib/rules/no-console.js index 540ff361e24..2a88b5e5844 100644 --- a/tests/lib/rules/no-console.js +++ b/tests/lib/rules/no-console.js @@ -32,7 +32,10 @@ ruleTester.run("no-console", rule, { { code: "console.info(foo)", options: [ { allow: ["warn", "info"] } ] }, { code: "console.warn(foo)", options: [ { allow: ["error", "warn"] } ] }, { code: "console.error(foo)", options: [ { allow: ["log", "error"] } ] }, - { code: "console.log(foo)", options: [ { allow: ["info", "log", "warn"] } ] } + { code: "console.log(foo)", options: [ { allow: ["info", "log", "warn"] } ] }, + + // https://github.com/eslint/eslint/issues/7010 + "var console = require('myconsole'); console.log(foo)", ], invalid: [ @@ -52,6 +55,9 @@ ruleTester.run("no-console", rule, { { code: "console.log(foo)", options: [ { allow: ["warn", "info"] } ], errors: [{ message: "Unexpected console statement.", type: "MemberExpression"}] }, { code: "console.error(foo)", options: [ { allow: ["warn", "info", "log"] } ], errors: [{ message: "Unexpected console statement.", type: "MemberExpression"}] }, { code: "console.info(foo)", options: [ { allow: ["warn", "error", "log"] } ], errors: [{ message: "Unexpected console statement.", type: "MemberExpression"}] }, - { code: "console.warn(foo)", options: [ { allow: ["info", "log"] } ], errors: [{ message: "Unexpected console statement.", type: "MemberExpression"}] } + { code: "console.warn(foo)", options: [ { allow: ["info", "log"] } ], errors: [{ message: "Unexpected console statement.", type: "MemberExpression"}] }, + + // In case that implicit global variable of 'console' exists + { code: "console.log(foo)", env: {node: true}, errors: [{ message: "Unexpected console statement.", type: "MemberExpression"}] }, ] });