Rule proposal: prefer-negative-index for things like foo.slice

[MrHen]

See https://github.com/sindresorhus/eslint-plugin-unicorn/pull/409/files#diff-1e364de46e721b05ccf3d616d4248ebdR37

Functions that support negative indexing should use that feature instead of calculating from length.

Fail

foo.slice(1, foo.length - 1);

Pass

foo.slice(1, -1);

Functions

• Array.prototype.includes but NOT String.prototype.includes
• Array.prototype.indexOf but NOT String.prototype.indexOf
• Array.prototype.lastIndexOf but NOT String.prototype.lastIndexOf
• Array.prototype.slice and String.prototype.slice
• Array.prototype.splice

[fisker]

It's good to have this rule to prevent this accident happen.

Can't remember any other method use negative index except substr(should not use at all)

[MrHen]

I added a few examples of where this is supported. Surprisingly, Array and String have different support.

[sindresorhus]

Good idea!

[fisker]

test on chrome

includes

'foobar'.includes('b', -5) // true
'foobar'.split('').includes('b', -5) // true
'foobar'.includes('b', -2) // true ???
'foobar'.split('').includes('b', -2) // false

indexOf

'foobar'.indexOf('b', -5) // 3
'foobar'.split('').indexOf('b', -5) // 3
'foobar'.indexOf('b', -2) // 3 ???
'foobar'.split('').indexOf('b', -2) // -1

lastIndexOf

'foobar'.lastIndexOf('b', -5) // -1
'foobar'.split('').lastIndexOf('b', -5) // -1
'foobar'.lastIndexOf('b', -2) // -1 ???
'foobar'.split('').lastIndexOf('b', -2) // 3

and there is no String.prototype.splice

[fisker]

If nobody is working on it, I'm going to do it

[fisker]

I've done basic implementation.

Need discussion

1. different literal member, but same result, this is already done

foo[1].slice(0, foo["1"].length - 1)

2. different type member, but same result

foo.bar.slice(0, foo['bar'].length - 1)

3. complicated length calculate

foo.slice(0, foo.length - 1 - 1)

4. call/apply/bind

Array.prototype.slice.call(foo, 0, foo.length - 1)
Array.prototype.slice.apply(foo, [0, foo.length - 1])
Array.prototype.slice.bind(foo)(0, foo.length - 1)
Array.prototype.slice.bind(foo, 0)(foo.length - 1)
Array.prototype.slice.bind(foo, 0, foo.length - 1)()

5. I simply remove foo.length, should we care about whitespace ?

@MrHen @sindresorhus

[MrHen]

I think we should flag any usage of .length as an error but only auto-fix if the parameter is trivial. Other test cases worth noting:

foo.slice(0, foo.length - bar)
foo.slice(0, foo.length /* comment */ - 1)

foo.slice(0, foo.length - 0)
foo.slice(0, foo.length + 1)

foo.slice(0, foo.length - foo[0])

[fisker]

I think we should flag any usage of .length as an error

Can't agree to this, we suppose to make length - n equal to - n, not taking care of haw he calculate index, one example

// get half array
foo.slice(0, Math.floor(foo.length / 2))

[MrHen]

Ah, yeah, I didn't think of that case.

Then I'd just limit it to looking for a single BinaryExpression with an operator of -.

Even more test cases:

foo.slice(0, foo.length - 1 / 1)
foo.slice(0, foo.length / 1 - 1)
foo.slice(0, (foo.length - 1))

To answer one of your other questions:

foo.slice(0, foo.length - 1 - 1)
foo.slice(0, foo.length - 1 + 1)
foo.slice(0, foo.length + 1 - 1)

This gets weird because the math is bad but giving slice a number that is far too big will be more correct than giving it a valid but incorrect index. I don't want to maintain some sort of detection for negation all the way down. I'd rather just flag it as an error and let the user fix it.

[fisker]

I just found simple remove foo.length is not right for many cases.

foo.slice(0, foo.length - n)
foo.slice(0, foo.length - 0)
foo.slice(0, foo.length - (-1))

n should be positive value, how do you suggest to fix? @MrHen

[fisker]

new proposal, only fix

foo.length - POSITIVE_LITERAL_VALUE
foo.length - POSITIVE_LITERAL_VALUE - POSITIVE_LITERAL_VALUE

and I think

looking for a single BinaryExpression with an operator of -

will be too annoying