From 94ff921689115f856578159564ee1968b4b914be Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Fri, 1 Nov 2019 20:18:31 +0100 Subject: [PATCH] Update: Add capIsConstructor option to no-invalid-this (fixes #12271) (#12308) * Update: Add capIsConstructor option to no-invalid-this (fixes #12271) * Update isDefaultThisBinding description * Remove newline in docs --- docs/rules/no-invalid-this.md | 49 +++++++++++++++++++++ lib/rules/no-invalid-this.js | 18 +++++++- lib/rules/utils/ast-utils.js | 18 ++++++-- tests/lib/rules/no-invalid-this.js | 70 ++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 6 deletions(-) diff --git a/docs/rules/no-invalid-this.md b/docs/rules/no-invalid-this.md index b6733eded56..7614b59a87b 100644 --- a/docs/rules/no-invalid-this.md +++ b/docs/rules/no-invalid-this.md @@ -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. diff --git a/lib/rules/no-invalid-this.js b/lib/rules/no-invalid-this.js index b1dddd9319c..5398fc3b5f8 100644 --- a/lib/rules/no-invalid-this.js +++ b/lib/rules/no-invalid-this.js @@ -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(); @@ -48,7 +61,8 @@ module.exports = { current.init = true; current.valid = !astUtils.isDefaultThisBinding( current.node, - sourceCode + sourceCode, + { capIsConstructor } ); } return current; diff --git a/lib/rules/utils/ast-utils.js b/lib/rules/utils/ast-utils.js index 17e056c240c..ffb8bace83c 100644 --- a/lib/rules/utils/ast-utils.js +++ b/lib/rules/utils/ast-utils.js @@ -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 } = {}) { + if ( + (capIsConstructor && isES5Constructor(node)) || + hasJSDocThisTag(node, sourceCode) + ) { return false; } const isAnonymous = node.id === null; @@ -671,6 +679,7 @@ module.exports = { return false; } if ( + capIsConstructor && isAnonymous && parent.left.type === "Identifier" && startsWithUpperCase(parent.left.name) @@ -685,6 +694,7 @@ module.exports = { */ case "VariableDeclarator": return !( + capIsConstructor && isAnonymous && parent.init === currentNode && parent.id.type === "Identifier" && diff --git a/tests/lib/rules/no-invalid-this.js b/tests/lib/rules/no-invalid-this.js index e4606d0e687..24b95405ba6 100644 --- a/tests/lib/rules/no-invalid-this.js +++ b/tests/lib/rules/no-invalid-this.js @@ -134,6 +134,22 @@ 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 }, @@ -141,6 +157,14 @@ const patterns = [ 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: { @@ -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 }, @@ -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] @@ -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]