From 55043c702a38be2e22c060e386910847bd933bae Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 8 Jan 2020 05:02:48 +0900 Subject: [PATCH 1/9] Fix: consider env in no-implied-eval (fixes #12733) --- lib/rules/no-implied-eval.js | 188 ++++++++++++++--------------- tests/lib/rules/no-implied-eval.js | 112 ++++++++++++++++- 2 files changed, 199 insertions(+), 101 deletions(-) diff --git a/lib/rules/no-implied-eval.js b/lib/rules/no-implied-eval.js index e0764d8223d..1c80f012716 100644 --- a/lib/rules/no-implied-eval.js +++ b/lib/rules/no-implied-eval.js @@ -5,6 +5,12 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const astUtils = require("./utils/ast-utils"); + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -24,91 +30,108 @@ module.exports = { }, create(context) { - const CALLEE_RE = /^(setTimeout|setInterval|execScript)$/u; + const EVAL_LIKE_FUNCS = Object.freeze(["setTimeout", "execScript", "setInterval"]); + const GLOBAL_CANDIDATES = Object.freeze(["global", "window"]); - /* - * Figures out if we should inspect a given binary expression. Is a stack - * of stacks, where the first element in each substack is a CallExpression. + /** + * Checks whether a node is evaluated as a string or not. + * @param {ASTNode} node A node to check. + * @returns {boolean} True if the node is evaluated as a string. */ - const impliedEvalAncestorsStack = []; - - //-------------------------------------------------------------------------- - // Helpers - //-------------------------------------------------------------------------- + function isEvaluatedSting(node) { + if ( + (node.type === "Literal" && typeof node.value === "string") || + node.type === "TemplateLiteral" + ) { + return true; + } + if (node.type === "BinaryExpression" && node.operator === "+") { + return isEvaluatedSting(node.left) || isEvaluatedSting(node.right); + } + return false; + } /** - * Get the last element of an array, without modifying arr, like pop(), but non-destructive. - * @param {Array} arr What to inspect - * @returns {*} The last element of arr - * @private + * Checks whether a node is an Identifier node named one of the specified names. + * @param {ASTNode} node A node to check. + * @param {string[]} specifiers Array of specified name. + * @returns {boolean} True if the node is a Identifier node which has specified name. */ - function last(arr) { - return arr ? arr[arr.length - 1] : null; + function isSpecifiedIdentifier(node, specifiers) { + return node.type === "Identifier" && specifiers.includes(node.name); } /** - * Checks if the given MemberExpression node is a potentially implied eval identifier on window. - * @param {ASTNode} node The MemberExpression node to check. - * @returns {boolean} Whether or not the given node is potentially an implied eval. - * @private + * Checks whether a node is a Literal node which has one of the specified values. + * @param {ASTNode} node A node to check. + * @param {string[]} specifiers Array of specified value. + * @returns {boolean} True if the node is a Literal node which has one of the specified values. */ - function isImpliedEvalMemberExpression(node) { - const object = node.object, - property = node.property, - hasImpliedEvalName = CALLEE_RE.test(property.name) || CALLEE_RE.test(property.value); - - return object.name === "window" && hasImpliedEvalName; + function isSpecifiedConstant(node, specifiers) { + switch (node.type) { + case "Literal": + return specifiers.includes(node.value); + + case "TemplateLiteral": + return ( + node.expressions.length === 0 && + specifiers.includes(node.quasis[0].value.cooked) + ); + + default: + return false; + } } /** - * Determines if a node represents a call to a potentially implied eval. - * - * This checks the callee name and that there's an argument, but not the type of the argument. - * @param {ASTNode} node The CallExpression to check. - * @returns {boolean} True if the node matches, false if not. - * @private + * Checks a given node is a MemberExpression node which has the specified name's + * property. + * @param {ASTNode} node A node to check. + * @param {string[]} specifiers Array of specified name. + * @returns {boolean} `true` if the node is a MemberExpression node which has + * the specified name's property */ - function isImpliedEvalCallExpression(node) { - const isMemberExpression = (node.callee.type === "MemberExpression"), - isIdentifier = (node.callee.type === "Identifier"), - isImpliedEvalCallee = - (isIdentifier && CALLEE_RE.test(node.callee.name)) || - (isMemberExpression && isImpliedEvalMemberExpression(node.callee)); - - return isImpliedEvalCallee && node.arguments.length; + function isSpecifiedMember(node, specifiers) { + return ( + node.type === "MemberExpression" && + (node.computed ? isSpecifiedConstant : isSpecifiedIdentifier)(node.property, specifiers) + ); } /** - * Checks that the parent is a direct descendent of an potential implied eval CallExpression, and if the parent is a CallExpression, that we're the first argument. - * @param {ASTNode} node The node to inspect the parent of. - * @returns {boolean} Was the parent a direct descendent, and is the child therefore potentially part of a dangerous argument? - * @private + * Reports if the `CallExpression` node has evaluated argument. + * @param {ASTNode} node A CallExpression to check. + * @returns {void} */ - function hasImpliedEvalParent(node) { - - // make sure our parent is marked - return node.parent === last(last(impliedEvalAncestorsStack)) && + function reportImpliedEvalCallExpression(node) { + const [firstArgument] = node.arguments; - // if our parent is a CallExpression, make sure we're the first argument - (node.parent.type !== "CallExpression" || node === node.parent.arguments[0]); + if (firstArgument && isEvaluatedSting(firstArgument)) { + context.report({ node, message: "Implied eval. Consider passing a function instead of a string." }); + } } /** - * Checks if our parent is marked as part of an implied eval argument. If - * so, collapses the top of impliedEvalAncestorsStack and reports on the - * original CallExpression. - * @param {ASTNode} node The CallExpression to check. - * @returns {boolean} True if the node matches, false if not. - * @private + * Reports calls of `implied eval` via the global references. + * @param {Variable} globalVar A global variable to check. + * @returns {void} */ - function checkString(node) { - if (hasImpliedEvalParent(node)) { + function reportImpliedEvalViaGlobal(globalVar) { + const { references, name } = globalVar; - // remove the entire substack, to avoid duplicate reports - const substack = impliedEvalAncestorsStack.pop(); + references.forEach(ref => { + const identifier = ref.identifier; + let node = identifier.parent; - context.report({ node: substack[0], message: "Implied eval. Consider passing a function instead of a string." }); - } + while (isSpecifiedMember(node, [name])) { + node = node.parent; + } + + if (isSpecifiedMember(node, EVAL_LIKE_FUNCS)) { + node = node.parent; + reportImpliedEvalCallExpression(node); + } + }); } //-------------------------------------------------------------------------- @@ -117,45 +140,18 @@ module.exports = { return { CallExpression(node) { - if (isImpliedEvalCallExpression(node)) { - - // call expressions create a new substack - impliedEvalAncestorsStack.push([node]); - } - }, - - "CallExpression:exit"(node) { - if (node === last(last(impliedEvalAncestorsStack))) { - - /* - * Destroys the entire sub-stack, rather than just using - * last(impliedEvalAncestorsStack).pop(), as a CallExpression is - * always the bottom of a impliedEvalAncestorsStack substack. - */ - impliedEvalAncestorsStack.pop(); + if (isSpecifiedIdentifier(node.callee, EVAL_LIKE_FUNCS)) { + reportImpliedEvalCallExpression(node); } }, + "Program:exit"() { + const globalScope = context.getScope(); - BinaryExpression(node) { - if (node.operator === "+" && hasImpliedEvalParent(node)) { - last(impliedEvalAncestorsStack).push(node); - } - }, - - "BinaryExpression:exit"(node) { - if (node === last(last(impliedEvalAncestorsStack))) { - last(impliedEvalAncestorsStack).pop(); - } - }, - - Literal(node) { - if (typeof node.value === "string") { - checkString(node); - } - }, + const globalVariables = GLOBAL_CANDIDATES + .map(candidate => astUtils.getVariableByName(globalScope, candidate)) + .filter(globalVar => !!globalVar); - TemplateLiteral(node) { - checkString(node); + globalVariables.forEach(reportImpliedEvalViaGlobal); } }; diff --git a/tests/lib/rules/no-implied-eval.js b/tests/lib/rules/no-implied-eval.js index 68166f16a22..be9decbd032 100644 --- a/tests/lib/rules/no-implied-eval.js +++ b/tests/lib/rules/no-implied-eval.js @@ -23,14 +23,49 @@ const ruleTester = new RuleTester(), ruleTester.run("no-implied-eval", rule, { valid: [ + "window.setTimeout('foo')", + "window.setInterval('foo')", + "window['setTimeout']('foo')", + "window['setInterval']('foo')", + + { code: "window.setTimeout('foo')", env: { node: true } }, + { code: "window.setInterval('foo')", env: { node: true } }, + { code: "window['setTimeout']('foo')", env: { node: true } }, + { code: "window['setInterval']('foo')", env: { node: true } }, + { code: "global.setTimeout('foo')", env: { browser: true } }, + { code: "global.setInterval('foo')", env: { browser: true } }, + { code: "global['setTimeout']('foo')", env: { browser: true } }, + { code: "global['setInterval']('foo')", env: { browser: true } }, + + { code: "window[`SetTimeOut`]('foo', 100);", parserOptions: { ecmaVersion: 6 }, env: { browser: true } }, + { code: "global[`SetTimeOut`]('foo', 100);", parserOptions: { ecmaVersion: 6 }, env: { node: true } }, + { code: "global[`setTimeout${foo}`]('foo', 100);", parserOptions: { ecmaVersion: 6 }, env: { browser: true } }, + { code: "global[`setTimeout${foo}`]('foo', 100);", parserOptions: { ecmaVersion: 6 }, env: { node: true } }, + // normal usage - "setInterval(function() { x = 1; }, 100);", + "setTimeout(function() { x = 1; }, 100);", + "setInterval(function() { x = 1; }, 100)", + "execScript(function() { x = 1; }, 100)", + { code: "window.setTimeout(function() { x = 1; }, 100);", env: { browser: true } }, + { code: "window.setInterval(function() { x = 1; }, 100);", env: { browser: true } }, + { code: "window.execScript(function() { x = 1; }, 100);", env: { browser: true } }, + { code: "window.setTimeout(foo, 100);", env: { browser: true } }, + { code: "window.setInterval(foo, 100);", env: { browser: true } }, + { code: "window.execScript(foo, 100);", env: { browser: true } }, + { code: "global.setTimeout(function() { x = 1; }, 100);", env: { node: true } }, + { code: "global.setInterval(function() { x = 1; }, 100);", env: { node: true } }, + { code: "global.execScript(function() { x = 1; }, 100);", env: { node: true } }, + { code: "global.setTimeout(foo, 100);", env: { node: true } }, + { code: "global.setInterval(foo, 100);", env: { node: true } }, + { code: "global.execScript(foo, 100);", env: { node: true } }, // only checks on top-level statements or window.* "foo.setTimeout('hi')", // identifiers are fine "setTimeout(foo, 10)", + "setInteval(1, 10)", + "execScript(2)", // as are function expressions "setTimeout(function() {}, 10)", @@ -66,19 +101,40 @@ ruleTester.run("no-implied-eval", rule, { { code: "execScript(\"x = 1;\");", errors: [expectedError] }, // member expressions - { code: "window.setTimeout('foo')", errors: [expectedError] }, - { code: "window.setInterval('foo')", errors: [expectedError] }, - { code: "window['setTimeout']('foo')", errors: [expectedError] }, - { code: "window['setInterval']('foo')", errors: [expectedError] }, + { code: "window.setTimeout('foo')", env: { browser: true }, errors: [expectedError] }, + { code: "window.setInterval('foo')", env: { browser: true }, errors: [expectedError] }, + { code: "window['setTimeout']('foo')", env: { browser: true }, errors: [expectedError] }, + { code: "window['setInterval']('foo')", env: { browser: true }, errors: [expectedError] }, + { code: "window[`setInterval`]('foo')", parserOptions: { ecmaVersion: 6 }, env: { browser: true }, errors: [expectedError] }, + { code: "window.window['setInterval']('foo')", env: { browser: true }, errors: [expectedError] }, + { code: "global.setTimeout('foo')", env: { node: true }, errors: [expectedError] }, + { code: "global.setInterval('foo')", env: { node: true }, errors: [expectedError] }, + { code: "global['setTimeout']('foo')", env: { node: true }, errors: [expectedError] }, + { code: "global['setInterval']('foo')", env: { node: true }, errors: [expectedError] }, + { code: "global[`setInterval`]('foo')", parserOptions: { ecmaVersion: 6 }, env: { node: true }, errors: [expectedError] }, + { code: "global.global['setInterval']('foo')", env: { node: true }, errors: [expectedError] }, // template literals { code: "setTimeout(`foo${bar}`)", parserOptions: { ecmaVersion: 6 }, errors: [expectedError] }, + { code: "window.setTimeout(`foo${bar}`)", parserOptions: { ecmaVersion: 6 }, env: { browser: true }, errors: [expectedError] }, + { code: "window.window.setTimeout(`foo${bar}`)", parserOptions: { ecmaVersion: 6 }, env: { browser: true }, errors: [expectedError] }, + { code: "global.global.setTimeout(`foo${bar}`)", parserOptions: { ecmaVersion: 6 }, env: { node: true }, errors: [expectedError] }, // string concatination { code: "setTimeout('foo' + bar)", errors: [expectedError] }, { code: "setTimeout(foo + 'bar')", errors: [expectedError] }, { code: "setTimeout(`foo` + bar)", parserOptions: { ecmaVersion: 6 }, errors: [expectedError] }, { code: "setTimeout(1 + ';' + 1)", errors: [expectedError] }, + { code: "window.setTimeout('foo' + bar)", env: { browser: true }, errors: [expectedError] }, + { code: "window.setTimeout(foo + 'bar')", env: { browser: true }, errors: [expectedError] }, + { code: "window.setTimeout(`foo` + bar)", parserOptions: { ecmaVersion: 6 }, env: { browser: true }, errors: [expectedError] }, + { code: "window.setTimeout(1 + ';' + 1)", env: { browser: true }, errors: [expectedError] }, + { code: "window.window.setTimeout(1 + ';' + 1)", env: { browser: true }, errors: [expectedError] }, + { code: "global.setTimeout('foo' + bar)", env: { node: true }, errors: [expectedError] }, + { code: "global.setTimeout(foo + 'bar')", env: { node: true }, errors: [expectedError] }, + { code: "global.setTimeout(`foo` + bar)", parserOptions: { ecmaVersion: 6 }, env: { node: true }, errors: [expectedError] }, + { code: "global.setTimeout(1 + ';' + 1)", env: { node: true }, errors: [expectedError] }, + { code: "global.global.setTimeout(1 + ';' + 1)", env: { node: true }, errors: [expectedError] }, // gives the correct node when dealing with nesting { @@ -95,6 +151,52 @@ ruleTester.run("no-implied-eval", rule, { line: 1 }, + // no error on line 2 + { + message: expectedErrorMessage, + type: "CallExpression", + line: 3 + } + ] + }, + { + code: + "window.setTimeout('foo' + (function() {\n" + + " setTimeout(helper);\n" + + " window.execScript('str');\n" + + " return 'bar';\n" + + "})())", + env: { browser: true }, + errors: [ + { + message: expectedErrorMessage, + type: "CallExpression", + line: 1 + }, + + // no error on line 2 + { + message: expectedErrorMessage, + type: "CallExpression", + line: 3 + } + ] + }, + { + code: + "global.setTimeout('foo' + (function() {\n" + + " setTimeout(helper);\n" + + " global.execScript('str');\n" + + " return 'bar';\n" + + "})())", + env: { node: true }, + errors: [ + { + message: expectedErrorMessage, + type: "CallExpression", + line: 1 + }, + // no error on line 2 { message: expectedErrorMessage, From 9d6c2608febe09b64bb2f4bfaef10484f8056b39 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 15 Jan 2020 12:41:46 +0900 Subject: [PATCH 2/9] fix typo, refactor to chaining --- lib/rules/no-implied-eval.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/rules/no-implied-eval.js b/lib/rules/no-implied-eval.js index 637538f0bea..594017f32bb 100644 --- a/lib/rules/no-implied-eval.js +++ b/lib/rules/no-implied-eval.js @@ -42,7 +42,7 @@ module.exports = { * @param {ASTNode} node A node to check. * @returns {boolean} True if the node is evaluated as a string. */ - function isEvaluatedSting(node) { + function isEvaluatedString(node) { if ( (node.type === "Literal" && typeof node.value === "string") || node.type === "TemplateLiteral" @@ -50,7 +50,7 @@ module.exports = { return true; } if (node.type === "BinaryExpression" && node.operator === "+") { - return isEvaluatedSting(node.left) || isEvaluatedSting(node.right); + return isEvaluatedString(node.left) || isEvaluatedString(node.right); } return false; } @@ -110,7 +110,7 @@ module.exports = { function reportImpliedEvalCallExpression(node) { const [firstArgument] = node.arguments; - if (firstArgument && isEvaluatedSting(firstArgument)) { + if (firstArgument && isEvaluatedString(firstArgument)) { context.report({ node, messageId: "impliedEval" @@ -154,11 +154,10 @@ module.exports = { "Program:exit"() { const globalScope = context.getScope(); - const globalVariables = GLOBAL_CANDIDATES + GLOBAL_CANDIDATES .map(candidate => astUtils.getVariableByName(globalScope, candidate)) - .filter(globalVar => !!globalVar); - - globalVariables.forEach(reportImpliedEvalViaGlobal); + .filter(globalVar => !!globalVar) + .forEach(reportImpliedEvalViaGlobal); } }; From e6a85c2af30d6d849a15b21062ec9aea89dc88cb Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 12 Feb 2020 19:47:30 +0900 Subject: [PATCH 3/9] change to check only CallExpression, refactoring --- lib/rules/no-implied-eval.js | 31 +++++------------------------- tests/lib/rules/no-implied-eval.js | 10 ++++++++++ 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/lib/rules/no-implied-eval.js b/lib/rules/no-implied-eval.js index 637538f0bea..5090b2fb12b 100644 --- a/lib/rules/no-implied-eval.js +++ b/lib/rules/no-implied-eval.js @@ -65,28 +65,6 @@ module.exports = { return node.type === "Identifier" && specifiers.includes(node.name); } - /** - * Checks whether a node is a Literal node which has one of the specified values. - * @param {ASTNode} node A node to check. - * @param {string[]} specifiers Array of specified value. - * @returns {boolean} True if the node is a Literal node which has one of the specified values. - */ - function isSpecifiedConstant(node, specifiers) { - switch (node.type) { - case "Literal": - return specifiers.includes(node.value); - - case "TemplateLiteral": - return ( - node.expressions.length === 0 && - specifiers.includes(node.quasis[0].value.cooked) - ); - - default: - return false; - } - } - /** * Checks a given node is a MemberExpression node which has the specified name's * property. @@ -96,10 +74,7 @@ module.exports = { * the specified name's property */ function isSpecifiedMember(node, specifiers) { - return ( - node.type === "MemberExpression" && - (node.computed ? isSpecifiedConstant : isSpecifiedIdentifier)(node.property, specifiers) - ); + return node.type === "MemberExpression" && specifiers.includes(astUtils.getStaticPropertyName(node)); } /** @@ -108,6 +83,10 @@ module.exports = { * @returns {void} */ function reportImpliedEvalCallExpression(node) { + if (node.type !== "CallExpression") { + return; + } + const [firstArgument] = node.arguments; if (firstArgument && isEvaluatedSting(firstArgument)) { diff --git a/tests/lib/rules/no-implied-eval.js b/tests/lib/rules/no-implied-eval.js index 31f41e9e056..bc52f796782 100644 --- a/tests/lib/rules/no-implied-eval.js +++ b/tests/lib/rules/no-implied-eval.js @@ -21,6 +21,16 @@ const ruleTester = new RuleTester(), ruleTester.run("no-implied-eval", rule, { valid: [ + { code: "setTimeout;", env: { browser: true } }, + { code: "setTimeout = foo;", env: { browser: true } }, + { code: "window.setTimeout;", env: { browser: true } }, + { code: "window.setTimeout = foo;", env: { browser: true } }, + { code: "window['setTimeout'];", env: { browser: true } }, + { code: "window['setTimeout'] = foo;", env: { browser: true } }, + { code: "global.setTimeout;", env: { node: true } }, + { code: "global.setTimeout = foo;", env: { node: true } }, + { code: "global['setTimeout'];", env: { node: true } }, + { code: "global['setTimeout'] = foo;", env: { node: true } }, "window.setTimeout('foo')", "window.setInterval('foo')", From 4fc6223ef5897baab614eb3cb932ef5e609b48d6 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Tue, 18 Feb 2020 23:46:32 +0900 Subject: [PATCH 4/9] check defs --- lib/rules/no-implied-eval.js | 2 +- tests/lib/rules/no-implied-eval.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-implied-eval.js b/lib/rules/no-implied-eval.js index 6e40695b046..7f663f9c647 100644 --- a/lib/rules/no-implied-eval.js +++ b/lib/rules/no-implied-eval.js @@ -135,7 +135,7 @@ module.exports = { GLOBAL_CANDIDATES .map(candidate => astUtils.getVariableByName(globalScope, candidate)) - .filter(globalVar => !!globalVar) + .filter(globalVar => !!globalVar && globalVar.defs.length === 0) .forEach(reportImpliedEvalViaGlobal); } }; diff --git a/tests/lib/rules/no-implied-eval.js b/tests/lib/rules/no-implied-eval.js index bc52f796782..a9305995872 100644 --- a/tests/lib/rules/no-implied-eval.js +++ b/tests/lib/rules/no-implied-eval.js @@ -100,7 +100,10 @@ ruleTester.run("no-implied-eval", rule, { "setTimeout(function() { return 'foobar'; }, 10)", // https://github.com/eslint/eslint/issues/7821 - "setTimeoutFooBar('Foo Bar')" + "setTimeoutFooBar('Foo Bar')", + + { code: "var window; window.setTimeout('foo', 100);", env: { browser: true } }, + { code: "var global; global.setTimeout('foo', 100);", env: { node: true } } ], invalid: [ From ada0c42b33bfb42570d446832b83ef0231f60ca9 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 19 Feb 2020 21:36:31 +0900 Subject: [PATCH 5/9] checks callee --- lib/rules/no-implied-eval.js | 11 +++++------ tests/lib/rules/no-implied-eval.js | 8 +++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/rules/no-implied-eval.js b/lib/rules/no-implied-eval.js index 7f663f9c647..561dde4f9e9 100644 --- a/lib/rules/no-implied-eval.js +++ b/lib/rules/no-implied-eval.js @@ -83,10 +83,6 @@ module.exports = { * @returns {void} */ function reportImpliedEvalCallExpression(node) { - if (node.type !== "CallExpression") { - return; - } - const [firstArgument] = node.arguments; if (firstArgument && isEvaluatedString(firstArgument)) { @@ -114,8 +110,11 @@ module.exports = { } if (isSpecifiedMember(node, EVAL_LIKE_FUNCS)) { - node = node.parent; - reportImpliedEvalCallExpression(node); + const parent = node.parent; + + if (parent.type === "CallExpression" && parent.callee === node) { + reportImpliedEvalCallExpression(parent); + } } }); } diff --git a/tests/lib/rules/no-implied-eval.js b/tests/lib/rules/no-implied-eval.js index a9305995872..d4a334aa5e3 100644 --- a/tests/lib/rules/no-implied-eval.js +++ b/tests/lib/rules/no-implied-eval.js @@ -102,8 +102,14 @@ ruleTester.run("no-implied-eval", rule, { // https://github.com/eslint/eslint/issues/7821 "setTimeoutFooBar('Foo Bar')", + { code: "foo.window.setTimeout('foo', 100);", env: { browser: true } }, + { code: "foo.global.setTimeout('foo', 100);", env: { node: true } }, { code: "var window; window.setTimeout('foo', 100);", env: { browser: true } }, - { code: "var global; global.setTimeout('foo', 100);", env: { node: true } } + { code: "var global; global.setTimeout('foo', 100);", env: { node: true } }, + { code: "function foo(window) { window.setTimeout('foo', 100); }", env: { browser: true } }, + { code: "function foo(global) { global.setTimeout('foo', 100); }", env: { node: true } }, + { code: "foo('', window.setTimeout);", env: { browser: true } }, + { code: "foo('', global.setTimeout);", env: { node: true } } ], invalid: [ From 8776e5999f78f609ad784f0ab21b74aa9044f91c Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Tue, 3 Mar 2020 20:57:47 +0900 Subject: [PATCH 6/9] check static string value --- lib/rules/no-implied-eval.js | 7 ++++++- tests/lib/rules/no-implied-eval.js | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-implied-eval.js b/lib/rules/no-implied-eval.js index 561dde4f9e9..8d678ed8ff7 100644 --- a/lib/rules/no-implied-eval.js +++ b/lib/rules/no-implied-eval.js @@ -10,6 +10,7 @@ //------------------------------------------------------------------------------ const astUtils = require("./utils/ast-utils"); +const { getStaticValue } = require("eslint-utils"); //------------------------------------------------------------------------------ // Rule Definition @@ -85,7 +86,11 @@ module.exports = { function reportImpliedEvalCallExpression(node) { const [firstArgument] = node.arguments; - if (firstArgument && isEvaluatedString(firstArgument)) { + const staticValue = getStaticValue(firstArgument, context.getScope()); + const isStaticString = staticValue && typeof staticValue.value === "string"; + const isString = isStaticString || isEvaluatedString(firstArgument); + + if (firstArgument && isString) { context.report({ node, messageId: "impliedEval" diff --git a/tests/lib/rules/no-implied-eval.js b/tests/lib/rules/no-implied-eval.js index d4a334aa5e3..35299d9bc5a 100644 --- a/tests/lib/rules/no-implied-eval.js +++ b/tests/lib/rules/no-implied-eval.js @@ -118,6 +118,9 @@ ruleTester.run("no-implied-eval", rule, { { code: "setInterval(\"x = 1;\");", errors: [expectedError] }, { code: "execScript(\"x = 1;\");", errors: [expectedError] }, + { code: "const s = 'x=1'; setTimeout(s, 100);", parserOptions: { ecmaVersion: 6 }, errors: [expectedError] }, + { code: "setTimeout(String('x=1'), 100);", parserOptions: { ecmaVersion: 6 }, errors: [expectedError] }, + // member expressions { code: "window.setTimeout('foo')", env: { browser: true }, errors: [expectedError] }, { code: "window.setInterval('foo')", env: { browser: true }, errors: [expectedError] }, From cbd34aad5e8f2ad6ffbc1fe97040b74a5a4a8e0b Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 5 Mar 2020 01:44:18 +0900 Subject: [PATCH 7/9] check falsy - first argument --- lib/rules/no-implied-eval.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/no-implied-eval.js b/lib/rules/no-implied-eval.js index 8d678ed8ff7..7fd37830b8b 100644 --- a/lib/rules/no-implied-eval.js +++ b/lib/rules/no-implied-eval.js @@ -86,7 +86,7 @@ module.exports = { function reportImpliedEvalCallExpression(node) { const [firstArgument] = node.arguments; - const staticValue = getStaticValue(firstArgument, context.getScope()); + const staticValue = firstArgument && getStaticValue(firstArgument, context.getScope()); const isStaticString = staticValue && typeof staticValue.value === "string"; const isString = isStaticString || isEvaluatedString(firstArgument); From 37d37d60a812e0fa8038c8fadbaab5a19b6b5e18 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 5 Mar 2020 09:44:30 +0900 Subject: [PATCH 8/9] check empty arg --- lib/rules/no-implied-eval.js | 22 +++++++++++++--------- tests/lib/rules/no-implied-eval.js | 2 ++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/lib/rules/no-implied-eval.js b/lib/rules/no-implied-eval.js index 7fd37830b8b..ea5c6b8636b 100644 --- a/lib/rules/no-implied-eval.js +++ b/lib/rules/no-implied-eval.js @@ -86,16 +86,20 @@ module.exports = { function reportImpliedEvalCallExpression(node) { const [firstArgument] = node.arguments; - const staticValue = firstArgument && getStaticValue(firstArgument, context.getScope()); - const isStaticString = staticValue && typeof staticValue.value === "string"; - const isString = isStaticString || isEvaluatedString(firstArgument); - - if (firstArgument && isString) { - context.report({ - node, - messageId: "impliedEval" - }); + if (firstArgument) { + + const staticValue = getStaticValue(firstArgument, context.getScope()); + const isStaticString = staticValue && typeof staticValue.value === "string"; + const isString = isStaticString || isEvaluatedString(firstArgument); + + if (isString) { + context.report({ + node, + messageId: "impliedEval" + }); + } } + } /** diff --git a/tests/lib/rules/no-implied-eval.js b/tests/lib/rules/no-implied-eval.js index 35299d9bc5a..6b14e74765d 100644 --- a/tests/lib/rules/no-implied-eval.js +++ b/tests/lib/rules/no-implied-eval.js @@ -21,6 +21,8 @@ const ruleTester = new RuleTester(), ruleTester.run("no-implied-eval", rule, { valid: [ + "setTimeout();", + { code: "setTimeout;", env: { browser: true } }, { code: "setTimeout = foo;", env: { browser: true } }, { code: "window.setTimeout;", env: { browser: true } }, From c00cb77d13f7bcb34787376fe1a1031607f9d2de Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Mon, 16 Mar 2020 09:01:58 +0900 Subject: [PATCH 9/9] fix typo --- tests/lib/rules/no-implied-eval.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/rules/no-implied-eval.js b/tests/lib/rules/no-implied-eval.js index 6b14e74765d..227e02b6bca 100644 --- a/tests/lib/rules/no-implied-eval.js +++ b/tests/lib/rules/no-implied-eval.js @@ -75,7 +75,7 @@ ruleTester.run("no-implied-eval", rule, { // identifiers are fine "setTimeout(foo, 10)", - "setInteval(1, 10)", + "setInterval(1, 10)", "execScript(2)", // as are function expressions