[[FIX]] Correct restriction on function name

The ECMAScript specification details the following early error for
function-defining productions:

> - If the source code matching this production is strict mode code, it
>   is a Syntax Error if BindingIdentifier is present and the
>   StringValue of BindingIdentifier is "eval" or "arguments".

There were two bugs in the original implementation of this error:

- violations from function expressions with empty bodies would not be
  detected
- violations from function declarations contained within strict mode
  code would be reported twice

Correct the first issue by ensuring that the "(isStrict)" property is
accurately set for functions with empty bodies. Correct the second issue
by re-locating the error detection logic to within the processing logic
for expressions and declarations and inserting an additional condition
within the latter.

(This correction does not require modification to the Test262 whitelist
file, although that test suite does offer coverage for this behavior. At
the revision in use by JSHint, the relevant tests are expressed using
`eval` and therefore unusable by parsers. Those tests have since been
reformatted to correct this; they will be incorporated when JSHint next
updates its reference to Test262.)

src/jshint.js

@@ -1764,6 +1764,9 @@ var JSHINT = (function() {
         metrics.statementCount += a.length;
 
         indent -= state.option.indent;
+      } else if (isfunc) {
+        // Ensure property is set for functions with empty bodies.
+        state.funct["(isStrict)"] = state.isStrict();
       }
 
       advance("}", t);
@@ -3055,11 +3058,6 @@ var JSHINT = (function() {
       warning("W124", state.tokens.curr);
     }
 
-    if (state.funct["(isStrict)"] === true &&
-      (name === "arguments" || name === "eval")) {
-      error("E008", token);
-    }
-
     state.funct["(metrics)"].verifyMaxStatementsPerFunction();
     state.funct["(metrics)"].verifyMaxComplexityPerFunction();
     state.funct["(unusedOption)"] = state.option.unused;
@@ -3876,27 +3874,40 @@ var JSHINT = (function() {
     if (inblock) {
       warning("W082", state.tokens.curr);
     }
-    var i = optionalidentifier();
+    var nameToken = optionalidentifier() ? state.tokens.curr : null;
 
-    if (i === undefined) {
+    if (!nameToken) {
       warning("W025");
     } else {
-      state.funct["(scope)"].addlabel(i, {
+      state.funct["(scope)"].addlabel(nameToken.value, {
         type: generator ? "generator function" : "function",
         token: state.tokens.curr,
         initialized: true });
 
       if (inexport) {
-        state.funct["(scope)"].setExported(i, state.tokens.prev);
+        state.funct["(scope)"].setExported(nameToken.value, state.tokens.prev);
       }
     }
 
-    doFunction({
-      name: i,
+    var f = doFunction({
+      name: nameToken && nameToken.value,
       statement: this,
       type: generator ? "generator" : null,
       ignoreLoopFunc: inblock // a declaration may already have warned
     });
+
+    // If the function declaration is strict because the surrounding code is
+    // strict, the invalid name will trigger E008 when the scope manager
+    // attempts to create a binding in the strict environment record. An error
+    // should only be signaled here when the function itself enables strict
+    // mode (the scope manager will not report an error because a declaration
+    // does not introduce a binding into the function's environment record).
+    var enablesStrictMode = f["(isStrict)"] && !state.isStrict();
+    if (nameToken && (f["(name)"] === "arguments" || f["(name)"] === "eval") &&
+      enablesStrictMode) {
+      error("E008", nameToken);
+    }
+
     if (state.tokens.next.id === "(" && state.tokens.next.line === state.tokens.curr.line) {
       error("E039");
     }
@@ -3914,8 +3925,17 @@ var JSHINT = (function() {
       generator = true;
     }
 
-    var i = optionalidentifier();
-    doFunction({ name: i, type: generator ? "generator" : null });
+    var nameToken = optionalidentifier() ? state.tokens.curr : null;
+
+    var f = doFunction({
+      name: nameToken && nameToken.value,
+      type: generator ? "generator" : null
+    });
+
+    if (nameToken && (f["(name)"] === "arguments" || f["(name)"] === "eval") &&
+      f["(isStrict)"]) {
+      error("E008", nameToken);
+    }
     return this;
   });
 

tests/unit/parser.js

@@ -3679,17 +3679,49 @@ exports["strict violation - use of arguments and eval"] = function (test) {
   TestRun(test, "as function declaration binding (invalid)")
     .addError(1, 10, "Strict violation.")
     .addError(2, 10, "Strict violation.")
+    .addError(5, 12, "Strict violation.")
+    .addError(6, 12, "Strict violation.")
+    .addError(10, 12, "Strict violation.")
+    .addError(10, 26, "Unnecessary directive \"use strict\".")
+    .addError(11, 12, "Strict violation.")
+    .addError(11, 21, "Unnecessary directive \"use strict\".")
     .test([
       "function arguments() { 'use strict'; }",
-      "function eval() { 'use strict'; }"
+      "function eval() { 'use strict'; }",
+      "(function() {",
+      "  'use strict';",
+      "  function arguments() {}",
+      "  function eval() {}",
+      "}());",
+      "(function() {",
+      "  'use strict';",
+      "  function arguments() { 'use strict'; }",
+      "  function eval() { 'use strict'; }",
+      "}());"
     ]);
 
   TestRun(test, "as function expression binding (invalid)")
     .addError(1, 15, "Strict violation.")
     .addError(2, 15, "Strict violation.")
+    .addError(5, 17, "Strict violation.")
+    .addError(6, 17, "Strict violation.")
+    .addError(10, 17, "Strict violation.")
+    .addError(10, 31, "Unnecessary directive \"use strict\".")
+    .addError(11, 17, "Strict violation.")
+    .addError(11, 26, "Unnecessary directive \"use strict\".")
     .test([
       "void function arguments() { 'use strict'; };",
-      "void function eval() { 'use strict'; };"
+      "void function eval() { 'use strict'; };",
+      "(function() {",
+      "  'use strict';",
+      "  void function arguments() {};",
+      "  void function eval() {};",
+      "}());",
+      "(function() {",
+      "  'use strict';",
+      "  void function arguments() { 'use strict'; };",
+      "  void function eval() { 'use strict'; };",
+      "}());"
     ]);
 
   test.done();