Add consistent-existence-index-check rule

[axetroy]

Close #1024

[sindresorhus]

I think the rule should be called consistent-indexof-check.

[axetroy]

/cc @sindresorhus @fisker Now is ready for review

[sindresorhus]

#2425 (comment)

[axetroy]

#2425 (comment)

I do remember I updated it 😢

[axetroy]

Perhaps, it should be renamed to consistent-existence-index-check? since this rule check for indexOf(), lastIndexOf(), findIndex(), and findLastIndex()

[fisker]

What do you think listen on assignment const index = ...indexOf(..) then look for comparison instead?

[axetroy]

This implementation may get a better performance. (Because it searches for const index = ...indexOf(..) statement first, and then searches for BinaryExpression)

But it is also more complicated, There are countless possibilities for BinaryExpression to appear, and traverse almost the entire AST, I think it is not worth it.

for example:

const index = "foo".indexOf("o"); // ✅ Detected here

function test() {
	class Man {
		method() {
			return {
				nest () {
					if (index > -1) { // ❌ You have to traverse from the top level to here
					
					}
				},
				noop () {
					// ❌ Even if there is no node here, you must traverse here to check
				}
			}
		}
	}
}

[fisker]

It's easy to find them, since index variable already include all the references.

[axetroy]

All right, I will try it later

[axetroy]

Done in 732ae05

[axetroy]

Should we catch the case of AssignmentExpression ?

let index = 0;

// do something

index = foo.indexOf("xx")

// ...

if (index > -1) {

}

Implemented

[fisker]

Should we catch the case of AssignmentExpression ?

No, should only check declaration with const

[axetroy]

No, should only check declaration with const

Is there any reason for this?

[fisker]

We can't know how it will be used, we don't want false positives.

[axetroy]

Update in 4d88f25

[fisker]

@axetroy I simplified the rule only check index < 0, index >= 0, index > -1. I think it's enough, what do you think?

[axetroy]

@axetroy I simplified the rule only check index < 0, index >= 0, index > -1. I think it's enough, what do you think?

I think we should support as many edge cases as possible, for example

Even if index < -100 and index < 0 are no different, it would be better if it can be fixed.

Of course, if it is not easy to do, then I am fine with it.

[sindresorhus]

Perhaps, it should be renamed to consistent-existence-index-check?

👍

[axetroy]

Perhaps, it should be renamed to consistent-existence-index-check?

👍

@sindresorhus Renamed