Rule proposal: prefer-set-has

[fisker]

Enforce use Set for existence check over Array#includes.

Fail

const array = ['foo', 'bar'];

if (array.includes('foo')) {}

Pass

const set = new Set(['foo', 'bar']);

if (set.has('foo')) {}

This rule might should not fail on this

if (['foo', 'bar'].includes('foo')) {}

Maybe only check variables and check references. Also perhaps the rule name prefer-set-has ?

real world case:

eslint-plugin-unicorn/rules/prefer-node-append.js

Lines 6 to 17 in d638e54

	const ignoredParentTypes = [
	'ArrayExpression',
	'IfStatement',
	'MemberExpression',
	'Property',
	'ReturnStatement',
	'VariableDeclarator'
	];
	const ignoredGrandparentTypes = [
	'ExpressionStatement'
	];

[yakov116]

@fisker your would need to know that the array is never used anywhere else. Maybe items are pushed on laster that will have duplicate values.

[fisker]

@yakov116 Yes, need check all references only called #includes() or #indexOf(), and not exported.

[sindresorhus]

Also perhaps the rule name prefer-set-has ?

👍

Yes, need check all references only called #includes() or #indexOf(), and not exported.

I don't think it could consider indexOf as it's not just used for existence checks, and we already have a rule to enforce includes over indexOf.

---

I think this would be a useful rule, but I don't think it should be auto-fixable.

[sindresorhus]

Accepted

[fisker]

@sindresorhus Why do you think it should not auto-fixable?

I've made a prototype works fine with auto-fix.

[sindresorhus]

I guess if you can confidently check all references, it's fine.

[silverwind]

What would be the limitations of this rule? Say a external API passes a Array. Would the rule require me to convert that to a Set first? Or does it only operate on arrays declared via literals?

[fisker]

@silverwind I'm going to make it only work on ArrayExpression first, so external Array will not reported. You can help review #604

Update: I didn't exclude SpreadElement, so

Pass

import externalArray from './some-where';

const array = externalArray;
if (array.includes('foo')) {}

Fail

import externalArray from './some-where';

const array = [...externalArray]; // `array` check result can't be effected by modifying `externalArray`
if (array.includes('foo')) {}

[silverwind]

I also wonder for cases where I have a small Array which I need to check for existance of an item but which I'd then need to pass on as Array to an external consumer. I imagine the conversation to Set and back may end up being worse performance than just calling .includes on a short array.

[fisker]

You can alway use eslint-disable to disable it on small array, or we add can option to do add length check later.

[silverwind]

Yeah of course. I generally try to avoid using eslint-disable, but this rule should still be good for manually checking a code base in a one-off fashion, but probably not something I'd generally enable.