Permalink
Browse files

[[FIX]] Do not require global USD for any envs

In web browsers, enabling ES5 "strict mode" at a global level effects
all scripts executed in the same context. For this reason, the `strict`
option was originally implemented to enforce the directive's use in
function scope only.  However: the Browserify, Node.js, and PhantomJS
environments do not suffer from this problem.

Recent improvements to the option's behavior included making JSHint's
interpretation of `strict: true` to be contextual: in those environments
where a global `use strict` directive is "safe," enabling the option
would require the global directive.

Although this change is an improvement (because it encourages more code
to be written in strict mode), it is backwards incompatible. Moreover,
the option's documentation in JSHint 2.8.0 explicitly described the
sub-optimal behavior:

> *Note:* This option enables strict mode for function scope only. It
> *prohibits* the global scoped strict mode because it might break
> third-party widgets on your page. If you really want to use global
> strict mode, see the *globalstrict* option.

For this reason, `strict: true` should continue to trigger the "legacy"
behavior. The change brings that configuration completely in-line with
the recently-introduced `strict: func`, and because `strict: func` has
not been released in a stable version, it can safely be removed.
  • Loading branch information...
jugglinmike committed Jan 1, 2016
1 parent cfe6c43 commit 3fa9ecef7ef1f2f9a40f4d6517b844feb068bc23
Showing with 42 additions and 28 deletions.
  1. +0 −10 src/jshint.js
  2. +5 −4 src/options.js
  3. +0 −4 tests/unit/envs.js
  4. +37 −10 tests/unit/options.js
@@ -248,9 +248,6 @@ var JSHINT = (function() {

if (state.option.phantom) {
combine(predefined, vars.phantom);
if (state.option.strict === true) {
state.option.strict = "global";
}
}

if (state.option.prototypejs) {
@@ -260,9 +257,6 @@ var JSHINT = (function() {
if (state.option.node) {
combine(predefined, vars.node);
combine(predefined, vars.typed);
if (state.option.strict === true) {
state.option.strict = "global";
}
}

if (state.option.devel) {
@@ -282,9 +276,6 @@ var JSHINT = (function() {
combine(predefined, vars.browser);
combine(predefined, vars.typed);
combine(predefined, vars.browserify);
if (state.option.strict === true) {
state.option.strict = "global";
}
}

if (state.option.nonstandard) {
@@ -651,7 +642,6 @@ var JSHINT = (function() {
case "false":
state.option.strict = false;
break;
case "func":
case "global":
case "implied":
state.option.strict = val;
@@ -885,13 +885,14 @@ exports.val = {
* them to produce errors. It also fixes mistakes that made it difficult
* for the JavaScript engines to perform certain optimizations.
*
* - "func" - there must be a `"use strict";` directive at function level
* - "global" - there must be a `"use strict";` directive at global level
* - "implied" - lint the code as if there is the `"use strict";` directive
* - false - disable warnings about strict mode
* - true - same as `"func"`, but environment options have precedence over
* this (e.g. `node`, `module`, `browserify` and `phantomjs` can
* set `strict: global`)
* - true - there must be a `"use strict";` directive at function level;
* this is preferable for scripts intended to be loaded in web
* browsers directly because enabling strict mode globally
* could adversely effect other scripts running on the same
* page
*/
strict : true,

@@ -89,10 +89,6 @@ exports.node = function (test) {
TestRun(test, "gh-2657")
.test("'use strict';var a;", { node: true });

// Implied `strict: global` if `strict` is `true`
JSHINT("", { node: true, strict: true });
test.strictEqual(JSHINT.data().options.strict, "global");

test.done();
};

@@ -1678,12 +1678,10 @@ exports.strict = function (test) {
var run = TestRun(test)
.addError(1, 'Missing "use strict" statement.');
run.test(code, { es3: true, strict: true });
run.test(code, { es3: true, strict: "func" });
run.test(code, { es3: true, strict: "global" });
TestRun(test).test(code, { es3: true, strict: "implied" });

TestRun(test).test(code1, { es3: true, strict: true });
TestRun(test).test(code1, { es3: true, strict: "func" });
TestRun(test).test(code1, { es3: true, strict: "global" });
TestRun(test)
.addError(1, 'Unnecessary directive "use strict".')
@@ -1695,7 +1693,6 @@ exports.strict = function (test) {
.addError(7, 'Strict violation.')
.addError(8, 'Strict violation.');
run.test(src, { es3: true, strict: true });
run.test(src, { es3: true, strict: "func" });
run.test(src, { es3: true, strict: "global" });

run = TestRun(test)
@@ -1709,7 +1706,6 @@ exports.strict = function (test) {
.test(src3, {es3 : true});

TestRun(test).test(code2, { es3: true, strict: true });
TestRun(test).test(code2, { es3: true, strict: "func" });
TestRun(test)
.addError(1, 'Missing "use strict" statement.')
.test(code2, { es3: true, strict: "global" });
@@ -1718,11 +1714,10 @@ exports.strict = function (test) {
run = TestRun(test)
.addError(1, 'Use the function form of "use strict".');
run.test(code3, { strict: true });
run.test(code3, { strict: "func" });
run.addError(1, 'Unnecessary directive "use strict".')
.test(code3, { strict: "implied" });

[ true, false, "global", "func", "implied" ].forEach(function(val) {
[ true, false, "global", "implied" ].forEach(function(val) {
JSHINT("/*jshint strict: " + val + " */");
test.strictEqual(JSHINT.data().options.strict, val);
});
@@ -1734,17 +1729,49 @@ exports.strict = function (test) {
TestRun(test, "environments have precedence over 'strict: true'")
.test(code3, { strict: true, node: true });

TestRun(test, "environments don't have precedence over 'strict: func'")
.addError(1, 'Use the function form of "use strict".')
.test(code3, { strict: "func", node: true });

TestRun(test, "gh-2668")
.addError(1, "Missing \"use strict\" statement.")
.test("a = 2;", { strict: "global" });

test.done();
};

/**
* This test asserts sub-optimal behavior.
*
* In the "browserify", "node" and "phantomjs" environments, user code is not
* executed in the global scope directly. This means that top-level `use
* strict` directives, although seemingly global, do *not* enable ES5 strict
* mode for other scripts executed in the same environment. Because of this,
* the `strict` option should enforce a top-level `use strict` directive in
* those environments.
*
* The `strict` option was implemented without consideration for these
* environments, so the sub-optimal behavior must be preserved for backwards
* compatability.
*
* TODO: Interpret `strict: true` as `strict: global` in the Browserify,
* Node.js, and PhantomJS environments, and remove this test in JSHint 3
*/
exports.strictEnvs = function (test) {
var partialStrict = [
"void 0;",
"(function() { void 0; }());",
"(function() { 'use strict'; void 0; }());"
];
TestRun(test, "")
.addError(2, "Missing \"use strict\" statement.")
.test(partialStrict, { strict: true, browserify: true });
TestRun(test, "")
.addError(2, "Missing \"use strict\" statement.")
.test(partialStrict, { strict: true, node: true });
TestRun(test, "")
.addError(2, "Missing \"use strict\" statement.")
.test(partialStrict, { strict: true, phantom: true });

test.done();
};

/** Option `globalstrict` allows you to use global "use strict"; */
exports.globalstrict = function (test) {
var code = [

0 comments on commit 3fa9ece

Please sign in to comment.