From 685a67a62bdea19ca9ce12008a034b8d31162422 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Fri, 25 Mar 2022 23:20:11 +0100 Subject: [PATCH] feat: fix logic for top-level `this` in no-invalid-this and no-eval (#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` --- docs/rules/no-invalid-this.md | 22 ++++++++++---------- lib/rules/no-eval.js | 8 ++++++-- lib/rules/no-invalid-this.js | 6 +++--- tests/lib/rules/no-eval.js | 9 ++++++++ tests/lib/rules/no-invalid-this.js | 33 +++++++++++++++++++----------- 5 files changed, 50 insertions(+), 28 deletions(-) diff --git a/docs/rules/no-invalid-this.md b/docs/rules/no-invalid-this.md index 91e460da797..bbdc79cbcf5 100644 --- a/docs/rules/no-invalid-this.md +++ b/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: @@ -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. @@ -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); @@ -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() { @@ -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; diff --git a/lib/rules/no-eval.js b/lib/rules/no-eval.js index ae6d71b80c9..dab04dd77e2 100644 --- a/lib/rules/no-eval.js +++ b/lib/rules/no-eval.js @@ -87,6 +87,7 @@ module.exports = { upper: funcInfo, node, strict, + isTopLevelOfScript: false, defaultThis: false, initialized: strict }; @@ -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 }; @@ -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); diff --git a/lib/rules/no-invalid-this.js b/lib/rules/no-invalid-this.js index 64e4d964a84..a97696b8562 100644 --- a/lib/rules/no-invalid-this.js +++ b/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 */ @@ -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" }, @@ -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) ) diff --git a/tests/lib/rules/no-eval.js b/tests/lib/rules/no-eval.js index b084892cf2e..fdd961f6546 100644 --- a/tests/lib/rules/no-eval.js +++ b/tests/lib/rules/no-eval.js @@ -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'); }}", @@ -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 }] }, @@ -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 }] }, @@ -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()]; }", diff --git a/tests/lib/rules/no-invalid-this.js b/tests/lib/rules/no-invalid-this.js index 6b54d9a965e..b106a706c0f 100644 --- a/tests/lib/rules/no-invalid-this.js +++ b/tests/lib/rules/no-invalid-this.js @@ -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));", @@ -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. @@ -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 }, { @@ -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" }] }, { @@ -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" }] },