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

Update: Add capIsConstructor option to no-invalid-this (fixes #12271) #12308

Merged
merged 3 commits into from Nov 1, 2019
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
49 changes: 49 additions & 0 deletions docs/rules/no-invalid-this.md
Expand Up @@ -197,6 +197,55 @@ function foo() {
}
```

## Options

This rule has an object option, with one option:

* `"capIsConstructor": false` (default `true`) disables the assumption that a function which name starts with an uppercase is a constructor.

### capIsConstructor

By default, this rule always allows the use of `this` in functions which name starts with an uppercase and anonymous functions that are assigned to a variable which name starts with an uppercase, assuming that those functions are used as constructor functions.

Set `"capIsConstructor"` to `false` if you want those functions to be treated as 'regular' functions.

Examples of **incorrect** code for this rule with `"capIsConstructor"` option set to `false`:

```js
/*eslint no-invalid-this: ["error", { "capIsConstructor": false }]*/

"use strict";

function Foo() {
this.a = 0;
}

var bar = function Foo() {
this.a = 0;
}

var Bar = function() {
this.a = 0;
};

Baz = function() {
this.a = 0;
};
```

Examples of **correct** code for this rule with `"capIsConstructor"` option set to `false`:

```js
/*eslint no-invalid-this: ["error", { "capIsConstructor": false }]*/

"use strict";

obj.Foo = function Foo() {
// OK, this is in a method.
this.a = 0;
};
```

## When Not To Use It

If you don't want to be notified about usage of `this` keyword outside of classes or class-like objects, you can safely disable this rule.
18 changes: 16 additions & 2 deletions lib/rules/no-invalid-this.js
Expand Up @@ -26,10 +26,23 @@ module.exports = {
url: "https://eslint.org/docs/rules/no-invalid-this"
},

schema: []
schema: [
{
type: "object",
properties: {
capIsConstructor: {
type: "boolean",
default: true
}
},
additionalProperties: false
}
]
},

create(context) {
const options = context.options[0] || {};
const capIsConstructor = options.capIsConstructor !== false;
const stack = [],
sourceCode = context.getSourceCode();

Expand All @@ -48,7 +61,8 @@ module.exports = {
current.init = true;
current.valid = !astUtils.isDefaultThisBinding(
current.node,
sourceCode
sourceCode,
{ capIsConstructor }
);
}
return current;
Expand Down
18 changes: 14 additions & 4 deletions lib/rules/utils/ast-utils.js
Expand Up @@ -581,23 +581,31 @@ module.exports = {
*
* First, this checks the node:
*
* - The function name does not start with uppercase (it's a constructor).
* - 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.
*
* Next, this checks the location of the node.
* If the location is below, this judges `this` is valid.
*
* - The location is not on an object literal.
* - The location is not assigned to a variable which starts with an uppercase letter.
* - The location is not assigned to a variable which starts with an uppercase letter. Applies to anonymous
* functions only, as the name of the variable is considered to be the name of the function in this case.
* This check is not performed if `capIsConstructor` is set to `false`.
* - 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 {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.
* @returns {boolean} The function node is the default `this` binding.
*/
isDefaultThisBinding(node, sourceCode) {
if (isES5Constructor(node) || hasJSDocThisTag(node, sourceCode)) {
isDefaultThisBinding(node, sourceCode, { capIsConstructor = true } = {}) {
kaicataldo marked this conversation as resolved.
Show resolved Hide resolved
if (
(capIsConstructor && isES5Constructor(node)) ||
hasJSDocThisTag(node, sourceCode)
) {
return false;
}
const isAnonymous = node.id === null;
Expand Down Expand Up @@ -671,6 +679,7 @@ module.exports = {
return false;
}
if (
capIsConstructor &&
isAnonymous &&
parent.left.type === "Identifier" &&
startsWithUpperCase(parent.left.name)
Expand All @@ -685,6 +694,7 @@ module.exports = {
*/
case "VariableDeclarator":
return !(
capIsConstructor &&
isAnonymous &&
parent.init === currentNode &&
parent.id.type === "Identifier" &&
Expand Down
70 changes: 70 additions & 0 deletions tests/lib/rules/no-invalid-this.js
Expand Up @@ -134,13 +134,37 @@ const patterns = [
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "function foo() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{ capIsConstructor: false }], // test that the option doesn't reverse the logic and mistakenly allows lowercase functions
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "function Foo() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{ capIsConstructor: false }],
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "function foo() { \"use strict\"; console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
errors,
valid: [],
invalid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "function Foo() { \"use strict\"; console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{ capIsConstructor: false }],
errors,
valid: [],
invalid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "return function() { console.log(this); z(x => console.log(x, this)); };",
parserOptions: {
Expand Down Expand Up @@ -226,6 +250,20 @@ const patterns = [
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "function Foo() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{}], // test the default value in schema
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "function Foo() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{ capIsConstructor: true }], // test explicitly set option to the default value
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "var Foo = function Foo() { console.log(this); z(x => console.log(x, this)); };",
parserOptions: { ecmaVersion: 6 },
Expand Down Expand Up @@ -556,9 +594,25 @@ const patterns = [
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "var Ctor = function() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{ capIsConstructor: false }],
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "var func = function() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "var func = function() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{ capIsConstructor: false }],
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
Expand All @@ -569,9 +623,25 @@ const patterns = [
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "Ctor = function() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{ capIsConstructor: false }],
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "func = function() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "func = function() { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 6 },
options: [{ capIsConstructor: false }],
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
Expand Down