Repository: jshint/jshint
Pull request #2188 (2015-02-23 04:30:15)

[[FIX]] Permit "eval" as object key
Makes it possible to use "eval" as a key in an object. Previously this
would have emitted a "W061" warning. "eval" used as the key for a
presumed global ("window" in "browser:true" and "global" in
"node:true") will still emit the warning.

Fixes https://github.com/jshint/jshint/issues/738

Adds unit tests for permitting using "eval" as object key

coveralls: 
[![Coverage Status](https://coveralls.io/builds/1976204/badge)](https://coveralls.io/builds/1976204)

Coverage increased (+0.02%) to 95.52% when pulling **5b6f804bb973e34ba0873cc36baa6d903386331c on mbildner:eval-key-not-harmful** into **69b318788de03623b5750079dad449f8fceddf9e on jshint:master**.


Patch:
diff --git a/src/jshint.js b/src/jshint.js
index 43d29d42..b1df8e68 100644
--- a/src/jshint.js
+++ b/src/jshint.js
@@ -1242,6 +1242,27 @@ var JSHINT = (function() {
     return false;
   }
 
+  function isGlobalEval(left, state, funct) {
+    var isGlobal = false;
+
+    // permit methods to refer to an "eval" key in their own context
+    if (left.type === "this" && funct["(context)"] === null) {
+      isGlobal = true;
+    }
+    // permit use of "eval" members of objects
+    else if (left.type === "(identifier)") {
+      if (state.option.node && left.value === "global") {
+        isGlobal = true;
+      }
+
+      else if (state.option.browser && (left.value === "window" || left.value === "document")) {
+        isGlobal = true;
+      }
+    }
+
+    return isGlobal;
+  }
+
   function findNativePrototype(left) {
     var natives = [
       "Array", "ArrayBuffer", "Boolean", "Collator", "DataView", "Date",
@@ -2396,7 +2417,9 @@ var JSHINT = (function() {
     }
 
     if (!state.option.evil && (m === "eval" || m === "execScript")) {
-      warning("W061");
+      if (isGlobalEval(left, state, funct)) {
+        warning("W061");
+      }
     }
 
     return that;
@@ -2588,7 +2611,9 @@ var JSHINT = (function() {
     var e = expression(10), s;
     if (e && e.type === "(string)") {
       if (!state.option.evil && (e.value === "eval" || e.value === "execScript")) {
-        warning("W061", that);
+        if (isGlobalEval(left, state, funct)) {
+          warning("W061");
+        }
       }
 
       countMember(e.value);


Test patch:
diff --git a/tests/unit/core.js b/tests/unit/core.js
index 5d12e41f..83b8fc2b 100644
--- a/tests/unit/core.js
+++ b/tests/unit/core.js
@@ -1139,3 +1139,34 @@ exports.testUnCleanedForinifcheckneeded = function (test) {
 
   test.done();
 };
+
+// gh-738 "eval" as an object key should not cause `W061` warnngs
+exports.testPermitEvalAsKey = function (test) {
+  var srcNode = fs.readFileSync(__dirname + "/fixtures/gh-738-node.js", "utf8");
+  var srcBrowser = fs.readFileSync(__dirname + "/fixtures/gh-738-browser.js", "utf8");
+  // global calls to eval should still cause warning.
+  // test a mixture of permitted and disallowed calls
+  // `global#eval` in `node:true` should still cause warning
+  // `(document|window)#eval` in `browser:true` should still cause warning
+
+  // browser globals
+  TestRun(test)
+  .addError(17, "eval can be harmful.")
+  .addError(19, "eval can be harmful.")
+  .addError(20, "eval can be harmful.")
+  .addError(22, "eval can be harmful.")
+  .addError(23, "eval can be harmful.")
+  .addError(25, "eval can be harmful.")
+  .test(srcBrowser, { browser: true });
+
+  // node globals
+  TestRun(test)
+  .addError(18, "eval can be harmful.")
+  .addError(19, "eval can be harmful.")
+  .addError(20, "eval can be harmful.")
+  .addError(22, "eval can be harmful.")
+  .test(srcNode, { node: true });
+
+  test.done();
+
+};
diff --git a/tests/unit/fixtures/gh-738-browser.js b/tests/unit/fixtures/gh-738-browser.js
new file mode 100644
index 00000000..1db10df3
--- /dev/null
+++ b/tests/unit/fixtures/gh-738-browser.js
@@ -0,0 +1,25 @@
+// object with "eval" key
+var obj = {
+	eval: function (str) {
+		return str;
+	},
+	wrapper: function (str) {
+		// method calling "eval" key from context
+		// permitted use
+		return this.eval(str);
+	}
+};
+
+// object-key use, permitted
+obj["eval"]("console.log('hello world');");
+obj.eval("console.log('hello world');");
+
+eval("console.log('hello world');");
+
+window.eval("4+2");
+window["eval"]("4+2");
+
+document.eval("4+2");
+document["eval"]("4+2");
+
+this.eval("2+2");
diff --git a/tests/unit/fixtures/gh-738-node.js b/tests/unit/fixtures/gh-738-node.js
new file mode 100644
index 00000000..d749e27e
--- /dev/null
+++ b/tests/unit/fixtures/gh-738-node.js
@@ -0,0 +1,22 @@
+// object with "eval" key
+var obj = {
+	eval: function (str) {
+		return str;
+	},
+	wrapper: function (str) {
+		// method calling "eval" key from context
+		// permitted use
+		return this.eval(str);
+	}
+};
+
+// object-key use, permitted
+obj["eval"]("console.log('hello world');");
+obj.eval("console.log('hello world');");
+
+// global use, forbidden
+global["eval"]("console.log('hello world');");
+global.eval("1+1");
+eval("console.log('hello world');");
+
+this.eval("2+2");