Skip to content

Commit

Permalink
feat: fix logic for top-level this in no-invalid-this and no-eval (#…
Browse files Browse the repository at this point in the history
…15712)

* feat: fix logic for top-level `this` in no-invalid-this and no-eval

* add test with `this.eval` to no-invalid-this

* update docs to clarify `no-invalid-this`
  • Loading branch information
mdjermanovic committed Mar 25, 2022
1 parent 18f5e05 commit 685a67a
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 28 deletions.
22 changes: 11 additions & 11 deletions docs/rules/no-invalid-this.md
@@ -1,14 +1,18 @@
# no-invalid-this

Disallows `this` keywords outside of classes or class-like objects.
Disallows use of `this` in contexts where the value of `this` is `undefined`.

Under the strict mode, `this` keywords outside of classes or class-like objects might be `undefined` and raise a `TypeError`.

## Rule Details

This rule aims to flag usage of `this` keywords outside of classes or class-like objects.
This rule aims to flag usage of `this` keywords in contexts where the value of `this` is `undefined`.

Basically, this rule checks whether or not a function containing `this` keyword is a constructor or a method.
Top-level `this` in scripts is always considered valid because it refers to the global object regardless of the strict mode.

Top-level `this` in ECMAScript modules is always considered invalid because its value is `undefined`.

For `this` inside functions, this rule basically checks whether or not the function containing `this` keyword is a constructor or a method. Note that arrow functions have lexical `this`, and that therefore this rule checks their enclosing contexts.

This rule judges from following conditions whether or not the function is a constructor:

Expand All @@ -30,6 +34,7 @@ And this rule allows `this` keywords in functions below:

And this rule always allows `this` keywords in the following contexts:

* At the top level of scripts.
* In class field initializers.
* In class static blocks.

Expand All @@ -46,9 +51,6 @@ Examples of **incorrect** code for this rule in strict mode:

"use strict";

this.a = 0;
baz(() => this);

(function() {
this.a = 0;
baz(() => this);
Expand All @@ -69,11 +71,6 @@ foo(function() {
baz(() => this);
});

obj.foo = () => {
// `this` of arrow functions is the outer scope's.
this.a = 0;
};

var obj = {
aaa: function() {
return function foo() {
Expand All @@ -98,6 +95,9 @@ Examples of **correct** code for this rule in strict mode:

"use strict";

this.a = 0;
baz(() => this);

function Foo() {
// OK, this is in a legacy style constructor.
this.a = 0;
Expand Down
8 changes: 6 additions & 2 deletions lib/rules/no-eval.js
Expand Up @@ -87,6 +87,7 @@ module.exports = {
upper: funcInfo,
node,
strict,
isTopLevelOfScript: false,
defaultThis: false,
initialized: strict
};
Expand Down Expand Up @@ -222,12 +223,14 @@ module.exports = {
strict =
scope.isStrict ||
node.sourceType === "module" ||
(features.globalReturn && scope.childScopes[0].isStrict);
(features.globalReturn && scope.childScopes[0].isStrict),
isTopLevelOfScript = node.sourceType !== "module" && !features.globalReturn;

funcInfo = {
upper: null,
node,
strict,
isTopLevelOfScript,
defaultThis: true,
initialized: true
};
Expand Down Expand Up @@ -269,7 +272,8 @@ module.exports = {
);
}

if (!funcInfo.strict && funcInfo.defaultThis) {
// `this` at the top level of scripts always refers to the global object
if (funcInfo.isTopLevelOfScript || (!funcInfo.strict && funcInfo.defaultThis)) {

// `this.eval` is possible built-in `eval`.
report(node.parent);
Expand Down
6 changes: 3 additions & 3 deletions lib/rules/no-invalid-this.js
@@ -1,5 +1,5 @@
/**
* @fileoverview A rule to disallow `this` keywords outside of classes or class-like objects.
* @fileoverview A rule to disallow `this` keywords in contexts where the value of `this` is `undefined`.
* @author Toru Nagashima
*/

Expand Down Expand Up @@ -36,7 +36,7 @@ module.exports = {
type: "suggestion",

docs: {
description: "disallow `this` keywords outside of classes or class-like objects",
description: "disallow use of `this` in contexts where the value of `this` is `undefined`",
recommended: false,
url: "https://eslint.org/docs/rules/no-invalid-this"
},
Expand Down Expand Up @@ -98,11 +98,11 @@ module.exports = {
const scope = context.getScope();
const features = context.parserOptions.ecmaFeatures || {};

// `this` at the top level of scripts always refers to the global object
stack.push({
init: true,
node,
valid: !(
scope.isStrict ||
node.sourceType === "module" ||
(features.globalReturn && scope.childScopes[0].isStrict)
)
Expand Down
9 changes: 9 additions & 0 deletions tests/lib/rules/no-eval.js
Expand Up @@ -42,6 +42,8 @@ ruleTester.run("no-eval", rule, {
{ code: "function foo() { var eval = 'foo'; globalThis[eval]('foo') }", env: { es2020: true } },
"this.noeval('foo');",
"function foo() { 'use strict'; this.eval('foo'); }",
{ code: "'use strict'; this.eval('foo');", parserOptions: { ecmaFeatures: { globalReturn: true } } },
{ code: "this.eval('foo');", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
{ code: "function foo() { this.eval('foo'); }", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
{ code: "function foo() { this.eval('foo'); }", parserOptions: { ecmaFeatures: { impliedStrict: true } } },
"var obj = {foo: function() { this.eval('foo'); }}",
Expand Down Expand Up @@ -92,6 +94,7 @@ ruleTester.run("no-eval", rule, {
{ code: "(0, window['eval'])('foo')", env: { browser: true }, errors: [{ messageId: "unexpected", type: "MemberExpression", column: 12, endColumn: 18 }] },
{ code: "var EVAL = eval; EVAL('foo')", errors: [{ messageId: "unexpected", type: "Identifier", column: 12, endColumn: 16 }] },
{ code: "var EVAL = this.eval; EVAL('foo')", errors: [{ messageId: "unexpected", type: "MemberExpression", column: 17, endColumn: 21 }] },
{ code: "'use strict'; var EVAL = this.eval; EVAL('foo')", errors: [{ messageId: "unexpected", type: "MemberExpression", column: 31, endColumn: 35 }] },
{ code: "(function(exe){ exe('foo') })(eval);", errors: [{ messageId: "unexpected", type: "Identifier", column: 31, endColumn: 35 }] },
{ code: "window.eval('foo')", env: { browser: true }, errors: [{ messageId: "unexpected", type: "CallExpression", column: 8, endColumn: 12 }] },
{ code: "window.window.eval('foo')", env: { browser: true }, errors: [{ messageId: "unexpected", type: "CallExpression", column: 15, endColumn: 19 }] },
Expand All @@ -100,6 +103,7 @@ ruleTester.run("no-eval", rule, {
{ code: "global.global.eval('foo')", env: { node: true }, errors: [{ messageId: "unexpected", type: "CallExpression", column: 15, endColumn: 19 }] },
{ code: "global.global[`eval`]('foo')", parserOptions: { ecmaVersion: 6 }, env: { node: true }, errors: [{ messageId: "unexpected", type: "CallExpression", column: 15, endColumn: 21 }] },
{ code: "this.eval('foo')", errors: [{ messageId: "unexpected", type: "CallExpression", column: 6, endColumn: 10 }] },
{ code: "'use strict'; this.eval('foo')", errors: [{ messageId: "unexpected", type: "CallExpression", column: 20, endColumn: 24 }] },
{ code: "function foo() { this.eval('foo') }", errors: [{ messageId: "unexpected", type: "CallExpression", column: 23, endColumn: 27 }] },
{ code: "var EVAL = globalThis.eval; EVAL('foo')", env: { es2020: true }, errors: [{ messageId: "unexpected", type: "MemberExpression", column: 23, endColumn: 27 }] },
{ code: "globalThis.eval('foo')", env: { es2020: true }, errors: [{ messageId: "unexpected", type: "CallExpression", column: 12, endColumn: 16 }] },
Expand Down Expand Up @@ -134,6 +138,11 @@ ruleTester.run("no-eval", rule, {
parserOptions: { ecmaVersion: 2022 },
errors: [{ messageId: "unexpected" }]
},
{
code: "'use strict'; class C { [this.eval('foo')] }",
parserOptions: { ecmaVersion: 2022 },
errors: [{ messageId: "unexpected" }]
},

{
code: "class A { static {} [this.eval()]; }",
Expand Down
33 changes: 21 additions & 12 deletions tests/lib/rules/no-invalid-this.js
Expand Up @@ -106,8 +106,8 @@ const patterns = [
code: "console.log(this); z(x => console.log(x, this));",
parserOptions: { ecmaVersion: 6 },
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT],
invalid: [MODULES]
},
{
code: "console.log(this); z(x => console.log(x, this));",
Expand All @@ -125,8 +125,17 @@ const patterns = [
ecmaVersion: 6
},
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT],
invalid: [MODULES]
},
{
code: "this.eval('foo');",
parserOptions: {
ecmaVersion: 6
},
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }],
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT],
invalid: [MODULES]
},

// IIFE.
Expand Down Expand Up @@ -374,8 +383,8 @@ const patterns = [
{
code: "obj.foo = (() => () => { console.log(this); z(x => console.log(x, this)); })();",
parserOptions: { ecmaVersion: 6 },
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES],
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT],
invalid: [MODULES],
errors
},
{
Expand Down Expand Up @@ -793,8 +802,8 @@ const patterns = [
{
code: "class C { [this.foo]; }",
parserOptions: { ecmaVersion: 2022 },
valid: [NORMAL], // the global this in non-strict mode is OK.
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES],
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT], // `this` is the top-level `this`
invalid: [MODULES],
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }]
},
{
Expand Down Expand Up @@ -855,15 +864,15 @@ const patterns = [
{
code: "class C { static {} [this]; }",
parserOptions: { ecmaVersion: 2022 },
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES],
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT],
invalid: [MODULES],
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }]
},
{
code: "class C { static {} [this.x]; }",
parserOptions: { ecmaVersion: 2022 },
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES],
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT],
invalid: [MODULES],
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }]
},

Expand Down

0 comments on commit 685a67a

Please sign in to comment.