Rule proposal: Prefer String#includes over String#indexOf

[sindresorhus]

With ES2015, it's more succinct to use 'foobar'.includes('foo') instead of 'foobar'.indexOf('foo') !== -1 or 'foobar'.indexOf('foo') >= 0 for checking whether a string contains a substring. I would like a rule that enforces this in those two scenarios and the negative cases of that: 'foobar'.indexOf('foo') === -1 and 'foobar'.indexOf('foo') < 0.

Suggested rule name: prefer-string-includes

I think this would be a good rule as .includes makes the code intent clearer.

There's already other rules that enforces new ES2015 features, so it seems natural to have a rule for this too.

[eslintbot]

Thanks for the issue! If you're reporting a bug, please be sure to include:

1. The version of ESLint you are using (run eslint -v)
2. What you did (the source code and ESLint configuration)
3. The actual ESLint output complete with numbers
4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

[nzakas]

Seems reasonable. Do you want to submit a PR?

[nzakas]

Looking at this again, it looks like this would only be possible with string literals since we have no way of knowing if a variable contains a string or not. Given that, I think I was too hasty in marking this accepted, as I don't think there's a good way to do this nor do I think a rule that applies only to string literals is all that useful (since if you have a string literal, you can see if a substring is included).

When includes is added for arrays, we can probably look at a more general rule that always suggests it regardless of type. It seems we are just too early for that.

[sindresorhus]

I guess this is yet another case where some kind of type inference would be useful.

[nzakas]

Yup. Or alternately, if every type that has indexOf also has includes, then the type doesn't matter. Sadly since Array.prototype.includes didn't make it into ES6, we are left with weirdness.

[sindresorhus]

TypedArray also has includes btw (ES2016). Even though Array#includes is ES2016, it's already in Chrome 47, Firefox 43, Safari 9.

[xjamundx]

We have a stupidly simple version of this that we use at PayPal:

module.exports = function(context) {
     return {
        'MemberExpression': function(node) {
            if (node.property.name === 'indexOf') {
                context.report(node, 'Expected .includes() instead of .indexOf().');
            }
        }
    };
};

[nzakas]

Yeah, it's not hard to make, it's just inaccurate, which is why we can't have it as part of core right now.

[platinumazure]

@nzakas Noticing this issue is open and has "help wanted" label, but not "accepted" (and comments seem to indicate this won't be accepted). Should the issue be closed and/or should the "help wanted" label be removed?

[nzakas]

Hmm, good point. Should be closed, thanks.

[sindresorhus]

@nzakas I noticed you added the array-callback-return rule, which says:

This rule checks callback functions of methods with the given names, even if the object which has the method is not an array.

That was pretty much the main argument against this rule, so would you be willing to reconsider this?

[nzakas]

Not quite the same, but since it looks like includes is making its way into browsers for arrays, it's probably less of an issue now.

Even so, I'm not sure this is important enough to be a core rule. I'll reopen to see if anyone feels differently.

[nzakas]

Okay, it's been another three months without movement, so closing.

[rodrigok]

@alxndr are you planing to submit a PR?

[alxndr]

I wasn't, but I can make one if there's still interest.

[ilyavolodin]

This can now be implemented as an option for no-restricted-syntax rule:

"no-restricted-syntax": ["error", "BinaryExpression > CallExpression > MemberExpression > Identifier[name = 'indexOf']"]

So I don't think this should be added to the core.

[vitorbal]

Agreed with @ilyavolodin, especially since very soon you will also be able to provide custom messages for no-restricted-syntax: #8298

[mnoorenberghe]

This can now be implemented as an option for no-restricted-syntax rule:

"no-restricted-syntax": ["error", "BinaryExpression > CallExpression > MemberExpression > Identifier[name = 'indexOf']"]

That doesn't address the original comment of only being an error when comparing with -1 and 0 since indexOf is the correct tool otherwise. Does "no-restricted-syntax" handle that?

[rodrigok]

I don't think no-restricted-syntax as a good option for that, or we should use it for all rules 😄
It can be a work around, but will not be easy to configure for important things like replace indexOf by the new method includes