Repository: ember-cli/eslint-plugin-ember
Pull request #1621 (2022-10-10 15:14:45)

Add `sortBy` to `sort` autofixer for `no-array-prototype-extensions` rule
## Summary
Added auto-fixer for `no-array-prototype-extensions` which replaces `sortBy` with `sort`.

## Description
Recently, the proposal to deprecate array prototype extensions has been approved and got merged. The plan is to add autoFixers to replace ember array prototype extensions with the native array methods. In this PR, an auto fixer has been added which will replace the ember array method `sortBy` with the native array method `sort`.

## Testing
Modified test case to check if the right output is generated after the fix.

tgvrssanthosh: @bmish Can you review this PR as well?
tgvrssanthosh: > @bmish Can you review this PR as well?

@bmish Just found a small issue. Will resolve it and will ping you.
tgvrssanthosh: @bmish Addressed the comment. Can you take a relook?

Patch:
diff --git a/lib/rules/no-array-prototype-extensions.js b/lib/rules/no-array-prototype-extensions.js
index a894dca5..3565428c 100644
--- a/lib/rules/no-array-prototype-extensions.js
+++ b/lib/rules/no-array-prototype-extensions.js
@@ -272,6 +272,46 @@ function applyFix(callExpressionNode, fixer, context, options = {}) {
 
       return [];
     }
+    case 'sortBy': {
+      const fixes = [];
+
+      if (callArgs.length > 0) {
+        // default to `compare` if the `compare` hasn't already been imported.
+        const importedCompareName = options.importedCompareName ?? 'compare';
+        // default to `compare` if the `compare` hasn't already been imported.
+        const importedGetName = options.importedGetName ?? 'get';
+
+        const sortFn = `(a, b) => {
+          for (const key of [${callArgs.map((arg) => sourceCode.getText(arg)).join(', ')}]) {
+            const compareValue = ${importedCompareName}(${importedGetName}(a, key), ${importedGetName}(b, key));
+            if (compareValue) {
+              return compareValue;
+            }
+          }
+          return 0;
+        }`;
+
+        fixes.push(
+          fixer.replaceText(calleeProp, 'sort'),
+          // Replacing the content starting from open parenthesis to close parenthesis
+          fixer.replaceTextRange([openParenToken.range[0], closeParenToken.range[1]], `(${sortFn})`)
+        );
+
+        // Add `get` import statement only if it is not imported already
+        if (!options.importedGetName) {
+          fixes.push(insertImportDeclaration(sourceCode, fixer, '@ember/object', importedGetName));
+        }
+
+        // Add `compare` import statement only if it is not imported already
+        if (!options.importedCompareName) {
+          fixes.push(
+            insertImportDeclaration(sourceCode, fixer, '@ember/utils', importedCompareName)
+          );
+        }
+      }
+
+      return fixes;
+    }
     case 'toArray': {
       if (callArgs.length === 0) {
         return [
@@ -351,6 +391,7 @@ module.exports = {
     const sourceCode = context.getSourceCode();
     const { scopeManager } = sourceCode;
     let importedGetName;
+    let importedCompareName;
 
     // Track some information about the current class we're inside.
     const classStack = new Stack();
@@ -360,6 +401,10 @@ module.exports = {
         if (node.source.value === '@ember/object') {
           importedGetName = importedGetName || getImportIdentifier(node, '@ember/object', 'get');
         }
+        if (node.source.value === '@ember/utils') {
+          importedCompareName =
+            importedCompareName || getImportIdentifier(node, '@ember/utils', 'compare');
+        }
       },
       /**
        * Cover cases when `EXTENSION_METHODS` is getting called.
@@ -445,6 +490,7 @@ module.exports = {
             messageId: 'main',
             fix(fixer) {
               return applyFix(node, fixer, context, {
+                importedCompareName,
                 importedGetName,
               });
             },


Test patch:
diff --git a/tests/lib/rules/no-array-prototype-extensions.js b/tests/lib/rules/no-array-prototype-extensions.js
index 4bae0854..b9415ec4 100644
--- a/tests/lib/rules/no-array-prototype-extensions.js
+++ b/tests/lib/rules/no-array-prototype-extensions.js
@@ -565,10 +565,153 @@ import { get as g } from 'dummy';
       errors: [{ messageId: 'main', type: 'CallExpression' }],
     },
     {
+      // When unexpected number of arguments are passed, auto-fixer will not run
       code: 'something.sortBy()',
       output: null,
       errors: [{ messageId: 'main', type: 'CallExpression' }],
     },
+    {
+      // Arguments containing strings
+      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;
+            }
+          }
+          return 0;
+        })`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // Arguments other than strings
+      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;
+            }
+          }
+          return 0;
+        })`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // When compare method is already imported from @ember/utils.
+      code: `import { compare } from '@ember/utils';
+      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;
+            }
+          }
+          return 0;
+        })`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // When compare method is already imported with alias from @ember/utils.
+      code: `import { compare as comp } 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;
+            }
+          }
+          return 0;
+        })`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // When compare method is already imported from a package other than @ember/utils.
+      code: `import { compare as comp } from '@custom/utils';
+      something.sortBy('abc', 'def')`,
+      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;
+            }
+          }
+          return 0;
+        })`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // When get method is already imported from @ember/object.
+      code: `import { compare as comp } from '@custom/utils';
+      import { get } from '@ember/object';
+      something.sortBy('abc', 'def')`,
+      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;
+            }
+          }
+          return 0;
+        })`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // When get method is already imported from a package other than @ember/object.
+      code: `import { compare as comp } from '@custom/utils';
+      import { get as g } from '@custom/object';
+      something.sortBy('abc', 'def')`,
+      output: `import { get } from '@ember/object';
+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;
+            }
+          }
+          return 0;
+        })`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // When get method is already imported with an alias from @ember/object package.
+      code: `import { compare as comp } from '@custom/utils';
+      import { get as g } from '@ember/object';
+      something.sortBy('abc', 'def')`,
+      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;
+            }
+          }
+          return 0;
+        })`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
     {
       // When unexpected number of params are passed, we will skip auto-fixing
       code: 'something.toArray(1)',