Merge pull request #747 from bmish/no-get-with-default-autofixer

Add autofixer to `no-get-with-default` rule

Author: bmish

README.md

@@ -120,7 +120,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
 |    | Rule ID | Description |
 |:---|:--------|:------------|
 | :white_check_mark: | [avoid-leaking-state-in-ember-objects](./docs/rules/avoid-leaking-state-in-ember-objects.md) | disallow state leakage |
-| :white_check_mark: | [no-get-with-default](./docs/rules/no-get-with-default.md) | disallow usage of the Ember's `getWithDefault` function |
+| :white_check_mark::wrench: | [no-get-with-default](./docs/rules/no-get-with-default.md) | disallow usage of the Ember's `getWithDefault` function |
 | :white_check_mark::wrench: | [no-get](./docs/rules/no-get.md) | require using ES5 getters instead of Ember's `get` / `getProperties` functions |
 |  | [no-proxies](./docs/rules/no-proxies.md) | disallow using array or object proxies |
 | :white_check_mark: | [require-super-in-init](./docs/rules/require-super-in-init.md) | require `this._super` to be called in `init` hooks |

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

@@ -1,12 +1,15 @@
 # no-get-with-default
 
+:wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.
+
 This rule attempts to catch and prevent the use of `getWithDefault`.
 
 ## Rule Details
 
-Even though the behavior for `getWithDefault` is more defined such that it only falls back to the default value on `undefined`,
-its inconsistency with the native `||` is confusing to many developers who assume otherwise. This rule encourages developers to use
-the native `||` operator instead.
+Even though the behavior for `getWithDefault` is more defined such that it only falls back to the default value on `undefined`, its inconsistency with the native `||` is confusing to many developers who assume otherwise. Instead, this rule encourages developers to use:
+
+- `||` operator
+- ternary operator
 
 In addition, [Nullish Coalescing Operator `??`](https://github.com/tc39/proposal-nullish-coalescing) will land in the JavaScript language soon so developers can leverage safe property access with native support instead of using `getWithDefault`.
 

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

@@ -15,6 +15,7 @@ module.exports = {
       url:
         'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-get-with-default.md',
     },
+    fixable: 'code',
     schema: [],
   },
   create(context) {
@@ -28,7 +29,13 @@ module.exports = {
           node.arguments.length === 2
         ) {
           // Example: this.getWithDefault('foo', 'bar');
-          context.report(node, ERROR_MESSAGE);
+          context.report({
+            node,
+            message: ERROR_MESSAGE,
+            fix(fixer) {
+              return fix(fixer, context, node, node.arguments[0], node.arguments[1], false);
+            },
+          });
         }
 
         if (
@@ -38,9 +45,37 @@ module.exports = {
           types.isThisExpression(node.arguments[0])
         ) {
           // Example: getWithDefault(this, 'foo', 'bar');
-          context.report(node, ERROR_MESSAGE);
+          context.report({
+            node,
+            message: ERROR_MESSAGE,
+            fix(fixer) {
+              return fix(fixer, context, node, node.arguments[1], node.arguments[2], true);
+            },
+          });
         }
       },
     };
   },
 };
+
+/**
+ * @param {fixer} fixer
+ * @param {context} context
+ * @param {node} node - node with: this.getWithDefault('foo', 'bar');
+ * @param {node} nodeProperty - node with: 'foo'
+ * @param {node} nodeDefault - node with: 'bar'
+ */
+function fix(fixer, context, node, nodeProperty, nodeDefault, useImportedGet) {
+  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.
+  // 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}))`
+    : `(this.get(${nodePropertySourceText}) === undefined ? ${nodeDefaultSourceText} : this.get(${nodePropertySourceText}))`;
+
+  return fixer.replaceText(node, fixed);
+}

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

@@ -17,25 +17,70 @@ ruleTester.run('no-get-with-default', rule, {
     "getWithDefault.testMethod(testClass, 'key', [])",
   ],
   invalid: [
+    // this.getWithDefault
     {
-      code: "const test = this.getWithDefault('key', []);",
+      code: "const test = this.getWithDefault('key', []);", // With a string property.
+      output: "const test = (this.get('key') === undefined ? [] : this.get('key'));",
       errors: [
         {
           message: ERROR_MESSAGE,
           type: 'CallExpression',
         },
       ],
-      output: null,
     },
     {
-      code: "const test = getWithDefault(this, 'key', []);",
+      code: 'const test = this.getWithDefault(SOME_VARIABLE, []);', // With a variable property.
+      output:
+        'const test = (this.get(SOME_VARIABLE) === undefined ? [] : this.get(SOME_VARIABLE));',
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: "this.getWithDefault('name', '').trim()",
+      output: "(this.get('name') === undefined ? '' : this.get('name')).trim()", // Parenthesis matter here.
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
+
+    // getWithDefault (imported)
+    {
+      code: "const test = getWithDefault(this, 'key', []);", // With a string property.
+      output: "const test = (get(this, 'key') === undefined ? [] : get(this, 'key'));",
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: 'const test = getWithDefault(this, SOME_VARIABLE, []);', // With a variable property.
+      output:
+        'const test = (get(this, SOME_VARIABLE) === undefined ? [] : get(this, SOME_VARIABLE));',
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: "getWithDefault(this, 'name', '').trim()",
+      output: "(get(this, 'name') === undefined ? '' : get(this, 'name')).trim()", // Parenthesis matter here.
       errors: [
         {
           message: ERROR_MESSAGE,
           type: 'CallExpression',
         },
       ],
-      output: null,
     },
   ],
 });