Add `prefer-array-index-of` rule #920

Fabrice-TIERCELIN · 2020-12-06T18:24:49Z

Add prefer-indexof rule

I have successfully run npm test. I have successfully manually tested the lint and the fix on a project. I have run npm run integration but my node is too old.

Fixes #919

Add `prefer-indexof` rule

rules/prefer-indexof.js

rules/prefer-array-index-of.js

Fabrice-TIERCELIN · 2020-12-09T07:35:07Z

May node.callee.object be undefined?

fisker · 2020-12-09T07:43:53Z

No, it can't

Fabrice-TIERCELIN · 2020-12-09T07:49:53Z

Have you got common code to check extended literals like:

-8
1 + 1
'foo' + 'bar'
10 * (5 - 9)
...

?

fisker · 2020-12-09T07:51:39Z

No, you are not doing right, you should not care about the searching value, you only need care about left/right is the same as element

fisker · 2020-12-09T07:52:40Z

Meaning foo.findIndex(bar => bar === a.b.c) should be fixed to foo.indexOf(a.b.c) too, WDYT?

Fabrice-TIERCELIN · 2020-12-09T07:59:22Z

I think I should only check it's an expression that doesn't change, that is to say not that for example:

foo.findIndex(bar => bar === a.getValueAndUpdateIt())

In this case, check that it's a literal is just a first objective that can be improved in the future.

fisker · 2020-12-09T08:03:10Z

Yes, I considered that, you can use hasSideEffect to make sure it don't has SideEffect. So we can fix it, otherwise, I think we can use ESLint suggestion.

And I suggest still report it , because I can't image anyone really do something in findIndex call

rules/prefer-array-index-of.js

Fabrice-TIERCELIN · 2020-12-14T18:11:15Z

Yes, it is parsed as FunctionExpression. I thought we were using esquery 😞

Thanx for implementing the suggestion. I don't know if I have badly understood your comment or wanted not to understand it 😄

fisker · 2020-12-16T10:01:31Z

I don't know if I have badly understood your comment or wanted not to understand it

Sorry for my bad English, I'm not a native speaker.

fisker · 2020-12-16T10:02:20Z

docs/rules/prefer-array-index-of.md

@@ -0,0 +1,29 @@
+# Prefer [`Array#indexOf()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf) over [`Array#findIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex) when looking for the index of an item


We normally add links in details. and this should be the same as the one in readme.md

fisker · 2020-12-16T10:06:18Z

docs/rules/prefer-array-index-of.md

+
+```js
+[].findIndex(i => i === list[i]);
+[].findIndex(x => x === 'foo' && isValid);


This one is not valid cases anymore, please fix it.

Also, please put these in separate markdown code blocks. example https://github.com/sindresorhus/eslint-plugin-unicorn/blob/master/docs/rules/error-message.md#fail

fisker · 2020-12-23T09:37:17Z

docs/rules/prefer-array-index-of.md

+
+```js
+values.findIndex(function (x) {return x !== 'foo';});
+```


Since we don't care about the callee.object, let's use const index = foo.findIndex in these examples,

please add this to "pass" section

const index = foo.indexOf('foo');

fisker

LGTM, thank you!

sindresorhus · 2020-12-24T07:03:47Z

Can you fix the merge conflict?

fisker · 2020-12-24T07:36:04Z

docs/rules/prefer-array-index-of.md

@@ -1,6 +1,6 @@
 # Prefer [`Array#indexOf()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf) over [`Array#findIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex) when looking for the index of an item

-[`Array#findIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex) is made for more complex criteria. If you are just looking for the index where a given object is present, then the code can be reduced. It concerns any search with a literal, a variable or any expression that hasn't any side effect. However, if the expression you are looking for has side effect, relies on element related to the function (the name of the function, its arguments...), the case is still valid.
+[`Array#findIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex) is intended for more complex needs. If you are just looking for the index where the given item is present, then the code can be simplified. This applies to any search with a literal, a variable, or any expression that doesn't have any side effects. However, if the expression you are looking for has side effects, relies on element related to the function (the name of the function, its arguments, etc.), the case is still valid.


However, if the expression you are looking for has side effects, relies on element related to the function (the name of the function, its arguments, etc.), the case is still valid.

This part seems not right, the searching value has "side effects" not valid.

const index = foo.findIndex(x => x === call());

This call has "side effects", but it will still reported without "auto-fix".

I have added explicit side effects.

In getBinaryExpressionSelector, I want to add a criteria that avoids the existence of call(), something like:

:not(:has(*[expression.callee.name="call"][expression.type="CallExpression"]))

But I don't know how to tell to search only in the binary expression.

Fabrice-TIERCELIN added 4 commits December 6, 2020 18:40

Add prefer-indexof rule

cb8b632

Add `prefer-indexof` rule

Should not be here

f4fdd98

Add tests

416e3a4

Handle arrow functions with block

4568627

fisker changed the title ~~Fixes #919; Add prefer-indexof rule~~ Add prefer-indexof rule Dec 7, 2020

fisker reviewed Dec 7, 2020

View reviewed changes

rules/prefer-indexof.js Outdated Show resolved Hide resolved

Fabrice-TIERCELIN added 5 commits December 8, 2020 06:23

Rename to prefer-array-index-of

65e9c55

Rename to prefer-array-index-of

6944415

Rename to prefer-array-index-of

a2b689d

Rename to prefer-array-index-of

103eb5b

Rename to prefer-array-index-of

28dee28

Fabrice-TIERCELIN changed the title ~~Add prefer-indexof rule~~ Add prefer-array-index-of rule Dec 8, 2020

Refactor to use selector

06a03d5

fisker reviewed Dec 8, 2020

View reviewed changes

rules/prefer-array-index-of.js Show resolved Hide resolved

Fabrice-TIERCELIN marked this pull request as draft December 9, 2020 06:54

Fabrice-TIERCELIN added 3 commits December 9, 2020 08:06

New implementation with selector

1ce8761

Update prefer-array-index-of.js

d278993

Update prefer-array-index-of.js

2afc4ec

Fabrice-TIERCELIN marked this pull request as ready for review December 9, 2020 07:50

fisker reviewed Dec 9, 2020

View reviewed changes

rules/prefer-array-index-of.js Outdated Show resolved Hide resolved

Fabrice-TIERCELIN added 3 commits December 9, 2020 18:52

Reduce the fix area

558d55b

Update prefer-array-index-of.js

375016b

Update prefer-array-index-of.js

510bed0

Fabrice-TIERCELIN added 2 commits December 14, 2020 19:20

Tell it the same way as the message id

f85a611

Add details

5c081b4

fisker reviewed Dec 16, 2020

View reviewed changes

Fix the valid case and separate in blocks

8abe391

fisker requested changes Dec 23, 2020

View reviewed changes

Fabrice-TIERCELIN and others added 3 commits December 23, 2020 14:51

Reuse the text of prefer-array-index-of.md

3c4bad7

const index = foo.indexOf('foo');

00ed43c

Update docs

ae2cb32

fisker approved these changes Dec 24, 2020

View reviewed changes

Fabrice-TIERCELIN requested a review from sindresorhus December 24, 2020 05:47

Update prefer-array-index-of.md

ecb7880

fisker requested changes Dec 24, 2020

View reviewed changes

Fabrice-TIERCELIN added 2 commits December 28, 2020 08:22

Fix conflict

623a9aa

Fix conflict in readme

6220845

Fabrice-TIERCELIN marked this pull request as draft December 28, 2020 08:07

Fabrice-TIERCELIN closed this Dec 28, 2020

Fabrice-TIERCELIN deleted the prefer-indexof-over-findindex branch December 28, 2020 08:09

Fabrice-TIERCELIN restored the prefer-indexof-over-findindex branch December 28, 2020 08:34

Fabrice-TIERCELIN reopened this Dec 28, 2020

Merge branch 'master' into prefer-indexof-over-findindex

fbe1d28

Fabrice-TIERCELIN marked this pull request as ready for review December 28, 2020 08:42

Fabrice-TIERCELIN and others added 2 commits December 28, 2020 09:44

Explicit side effects

6d8e1e1

Update prefer-array-index-of.md

67ef543

fisker approved these changes Dec 29, 2020

View reviewed changes

Update prefer-array-index-of.md

92fe1ff

sindresorhus merged commit 517a782 into sindresorhus:master Jan 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `prefer-array-index-of` rule #920

Add `prefer-array-index-of` rule #920

Fabrice-TIERCELIN commented Dec 6, 2020 •

edited by fisker

Fabrice-TIERCELIN commented Dec 9, 2020

fisker commented Dec 9, 2020

Fabrice-TIERCELIN commented Dec 9, 2020

fisker commented Dec 9, 2020

fisker commented Dec 9, 2020

Fabrice-TIERCELIN commented Dec 9, 2020

fisker commented Dec 9, 2020 •

edited

Fabrice-TIERCELIN commented Dec 14, 2020

fisker commented Dec 16, 2020

fisker Dec 16, 2020 •

edited

fisker Dec 16, 2020

fisker Dec 23, 2020

fisker left a comment

sindresorhus commented Dec 24, 2020

fisker Dec 24, 2020

Fabrice-TIERCELIN Dec 28, 2020

Fabrice-TIERCELIN Dec 29, 2020

		@@ -0,0 +1,29 @@
		# Prefer [`Array#indexOf()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf) over [`Array#findIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex) when looking for the index of an item

Add prefer-array-index-of rule #920

Add prefer-array-index-of rule #920

Conversation

Fabrice-TIERCELIN commented Dec 6, 2020 • edited by fisker

Fabrice-TIERCELIN commented Dec 9, 2020

fisker commented Dec 9, 2020

Fabrice-TIERCELIN commented Dec 9, 2020

fisker commented Dec 9, 2020

fisker commented Dec 9, 2020

Fabrice-TIERCELIN commented Dec 9, 2020

fisker commented Dec 9, 2020 • edited

Fabrice-TIERCELIN commented Dec 14, 2020

fisker commented Dec 16, 2020

fisker Dec 16, 2020 • edited

Choose a reason for hiding this comment

fisker Dec 16, 2020

Choose a reason for hiding this comment

fisker Dec 23, 2020

Choose a reason for hiding this comment

fisker left a comment

Choose a reason for hiding this comment

sindresorhus commented Dec 24, 2020

fisker Dec 24, 2020

Choose a reason for hiding this comment

Fabrice-TIERCELIN Dec 28, 2020

Choose a reason for hiding this comment

Fabrice-TIERCELIN Dec 29, 2020

Choose a reason for hiding this comment

Add `prefer-array-index-of` rule #920

Add `prefer-array-index-of` rule #920

Fabrice-TIERCELIN commented Dec 6, 2020 •

edited by fisker

fisker commented Dec 9, 2020 •

edited

fisker Dec 16, 2020 •

edited