From de4c13fe57d8a9ee82c1c8a9ac39ca709f90ca1e Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Tue, 24 Sep 2019 08:58:26 +0200 Subject: [PATCH 01/13] Allow RegExp for exceptMethods rule definition --- lib/rules/class-methods-use-this.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 4bf17090abc..0cb675beec8 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -45,7 +45,7 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); - const exceptMethods = new Set(config.exceptMethods || []); + const exceptMethods = new Set(config.exceptMethods.map(e => new RegExp(e)) || []); const stack = []; @@ -77,7 +77,7 @@ module.exports = { */ function isIncludedInstanceMethod(node) { return isInstanceMethod(node) && - (node.computed || !exceptMethods.has(node.key.name)); + (node.computed || !exceptMethods.find(e => e.test(node.key.name))); } /** From b43cb82dd4d945caf6bb15c6f0c74ecda24c7680 Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Tue, 24 Sep 2019 09:03:02 +0200 Subject: [PATCH 02/13] Update doc for rule class-methods-use-this --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 89f6762b222..d4581c0c08b 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -94,7 +94,7 @@ class A { "class-methods-use-this": [, { "exceptMethods": [<...exceptions>] }] ``` -The `exceptMethods` option allows you to pass an array of method names for which you would like to ignore warnings. For example, you might have a spec from an external library that requires you to overwrite a method as a regular function (and not as a static method) and does not use `this` inside the function body. In this case, you can add that method to ignore in the warnings. +The `exceptMethods` option allows you to pass an array of method names (accept regex) for which you would like to ignore warnings. For example, you might have a spec from an external library that requires you to overwrite a method as a regular function (and not as a static method) and does not use `this` inside the function body. In this case, you can add that method to ignore in the warnings. Examples of **incorrect** code for this rule when used without exceptMethods: From 08921048d8397c4cf144729561554775a0b89e03 Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Tue, 24 Sep 2019 09:12:56 +0200 Subject: [PATCH 03/13] Fix: use array instead of set --- lib/rules/class-methods-use-this.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 0cb675beec8..db9adec5153 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -45,7 +45,7 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); - const exceptMethods = new Set(config.exceptMethods.map(e => new RegExp(e)) || []); + const exceptMethods = config.exceptMethods.map(e => new RegExp(e)) || []; const stack = []; From 9a810c737b0f2a14fa45cba2aa20e653efeadca6 Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Tue, 24 Sep 2019 09:14:37 +0200 Subject: [PATCH 04/13] Fix: use array correctly... --- lib/rules/class-methods-use-this.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index db9adec5153..26d1614fb9f 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -45,7 +45,7 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); - const exceptMethods = config.exceptMethods.map(e => new RegExp(e)) || []; + const exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e)); const stack = []; From 4be253220864dc59e0298e5993c054baa41ef819 Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Tue, 24 Sep 2019 09:25:20 +0200 Subject: [PATCH 05/13] Fix lint error: Add unicode flag to regex --- lib/rules/class-methods-use-this.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 26d1614fb9f..64da0265abf 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -45,7 +45,7 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); - const exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e)); + const exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e, 'u')); const stack = []; From 07c525dd700cf4bacb93df784bd1b9116a8d0b9a Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Tue, 24 Sep 2019 09:30:10 +0200 Subject: [PATCH 06/13] Fix lint issue: use doublequotes --- lib/rules/class-methods-use-this.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 64da0265abf..57af4d71455 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -45,7 +45,7 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); - const exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e, 'u')); + const exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e, "u")); const stack = []; From 2d5c98def4c7f48514ebf4539abc1b9db44410c8 Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Tue, 24 Sep 2019 17:51:09 +0200 Subject: [PATCH 07/13] Add useRegExp flag to avoid breaking change --- docs/rules/class-methods-use-this.md | 6 ++++-- lib/rules/class-methods-use-this.js | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index d4581c0c08b..8b2dc9f292c 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -91,10 +91,12 @@ class A { ### Exceptions ``` -"class-methods-use-this": [, { "exceptMethods": [<...exceptions>] }] +"class-methods-use-this": [, { "exceptMethods": [<...exceptions>], "useRegExp": }] ``` -The `exceptMethods` option allows you to pass an array of method names (accept regex) for which you would like to ignore warnings. For example, you might have a spec from an external library that requires you to overwrite a method as a regular function (and not as a static method) and does not use `this` inside the function body. In this case, you can add that method to ignore in the warnings. +The `exceptMethods` option allows you to pass an array of method names for which you would like to ignore warnings. For example, you might have a spec from an external library that requires you to overwrite a method as a regular function (and not as a static method) and does not use `this` inside the function body. In this case, you can add that method to ignore in the warnings. + +The option `useRegExp` option enables the use of regex expressions. Examples of **incorrect** code for this rule when used without exceptMethods: diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 57af4d71455..d1531f7ee0d 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -34,6 +34,9 @@ module.exports = { items: { type: "string" } + }, + useRegExp: { + type: "boolean" } }, additionalProperties: false @@ -45,7 +48,12 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); - const exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e, "u")); + let exceptMethods; + if (config.useRegExp) { + exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e, "u")); + } else { + exceptMethods = new Set(config.exceptMethods || []) + } const stack = []; @@ -76,8 +84,12 @@ module.exports = { * @private */ function isIncludedInstanceMethod(node) { + if (config.useRegExp) { + return isInstanceMethod(node) && + (node.computed || !exceptMethods.find(e => e.test(node.key.name))); + } return isInstanceMethod(node) && - (node.computed || !exceptMethods.find(e => e.test(node.key.name))); + (node.computed || !exceptMethods.has(node.key.name)); } /** From 8495f2a7c195547133821b7a056374df04575b98 Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Tue, 24 Sep 2019 18:05:28 +0200 Subject: [PATCH 08/13] Fix lint errors --- lib/rules/class-methods-use-this.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index d1531f7ee0d..0a100f22d39 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -49,10 +49,11 @@ module.exports = { create(context) { const config = Object.assign({}, context.options[0]); let exceptMethods; + if (config.useRegExp) { exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e, "u")); } else { - exceptMethods = new Set(config.exceptMethods || []) + exceptMethods = new Set(config.exceptMethods || []); } const stack = []; From d960d1195bb6a2796807c6ee4a1b1c50f0dbbb3b Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Fri, 27 Sep 2019 06:08:52 +0200 Subject: [PATCH 09/13] Use own array to define exceptions regex instead of flag --- docs/rules/class-methods-use-this.md | 9 ++++++++- lib/rules/class-methods-use-this.js | 21 ++++++++------------ tests/lib/rules/class-methods-use-this.js | 24 +++++++++++++++++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 8b2dc9f292c..7830e1bafff 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -96,7 +96,7 @@ class A { The `exceptMethods` option allows you to pass an array of method names for which you would like to ignore warnings. For example, you might have a spec from an external library that requires you to overwrite a method as a regular function (and not as a static method) and does not use `this` inside the function body. In this case, you can add that method to ignore in the warnings. -The option `useRegExp` option enables the use of regex expressions. +The option `exceptMethodsForRegex` works like exceptMethods just for regular expressions. Examples of **incorrect** code for this rule when used without exceptMethods: @@ -118,6 +118,13 @@ class A { foo() { } } + +/*eslint class-methods-use-this: ["error", { "exceptMethodsForRegex": ["^foo.*"] }] */ + +class B { + foobar() { + } +} ``` ## Further Reading diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index 0a100f22d39..e4adb3ab36b 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -35,8 +35,11 @@ module.exports = { type: "string" } }, - useRegExp: { - type: "boolean" + exceptMethodsForRegex: { + type: "array", + items: { + type: "string" + } } }, additionalProperties: false @@ -48,13 +51,9 @@ module.exports = { }, create(context) { const config = Object.assign({}, context.options[0]); - let exceptMethods; - if (config.useRegExp) { - exceptMethods = (config.exceptMethods || []).map(e => new RegExp(e, "u")); - } else { - exceptMethods = new Set(config.exceptMethods || []); - } + const exceptMethods = new Set(config.exceptMethods || []); + const exceptMethodsForRegex = (config.exceptMethodsForRegex || []).map(e => new RegExp(e, "u")); const stack = []; @@ -85,12 +84,8 @@ module.exports = { * @private */ function isIncludedInstanceMethod(node) { - if (config.useRegExp) { - return isInstanceMethod(node) && - (node.computed || !exceptMethods.find(e => e.test(node.key.name))); - } return isInstanceMethod(node) && - (node.computed || !exceptMethods.has(node.key.name)); + (node.computed || !exceptMethods.has(node.key.name) || !exceptMethodsForRegex.find(e => e.test(node.key.name))); } /** diff --git a/tests/lib/rules/class-methods-use-this.js b/tests/lib/rules/class-methods-use-this.js index 5238b9d68b7..77c9177704d 100644 --- a/tests/lib/rules/class-methods-use-this.js +++ b/tests/lib/rules/class-methods-use-this.js @@ -98,6 +98,14 @@ ruleTester.run("class-methods-use-this", rule, { { type: "FunctionExpression", line: 1, column: 34, messageId: "missingThis", data: { name: "method 'hasOwnProperty'" } } ] }, + { + code: "class A { foobar() {} hasOwnProperty() {} }", + options: [{ exceptMethodsForRegex: ["^foo.*"] }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { type: "FunctionExpression", line: 1, column: 34, messageId: "missingThis", data: { name: "method 'hasOwnProperty'" } } + ] + }, { code: "class A { [foo]() {} }", options: [{ exceptMethods: ["foo"] }], @@ -106,6 +114,22 @@ ruleTester.run("class-methods-use-this", rule, { { type: "FunctionExpression", line: 1, column: 16, messageId: "missingThis", data: { name: "method" } } ] }, + { + code: "class A { [foobar]() {} }", + options: [{ exceptMethodsForRegex: ["^foo.*"] }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { type: "FunctionExpression", line: 1, column: 16, messageId: "missingThis", data: { name: "method" } } + ] + }, + { + code: "class A { foo() {} bar() {} afoobar() {} }", + options: [{ exceptMethods: ["bar"], exceptMethodsForRegex: ["afoo.*"] }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } } + ] + }, { code: "class A { foo(){} 'bar'(){} 123(){} [`baz`](){} [a](){} [f(a)](){} get quux(){} set[a](b){} *quuux(){} }", parserOptions: { ecmaVersion: 6 }, From da6bb73407a2c2ad47da0becbabe4d85bab77afb Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Fri, 27 Sep 2019 06:10:35 +0200 Subject: [PATCH 10/13] Fix: docu --- docs/rules/class-methods-use-this.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/class-methods-use-this.md b/docs/rules/class-methods-use-this.md index 7830e1bafff..9b551e70ff0 100644 --- a/docs/rules/class-methods-use-this.md +++ b/docs/rules/class-methods-use-this.md @@ -91,7 +91,7 @@ class A { ### Exceptions ``` -"class-methods-use-this": [, { "exceptMethods": [<...exceptions>], "useRegExp": }] +"class-methods-use-this": [, { "exceptMethods": [<...exceptions>], "exceptMethodsForRegex": [<...exceptions>] }] ``` The `exceptMethods` option allows you to pass an array of method names for which you would like to ignore warnings. For example, you might have a spec from an external library that requires you to overwrite a method as a regular function (and not as a static method) and does not use `this` inside the function body. In this case, you can add that method to ignore in the warnings. From af1cef81e4b4d075b0f5a865151676c3577edd22 Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Fri, 27 Sep 2019 06:16:50 +0200 Subject: [PATCH 11/13] Fix: Rule and exception logic --- lib/rules/class-methods-use-this.js | 2 +- tests/lib/rules/class-methods-use-this.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index e4adb3ab36b..fc51e2bbfab 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -85,7 +85,7 @@ module.exports = { */ function isIncludedInstanceMethod(node) { return isInstanceMethod(node) && - (node.computed || !exceptMethods.has(node.key.name) || !exceptMethodsForRegex.find(e => e.test(node.key.name))); + (node.computed || !(exceptMethods.has(node.key.name) && exceptMethodsForRegex.find(e => e.test(node.key.name)))); } /** diff --git a/tests/lib/rules/class-methods-use-this.js b/tests/lib/rules/class-methods-use-this.js index 77c9177704d..f66c9c24502 100644 --- a/tests/lib/rules/class-methods-use-this.js +++ b/tests/lib/rules/class-methods-use-this.js @@ -124,7 +124,7 @@ ruleTester.run("class-methods-use-this", rule, { }, { code: "class A { foo() {} bar() {} afoobar() {} }", - options: [{ exceptMethods: ["bar"], exceptMethodsForRegex: ["afoo.*"] }], + options: [{ exceptMethods: ["bar"], exceptMethodsForRegex: ["^afoo.*"] }], parserOptions: { ecmaVersion: 6 }, errors: [ { type: "FunctionExpression", line: 1, column: 14, messageId: "missingThis", data: { name: "method 'foo'" } } From 5bc25305fe7e67caa6bb5a4627e71c3d50e3beef Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Fri, 27 Sep 2019 06:24:35 +0200 Subject: [PATCH 12/13] Fix: exception logic --- lib/rules/class-methods-use-this.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/class-methods-use-this.js b/lib/rules/class-methods-use-this.js index fc51e2bbfab..97ef493e0d7 100644 --- a/lib/rules/class-methods-use-this.js +++ b/lib/rules/class-methods-use-this.js @@ -85,7 +85,7 @@ module.exports = { */ function isIncludedInstanceMethod(node) { return isInstanceMethod(node) && - (node.computed || !(exceptMethods.has(node.key.name) && exceptMethodsForRegex.find(e => e.test(node.key.name)))); + (node.computed || (!exceptMethods.has(node.key.name) && !exceptMethodsForRegex.find(e => e.test(node.key.name)))); } /** From d191617ed6666782463a1ccaf0e43cc9ea551f14 Mon Sep 17 00:00:00 2001 From: Jonas Brenner Date: Fri, 27 Sep 2019 06:30:18 +0200 Subject: [PATCH 13/13] Fix: test cases --- tests/lib/rules/class-methods-use-this.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/class-methods-use-this.js b/tests/lib/rules/class-methods-use-this.js index f66c9c24502..5ff01045b20 100644 --- a/tests/lib/rules/class-methods-use-this.js +++ b/tests/lib/rules/class-methods-use-this.js @@ -103,7 +103,7 @@ ruleTester.run("class-methods-use-this", rule, { options: [{ exceptMethodsForRegex: ["^foo.*"] }], parserOptions: { ecmaVersion: 6 }, errors: [ - { type: "FunctionExpression", line: 1, column: 34, messageId: "missingThis", data: { name: "method 'hasOwnProperty'" } } + { type: "FunctionExpression", line: 1, column: 37, messageId: "missingThis", data: { name: "method 'hasOwnProperty'" } } ] }, { @@ -119,7 +119,7 @@ ruleTester.run("class-methods-use-this", rule, { options: [{ exceptMethodsForRegex: ["^foo.*"] }], parserOptions: { ecmaVersion: 6 }, errors: [ - { type: "FunctionExpression", line: 1, column: 16, messageId: "missingThis", data: { name: "method" } } + { type: "FunctionExpression", line: 1, column: 19, messageId: "missingThis", data: { name: "method" } } ] }, {