Add prefer-negative-index rule (#417)

docs/rules/prefer-negative-index.md

@@ -0,0 +1,23 @@
+# Prefer negative index over `.length - index` for `{String,Array,TypedArray}#slice()` and `Array#splice()`
+
+Prefer negative index over calculating from `.length` for [`String#slice()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/slice), [`Array#slice`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice), [`TypedArray#slice`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/slice) and [`Array#splice()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice)
+
+This rule is fixable.
+
+## Fail
+
+```js
+foo.slice(foo.length - 2, foo.length - 1);
+foo.splice(foo.length - 1, 1);
+Array.prototype.slice.call(foo, foo.length - 2, foo.length - 1);
+Array.prototype.slice.apply(foo, [foo.length - 2, foo.length - 1]);
+```
+
+## Pass
+
+```js
+foo.slice(-2, -1);
+foo.splice(-1, 1);
+Array.prototype.slice.call(foo, -2, -1);
+Array.prototype.slice.apply(foo, [-2, -1]);
+```

index.js

@@ -49,6 +49,7 @@ module.exports = {
 				'unicorn/prefer-exponentiation-operator': 'error',
 				'unicorn/prefer-flat-map': 'error',
 				'unicorn/prefer-includes': 'error',
+				'unicorn/prefer-negative-index': 'error',
 				'unicorn/prefer-node-append': 'error',
 				'unicorn/prefer-node-remove': 'error',
 				'unicorn/prefer-query-selector': 'error',

readme.md

@@ -67,6 +67,7 @@ Configure it in `package.json`.
 			"unicorn/prefer-exponentiation-operator": "error",
 			"unicorn/prefer-flat-map": "error",
 			"unicorn/prefer-includes": "error",
+			"unicorn/prefer-negative-index": "error",
 			"unicorn/prefer-node-append": "error",
 			"unicorn/prefer-node-remove": "error",
 			"unicorn/prefer-query-selector": "error",
@@ -119,6 +120,7 @@ Configure it in `package.json`.
 - [prefer-exponentiation-operator](docs/rules/prefer-exponentiation-operator.md) - Prefer the exponentiation operator over `Math.pow()` *(fixable)*
 - [prefer-flat-map](docs/rules/prefer-flat-map.md) - Prefer `.flatMap(…)` over `.map(…).flat()`. *(fixable)*
 - [prefer-includes](docs/rules/prefer-includes.md) - Prefer `.includes()` over `.indexOf()` when checking for existence or non-existence. *(fixable)*
+- [prefer-negative-index](docs/rules/prefer-negative-index.md) - Prefer negative index over `.length - index` for `{String,Array,TypedArray}#slice()` and `Array#splice()`. *(fixable)*
 - [prefer-node-append](docs/rules/prefer-node-append.md) - Prefer `Node#append()` over `Node#appendChild()`. *(fixable)*
 - [prefer-node-remove](docs/rules/prefer-node-remove.md) - Prefer `node.remove()` over `parentNode.removeChild(node)` and `parentElement.removeChild(node)`. *(fixable)*
 - [prefer-query-selector](docs/rules/prefer-query-selector.md) - Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`. *(partly fixable)*

rules/prefer-negative-index.js

@@ -0,0 +1,303 @@
+'use strict';
+const getDocumentationUrl = require('./utils/get-documentation-url');
+const isLiteralValue = require('./utils/is-literal-value');
+
+const methods = new Map([
+	[
+		'slice',
+		{
+			argumentsIndexes: [0, 1],
+			supportObjects: new Set([
+				'Array',
+				'String',
+				'ArrayBuffer',
+				'Int8Array',
+				'Uint8Array',
+				'Uint8ClampedArray',
+				'Int16Array',
+				'Uint16Array',
+				'Int32Array',
+				'Uint32Array',
+				'Float32Array',
+				'Float64Array',
+				'BigInt64Array',
+				'BigUint64Array'
+				// `{Blob,File}#slice()` are not generally used
+				// 'Blob'
+				// 'File'
+			])
+		}
+	],
+	[
+		'splice',
+		{
+			argumentsIndexes: [0],
+			supportObjects: new Set([
+				'Array'
+			])
+		}
+	]
+]);
+
+const OPERATOR_MINUS = '-';
+
+const isPropertiesEqual = (node1, node2) => properties => {
+	return properties.every(property => isEqual(node1[property], node2[property]));
+};
+
+const isTemplateElementEqual = (node1, node2) => {
+	return node1.value &&
+		node2.value &&
+		node1.tail === node2.tail &&
+		isPropertiesEqual(node1.value, node2.value)(['cooked', 'raw']);
+};
+
+const isTemplateLiteralEqual = (node1, node2) => {
+	const {quasis: quasis1} = node1;
+	const {quasis: quasis2} = node2;
+
+	return quasis1.length === quasis2.length &&
+		quasis1.every((templateElement, index) => isEqual(templateElement, quasis2[index]));
+};
+
+const isEqual = (node1, node2) => {
+	if (node1 === node2) {
+		return true;
+	}
+
+	const compare = isPropertiesEqual(node1, node2);
+
+	if (!compare(['type'])) {
+		return false;
+	}
+
+	const {type} = node1;
+
+	switch (type) {
+		case 'Identifier':
+			return compare(['name', 'computed']);
+		case 'Literal':
+			return compare(['value', 'raw']);
+		case 'TemplateLiteral':
+			return isTemplateLiteralEqual(node1, node2);
+		case 'TemplateElement':
+			return isTemplateElementEqual(node1, node2);
+		case 'BinaryExpression':
+			return compare(['operator', 'left', 'right']);
+		case 'MemberExpression':
+			return compare(['object', 'property']);
+		default:
+			return false;
+	}
+};
+
+const isLengthMemberExpression = node => node &&
+	node.type === 'MemberExpression' &&
+	node.property &&
+	node.property.type === 'Identifier' &&
+	node.property.name === 'length' &&
+	node.object;
+
+const isLiteralPositiveValue = node =>
+	node &&
+	node.type === 'Literal' &&
+	typeof node.value === 'number' &&
+	node.value > 0;
+
+const getLengthMemberExpression = node => {
+	if (!node) {
+		return;
+	}
+
+	const {type, operator, left, right} = node;
+
+	if (
+		type !== 'BinaryExpression' ||
+		operator !== OPERATOR_MINUS ||
+		!left ||
+		!isLiteralPositiveValue(right)
+	) {
+		return;
+	}
+
+	if (isLengthMemberExpression(left)) {
+		return left;
+	}
+
+	// Nested BinaryExpression
+	return getLengthMemberExpression(left);
+};
+
+const getRemoveAbleNode = (target, argument) => {
+	const lengthMemberExpression = getLengthMemberExpression(argument);
+
+	if (lengthMemberExpression && isEqual(target, lengthMemberExpression.object)) {
+		return lengthMemberExpression;
+	}
+};
+
+const getRemovalRange = (node, sourceCode) => {
+	let before = sourceCode.getTokenBefore(node);
+	let after = sourceCode.getTokenAfter(node);
+
+	let [start, end] = node.range;
+
+	let hasParentheses = true;
+
+	while (hasParentheses) {
+		hasParentheses =
+			(before.type === 'Punctuator' && before.value === '(') &&
+			(after.type === 'Punctuator' && after.value === ')');
+		if (hasParentheses) {
+			before = sourceCode.getTokenBefore(before);
+			after = sourceCode.getTokenAfter(after);
+			start = before.range[1];
+			end = after.range[0];
+		}
+	}
+
+	const [nextStart] = after.range;
+	const textBetween = sourceCode.text.slice(end, nextStart);
+
+	end += textBetween.match(/\S|$/).index;
+
+	return [start, end];
+};
+
+const getMemberName = node => {
+	const {type, property} = node;
+
+	if (
+		type === 'MemberExpression' &&
+		property &&
+		property.type === 'Identifier'
+	) {
+		return property.name;
+	}
+};
+
+function parse(node) {
+	const {callee, arguments: originalArguments} = node;
+
+	let method = callee.property.name;
+	let target = callee.object;
+	let argumentsNodes = originalArguments;
+
+	if (methods.has(method)) {
+		return {
+			method,
+			target,
+			argumentsNodes
+		};
+	}
+
+	if (method !== 'call' && method !== 'apply') {
+		return;
+	}
+
+	const isApply = method === 'apply';
+
+	method = getMemberName(callee.object);
+
+	if (!methods.has(method)) {
+		return;
+	}
+
+	const {
+		supportObjects
+	} = methods.get(method);
+
+	const parentCallee = callee.object.object;
+
+	if (
+		// [].{slice,splice}
+		(
+			parentCallee.type === 'ArrayExpression' &&
+			parentCallee.elements.length === 0
+		) ||
+		// ''.slice
+		(
+			method === 'slice' &&
+			isLiteralValue(parentCallee, '')
+		) ||
+		// {Array,String...}.prototype.slice
+		// Array.prototype.splice
+		(
+			getMemberName(parentCallee) === 'prototype' &&
+			parentCallee.object.type === 'Identifier' &&
+			supportObjects.has(parentCallee.object.name)
+		)
+	) {
+		[target] = originalArguments;
+
+		if (isApply) {
+			const [, secondArgument] = originalArguments;
+			if (secondArgument.type !== 'ArrayExpression') {
+				return;
+			}
+
+			argumentsNodes = secondArgument.elements;
+		} else {
+			argumentsNodes = originalArguments.slice(1);
+		}
+
+		return {
+			method,
+			target,
+			argumentsNodes
+		};
+	}
+}
+
+const create = context => ({
+	CallExpression: node => {
+		if (node.callee.type !== 'MemberExpression') {
+			return;
+		}
+
+		const parsed = parse(node);
+
+		if (!parsed) {
+			return;
+		}
+
+		const {
+			method,
+			target,
+			argumentsNodes
+		} = parsed;
+
+		const {argumentsIndexes} = methods.get(method);
+		const removableNodes = argumentsIndexes
+			.map(index => getRemoveAbleNode(target, argumentsNodes[index]))
+			.filter(Boolean);
+
+		if (removableNodes.length === 0) {
+			return;
+		}
+
+		context.report({
+			node,
+			message: `Prefer negative index over length minus index for \`${method}\`.`,
+			fix(fixer) {
+				const sourceCode = context.getSourceCode();
+				return removableNodes.map(
+					node => fixer.removeRange(
+						getRemovalRange(node, sourceCode)
+					)
+				);
+			}
+		});
+	}
+});
+
+module.exports = {
+	create,
+	meta: {
+		type: 'suggestion',
+		docs: {
+			url: getDocumentationUrl(__filename)
+		},
+		fixable: 'code'
+	}
+};