Skip to content

Commit

Permalink
fix: update no-invalid-this and no-eval for class static blocks (#15300)
Browse files Browse the repository at this point in the history
Fixes false positives of rules `no-invalid-this` and `no-eval` in cases where `this` is used in class static blocks.

Refs #15016
  • Loading branch information
mdjermanovic committed Nov 15, 2021
1 parent d3a267f commit 6d1c666
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 3 deletions.
8 changes: 8 additions & 0 deletions docs/rules/no-eval.md
Expand Up @@ -66,6 +66,14 @@ class A {

eval() {
}

static {
// This is a user-defined static method.
this.eval("var a = 0");
}

static eval() {
}
}
```

Expand Down
20 changes: 19 additions & 1 deletion docs/rules/no-invalid-this.md
Expand Up @@ -18,14 +18,19 @@ This rule judges from following conditions whether or not the function is a meth

* The function is on an object literal.
* The function is assigned to a property.
* The function is a method/getter/setter of ES2015 Classes. (excepts static methods)
* The function is a method/getter/setter of ES2015 Classes.

And this rule allows `this` keywords in functions below:

* The `call/apply/bind` method of the function is called directly.
* The function is a callback of array methods (such as `.forEach()`) if `thisArg` is given.
* The function has `@this` tag in its JSDoc comment.

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

* In class field initializers.
* In class static blocks.

Otherwise are considered problems.

This rule applies **only** in strict mode.
Expand Down Expand Up @@ -166,6 +171,13 @@ Foo.prototype.foo = function foo() {
};

class Foo {

// OK, this is in a class field initializer.
a = this.b;

// OK, static initializers also have valid this.
static a = this.b;

foo() {
// OK, this is in a method.
this.a = 0;
Expand All @@ -177,6 +189,12 @@ class Foo {
this.a = 0;
baz(() => this);
}

static {
// OK, static blocks also have valid this.
this.a = 0;
baz(() => this);
}
}

var foo = (function foo() {
Expand Down
2 changes: 2 additions & 0 deletions lib/rules/no-eval.js
Expand Up @@ -248,6 +248,8 @@ module.exports = {
"ArrowFunctionExpression:exit": exitVarScope,
"PropertyDefinition > *.value": enterVarScope,
"PropertyDefinition > *.value:exit": exitVarScope,
StaticBlock: enterVarScope,
"StaticBlock:exit": exitVarScope,

ThisExpression(node) {
if (!isMember(node.parent, "eval")) {
Expand Down
4 changes: 4 additions & 0 deletions lib/rules/no-invalid-this.js
Expand Up @@ -132,6 +132,10 @@ module.exports = {
"PropertyDefinition > *.value": enterFunction,
"PropertyDefinition > *.value:exit": exitFunction,

// Class static blocks are implicit functions.
StaticBlock: enterFunction,
"StaticBlock:exit": exitFunction,

// Reports if `this` of the current context is invalid.
ThisExpression(node) {
const current = stack.getCurrent();
Expand Down
12 changes: 10 additions & 2 deletions lib/rules/utils/ast-utils.js
Expand Up @@ -937,6 +937,8 @@ module.exports = {
*
* First, this checks the node:
*
* - The given node is not in `PropertyDefinition#value` position.
* - The given node is not `StaticBlock`.
* - The function name does not start with uppercase. It's a convention to capitalize the names
* of constructor functions. This check is not performed if `capIsConstructor` is set to `false`.
* - The function does not have a JSDoc comment that has a @this tag.
Expand All @@ -951,7 +953,8 @@ module.exports = {
* - The location is not on an ES2015 class.
* - Its `bind`/`call`/`apply` method is not called directly.
* - The function is not a callback of array methods (such as `.forEach()`) if `thisArg` is given.
* @param {ASTNode} node A function node to check.
* @param {ASTNode} node A function node to check. It also can be an implicit function, like `StaticBlock`
* or any expression that is `PropertyDefinition#value` node.
* @param {SourceCode} sourceCode A SourceCode instance to get comments.
* @param {boolean} [capIsConstructor = true] `false` disables the assumption that functions which name starts
* with an uppercase or are assigned to a variable which name starts with an uppercase are constructors.
Expand All @@ -964,7 +967,12 @@ module.exports = {
* Therefore, A expression node at `PropertyDefinition#value` is a function.
* In this case, `this` is always not default binding.
*/
if (node && node.parent && node.parent.type === "PropertyDefinition" && node.value === node) {
if (node.parent.type === "PropertyDefinition" && node.parent.value === node) {
return false;
}

// Class static blocks are implicit functions. In this case, `this` is always not default binding.
if (node.type === "StaticBlock") {
return false;
}

Expand Down
7 changes: 7 additions & 0 deletions tests/lib/rules/no-eval.js
Expand Up @@ -50,6 +50,7 @@ ruleTester.run("no-eval", rule, {
{ code: "class A { static foo() { this.eval(); } }", parserOptions: { ecmaVersion: 6 } },
{ code: "class A { field = this.eval(); }", parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { field = () => this.eval(); }", parserOptions: { ecmaVersion: 2022 } },
{ code: "class A { static { this.eval(); } }", parserOptions: { ecmaVersion: 2022 } },

// Allows indirect eval
{ code: "(0, eval)('foo')", options: [{ allowIndirect: true }] },
Expand Down Expand Up @@ -132,6 +133,12 @@ ruleTester.run("no-eval", rule, {
code: "class C { [this.eval('foo')] }",
parserOptions: { ecmaVersion: 2022 },
errors: [{ messageId: "unexpected" }]
},

{
code: "class A { static {} [this.eval()]; }",
parserOptions: { ecmaVersion: 2022 },
errors: [{ messageId: "unexpected" }]
}
]
});
48 changes: 48 additions & 0 deletions tests/lib/rules/no-invalid-this.js
Expand Up @@ -775,6 +775,54 @@ const patterns = [
valid: [NORMAL], // the global this in non-strict mode is OK.
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES],
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }]
},

// Class static blocks
{
code: "class C { static { this.x; } }",
parserOptions: { ecmaVersion: 2022 },
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "class C { static { () => { this.x; } } }",
parserOptions: { ecmaVersion: 2022 },
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "class C { static { class D { [this.x]; } } }",
parserOptions: { ecmaVersion: 2022 },
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "class C { static { function foo() { this.x; } } }",
parserOptions: { ecmaVersion: 2022 },
valid: [],
invalid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }]
},
{
code: "class C { static { (function() { this.x; }); } }",
parserOptions: { ecmaVersion: 2022 },
valid: [],
invalid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }]
},
{
code: "class C { static { (function() { this.x; })(); } }",
parserOptions: { ecmaVersion: 2022 },
valid: [],
invalid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }]
},
{
code: "class C { static {} [this.x]; }",
parserOptions: { ecmaVersion: 2022 },
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES],
errors: [{ messageId: "unexpectedThis", type: "ThisExpression" }]
}
];

Expand Down

0 comments on commit 6d1c666

Please sign in to comment.