Rule proposal: no-unnecessary-splice

[burtek]

Description

Saw splice being used instead of push/pop/shift/unshift in some projects. AFAIK it brings no performance gain and is harder to read/understand than the latter ones.

Basically bans:

• array.splice(index, 0) (can be removed as no-op)
• array.splice(0, 1) (as substitutable by array.shift())
• array.splice(0, 0, element) (as substitutable by array.unshift(element) - for 1 or more elements)
• array.splice(array.length - 1, 1) (as substitutable by array.pop())
• array.splice(array.length, 0, element) (as substitutable by array.push(element) - for 1 or more elements)

Should be easy to implement auto-fixer for those replacements.

Fail

array.splice(start, 0)

array.splice(0, 1)

array.splice(array.length, 0, element)

Pass

array.shift()

array.push(element)

Proposed rule name

no-unnecessary-splice

Additional Info

Happy to work on this once accepted (already have a stub).

This would make #2165 not needed any more

[fisker]

Fail

array.splice(index, 1, newValue);

Pass

array[index] = newValue;

array = array.with(index, newValue);

[burtek]

@fisker good one, but I'd prefer to have that configurable, as it depends on the targeted ECMA version if array.with is supported

EDIT: actually, this doesn't look like that good of a replacement. splice mutates the array and returns old values, same for push/pop/shift/unshift, but neither array[index] = newValue or array.with does it, so it changes the logic, especially since array can't be const for the array.with solution.

Will leave this one for the end

[burtek]

One thing to watch out for is whether splice's return value is used or not.

Another thing is for all calls like getArray().splice(4, 0) (no-op case), we cant just remove the whole thing, as we'd be removing call to getArray which might introduce bugs. A lot to consider for auto-fixer. Again, have some stub for this already, working on more

[sindresorhus]

Accepted

[burtek]

#2361 is WIP, will try finishing it in the coming week.