"use strict" in ES3

[mdjermanovic]

Per eslint/eslint#15456, "use strict" directives should not apply in ES3.

eslint-scope currently does apply "use strict" directives in ES3 mode. Given the above, that behavior looks like a bug.

This is observable with no-invalid-this rule, which uses Scope#isStrict.

/* eslint no-invalid-this: "error" */

function foo() {
    "use strict";
    this;
}

This rule aims to report undefined this. Lowercase functions are assumed to be called without this. If the function is strict code, then this is undefined and it should be reported. If the function is non-strict code, then this refers to the global object and therefore it shouldn't be reported.

Online Demo with ES3 - the rule reports error, which seems wrong because this isn't strict code in ES3.

[nzakas]

It seems like there are multiple areas trying to track strict mode.

isStrictModeSupported:

eslint-scope/lib/scope-manager.js

Line 80 in d756f1e

isStrictModeSupported() {

useDirective:

eslint-scope/lib/scope-manager.js

Line 56 in d756f1e

__useDirective() {

isStrictScope:

eslint-scope/lib/scope.js

Line 45 in 22a55c0

function isStrictScope(scope, block, isMethodDefinition, useDirective) {

Can you narrow down where the problem is coming from? I’m guessing from useDirective, which doesn’t seem necessary as a separate option from ecmaVersion.

[mdjermanovic]

__useDirective() returns the value of the configuration option directive.

(by "configuration option" I mean an option passed to analyze() on public API)

It doesn't control whether or not "use strict" directives will be processed and applied. It controls only how will eslint-scope technically look for "use strict" directives in given AST.

When the option is true, eslint-scope looks for DirectiveStatement nodes:

eslint-scope/lib/scope.js

Lines 86 to 97 in ceb8bdd

	if (useDirective) {
	for (let i = 0, iz = body.body.length; i < iz; ++i) {
	const stmt = body.body[i];
	if (stmt.type !== Syntax.DirectiveStatement) {
	break;
	}
	if (stmt.raw === "\"use strict\"" || stmt.raw === "'use strict'") {
	return true;
	}
	}
	} else {

That's probably an alternative AST spec that was actual when escope was originally written. ESTree spec doesn't have DirectiveStatement nodes.

When the option is false (default), eslint-scope looks for ExpressionStatement nodes:

eslint-scope/lib/scope.js

Lines 97 to 119 in ceb8bdd

	} else {
	for (let i = 0, iz = body.body.length; i < iz; ++i) {
	const stmt = body.body[i];
	if (stmt.type !== Syntax.ExpressionStatement) {
	break;
	}
	const expr = stmt.expression;
	if (expr.type !== Syntax.Literal || typeof expr.value !== "string") {
	break;
	}
	if (expr.raw !== null && expr.raw !== undefined) {
	if (expr.raw === "\"use strict\"" || expr.raw === "'use strict'") {
	return true;
	}
	} else {
	if (expr.value === "use strict") {
	return true;
	}
	}
	}
	}

That's per the ESTree spec.

ESLint doesn't set the directive option, so by default eslint-scope works with ESTree AST.

The conclusion is that it doesn't seem that __useDirective() is causing problems with ES3 described in this issue.

Off-topic, we could consider dropping the directive option and the DirectiveStatement branch in a future major version. We could also consider refactoring the other branch to use ESTree's directive property.

[mdjermanovic]

isStrictScope() is the main function that calculates whether a given scope is strict by looking at its upper scope, nodes, directives, etc.

isStrictModeSupported() is a helper function added in estools/escope#93 only to check if the impliedStrict option value should be respected given the other options. It's currently used only here:

eslint-scope/lib/referencer.js

Lines 409 to 425 in ceb8bdd

	Program(node) {
	this.scopeManager.__nestGlobalScope(node);
	if (this.scopeManager.__isNodejsScope()) {
	// Force strictness of GlobalScope to false when using node.js scope.
	this.currentScope().isStrict = false;
	this.scopeManager.__nestFunctionScope(node, false);
	}
	if (this.scopeManager.__isES6() && this.scopeManager.isModule()) {
	this.scopeManager.__nestModuleScope(node);
	}
	if (this.scopeManager.isStrictModeSupported() && this.scopeManager.isImpliedStrict()) {
	this.currentScope().isStrict = true;
	}

I believe this issue will be fixed by setting this.isStrict = false if scopeManager.isStrictModeSupported() returns false, here:

eslint-scope/lib/scope.js

Lines 262 to 266 in ceb8bdd

	/**
	* Whether 'use strict' is in effect in this scope.
	* @member {boolean} Scope#isStrict
	*/
	this.isStrict = isStrictScope(this, block, isMethodDefinition, scopeManager.__useDirective());

[mdjermanovic]

PR #87