[[FIX]] Account for implied closures

Some environments wrap code in an immediately-invoked function

expression prior to evaluation. This behavior has an effect on global

`var` declarations. Unlike in typical JavaScript source code, such

statements in these contexts do *not* create new entries in the

environment record, nor do they resolve to references to the global

binding (where defined).



Update the logic that tracks global definitions to account for such

environments, avoiding issuing warning W079 ("Redefinition of '{a}'.")

which is otherwise appropriate.



Note: a far preferable solution would involve updating JSHint's scope

tracking mechanism to create a new entry for this implicit function

scope. Unfortunately the legacy implementation of the environmental

options allowed for usage scenarios that are incompatible with that

approach (specifically: enabling the environment options after the

source text has been partially parsed).

src/jshint.js

@@ -3613,7 +3613,7 @@ var JSHINT = (function() {
       for (var t in tokens) {
         if (tokens.hasOwnProperty(t)) {
           t = tokens[t];
-          if (!implied && state.funct["(global)"]) {
+          if (!implied && state.funct["(global)"] && !state.impliedClosure()) {
             if (predefined[t.id] === false) {
               warning("W079", t.token, t.id);
             } else if (state.option.futurehostile === false) {

src/scope-manager.js

@@ -693,7 +693,7 @@ var scopeManager = function(state, predefined, exported, declared) {
 
         scopeManagerInst.funct.add(labelName, type, token, !isexported);
 
-        if (_currentFunctBody["(type)"] === "global") {
+        if (_currentFunctBody["(type)"] === "global" && !state.impliedClosure()) {
           usedPredefinedAndGlobals[labelName] = marker;
         }
       }

src/state.js

@@ -46,8 +46,17 @@ var state = {
 
   allowsGlobalUsd: function() {
     return this.option.strict === "global" || this.option.globalstrict ||
-      this.option.module || this.option.node || this.option.phantom ||
-      this.option.browserify;
+      this.option.module || this.impliedClosure();
+  },
+
+  /**
+   * Determine if the current configuration describes an environment that is
+   * wrapped in an immediately-invoked function expression prior to evaluation.
+   *
+   * @returns {boolean}
+   */
+  impliedClosure: function() {
+    return this.option.node || this.option.phantom || this.option.browserify;
   },
 
   // Assumption: chronologically ES3 < ES5 < ES6 < Moz

tests/unit/core.js

@@ -98,12 +98,16 @@ exports.testExportedDefinedGlobals = function (test) {
 exports.testGlobalVarDeclarations = function (test) {
   var src = "var a;";
 
-  // Test should pass
-  TestRun(test).test(src, { es3: true, node: true }, {});
+  TestRun(test).test(src, { es3: true }, {});
 
   var report = JSHINT.data();
   test.deepEqual(report.globals, ['a']);
 
+  TestRun(test).test(src, { es3: true, node: true }, {});
+
+  report = JSHINT.data();
+  test.strictEqual(report.globals, undefined);
+
   TestRun(test).test("var __proto__;", { proto: true });
   report = JSHINT.data();
   test.deepEqual(report.globals, ["__proto__"]);

tests/unit/envs.js

@@ -212,3 +212,88 @@ exports.phantom = function (test) {
 
   test.done();
 };
+
+exports.globals = function (test) {
+  var src = [
+    "/* global first */",
+    "var first;"
+  ];
+
+  TestRun(test)
+    .addError(2, "Redefinition of 'first'.")
+    .test(src);
+  TestRun(test)
+    .test(src, { browserify: true });
+  TestRun(test)
+    .test(src, { node: true });
+  TestRun(test)
+    .test(src, { phantom: true });
+
+  TestRun(test, "Late configuration of `browserify`")
+    .test([
+      "/* global first */",
+      "void 0;",
+      "// jshint browserify: true",
+      "var first;"
+    ]);
+
+  TestRun(test)
+    .test([
+      "// jshint browserify: true",
+      "/* global first */",
+      "var first;"
+    ]);
+
+  TestRun(test)
+    .test([
+      "/* global first */",
+      "// jshint browserify: true",
+      "var first;"
+    ]);
+
+  TestRun(test, "Late configuration of `node`")
+    .test([
+      "/* global first */",
+      "void 0;",
+      "// jshint node: true",
+      "var first;"
+    ]);
+
+  TestRun(test)
+    .test([
+      "// jshint node: true",
+      "/* global first */",
+      "var first;"
+    ]);
+
+  TestRun(test)
+    .test([
+      "/* global first */",
+      "// jshint node: true",
+      "var first;"
+    ]);
+
+  TestRun(test, "Late configuration of `phantom`")
+    .test([
+      "/* global first */",
+      "void 0;",
+      "// jshint phantom: true",
+      "var first;"
+    ]);
+
+  TestRun(test)
+    .test([
+      "// jshint phantom: true",
+      "/* global first */",
+      "var first;"
+    ]);
+
+  TestRun(test)
+    .test([
+      "/* global first */",
+      "// jshint phantom: true",
+      "var first;"
+    ]);
+
+  test.done();
+};

tests/unit/options.js

@@ -1769,6 +1769,22 @@ exports.strictEnvs = function (test) {
     .addError(2, "Missing \"use strict\" statement.")
     .test(partialStrict, { strict: true, phantom: true });
 
+  partialStrict = [
+    '(() =>',
+    '  void 0',
+    ')();',
+  ]
+
+  TestRun(test, "Block-less arrow functions in the Browserify env")
+    .addError(3, "Missing \"use strict\" statement.")
+    .test(partialStrict, { esversion: 6, strict: true, browserify: true });
+  TestRun(test, "Block-less arrow function in the Node.js environment")
+    .addError(3, "Missing \"use strict\" statement.")
+    .test(partialStrict, { esversion: 6, strict: true, node: true });
+  TestRun(test, "Block-less arrow function in the PhantomJS environment")
+    .addError(3, "Missing \"use strict\" statement.")
+    .test(partialStrict, { esversion: 6, strict: true, phantom: true });
+
   test.done();
 };