Repository: ember-cli/eslint-plugin-ember
Pull request #881 (2020-07-03 22:11:27)

Check imports in `no-get-with-default` rule
* Check imported names instead of looking for any function called `getWithDefault`
* Add import statement for `get` function in autofix if needed

CC: @mongoose700 


Patch:
diff --git a/lib/rules/no-get-with-default.js b/lib/rules/no-get-with-default.js
index cf7826f8..5aad6a4e 100644
--- a/lib/rules/no-get-with-default.js
+++ b/lib/rules/no-get-with-default.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const types = require('../utils/types');
+const { getImportIdentifier } = require('../utils/import');
 
 const ERROR_MESSAGE = 'Use `||` or the ternary operator instead of `getWithDefault()`';
 
@@ -19,7 +20,19 @@ module.exports = {
     schema: [],
   },
   create(context) {
+    let importedGetName;
+    let importedGetWithDefaultName;
+
     return {
+      ImportDeclaration(node) {
+        if (node.source.value === '@ember/object') {
+          importedGetName = importedGetName || getImportIdentifier(node, '@ember/object', 'get');
+          importedGetWithDefaultName =
+            importedGetWithDefaultName ||
+            getImportIdentifier(node, '@ember/object', 'getWithDefault');
+        }
+      },
+
       CallExpression(node) {
         if (
           types.isMemberExpression(node.callee) &&
@@ -33,14 +46,22 @@ module.exports = {
             node,
             message: ERROR_MESSAGE,
             fix(fixer) {
-              return fix(fixer, context, node, node.arguments[0], node.arguments[1], false);
+              return fix({
+                fixer,
+                context,
+                node,
+                nodeProperty: node.arguments[0],
+                nodeDefault: node.arguments[1],
+                isImported: false,
+                importedGetName,
+              });
             },
           });
         }
 
         if (
           types.isIdentifier(node.callee) &&
-          node.callee.name === 'getWithDefault' &&
+          node.callee.name === importedGetWithDefaultName &&
           node.arguments.length === 3 &&
           types.isThisExpression(node.arguments[0])
         ) {
@@ -49,7 +70,15 @@ module.exports = {
             node,
             message: ERROR_MESSAGE,
             fix(fixer) {
-              return fix(fixer, context, node, node.arguments[1], node.arguments[2], true);
+              return fix({
+                fixer,
+                context,
+                node,
+                nodeProperty: node.arguments[1],
+                nodeDefault: node.arguments[2],
+                isImported: true,
+                importedGetName,
+              });
             },
           });
         }
@@ -64,18 +93,29 @@ module.exports = {
  * @param {node} node - node with: this.getWithDefault('foo', 'bar');
  * @param {node} nodeProperty - node with: 'foo'
  * @param {node} nodeDefault - node with: 'bar'
+ * @param {boolean} isImported - whether we are dealing with the imported version of `getWithDefault`
+ * @param {string} importedGetName - name that `get` is imported under (if at all)
  */
-function fix(fixer, context, node, nodeProperty, nodeDefault, useImportedGet) {
+function fix({ fixer, context, node, nodeProperty, nodeDefault, isImported, importedGetName }) {
   const sourceCode = context.getSourceCode();
 
   const nodePropertySourceText = sourceCode.getText(nodeProperty);
   const nodeDefaultSourceText = sourceCode.getText(nodeDefault);
 
-  // We convert it to use `this.get('property')` here for safety.
+  // We convert it to use `this.get('property')` here for safety in case of nested paths.
   // The `no-get` rule can then convert it to ES5 getters (`this.property`) if safe.
-  const fixed = useImportedGet
-    ? `(get(this, ${nodePropertySourceText}) === undefined ? ${nodeDefaultSourceText} : get(this, ${nodePropertySourceText}))`
+  const getName = importedGetName ? importedGetName : 'get';
+  const fixed = isImported
+    ? `(${getName}(this, ${nodePropertySourceText}) === undefined ? ${nodeDefaultSourceText} : ${getName}(this, ${nodePropertySourceText}))`
     : `(this.get(${nodePropertySourceText}) === undefined ? ${nodeDefaultSourceText} : this.get(${nodePropertySourceText}))`;
 
-  return fixer.replaceText(node, fixed);
+  if (!isImported || importedGetName) {
+    return fixer.replaceText(node, fixed);
+  } else {
+    // Need to add import statement for `get`.
+    return [
+      fixer.insertTextBefore(sourceCode.ast, "import { get } from '@ember/object';\n"),
+      fixer.replaceText(node, fixed),
+    ];
+  }
 }


Test patch:
diff --git a/tests/lib/rules/no-get-with-default.js b/tests/lib/rules/no-get-with-default.js
index d55c9e30..2331015c 100644
--- a/tests/lib/rules/no-get-with-default.js
+++ b/tests/lib/rules/no-get-with-default.js
@@ -11,10 +11,14 @@ const ruleTester = new RuleTester({
 
 ruleTester.run('no-get-with-default', rule, {
   valid: [
-    "const test = this.get('key') || [];",
-    "const test = get(this, 'target') || [];",
+    // get
+    "this.get('key') || [];",
+    "import { get } from '@ember/object'; get(this, 'target') || [];",
+
+    // getWithDefault
     "testClass.getWithDefault('key', [])",
-    "getWithDefault.testMethod(testClass, 'key', [])",
+    "import { getWithDefault } from '@ember/object'; getWithDefault.testMethod(testClass, 'key', [])",
+    "getWithDefault(this, 'key', []);", // Missing import
   ],
   invalid: [
     // this.getWithDefault
@@ -40,8 +44,9 @@ ruleTester.run('no-get-with-default', rule, {
       ],
     },
     {
+      // Having parenthesis around the autofix matters in this example.
       code: "this.getWithDefault('name', '').trim()",
-      output: "(this.get('name') === undefined ? '' : this.get('name')).trim()", // Parenthesis matter here.
+      output: "(this.get('name') === undefined ? '' : this.get('name')).trim()",
       errors: [
         {
           message: ERROR_MESSAGE,
@@ -52,8 +57,21 @@ ruleTester.run('no-get-with-default', rule, {
 
     // getWithDefault (imported)
     {
-      code: "const test = getWithDefault(this, 'key', []);", // With a string property.
-      output: "const test = (get(this, 'key') === undefined ? [] : get(this, 'key'));",
+      code: "import { getWithDefault } from '@ember/object'; getWithDefault(this, 'key', []);", // With a string property.
+      output: `import { get } from '@ember/object';
+import { getWithDefault } from '@ember/object'; (get(this, 'key') === undefined ? [] : get(this, 'key'));`,
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      // With renamed `getWithDefault` import:
+      code: "import { getWithDefault as gwd } from '@ember/object'; gwd(this, 'key', []);",
+      output: `import { get } from '@ember/object';
+import { getWithDefault as gwd } from '@ember/object'; (get(this, 'key') === undefined ? [] : get(this, 'key'));`,
       errors: [
         {
           message: ERROR_MESSAGE,
@@ -62,9 +80,23 @@ ruleTester.run('no-get-with-default', rule, {
       ],
     },
     {
-      code: 'const test = getWithDefault(this, SOME_VARIABLE, []);', // With a variable property.
+      // With existing and renamed `get` import:
+      code:
+        "import { getWithDefault, get as g } from '@ember/object'; getWithDefault(this, 'key', []);",
       output:
-        'const test = (get(this, SOME_VARIABLE) === undefined ? [] : get(this, SOME_VARIABLE));',
+        "import { getWithDefault, get as g } from '@ember/object'; (g(this, 'key') === undefined ? [] : g(this, 'key'));",
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code:
+        "import { getWithDefault } from '@ember/object'; getWithDefault(this, SOME_VARIABLE, []);", // With a variable property.
+      output: `import { get } from '@ember/object';
+import { getWithDefault } from '@ember/object'; (get(this, SOME_VARIABLE) === undefined ? [] : get(this, SOME_VARIABLE));`,
       errors: [
         {
           message: ERROR_MESSAGE,
@@ -73,8 +105,11 @@ ruleTester.run('no-get-with-default', rule, {
       ],
     },
     {
-      code: "getWithDefault(this, 'name', '').trim()",
-      output: "(get(this, 'name') === undefined ? '' : get(this, 'name')).trim()", // Parenthesis matter here.
+      // Having parenthesis around the autofix matters in this example.
+      code:
+        "import { getWithDefault } from '@ember/object'; getWithDefault(this, 'name', '').trim()",
+      output: `import { get } from '@ember/object';
+import { getWithDefault } from '@ember/object'; (get(this, 'name') === undefined ? '' : get(this, 'name')).trim()`,
       errors: [
         {
           message: ERROR_MESSAGE,