From e8518f1b138b59e2b95a5f3ce357ff75840b03df Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 21 Oct 2020 15:41:47 +0800 Subject: [PATCH 01/14] Add `prefer-array-some` rule --- docs/rules/prefer-array-some.md | 29 ++++++ index.js | 1 + readme.md | 2 + rules/prefer-array-some.js | 53 ++++++++++ test/prefer-array-some.js | 120 +++++++++++++++++++++++ test/snapshots/prefer-array-some.js.md | 65 ++++++++++++ test/snapshots/prefer-array-some.js.snap | Bin 0 -> 466 bytes 7 files changed, 270 insertions(+) create mode 100644 docs/rules/prefer-array-some.md create mode 100644 rules/prefer-array-some.js create mode 100644 test/prefer-array-some.js create mode 100644 test/snapshots/prefer-array-some.js.md create mode 100644 test/snapshots/prefer-array-some.js.snap diff --git a/docs/rules/prefer-array-some.md b/docs/rules/prefer-array-some.md new file mode 100644 index 0000000000..ad25839f8e --- /dev/null +++ b/docs/rules/prefer-array-some.md @@ -0,0 +1,29 @@ +# Prefer `.some(…)` over `.find(…)`. + +Prefer use [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) over [`Array#find`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) when testing array. + +## Fail + +```js +if (array.find(element => element === 'πŸ¦„')) { +} +``` + +```js +const foo = array.find(element => element === 'πŸ¦„') ? bar : baz; +``` + +## Pass + +```js +if (array.some(element => element === 'πŸ¦„')) { +} +``` + +```js +const foo = array.some(element => element === 'πŸ¦„') ? bar : baz; +``` + +```js +const foo = bar ? array.find(element => element === 'πŸ¦„') : ''; +``` diff --git a/index.js b/index.js index 3f1d1f5750..48d3ac07bf 100644 --- a/index.js +++ b/index.js @@ -53,6 +53,7 @@ module.exports = { 'unicorn/numeric-separators-style': 'off', 'unicorn/prefer-add-event-listener': 'error', 'unicorn/prefer-array-find': 'error', + 'unicorn/prefer-array-some': 'error', 'unicorn/prefer-dataset': 'error', 'unicorn/prefer-event-key': 'error', // TODO: Enable this by default when targeting Node.js 12. diff --git a/readme.md b/readme.md index 754e794cff..13f33c17b7 100644 --- a/readme.md +++ b/readme.md @@ -33,6 +33,7 @@ Configure it in `package.json`. "unicorn" ], "rules": { + "unicorn/prefer-array-some": "error", "unicorn/better-regex": "error", "unicorn/catch-error-name": "error", "unicorn/consistent-function-scoping": "error", @@ -134,6 +135,7 @@ Configure it in `package.json`. - [numeric-separators-style](docs/rules/numeric-separators-style.md) - Enforce the style of numeric separators by correctly grouping digits. *(fixable)* - [prefer-add-event-listener](docs/rules/prefer-add-event-listener.md) - Prefer `.addEventListener()` and `.removeEventListener()` over `on`-functions. *(partly fixable)* - [prefer-array-find](docs/rules/prefer-array-find.md) - Prefer `.find(…)` over the first element from `.filter(…)`. *(partly fixable)* +- [prefer-array-some](docs/rules/prefer-array-some.md) - Prefer `.some(…)` over `.find(…)`. - [prefer-dataset](docs/rules/prefer-dataset.md) - Prefer using `.dataset` on DOM elements over `.setAttribute(…)`. *(fixable)* - [prefer-event-key](docs/rules/prefer-event-key.md) - Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`. *(partly fixable)* - [prefer-flat-map](docs/rules/prefer-flat-map.md) - Prefer `.flatMap(…)` over `.map(…).flat()`. *(fixable)* diff --git a/rules/prefer-array-some.js b/rules/prefer-array-some.js new file mode 100644 index 0000000000..4edd991905 --- /dev/null +++ b/rules/prefer-array-some.js @@ -0,0 +1,53 @@ +'use strict'; +const getDocumentationUrl = require('./utils/get-documentation-url'); +const methodSelector = require('./utils/method-selector'); + +const MESSAGE_ID_ERROR = 'error'; +const MESSAGE_ID_SUGGESTION = 'suggestion'; +const messages = { + [MESSAGE_ID_ERROR]: 'Prefer `.some(…)` over `.find(…)`.', + [MESSAGE_ID_SUGGESTION]: 'Replace `.find(…)` with `.some(…)`.', +}; + +const selector = [ + ':matches(IfStatement, ConditionalExpression)', + '>', + methodSelector({ + name: 'find', + min: 1, + max: 2 + }), + '.test', + '>', + '.callee', + '>', + '.property', +].join(''); + +const create = context => { + return { + [selector](node) { + context.report({ + node, + messageId: MESSAGE_ID_ERROR, + suggest: [ + { + messageId: MESSAGE_ID_SUGGESTION, + fix: fixer => fixer.replaceText(node, 'some') + } + ] + }); + } + } +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + messages + } +}; diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js new file mode 100644 index 0000000000..88ffb0b31c --- /dev/null +++ b/test/prefer-array-some.js @@ -0,0 +1,120 @@ +import {outdent} from 'outdent'; +import {flatten} from 'lodash'; +import {test} from './utils/test'; + +const MESSAGE_ID_ERROR = 'error'; +const MESSAGE_ID_SUGGESTION = 'suggestion'; + +const invalidCase = ({code, suggestionOutput}) => ({ + code, + output: code, + errors: [ + { + messageId: MESSAGE_ID_ERROR, + suggestions: [ + { + messageId: MESSAGE_ID_SUGGESTION, + output: suggestionOutput + } + ] + } + ] +}); + +test({ + valid: [ + 'if (foo.some(fn)) {}', + 'if (foo.every(fn)) {}', + + // Not `IfStatement`/`ConditionalExpression` `.test` + 'true ? foo.find(fn) : foo.find(fn)', + 'if (true) foo.find(fn); else foo.find(fn);', + 'if (true) { foo.find(fn); } else { foo.find(fn); }', + + // Not matched `CallExpression` + ...flatten( + [ + // Not `CallExpression` + 'new foo.find(fn)', + // Not `MemberExpression` + 'find(fn)', + // `callee.property` is not a `Identifier` + 'foo["find"](fn)', + 'foo["fi" + "nd"](fn)', + 'foo[`find`](fn)', + // Computed + 'foo[map](fn)', + // Not `.find` + 'foo.notFind(fn)', + // More or less argument(s) + 'foo.find()', + 'foo.find(fn, thisArgument, extraArgument)', + 'foo.find(...argumentsArray)' + ].map(code => [ + `${code} ? 1 : 2`, + `if (${code}) {}`, + ]) + ) + ], + invalid: [ + invalidCase({ + code: 'if (foo.find(fn)) {}', + suggestionOutput: 'if (foo.some(fn)) {}' + }), + invalidCase({ + code: 'if (foo.find(fn, thisArgument)) {}', + suggestionOutput: 'if (foo.some(fn, thisArgument)) {}' + }), + invalidCase({ + code: 'if (foo().bar.find(fn)) {}', + suggestionOutput: 'if (foo().bar.some(fn)) {}' + }), + invalidCase({ + code: 'console.log(foo.find(fn) ? a : b)', + suggestionOutput: 'console.log(foo.some(fn) ? a : b)' + }), + // Comments + invalidCase({ + code: 'console.log(foo /* comment 1 */ . /* comment 2 */ find /* comment 3 */ (fn) ? a : b)', + suggestionOutput: 'console.log(foo /* comment 1 */ . /* comment 2 */ some /* comment 3 */ (fn) ? a : b)' + }), + // This should not be reported, but `jQuery.find()` is always `truly`, + // Nobody use it in `IfStatement.test` + invalidCase({ + code: 'if(jQuery.find(".outer > div")) {}', + suggestionOutput: `if(jQuery.some(".outer > div")) {}` + }), + // Actual messages + { + code: 'if (foo.find(fn)) {}', + output: 'if (foo.find(fn)) {}', + errors: [ + { + message: 'Prefer `.some(…)` over `.find(…)`.', + suggestions: [ + { + desc: 'Replace `.find(…)` with `.some(…)`.', + output: 'if (foo.some(fn)) {}' + } + ] + } + ] + }, + ] +}); + +test.visualize([ + 'if (array.find(element => element === "πŸ¦„")) {}', + 'const foo = array.find(element => element === "πŸ¦„") ? bar : baz;', + outdent` + if ( + array + /* correct */.find(element => Array.isArray(element)) + /* incorrect */.find(element => element/* incorrect */.find(fn) ? 1 : 0) + ) { + console.log(jQuery/* correct */.find('div')); + } else { + console.log(array/* incorrect */.find(fn) ? 'yes' : 'no'); + } + ` +]); diff --git a/test/snapshots/prefer-array-some.js.md b/test/snapshots/prefer-array-some.js.md new file mode 100644 index 0000000000..a8843613d2 --- /dev/null +++ b/test/snapshots/prefer-array-some.js.md @@ -0,0 +1,65 @@ +# Snapshot report for `test/prefer-array-some.js` + +The actual snapshot is saved in `prefer-array-some.js.snap`. + +Generated by [AVA](https://avajs.dev). + +## prefer-array-some - #1 + +> Snapshot 1 + + `␊ + Error 1/1:␊ + > 1 | if (array.find(element => element === "πŸ¦„")) {}␊ + | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ + ` + +## prefer-array-some - #2 + +> Snapshot 1 + + `␊ + Error 1/1:␊ + > 1 | const foo = array.find(element => element === "πŸ¦„") ? bar : baz;␊ + | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ + ` + +## prefer-array-some - #3 + +> Snapshot 1 + + `␊ + Error 1/3:␊ + 1 | if (␊ + 2 | array␊ + 3 | /* correct */.find(element => Array.isArray(element))␊ + > 4 | /* incorrect */.find(element => element/* incorrect */.find(fn) ? 1 : 0)␊ + | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ + 5 | ) {␊ + 6 | console.log(jQuery/* correct */.find('div'));␊ + 7 | } else {␊ + 8 | console.log(array/* incorrect */.find(fn) ? 'yes' : 'no');␊ + 9 | }␊ + Error 2/3:␊ + 1 | if (␊ + 2 | array␊ + 3 | /* correct */.find(element => Array.isArray(element))␊ + > 4 | /* incorrect */.find(element => element/* incorrect */.find(fn) ? 1 : 0)␊ + | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ + 5 | ) {␊ + 6 | console.log(jQuery/* correct */.find('div'));␊ + 7 | } else {␊ + 8 | console.log(array/* incorrect */.find(fn) ? 'yes' : 'no');␊ + 9 | }␊ + Error 3/3:␊ + 1 | if (␊ + 2 | array␊ + 3 | /* correct */.find(element => Array.isArray(element))␊ + 4 | /* incorrect */.find(element => element/* incorrect */.find(fn) ? 1 : 0)␊ + 5 | ) {␊ + 6 | console.log(jQuery/* correct */.find('div'));␊ + 7 | } else {␊ + > 8 | console.log(array/* incorrect */.find(fn) ? 'yes' : 'no');␊ + | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ + 9 | }␊ + ` diff --git a/test/snapshots/prefer-array-some.js.snap b/test/snapshots/prefer-array-some.js.snap new file mode 100644 index 0000000000000000000000000000000000000000..780784f4e2e60eceeb8ce60c03a4bd0c287cb8f0 GIT binary patch literal 466 zcmV;@0WJPPRzV^lJs2zs7TNuHj=0F7( z(tDR}tV`PBq!wq%`iWJC5iF{~1~QKwte=sUL6E6|fy=e1D8ER-P~Xss%TB>ip++Gy zO+h2Es3@^gFD)}KMI$vQH8(Y{M8Vch0nW6wRZ#jce_4x?rlvx5Etdii)F>c891ti3 z6s4x67AYj?73b%sYCLLKrkS9SUj`Ndn+q1v<3e%HOf1ex&d)0@QAo?rSFlwe+F1(r z3Q37Y3RXa}${NFc6ua{g3*4Q?RzTxn!3Ja+0hyd&cLF)aAP%R#7SIz#MXAXp3R?OY zu6G3cDYF<%!z45{fxb6^>CVih5{!iC8408UgD98Yh(qMrWRFV@vVAFW|_LCCMY7zftqT8ZZ1v*n_+=&1~}yK_(i=k zwOAeKAN9O^b+Cb!AOoSXVKfY4gGexZBF&g~k)}ZXNTX&*+EF1S!HI~ Date: Wed, 21 Oct 2020 15:59:57 +0800 Subject: [PATCH 02/14] Style --- rules/prefer-array-some.js | 6 +++--- test/prefer-array-some.js | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rules/prefer-array-some.js b/rules/prefer-array-some.js index 4edd991905..fd1bfface7 100644 --- a/rules/prefer-array-some.js +++ b/rules/prefer-array-some.js @@ -6,7 +6,7 @@ const MESSAGE_ID_ERROR = 'error'; const MESSAGE_ID_SUGGESTION = 'suggestion'; const messages = { [MESSAGE_ID_ERROR]: 'Prefer `.some(…)` over `.find(…)`.', - [MESSAGE_ID_SUGGESTION]: 'Replace `.find(…)` with `.some(…)`.', + [MESSAGE_ID_SUGGESTION]: 'Replace `.find(…)` with `.some(…)`.' }; const selector = [ @@ -21,7 +21,7 @@ const selector = [ '>', '.callee', '>', - '.property', + '.property' ].join(''); const create = context => { @@ -38,7 +38,7 @@ const create = context => { ] }); } - } + }; }; module.exports = { diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js index 88ffb0b31c..415cd03972 100644 --- a/test/prefer-array-some.js +++ b/test/prefer-array-some.js @@ -26,7 +26,7 @@ test({ 'if (foo.some(fn)) {}', 'if (foo.every(fn)) {}', - // Not `IfStatement`/`ConditionalExpression` `.test` + // Not `{IfStatement,ConditionalExpression}.test` 'true ? foo.find(fn) : foo.find(fn)', 'if (true) foo.find(fn); else foo.find(fn);', 'if (true) { foo.find(fn); } else { foo.find(fn); }', @@ -52,7 +52,7 @@ test({ 'foo.find(...argumentsArray)' ].map(code => [ `${code} ? 1 : 2`, - `if (${code}) {}`, + `if (${code}) {}` ]) ) ], @@ -82,7 +82,7 @@ test({ // Nobody use it in `IfStatement.test` invalidCase({ code: 'if(jQuery.find(".outer > div")) {}', - suggestionOutput: `if(jQuery.some(".outer > div")) {}` + suggestionOutput: 'if(jQuery.some(".outer > div")) {}' }), // Actual messages { @@ -99,7 +99,7 @@ test({ ] } ] - }, + } ] }); From 547f20141f27e84fd69ff42ab0ec5aa081f966db Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 21 Oct 2020 16:15:52 +0800 Subject: [PATCH 03/14] Support `ForStatement, WhileStatement, DoWhileStatement` --- docs/rules/prefer-array-some.md | 12 ++++++++++++ rules/prefer-array-some.js | 2 +- test/prefer-array-some.js | 29 +++++++++++++++++++++++------ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/docs/rules/prefer-array-some.md b/docs/rules/prefer-array-some.md index ad25839f8e..9cae504135 100644 --- a/docs/rules/prefer-array-some.md +++ b/docs/rules/prefer-array-some.md @@ -13,6 +13,12 @@ if (array.find(element => element === 'πŸ¦„')) { const foo = array.find(element => element === 'πŸ¦„') ? bar : baz; ``` +```js +while (array.find(element => element === 'πŸ¦„')) { + array.shift(); +} +``` + ## Pass ```js @@ -27,3 +33,9 @@ const foo = array.some(element => element === 'πŸ¦„') ? bar : baz; ```js const foo = bar ? array.find(element => element === 'πŸ¦„') : ''; ``` + +```js +while (array.some(element => element === 'πŸ¦„')) { + array.shift(); +} +``` diff --git a/rules/prefer-array-some.js b/rules/prefer-array-some.js index fd1bfface7..8dc4dbe9e1 100644 --- a/rules/prefer-array-some.js +++ b/rules/prefer-array-some.js @@ -10,7 +10,7 @@ const messages = { }; const selector = [ - ':matches(IfStatement, ConditionalExpression)', + ':matches(IfStatement, ConditionalExpression, ForStatement, WhileStatement, DoWhileStatement)', '>', methodSelector({ name: 'find', diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js index 415cd03972..e5f21b64d1 100644 --- a/test/prefer-array-some.js +++ b/test/prefer-array-some.js @@ -26,10 +26,15 @@ test({ 'if (foo.some(fn)) {}', 'if (foo.every(fn)) {}', - // Not `{IfStatement,ConditionalExpression}.test` - 'true ? foo.find(fn) : foo.find(fn)', + // Not `{IfStatement, ConditionalExpression, ForStatement, WhileStatement, DoWhileStatement}.test` 'if (true) foo.find(fn); else foo.find(fn);', 'if (true) { foo.find(fn); } else { foo.find(fn); }', + 'true ? foo.find(fn) : foo.find(fn)', + 'for (foo.find(fn); true; foo.find(fn)) foo.find(fn);', + 'while(true) foo.find(fn);', + 'do foo.find(fn) ; while(true)', + // `SwitchCase.test` + 'switch (foo.find(fn)){ case foo.find(fn): foo.find(fn)}', // Not matched `CallExpression` ...flatten( @@ -61,6 +66,22 @@ test({ code: 'if (foo.find(fn)) {}', suggestionOutput: 'if (foo.some(fn)) {}' }), + invalidCase({ + code: 'console.log(foo.find(fn) ? a : b)', + suggestionOutput: 'console.log(foo.some(fn) ? a : b)' + }), + invalidCase({ + code: 'for(;foo.find(fn);) foo.shift();', + suggestionOutput: 'for(;foo.some(fn);) foo.shift();' + }), + invalidCase({ + code: 'while(foo.find(fn)) foo.shift();', + suggestionOutput: 'while(foo.some(fn)) foo.shift();' + }), + invalidCase({ + code: 'do {foo.shift();} while(foo.find(fn));', + suggestionOutput: 'do {foo.shift();} while(foo.some(fn));' + }), invalidCase({ code: 'if (foo.find(fn, thisArgument)) {}', suggestionOutput: 'if (foo.some(fn, thisArgument)) {}' @@ -69,10 +90,6 @@ test({ code: 'if (foo().bar.find(fn)) {}', suggestionOutput: 'if (foo().bar.some(fn)) {}' }), - invalidCase({ - code: 'console.log(foo.find(fn) ? a : b)', - suggestionOutput: 'console.log(foo.some(fn) ? a : b)' - }), // Comments invalidCase({ code: 'console.log(foo /* comment 1 */ . /* comment 2 */ find /* comment 3 */ (fn) ? a : b)', From 8b07dda6fc809f6891d03b0d1145db81bc7fb2b6 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 21 Oct 2020 16:21:31 +0800 Subject: [PATCH 04/14] Fix readme.md --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 13f33c17b7..7a500832de 100644 --- a/readme.md +++ b/readme.md @@ -33,7 +33,6 @@ Configure it in `package.json`. "unicorn" ], "rules": { - "unicorn/prefer-array-some": "error", "unicorn/better-regex": "error", "unicorn/catch-error-name": "error", "unicorn/consistent-function-scoping": "error", @@ -69,6 +68,7 @@ Configure it in `package.json`. "unicorn/numeric-separators-style": "off", "unicorn/prefer-add-event-listener": "error", "unicorn/prefer-array-find": "error", + "unicorn/prefer-array-some": "error", "unicorn/prefer-dataset": "error", "unicorn/prefer-event-key": "error", "unicorn/prefer-flat-map": "error", From 6d519c60d8ba0f9ecc98ca60a5a012eefa9b1e49 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 21 Oct 2020 16:30:39 +0800 Subject: [PATCH 05/14] Fix test --- test/prefer-array-some.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js index e5f21b64d1..3a0112a1fd 100644 --- a/test/prefer-array-some.js +++ b/test/prefer-array-some.js @@ -48,7 +48,7 @@ test({ 'foo["fi" + "nd"](fn)', 'foo[`find`](fn)', // Computed - 'foo[map](fn)', + 'foo[find](fn)', // Not `.find` 'foo.notFind(fn)', // More or less argument(s) From d1a7664654af8a791a649dea4941d160aa2c23f0 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Wed, 21 Oct 2020 12:38:25 +0200 Subject: [PATCH 06/14] Update prefer-array-some.md --- docs/rules/prefer-array-some.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/rules/prefer-array-some.md b/docs/rules/prefer-array-some.md index 9cae504135..1ff5de0d03 100644 --- a/docs/rules/prefer-array-some.md +++ b/docs/rules/prefer-array-some.md @@ -1,11 +1,12 @@ # Prefer `.some(…)` over `.find(…)`. -Prefer use [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) over [`Array#find`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) when testing array. +Prefer using [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) over [`Array#find`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) when testing array. ## Fail ```js if (array.find(element => element === 'πŸ¦„')) { + // … } ``` @@ -23,6 +24,7 @@ while (array.find(element => element === 'πŸ¦„')) { ```js if (array.some(element => element === 'πŸ¦„')) { + // … } ``` From 1e9dfe00a2f2d0103820badfb83ab846bf87d730 Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 24 Dec 2020 09:17:33 +0800 Subject: [PATCH 07/14] Extract functions from `explicit-length-check.js` --- rules/explicit-length-check.js | 66 +-------------------------- rules/utils/boolean.js | 83 ++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 65 deletions(-) create mode 100644 rules/utils/boolean.js diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 1cb404478f..c8c35fba9a 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -2,6 +2,7 @@ const {isParenthesized} = require('eslint-utils'); const getDocumentationUrl = require('./utils/get-documentation-url'); const isLiteralValue = require('./utils/is-literal-value'); +const {isBooleanNode, getBooleanAncestor} = require('./boolean'); const TYPE_NON_ZERO = 'non-zero'; const TYPE_ZERO = 'zero'; @@ -12,23 +13,6 @@ const messages = { [MESSAGE_ID_SUGGESTION]: 'Replace `.length` with `.length {{code}}`.' }; -const isLogicNot = node => - node && - node.type === 'UnaryExpression' && - node.operator === '!'; -const isLogicNotArgument = node => - isLogicNot(node.parent) && - node.parent.argument === node; -const isBooleanCall = node => - node && - node.type === 'CallExpression' && - node.callee && - node.callee.type === 'Identifier' && - node.callee.name === 'Boolean' && - node.arguments.length === 1; -const isBooleanCallArgument = node => - isBooleanCall(node.parent) && - node.parent.arguments[0] === node; const isCompareRight = (node, operator, value) => node.type === 'BinaryExpression' && node.operator === operator && @@ -72,23 +56,6 @@ const lengthSelector = [ '[property.name="length"]' ].join(''); -function getBooleanAncestor(node) { - let isNegative = false; - // eslint-disable-next-line no-constant-condition - while (true) { - if (isLogicNotArgument(node)) { - isNegative = !isNegative; - node = node.parent; - } else if (isBooleanCallArgument(node)) { - node = node.parent; - } else { - break; - } - } - - return {node, isNegative}; -} - function getLengthCheckNode(node) { node = node.parent; @@ -135,37 +102,6 @@ function getLengthCheckNode(node) { return {}; } -function isBooleanNode(node) { - if ( - isLogicNot(node) || - isLogicNotArgument(node) || - isBooleanCall(node) || - isBooleanCallArgument(node) - ) { - return true; - } - - const {parent} = node; - if ( - ( - parent.type === 'IfStatement' || - parent.type === 'ConditionalExpression' || - parent.type === 'WhileStatement' || - parent.type === 'DoWhileStatement' || - parent.type === 'ForStatement' - ) && - parent.test === node - ) { - return true; - } - - if (parent.type === 'LogicalExpression') { - return isBooleanNode(parent); - } - - return false; -} - function create(context) { const options = { 'non-zero': 'greater-than', diff --git a/rules/utils/boolean.js b/rules/utils/boolean.js new file mode 100644 index 0000000000..aa63562a6e --- /dev/null +++ b/rules/utils/boolean.js @@ -0,0 +1,83 @@ +'use strict'; + +const isLogicNot = node => + node && + node.type === 'UnaryExpression' && + node.operator === '!'; +const isLogicNotArgument = node => + isLogicNot(node.parent) && + node.parent.argument === node; +const isBooleanCallArgument = node => + isBooleanCall(node.parent) && + node.parent.arguments[0] === node; +const isBooleanCall = node => + node && + node.type === 'CallExpression' && + node.callee && + node.callee.type === 'Identifier' && + node.callee.name === 'Boolean' && + node.arguments.length === 1; + +/** +Check if the value of node is a `boolean`. + +@param {Node} node +@returns {boolean} +*/ +function isBooleanNode(node) { + if ( + isLogicNot(node) || + isLogicNotArgument(node) || + isBooleanCall(node) || + isBooleanCallArgument(node) + ) { + return true; + } + + const {parent} = node; + if ( + ( + parent.type === 'IfStatement' || + parent.type === 'ConditionalExpression' || + parent.type === 'WhileStatement' || + parent.type === 'DoWhileStatement' || + parent.type === 'ForStatement' + ) && + parent.test === node + ) { + return true; + } + + if (parent.type === 'LogicalExpression') { + return isBooleanNode(parent); + } + + return false; +} + +/** +Check if value a node is a `boolean`. + +@typedef {{ node: Node, isNegative: boolean }} Result + +@param {Node} node +@returns {Result} +*/ +function getBooleanAncestor(node) { + let isNegative = false; + // eslint-disable-next-line no-constant-condition + while (true) { + if (isLogicNotArgument(node)) { + isNegative = !isNegative; + node = node.parent; + } else if (isBooleanCallArgument(node)) { + node = node.parent; + } else { + break; + } + } + + return {node, isNegative}; +} + +module.exports = {isBooleanNode, getBooleanAncestor}; From e36d1c84d2c310d2c4205ae5930444fbd81ea3bd Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 24 Dec 2020 09:43:10 +0800 Subject: [PATCH 08/14] Update logic --- docs/rules/prefer-array-some.md | 20 +--- rules/explicit-length-check.js | 2 +- rules/prefer-array-some.js | 45 ++++----- test/prefer-array-some.js | 111 ++++++++--------------- test/snapshots/prefer-array-some.js.md | 39 ++------ test/snapshots/prefer-array-some.js.snap | Bin 466 -> 385 bytes 6 files changed, 71 insertions(+), 146 deletions(-) diff --git a/docs/rules/prefer-array-some.md b/docs/rules/prefer-array-some.md index 1ff5de0d03..f0a7729360 100644 --- a/docs/rules/prefer-array-some.md +++ b/docs/rules/prefer-array-some.md @@ -1,6 +1,6 @@ # Prefer `.some(…)` over `.find(…)`. -Prefer using [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) over [`Array#find`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) when testing array. +Prefer using [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) over [`Array#find`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) when ensuring at least one element in the array passes a given check. ## Fail @@ -15,9 +15,7 @@ const foo = array.find(element => element === 'πŸ¦„') ? bar : baz; ``` ```js -while (array.find(element => element === 'πŸ¦„')) { - array.shift(); -} +const foo = array.find(element => element === 'πŸ¦„') || bar; ``` ## Pass @@ -27,17 +25,3 @@ if (array.some(element => element === 'πŸ¦„')) { // … } ``` - -```js -const foo = array.some(element => element === 'πŸ¦„') ? bar : baz; -``` - -```js -const foo = bar ? array.find(element => element === 'πŸ¦„') : ''; -``` - -```js -while (array.some(element => element === 'πŸ¦„')) { - array.shift(); -} -``` diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index c8c35fba9a..9bdcabd154 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -2,7 +2,7 @@ const {isParenthesized} = require('eslint-utils'); const getDocumentationUrl = require('./utils/get-documentation-url'); const isLiteralValue = require('./utils/is-literal-value'); -const {isBooleanNode, getBooleanAncestor} = require('./boolean'); +const {isBooleanNode, getBooleanAncestor} = require('./utils/boolean'); const TYPE_NON_ZERO = 'non-zero'; const TYPE_ZERO = 'zero'; diff --git a/rules/prefer-array-some.js b/rules/prefer-array-some.js index 8dc4dbe9e1..4ec83b6bfd 100644 --- a/rules/prefer-array-some.js +++ b/rules/prefer-array-some.js @@ -1,6 +1,7 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); const methodSelector = require('./utils/method-selector'); +const {isBooleanNode} = require('./utils/boolean'); const MESSAGE_ID_ERROR = 'error'; const MESSAGE_ID_SUGGESTION = 'suggestion'; @@ -9,34 +10,28 @@ const messages = { [MESSAGE_ID_SUGGESTION]: 'Replace `.find(…)` with `.some(…)`.' }; -const selector = [ - ':matches(IfStatement, ConditionalExpression, ForStatement, WhileStatement, DoWhileStatement)', - '>', - methodSelector({ - name: 'find', - min: 1, - max: 2 - }), - '.test', - '>', - '.callee', - '>', - '.property' -].join(''); +const arrayFindCallSelector = methodSelector({ + name: 'find', + min: 1, + max: 2 +}); const create = context => { return { - [selector](node) { - context.report({ - node, - messageId: MESSAGE_ID_ERROR, - suggest: [ - { - messageId: MESSAGE_ID_SUGGESTION, - fix: fixer => fixer.replaceText(node, 'some') - } - ] - }); + [arrayFindCallSelector](node) { + if (isBooleanNode(node) || node.parent.type === 'LogicalExpression') { + node = node.callee.property; + context.report({ + node, + messageId: MESSAGE_ID_ERROR, + suggest: [ + { + messageId: MESSAGE_ID_SUGGESTION, + fix: fixer => fixer.replaceText(node, 'some') + } + ] + }); + } } }; }; diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js index 3a0112a1fd..c48d0becfe 100644 --- a/test/prefer-array-some.js +++ b/test/prefer-array-some.js @@ -23,83 +23,53 @@ const invalidCase = ({code, suggestionOutput}) => ({ test({ valid: [ - 'if (foo.some(fn)) {}', - 'if (foo.every(fn)) {}', - - // Not `{IfStatement, ConditionalExpression, ForStatement, WhileStatement, DoWhileStatement}.test` - 'if (true) foo.find(fn); else foo.find(fn);', - 'if (true) { foo.find(fn); } else { foo.find(fn); }', - 'true ? foo.find(fn) : foo.find(fn)', - 'for (foo.find(fn); true; foo.find(fn)) foo.find(fn);', - 'while(true) foo.find(fn);', - 'do foo.find(fn) ; while(true)', - // `SwitchCase.test` - 'switch (foo.find(fn)){ case foo.find(fn): foo.find(fn)}', + // Not `boolean` + 'const hasValue = foo.find(fn)', // Not matched `CallExpression` - ...flatten( - [ - // Not `CallExpression` - 'new foo.find(fn)', - // Not `MemberExpression` - 'find(fn)', - // `callee.property` is not a `Identifier` - 'foo["find"](fn)', - 'foo["fi" + "nd"](fn)', - 'foo[`find`](fn)', - // Computed - 'foo[find](fn)', - // Not `.find` - 'foo.notFind(fn)', - // More or less argument(s) - 'foo.find()', - 'foo.find(fn, thisArgument, extraArgument)', - 'foo.find(...argumentsArray)' - ].map(code => [ - `${code} ? 1 : 2`, - `if (${code}) {}` - ]) - ) + ...[ + // Not `CallExpression` + 'new foo.find(fn)', + // Not `MemberExpression` + 'find(fn)', + // `callee.property` is not a `Identifier` + 'foo["find"](fn)', + 'foo["fi" + "nd"](fn)', + 'foo[`find`](fn)', + // Computed + 'foo[find](fn)', + // Not `.find` + 'foo.notFind(fn)', + // More or less argument(s) + 'foo.find()', + 'foo.find(fn, thisArgument, extraArgument)', + 'foo.find(...argumentsArray)' + ].map(code => `if (${code}) {}`) ], invalid: [ - invalidCase({ - code: 'if (foo.find(fn)) {}', - suggestionOutput: 'if (foo.some(fn)) {}' - }), - invalidCase({ - code: 'console.log(foo.find(fn) ? a : b)', - suggestionOutput: 'console.log(foo.some(fn) ? a : b)' - }), - invalidCase({ - code: 'for(;foo.find(fn);) foo.shift();', - suggestionOutput: 'for(;foo.some(fn);) foo.shift();' - }), - invalidCase({ - code: 'while(foo.find(fn)) foo.shift();', - suggestionOutput: 'while(foo.some(fn)) foo.shift();' - }), - invalidCase({ - code: 'do {foo.shift();} while(foo.find(fn));', - suggestionOutput: 'do {foo.shift();} while(foo.some(fn));' - }), - invalidCase({ - code: 'if (foo.find(fn, thisArgument)) {}', - suggestionOutput: 'if (foo.some(fn, thisArgument)) {}' - }), - invalidCase({ - code: 'if (foo().bar.find(fn)) {}', - suggestionOutput: 'if (foo().bar.some(fn)) {}' - }), + ...[ + 'const bar = !foo.find(fn)', + 'const bar = Boolean(foo.find(fn))', + 'if (foo.find(fn)) {}', + 'const bar = foo.find(fn) ? 1 : 2', + 'while (foo.find(fn)) foo.shift();', + 'do {foo.shift();} while (foo.find(fn));', + 'for (; foo.find(fn); ) foo.shift();', + 'const bar = foo.find(fn) || baz' + ].map(code => invalidCase({ + code, + suggestionOutput: code.replace('find', 'some') + })), // Comments invalidCase({ code: 'console.log(foo /* comment 1 */ . /* comment 2 */ find /* comment 3 */ (fn) ? a : b)', suggestionOutput: 'console.log(foo /* comment 1 */ . /* comment 2 */ some /* comment 3 */ (fn) ? a : b)' }), // This should not be reported, but `jQuery.find()` is always `truly`, - // Nobody use it in `IfStatement.test` + // It should not use in a LogicalExpression invalidCase({ - code: 'if(jQuery.find(".outer > div")) {}', - suggestionOutput: 'if(jQuery.some(".outer > div")) {}' + code: 'const el = anotherElement || jQuery.find(".outer > div");', + suggestionOutput: 'const el = anotherElement || jQuery.some(".outer > div");' }), // Actual messages { @@ -126,12 +96,11 @@ test.visualize([ outdent` if ( array - /* correct */.find(element => Array.isArray(element)) - /* incorrect */.find(element => element/* incorrect */.find(fn) ? 1 : 0) + .find(element => Array.isArray(element)) + // ^^^^ This should NOT report + .find(x => x === 0) + // ^^^^ This should report ) { - console.log(jQuery/* correct */.find('div')); - } else { - console.log(array/* incorrect */.find(fn) ? 'yes' : 'no'); } ` ]); diff --git a/test/snapshots/prefer-array-some.js.md b/test/snapshots/prefer-array-some.js.md index a8843613d2..9678f3c996 100644 --- a/test/snapshots/prefer-array-some.js.md +++ b/test/snapshots/prefer-array-some.js.md @@ -29,37 +29,14 @@ Generated by [AVA](https://avajs.dev). > Snapshot 1 `␊ - Error 1/3:␊ - 1 | if (␊ - 2 | array␊ - 3 | /* correct */.find(element => Array.isArray(element))␊ - > 4 | /* incorrect */.find(element => element/* incorrect */.find(fn) ? 1 : 0)␊ - | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ - 5 | ) {␊ - 6 | console.log(jQuery/* correct */.find('div'));␊ - 7 | } else {␊ - 8 | console.log(array/* incorrect */.find(fn) ? 'yes' : 'no');␊ - 9 | }␊ - Error 2/3:␊ - 1 | if (␊ - 2 | array␊ - 3 | /* correct */.find(element => Array.isArray(element))␊ - > 4 | /* incorrect */.find(element => element/* incorrect */.find(fn) ? 1 : 0)␊ - | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ - 5 | ) {␊ - 6 | console.log(jQuery/* correct */.find('div'));␊ - 7 | } else {␊ - 8 | console.log(array/* incorrect */.find(fn) ? 'yes' : 'no');␊ - 9 | }␊ - Error 3/3:␊ + Error 1/1:␊ 1 | if (␊ 2 | array␊ - 3 | /* correct */.find(element => Array.isArray(element))␊ - 4 | /* incorrect */.find(element => element/* incorrect */.find(fn) ? 1 : 0)␊ - 5 | ) {␊ - 6 | console.log(jQuery/* correct */.find('div'));␊ - 7 | } else {␊ - > 8 | console.log(array/* incorrect */.find(fn) ? 'yes' : 'no');␊ - | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ - 9 | }␊ + 3 | .find(element => Array.isArray(element))␊ + 4 | // ^^^^ This should NOT report␊ + > 5 | .find(x => x === 0)␊ + | ^^^^ Prefer `.some(…)` over `.find(…)`.␊ + 6 | // ^^^^ This should report␊ + 7 | ) {␊ + 8 | }␊ ` diff --git a/test/snapshots/prefer-array-some.js.snap b/test/snapshots/prefer-array-some.js.snap index 780784f4e2e60eceeb8ce60c03a4bd0c287cb8f0..1515e231c84edc13c727bd43c109249ec81494c0 100644 GIT binary patch delta 168 zcmV;Z09XIg1Azk~K~_N^Q*L2!b7*gLAa*he0st=`_`uNLZx};T)$Zqbo>`G1Q3LA6 z1d(=?c9Ve-*_{gTU;{FZfJ{!XJAoWy5Qh`P!H!^WWEO*In1rS#P=g6bgT6k*Wg!`v z#R|n4`K3823V!|}3Pq^}`9&qbU@=89wgO~e1=#Nfnoyr|a+2gV1qCx))asdEYkCL^T0ssJ3)IzWT delta 250 zcmVvlvXn zBs4XFzBhsC&dkH3AIik4EG-Wd5{AHVFwlgC0w*UWp`f5(3N!;4=|Gkl$TU#waSpWCfO{7FA;Lt$IpknYyMXC?d>(nreY=NG?tVn_+=&1~}yK_(i=kwOAeKAN9O^ zb+Cb!AOoSXVKfY4gGexZBF&g~k)}ZXNTX&*+EF1S!HI~ Date: Thu, 24 Dec 2020 09:44:05 +0800 Subject: [PATCH 09/14] Update example --- test/prefer-array-some.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js index c48d0becfe..286f7370a2 100644 --- a/test/prefer-array-some.js +++ b/test/prefer-array-some.js @@ -24,7 +24,7 @@ const invalidCase = ({code, suggestionOutput}) => ({ test({ valid: [ // Not `boolean` - 'const hasValue = foo.find(fn)', + 'const bar = foo.find(fn)', // Not matched `CallExpression` ...[ From 705ada02d9ee9ad19f9433426643943f62f0e2e6 Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 24 Dec 2020 09:47:50 +0800 Subject: [PATCH 10/14] Fix linting --- test/prefer-array-some.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js index 286f7370a2..9475582cc6 100644 --- a/test/prefer-array-some.js +++ b/test/prefer-array-some.js @@ -1,5 +1,4 @@ import {outdent} from 'outdent'; -import {flatten} from 'lodash'; import {test} from './utils/test'; const MESSAGE_ID_ERROR = 'error'; From 8d3e7fb16410516c8876367bd4e58f8433e09192 Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 24 Dec 2020 09:52:34 +0800 Subject: [PATCH 11/14] Do not check `LogicalExpression` --- docs/rules/prefer-array-some.md | 8 ++++---- rules/prefer-array-some.js | 2 +- test/prefer-array-some.js | 7 ++++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/rules/prefer-array-some.md b/docs/rules/prefer-array-some.md index f0a7729360..ec8d820a02 100644 --- a/docs/rules/prefer-array-some.md +++ b/docs/rules/prefer-array-some.md @@ -14,10 +14,6 @@ if (array.find(element => element === 'πŸ¦„')) { const foo = array.find(element => element === 'πŸ¦„') ? bar : baz; ``` -```js -const foo = array.find(element => element === 'πŸ¦„') || bar; -``` - ## Pass ```js @@ -25,3 +21,7 @@ if (array.some(element => element === 'πŸ¦„')) { // … } ``` + +```js +const foo = array.find(element => element === 'πŸ¦„') || bar; +``` diff --git a/rules/prefer-array-some.js b/rules/prefer-array-some.js index 4ec83b6bfd..9574787ccf 100644 --- a/rules/prefer-array-some.js +++ b/rules/prefer-array-some.js @@ -19,7 +19,7 @@ const arrayFindCallSelector = methodSelector({ const create = context => { return { [arrayFindCallSelector](node) { - if (isBooleanNode(node) || node.parent.type === 'LogicalExpression') { + if (isBooleanNode(node)) { node = node.callee.property; context.report({ node, diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js index 9475582cc6..af65160d8f 100644 --- a/test/prefer-array-some.js +++ b/test/prefer-array-some.js @@ -24,6 +24,7 @@ test({ valid: [ // Not `boolean` 'const bar = foo.find(fn)', + 'const bar = foo.find(fn) || baz', // Not matched `CallExpression` ...[ @@ -65,10 +66,10 @@ test({ suggestionOutput: 'console.log(foo /* comment 1 */ . /* comment 2 */ some /* comment 3 */ (fn) ? a : b)' }), // This should not be reported, but `jQuery.find()` is always `truly`, - // It should not use in a LogicalExpression + // It should not use as a boolean invalidCase({ - code: 'const el = anotherElement || jQuery.find(".outer > div");', - suggestionOutput: 'const el = anotherElement || jQuery.some(".outer > div");' + code: 'if (jQuery.find(".outer > div")) {}', + suggestionOutput: 'if (jQuery.some(".outer > div")) {}' }), // Actual messages { From de0a90fdd18ef91c93f485fad058c9d14c28045c Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 24 Dec 2020 09:56:48 +0800 Subject: [PATCH 12/14] Fix lint --- rules/prefer-event-key.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rules/prefer-event-key.js b/rules/prefer-event-key.js index 17c1ff501b..ee70eb413d 100644 --- a/rules/prefer-event-key.js +++ b/rules/prefer-event-key.js @@ -171,7 +171,7 @@ const create = context => { if ( references && - references.find(reference => isPropertyOf(node, reference.identifier)) + references.some(reference => isPropertyOf(node, reference.identifier)) ) { report(node); } @@ -201,7 +201,7 @@ const create = context => { // Make sure initObject is a reference of eventVariable if ( references && - references.find(reference => reference.identifier === initObject) + references.some(reference => reference.identifier === initObject) ) { report(node.value); return; From 914ba5498ee13b7b571035a26b59c0034d9426ba Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 24 Dec 2020 10:02:02 +0800 Subject: [PATCH 13/14] Fix comment --- rules/utils/boolean.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/utils/boolean.js b/rules/utils/boolean.js index aa63562a6e..6eef61c96a 100644 --- a/rules/utils/boolean.js +++ b/rules/utils/boolean.js @@ -56,7 +56,7 @@ function isBooleanNode(node) { } /** -Check if value a node is a `boolean`. +Get the boolean type-casting ancestor. @typedef {{ node: Node, isNegative: boolean }} Result From efb01cb91a595740e6c0eb21951fecefc45e6a08 Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 24 Dec 2020 10:06:07 +0800 Subject: [PATCH 14/14] Fix test --- test/prefer-array-some.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/prefer-array-some.js b/test/prefer-array-some.js index af65160d8f..d77ac245f1 100644 --- a/test/prefer-array-some.js +++ b/test/prefer-array-some.js @@ -54,8 +54,7 @@ test({ 'const bar = foo.find(fn) ? 1 : 2', 'while (foo.find(fn)) foo.shift();', 'do {foo.shift();} while (foo.find(fn));', - 'for (; foo.find(fn); ) foo.shift();', - 'const bar = foo.find(fn) || baz' + 'for (; foo.find(fn); ) foo.shift();' ].map(code => invalidCase({ code, suggestionOutput: code.replace('find', 'some')