Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prefer-negative-index rule #417

Merged
merged 32 commits into from
Nov 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
f68080b
Add `prefer-negative-index` rule
fisker Oct 11, 2019
0f2b76c
fix identifier compare
fisker Oct 11, 2019
c9994ed
Merge branch 'master' into rule/prefer-negative-index
fisker Oct 13, 2019
e23be57
rebase
fisker Oct 13, 2019
14390d3
more cases
fisker Oct 16, 2019
391d946
Merge branch 'rule/prefer-negative-index' of github.com:fisker/eslint…
fisker Oct 16, 2019
38a5479
avoid ref issue
fisker Oct 16, 2019
3092586
fix test
fisker Oct 16, 2019
e524d20
disable wrong result
fisker Oct 16, 2019
44acc09
only fix `.length - 1`
fisker Oct 17, 2019
34d7182
handle spacing and parentheses
fisker Oct 17, 2019
79c65b5
disable some eslint rules
fisker Oct 17, 2019
d09006b
remove trimStart
fisker Oct 17, 2019
99827d6
update leadingSpacingLength
fisker Oct 17, 2019
208c512
coverage 100%
fisker Oct 17, 2019
309a870
name test code
fisker Oct 17, 2019
1428526
prototype support
fisker Oct 17, 2019
ff54f48
coverage
fisker Oct 17, 2019
fcae26f
fix function call
fisker Oct 17, 2019
bdf1fc1
style
fisker Oct 17, 2019
25c2704
simplify getMemberName
fisker Oct 17, 2019
bf59ecc
refactor
fisker Oct 17, 2019
0a1c77c
typo
fisker Oct 17, 2019
9706f83
more type
fisker Oct 17, 2019
9950d5d
style
fisker Oct 17, 2019
df6c7b1
code style
fisker Oct 25, 2019
9fae2ef
Merge branch 'master' into rule/prefer-negative-index
sindresorhus Nov 27, 2019
25f5a9d
Update docs/rules/prefer-negative-index.md
fisker Nov 27, 2019
3ea3880
Update docs/rules/prefer-negative-index.md
fisker Nov 27, 2019
f56647d
fix issues
fisker Nov 27, 2019
7f7209f
add comment
fisker Nov 27, 2019
6fee573
update comment
fisker Nov 27, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions docs/rules/prefer-negative-index.md
Original file line number Diff line number Diff line change
@@ -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]);
```
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)*
Expand Down
303 changes: 303 additions & 0 deletions rules/prefer-negative-index.js
Original file line number Diff line number Diff line change
@@ -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'
fisker marked this conversation as resolved.
Show resolved Hide resolved
])
}
],
[
'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];
Copy link
Collaborator Author

@fisker fisker Nov 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i mean these two line cant combine

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was not clear that you were referring to this as you commented on the other code.

}
}

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'
}
};
Loading