Repository: ember-cli/eslint-plugin-ember
Pull request #1614 (2022-10-07 13:31:50)

Add `findBy` to `find` autofixer for `no-array-prototype-extensions` rule
## Summary
Added auto-fixer for `no-array-prototype-extensions` which replaces `findBy` with `find`.

## 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 `findBy` with the native array method `find`.

## Testing
Modified test case to check if the right output is generated after the fix.


tgvrssanthosh: @bmish Can you please review this PR as well? Thank you!
tgvrssanthosh: > Can you also merge master in? There were some dependency updates merged recently.

Just rebased the changes.

Patch:
diff --git a/lib/rules/no-array-prototype-extensions.js b/lib/rules/no-array-prototype-extensions.js
index 016b0cb5..d68f4beb 100644
--- a/lib/rules/no-array-prototype-extensions.js
+++ b/lib/rules/no-array-prototype-extensions.js
@@ -3,6 +3,7 @@
 const { getName, getNodeOrNodeFromVariable } = require('../utils/utils');
 const { isClassPropertyOrPropertyDefinition } = require('../utils/types');
 const { getImportIdentifier } = require('../utils/import');
+const { insertImportDeclaration } = require('../utils/fixer');
 const Stack = require('../utils/stack');
 
 const ERROR_MESSAGE = "Don't use Ember's array prototype extensions";
@@ -197,46 +198,51 @@ function applyFix(callExpressionNode, fixer, context, options = {}) {
       }
       return fixes;
     }
-    case 'filterBy': {
+    case 'filterBy':
+    case 'findBy': {
+      const fixes = [];
       const callArgs = callExpressionNode.arguments;
-      const sourceCode = context.getSourceCode();
-      const hasSecondArg = callArgs.length > 1;
-      const firstArg = callArgs[0];
-      const secondArg = callArgs[1];
-      // default to "get" if the `get` hasn't already been imported.
-      const importedGetName = options.importedGetName ?? 'get';
-
-      const fixes = [
-        fixer.replaceText(callExpressionNode.callee.property, 'filter'),
-        // Replace the first argument with the necessary condition
-        // If the filterBy contains two arguments, the property (first argument) value will be compared against the second argument
-        // If the filterBy contains only one argument, the property's truthy value is used to filter
-        fixer.replaceText(
-          firstArg,
-          hasSecondArg
-            ? `item => ${importedGetName}(item, ${sourceCode.getText(
-                firstArg
-              )}) === ${sourceCode.getText(secondArg)}`
-            : `item => ${importedGetName}(item, ${sourceCode.getText(firstArg)})`
-        ),
-      ];
-
-      // Add `get` import statement only if it is not imported already
-      if (!options.importedGetName) {
+
+      if (callArgs.length > 0 && callArgs.length < 3) {
+        const sourceCode = context.getSourceCode();
+        const hasSecondArg = callArgs.length > 1;
+        const firstArg = callArgs[0];
+        const secondArg = callArgs[1];
+
+        // default to `get` if the `get` hasn't already been imported.
+        const importedGetName = options.importedGetName ?? 'get';
+        const calleeProp = callExpressionNode.callee.property;
+
+        // Get the open parenthesis immediately after the callee name
+        const openParenToken = sourceCode.getTokenAfter(calleeProp, {
+          filter(token) {
+            return token.type === TOKEN_TYPES.PUNCTUATOR && token.value === '(';
+          },
+        });
+        // Get the close parenthesis from the end of the callExpressionNode
+        const closeParenToken = sourceCode.getLastToken(callExpressionNode, {
+          filter(token) {
+            return token.type === TOKEN_TYPES.PUNCTUATOR && token.value === ')';
+          },
+        });
+
         fixes.push(
-          fixer.insertTextBefore(
-            sourceCode.ast,
-            `import { ${importedGetName} } from '@ember/object';\n`
+          fixer.replaceText(calleeProp, propertyName === 'findBy' ? 'find' : 'filter'),
+          // Replacing the content starting from open parenthesis to close parenthesis
+          // If the findBy contains two arguments, the property (first argument) value will be compared against the second argument
+          // If the filterBy contains only one argument, the property's truthy value is used to filter
+          fixer.replaceTextRange(
+            [openParenToken.range[0], closeParenToken.range[1]],
+            `(item => ${importedGetName}(item, ${sourceCode.getText(firstArg)})${
+              hasSecondArg ? ` === ${sourceCode.getText(secondArg)}` : ''
+            })`
           )
         );
-      }
 
-      if (hasSecondArg) {
-        fixes.push(
-          // Required to get rid of `, ` followed by the first argument since the second argument will be removed
-          fixer.removeRange([firstArg.range[1], secondArg.range[0]]),
-          fixer.remove(secondArg)
-        );
+        // Add `get` import statement only if it is not imported already
+        if (!options.importedGetName) {
+          fixes.push(insertImportDeclaration(sourceCode, fixer, '@ember/object', importedGetName));
+        }
       }
 
       return fixes;
diff --git a/lib/utils/fixer.js b/lib/utils/fixer.js
index 88b2c237..e39cbd43 100644
--- a/lib/utils/fixer.js
+++ b/lib/utils/fixer.js
@@ -3,6 +3,7 @@
 const types = require('./types');
 
 module.exports = {
+  insertImportDeclaration,
   removeCommaSeparatedNode,
 };
 
@@ -17,3 +18,20 @@ function removeCommaSeparatedNode(node, sourceCode, fixer) {
 
   return [removeComma, removeNode];
 }
+
+/**
+ * Returns an ESLint fixing object that inserts an import declaration depending on the passed arguments
+ *
+ * @param {Object} sourceCode The ESLint source code object
+ * @param {Object} fixer The ESLint fixer object which will be used to apply fixes.
+ * @param {String} source The name of the package from where the methods/properties will be imported.
+ * @param {String} specifier The name of the method/property that needs to be imported.
+ * @param {String} defaultSpecifier The name of the default specifier that needs to be imported.
+ * @returns {Object} ESLint fixing object
+ */
+function insertImportDeclaration(sourceCode, fixer, source, specifier, defaultSpecifier) {
+  return fixer.insertTextBefore(
+    sourceCode.ast,
+    `import ${defaultSpecifier ? `${defaultSpecifier}, ` : ''}{ ${specifier} } from '${source}';\n`
+  );
+}


Test patch:
diff --git a/tests/lib/rules/no-array-prototype-extensions.js b/tests/lib/rules/no-array-prototype-extensions.js
index f73e01fc..defab019 100644
--- a/tests/lib/rules/no-array-prototype-extensions.js
+++ b/tests/lib/rules/no-array-prototype-extensions.js
@@ -16,6 +16,8 @@ ruleTester.run('no-array-prototype-extensions', rule, {
   valid: [
     '[1, 2, 4].filter(el => { /* ... */ })',
     'something.filter(el => { /* ... */ })',
+    'something.find(item => item.isValid)',
+    'something.find(item => item.age === 18, {})',
     "filterBy(something, 'foo')",
     'this.filterBy()',
     'something[0]',
@@ -408,10 +410,56 @@ set.filter(item => get(item, "age") === 18);`,
       errors: [{ messageId: 'main', type: 'CallExpression' }],
     },
     {
+      // When unexpected number of arguments are passed, auto-fixer doesn't get triggered
       code: 'something.findBy()',
       output: null,
       errors: [{ messageId: 'main', type: 'CallExpression' }],
     },
+    {
+      // When unexpected number of arguments are passed, auto-fixer doesn't get triggered
+      code: 'something.findBy(1, 2, 3)',
+      output: null,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // findBy with single argument
+      code: "something.findBy('abc')",
+      output: `import { get } from '@ember/object';
+something.find(item => get(item, 'abc'))`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // findBy with two arguments
+      code: "something.findBy('abc', 'def')",
+      output: `import { get } from '@ember/object';
+something.find(item => get(item, 'abc') === 'def')`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // findBy with two arguments and @ember/object's `get` is already imported
+      code: `import { get } from '@ember/object';
+      something.findBy('abc', 'def')`,
+      output: `import { get } from '@ember/object';
+      something.find(item => get(item, 'abc') === 'def')`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // findBy with two arguments and @ember/object's `get` is imported with an alias
+      code: `import { get as g } from '@ember/object';
+      something.findBy('abc', 'def')`,
+      output: `import { get as g } from '@ember/object';
+      something.find(item => g(item, 'abc') === 'def')`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
+    {
+      // findBy with two arguments and `get` is imported from package other than @ember/object
+      code: `import { get as g } from 'dummy';
+      something.findBy('abc', 'def')`,
+      output: `import { get } from '@ember/object';
+import { get as g } from 'dummy';
+      something.find(item => get(item, 'abc') === 'def')`,
+      errors: [{ messageId: 'main', type: 'CallExpression' }],
+    },
     {
       code: 'something.getEach()',
       output: null,