Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: fix logic for top-level this in no-invalid-this and no-eval #15712

Merged
merged 3 commits into from Mar 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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" } },
aladdin-add marked this conversation as resolved.
Show resolved Hide resolved
{ 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