Skip to content

Commit

Permalink
Avoid in-place sorting in sortBy autofixer in `no-array-prototype-e…
Browse files Browse the repository at this point in the history 
…xtensions` rule (#1640)

Co-authored-by: Bryan Mishkin <698306+bmish@users.noreply.github.com>
  • Loading branch information
@tgvrssanthosh @bmish
tgvrssanthosh and bmish authored Oct 27, 2022
1 parent 9864569 commit 8e862a2
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 87 deletions.
41 changes: 24 additions & 17 deletions lib/rules/no-array-prototype-extensions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,28 +424,35 @@ function applyFix(callExpressionNode, fixer, context, options = {}) {
// default to `compare` if the `compare` hasn't already been imported.
const importedGetName = options.importedGetName ?? 'get';

const compareValueCheckText = `const compareValue = ${importedCompareName}(${importedGetName}(a, key), ${importedGetName}(b, key));
if (compareValue) {
return compareValue;
}`;
const argsText = callArgs.map((arg) => sourceCode.getText(arg)).join(', ');

// Loop through keys if more than one of them.
const sortFn =
callArgs.length === 1
? `(a, b) => {
const key = ${argsText};
${compareValueCheckText}
let sortFn;
const comparisonFnInString = (key) =>
`${importedCompareName}(${importedGetName}(a, ${key}), ${importedGetName}(b, ${key}))`;
if (callArgs.length === 1 && callArgs[0].type !== 'SpreadElement') {
sortFn =
callArgs[0].type === 'Identifier' || callArgs[0].type === 'Literal'
? `(a, b) => ${comparisonFnInString(argsText)}`
: `(a, b) => {
const key = ${argsText};
return ${comparisonFnInString('key')};
}`;
} else {
// Loop through keys if there are more than one argument
sortFn = `(a, b) => {
for (const key of [${argsText}]) {
const compareValue = ${comparisonFnInString('key')};
if (compareValue) {
return compareValue;
}
}
return 0;
}`
: `(a, b) => {
for (const key of [${argsText}]) {
${compareValueCheckText}
}
return 0;
}`;
}`;
}

fixes.push(
// Duplicate array
fixer.replaceText(calleeObj, `[...${sourceCode.getText(calleeObj)}]`),
fixer.replaceText(calleeProp, 'sort'),
// Replacing the content starting from open parenthesis to close parenthesis
fixer.replaceTextRange([openParenToken.range[0], closeParenToken.range[1]], `(${sortFn})`)
Expand Down
160 changes: 90 additions & 70 deletions tests/lib/rules/no-array-prototype-extensions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -832,27 +832,47 @@ import { set as s } from '@custom/object';
code: "something.sortBy('abc', 'def')",
output: `import { get } from '@ember/object';
import { compare } from '@ember/utils';
something.sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
}
}
}
return 0;
})`,
return 0;
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
// Single argument.
// Single call expression argument.
code: 'something.sortBy(getKey())',
output: `import { get } from '@ember/object';
import { compare } from '@ember/utils';
something.sort((a, b) => {
const key = getKey();
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
const key = getKey();
return compare(get(a, key), get(b, key));
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
// Single non CallExpression argument.
code: "something.sortBy('abc')",
output: `import { get } from '@ember/object';
import { compare } from '@ember/utils';
[...something].sort((a, b) => compare(get(a, 'abc'), get(b, 'abc')))`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
// Spread element as argument
code: 'something.sortBy(...abc)',
output: `import { get } from '@ember/object';
import { compare } from '@ember/utils';
[...something].sort((a, b) => {
for (const key of [...abc]) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
}
}
return 0;
})`,
Expand All @@ -863,15 +883,15 @@ something.sort((a, b) => {
code: "something.sortBy('abc', def)",
output: `import { get } from '@ember/object';
import { compare } from '@ember/utils';
something.sort((a, b) => {
for (const key of ['abc', def]) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
for (const key of ['abc', def]) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
}
}
}
return 0;
})`,
return 0;
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
Expand All @@ -880,15 +900,15 @@ something.sort((a, b) => {
something.sortBy('abc', 'def')`,
output: `import { get } from '@ember/object';
import { compare } from '@ember/utils';
something.sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
}
}
}
return 0;
})`,
return 0;
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
Expand All @@ -897,15 +917,15 @@ import { compare } from '@ember/utils';
something.sortBy('abc', 'def')`,
output: `import { get } from '@ember/object';
import { compare as comp } from '@ember/utils';
something.sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = comp(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = comp(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
}
}
}
return 0;
})`,
return 0;
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
Expand All @@ -915,15 +935,15 @@ import { compare as comp } from '@ember/utils';
output: `import { get } from '@ember/object';
import { compare } from '@ember/utils';
import { compare as comp } from '@custom/utils';
something.sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
}
}
}
return 0;
})`,
return 0;
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
Expand All @@ -934,15 +954,15 @@ import { compare as comp } from '@custom/utils';
output: `import { compare } from '@ember/utils';
import { compare as comp } from '@custom/utils';
import { get } from '@ember/object';
something.sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
}
}
}
return 0;
})`,
return 0;
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
Expand All @@ -954,15 +974,15 @@ import { compare as comp } from '@custom/utils';
import { compare } from '@ember/utils';
import { compare as comp } from '@custom/utils';
import { get as g } from '@custom/object';
something.sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(get(a, key), get(b, key));
if (compareValue) {
return compareValue;
}
}
}
return 0;
})`,
return 0;
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
Expand All @@ -973,15 +993,15 @@ import { compare as comp } from '@custom/utils';
output: `import { compare } from '@ember/utils';
import { compare as comp } from '@custom/utils';
import { get as g } from '@ember/object';
something.sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(g(a, key), g(b, key));
if (compareValue) {
return compareValue;
[...something].sort((a, b) => {
for (const key of ['abc', 'def']) {
const compareValue = compare(g(a, key), g(b, key));
if (compareValue) {
return compareValue;
}
}
}
return 0;
})`,
return 0;
})`,
errors: [{ messageId: 'main', type: 'CallExpression' }],
},
{
Expand Down

0 comments on commit 8e862a2

Please sign in to comment.