feat: add catchSafeObjects and catchUnsafeObjects options to no-get-with-default rule

Author: bmish

docs/rules/no-get-with-default.md

@@ -40,6 +40,13 @@ const test = this.key === undefined ? [] : this.key;
 const test = this.key || [];
 ```
 
+## Configuration
+
+This rule takes an optional object containing:
+
+- `boolean` -- `catchSafeObjects` -- whether the rule should catch non-`this` imported usages like `getWithDefault(person, 'name', '')` (default `false`, TODO: enable in next major release)
+- `boolean` -- `catchUnsafeObjects` -- whether the rule should catch non-`this` usages like `person.getWithDefault('name', '')` even though we don't know for sure if `person` is an Ember object (default `false`, TODO: enable in next major release)
+
 ## References
 
 - [RFC](https://github.com/emberjs/rfcs/pull/554/) to deprecate `getWithDefault`

docs/rules/no-get.md

@@ -94,8 +94,8 @@ This rule takes an optional object containing:
 * `boolean` -- `ignoreGetProperties` -- whether the rule should ignore `getProperties` (default `false`)
 * `boolean` -- `ignoreNestedPaths` -- whether the rule should ignore `this.get('some.nested.property')` (default `false`)
 * `boolean` -- `useOptionalChaining` -- whether the rule should use the [optional chaining operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining) `?.` to autofix nested paths such as `this.get('some.nested.property')` to `this.some?.nested?.property` (when this option is off, these nested paths won't be autofixed at all) (default `false`)
-* `boolean` -- `catchSafeObjects` -- whether the rule should catch `get(foo, 'bar')` (default `true`)
-* `boolean` -- `catchUnsafeObjects` -- whether the rule should catch `foo.get('bar')` even though we don't know for sure if `foo` is an Ember object (default `false`)
+* `boolean` -- `catchSafeObjects` -- whether the rule should catch non-`this` imported usages like `get(foo, 'bar')` (default `true`)
+* `boolean` -- `catchUnsafeObjects` -- whether the rule should catch non-`this` usages like `foo.get('bar')` even though we don't know for sure if `foo` is an Ember object (default `false`)
 
 ## Related Rules
 

lib/rules/no-get-with-default.js

@@ -17,12 +17,30 @@ module.exports = {
         'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-get-with-default.md',
     },
     fixable: 'code',
-    schema: [],
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          catchSafeObjects: {
+            type: 'boolean',
+            default: false,
+          },
+          catchUnsafeObjects: {
+            type: 'boolean',
+            default: false,
+          },
+        },
+        additionalProperties: false,
+      },
+    ],
   },
   create(context) {
     let importedGetName;
     let importedGetWithDefaultName;
 
+    const catchSafeObjects = context.options[0] && context.options[0].catchSafeObjects;
+    const catchUnsafeObjects = context.options[0] && context.options[0].catchUnsafeObjects;
+
     return {
       ImportDeclaration(node) {
         if (node.source.value === '@ember/object') {
@@ -36,7 +54,7 @@ module.exports = {
       CallExpression(node) {
         if (
           types.isMemberExpression(node.callee) &&
-          types.isThisExpression(node.callee.object) &&
+          (types.isThisExpression(node.callee.object) || catchUnsafeObjects) &&
           types.isIdentifier(node.callee.property) &&
           node.callee.property.name === 'getWithDefault' &&
           node.arguments.length === 2
@@ -50,6 +68,7 @@ module.exports = {
                 fixer,
                 context,
                 node,
+                nodeObject: node.callee.object,
                 nodeProperty: node.arguments[0],
                 nodeDefault: node.arguments[1],
                 isImported: false,
@@ -63,7 +82,7 @@ module.exports = {
           types.isIdentifier(node.callee) &&
           node.callee.name === importedGetWithDefaultName &&
           node.arguments.length === 3 &&
-          types.isThisExpression(node.arguments[0])
+          (types.isThisExpression(node.arguments[0]) || catchSafeObjects)
         ) {
           // Example: getWithDefault(this, 'foo', 'bar');
           context.report({
@@ -74,6 +93,7 @@ module.exports = {
                 fixer,
                 context,
                 node,
+                nodeObject: node.arguments[0],
                 nodeProperty: node.arguments[1],
                 nodeDefault: node.arguments[2],
                 isImported: true,
@@ -91,23 +111,34 @@ module.exports = {
  * @param {fixer} fixer
  * @param {context} context
  * @param {node} node - node with: this.getWithDefault('foo', 'bar');
+ * @param {node} nodeObject - node with: 'this'
  * @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, isImported, importedGetName }) {
+function fix({
+  fixer,
+  context,
+  node,
+  nodeObject,
+  nodeProperty,
+  nodeDefault,
+  isImported,
+  importedGetName,
+}) {
   const sourceCode = context.getSourceCode();
 
+  const nodeObjectSourceText = sourceCode.getText(nodeObject);
   const nodePropertySourceText = sourceCode.getText(nodeProperty);
   const nodeDefaultSourceText = sourceCode.getText(nodeDefault);
 
   // 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 getName = importedGetName ? importedGetName : 'get';
   const fixed = isImported
-    ? `(${getName}(this, ${nodePropertySourceText}) === undefined ? ${nodeDefaultSourceText} : ${getName}(this, ${nodePropertySourceText}))`
-    : `(this.get(${nodePropertySourceText}) === undefined ? ${nodeDefaultSourceText} : this.get(${nodePropertySourceText}))`;
+    ? `(${getName}(${nodeObjectSourceText}, ${nodePropertySourceText}) === undefined ? ${nodeDefaultSourceText} : ${getName}(${nodeObjectSourceText}, ${nodePropertySourceText}))`
+    : `(${nodeObjectSourceText}.get(${nodePropertySourceText}) === undefined ? ${nodeDefaultSourceText} : ${nodeObjectSourceText}.get(${nodePropertySourceText}))`;
 
   return !isImported || importedGetName
     ? fixer.replaceText(node, fixed)

tests/lib/rules/no-get-with-default.js

@@ -19,6 +19,20 @@ ruleTester.run('no-get-with-default', rule, {
     "testClass.getWithDefault('key', [])",
     "import { getWithDefault } from '@ember/object'; getWithDefault.testMethod(testClass, 'key', [])",
     "getWithDefault(this, 'key', []);", // Missing import
+
+    // With catchSafeObjects: false
+    "import { getWithDefault } from '@ember/object'; getProperties('person', 'name', '');",
+    {
+      code: "import { getWithDefault } from '@ember/object'; getProperties('person', 'name', '');",
+      options: [{ catchSafeObjects: false }],
+    },
+
+    // With catchUnsafeObjects: false
+    "person.getWithDefault('name', '');",
+    {
+      code: "person.getWithDefault('name', '');",
+      options: [{ catchUnsafeObjects: false }],
+    },
   ],
   invalid: [
     // this.getWithDefault
@@ -117,5 +131,32 @@ import { getWithDefault } from '@ember/object'; (get(this, 'name') === undefined
         },
       ],
     },
+
+    // With catchSafeObjects: true
+    {
+      code: "import { getWithDefault } from '@ember/object'; getWithDefault(person, 'name', '');",
+      options: [{ catchSafeObjects: true }],
+      output: `import { get } from '@ember/object';
+import { getWithDefault } from '@ember/object'; (get(person, 'name') === undefined ? '' : get(person, 'name'));`,
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
+
+    // With catchUnsafeObjects: true
+    {
+      code: "person.getWithDefault('name', '');",
+      options: [{ catchUnsafeObjects: true }],
+      output: "(person.get('name') === undefined ? '' : person.get('name'));",
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
   ],
 });