Update the no-get rule to also handle the getProperties function, and mark the no-get-properties rule as deprecated

README.md

@@ -89,8 +89,7 @@ The `--fix` option on the command line automatically fixes problems reported by
 | :white_check_mark: | [new-module-imports](./docs/rules/new-module-imports.md) |  Use "New Module Imports" from Ember RFC #176 |
 |  | [no-empty-attrs](./docs/rules/no-empty-attrs.md) | Prevents usage of empty attributes in ember data models |
 | :white_check_mark: | [no-function-prototype-extensions](./docs/rules/no-function-prototype-extensions.md) | Prevents usage of Ember's `function` prototype extensions |
-|  | [no-get-properties](./docs/rules/no-get-properties.md) | Disallow unnecessary usage of Ember's `getProperties` function |
-|  | [no-get](./docs/rules/no-get.md) | Disallow unnecessary usage of Ember's `get` function |
+|  | [no-get](./docs/rules/no-get.md) | Require ES5 getters instead of Ember's `get` / `getProperties` functions |
 | :white_check_mark: | [no-global-jquery](./docs/rules/no-global-jquery.md) | Prevents usage of global jQuery object |
 |  | [no-jquery](./docs/rules/no-jquery.md) | Disallow any usage of jQuery |
 |  | [no-new-mixins](./docs/rules/no-new-mixins.md) | Prevents creation of new mixins |
@@ -158,6 +157,7 @@ The `--fix` option on the command line automatically fixes problems reported by
 |:--------|:------------|
 | [avoid-leaking-state-in-components](./docs/rules/avoid-leaking-state-in-components.md) | [avoid-leaking-state-in-ember-objects](./docs/rules/avoid-leaking-state-in-ember-objects.md) |
 | [local-modules](./docs/rules/local-modules.md) | [new-module-imports](./docs/rules/new-module-imports.md) |
+| [no-get-properties](./docs/rules/no-get-properties.md) | [no-get](./docs/rules/no-get.md) |
 
 <!--RULES_TABLE_END-->
 

docs/rules/no-get-properties.md

@@ -2,6 +2,8 @@
 
 ### Rule name: `no-get-properties`
 
+**NOTE**: this rule is deprecated as it has been consolidated into the [no-get](no-get.md) rule.
+
 Starting in Ember 3.1, native ES5 getters are available, which eliminates much of the need to use `get` and `getProperties` on Ember objects. In particular, `getProperties` no longer needs to be used with destructuring assignments.
 
 ### Rule Details

docs/rules/no-get.md

@@ -1,12 +1,15 @@
 # no-get
 
-Starting in Ember 3.1, native ES5 getters are available, which eliminates much of the need to use `get` on Ember objects.
+Starting in Ember 3.1, native ES5 getters are available, which eliminates much of the need to use `get` / `getProperties` on Ember objects.
 
 ## Rule Details
 
-This rule disallows using `this.get('someProperty')` when `this.someProperty` can be used.
+This rule disallows:
 
-**WARNING**: there are a number of circumstances where `get` still needs to be used, and you may need to manually disable the rule for these:
+* `this.get('someProperty')` when `this.someProperty` can be used
+* `this.getProperties('prop1', 'prop2')` when `{ this.prop1, this.prop2 }` can be used
+
+**WARNING**: there are a number of circumstances where `get` / `getProperties` still need to be used, and you may need to manually disable the rule for these:
 
 * Ember proxy objects (`ObjectProxy`, `ArrayProxy`)
 * Objects implementing the `unknownProperty` method
@@ -24,6 +27,15 @@ import { get } from '@ember/object';
 const foo = get(this, 'someProperty');
 ```
 
+```js
+const { prop1, prop2 } = this.getProperties('prop1', 'prop2');
+```
+
+```js
+import { getProperties } from '@ember/object';
+const foo = getProperties(this, 'prop1', 'prop2');
+```
+
 Examples of **correct** code for this rule:
 
 
@@ -35,14 +47,19 @@ const foo = this.someProperty;
 const foo = this.get('some.nested.property'); // Allowed because of nested path.
 ```
 
+```js
+const { prop1, prop2 } = this;
+```
+
+```js
+const foo = { this.prop1, this.prop2 };
+```
+
 ## References
 
 * [Ember 3.1 Release Notes](https://blog.emberjs.com/2018/04/13/ember-3-1-released.html) describing "ES5 Getters for Computed Properties"
 * [Ember get Spec](https://api.emberjs.com/ember/release/functions/@ember%2Fobject/get)
+* [Ember getProperties Spec](https://api.emberjs.com/ember/release/functions/@ember%2Fobject/getProperties)
 * [Ember ES5 Getter RFC](https://github.com/emberjs/rfcs/blob/master/text/0281-es5-getters.md)
 * [es5-getter-ember-codemod](https://github.com/rondale-sc/es5-getter-ember-codemod)
 * [More context](https://github.com/emberjs/ember.js/issues/16148) about the proxy object exception to this rule
-
-## Related Rules
-
-* [no-get-properties](no-get-properties.md)

lib/rules/no-get-properties.js

@@ -13,8 +13,10 @@ module.exports = {
     docs: {
       description: 'Disallow unnecessary usage of Ember\'s `getProperties` function',
       category: 'Best Practices',
+      replacedBy: ['no-get'],
       recommended: false
     },
+    deprecated: true,
     ERROR_MESSAGE
   },
 

lib/rules/no-get.js

@@ -2,34 +2,42 @@
 
 const utils = require('../utils/utils');
 
-function makeErrorMessage(property, isImportedGet) {
+function makeErrorMessageForGet(property, isImportedGet) {
   return isImportedGet
     ? `Use \`this.${property}\` instead of \`get(this, '${property}')\``
     : `Use \`this.${property}\` instead of \`this.get('${property}')\``;
 }
 
+const ERROR_MESSAGE_GET_PROPERTIES = "Use `{ this.prop1, this.prop2, ... }` instead of Ember's `getProperties` function";
+
 module.exports = {
-  makeErrorMessage,
+  makeErrorMessageForGet,
+  ERROR_MESSAGE_GET_PROPERTIES,
   meta: {
     docs: {
-      description: "Disallow unnecessary usage of Ember's `get` function",
+      description: "Require ES5 getters instead of Ember's `get` / `getProperties` functions",
       category: 'Best Practices',
       recommended: false
     }
   },
   create(context) {
     return {
       CallExpression(node) {
+        // **************************
+        // get
+        // **************************
+
         if (
           utils.isMemberExpression(node.callee) &&
           utils.isThisExpression(node.callee.object) &&
+          utils.isIdentifier(node.callee.property) &&
           node.callee.property.name === 'get' &&
           node.arguments.length === 1 &&
           utils.isStringLiteral(node.arguments[0]) &&
           !node.arguments[0].value.includes('.')
         ) {
           // Example: this.get('foo');
-          context.report(node, makeErrorMessage(node.arguments[0].value), false);
+          context.report(node, makeErrorMessageForGet(node.arguments[0].value), false);
         }
 
         if (
@@ -41,9 +49,42 @@ module.exports = {
           !node.arguments[1].value.includes('.')
         ) {
           // Example: get(this, 'foo');
-          context.report(node, makeErrorMessage(node.arguments[1].value, true));
+          context.report(node, makeErrorMessageForGet(node.arguments[1].value, true));
+        }
+
+        // **************************
+        // getProperties
+        // **************************
+
+        if (
+          utils.isMemberExpression(node.callee) &&
+          utils.isThisExpression(node.callee.object) &&
+          utils.isIdentifier(node.callee.property) &&
+          node.callee.property.name === 'getProperties' &&
+          validateGetPropertiesArguments(node.arguments)
+        ) {
+          // Example: this.getProperties('foo', 'bar');
+          context.report(node, ERROR_MESSAGE_GET_PROPERTIES);
+        }
+
+        if (
+          utils.isIdentifier(node.callee) &&
+          node.callee.name === 'getProperties' &&
+          utils.isThisExpression(node.arguments[0]) &&
+          validateGetPropertiesArguments(node.arguments.slice(1))
+        ) {
+          // Example: getProperties(this, 'foo', 'bar');
+          context.report(node, ERROR_MESSAGE_GET_PROPERTIES);
         }
       }
     };
   }
 };
+
+function validateGetPropertiesArguments(args) {
+  if (args.length === 1 && utils.isArrayExpression(args[0])) {
+    return validateGetPropertiesArguments(args[0].elements);
+  }
+  // We can only handle string arguments without nested property paths.
+  return args.every(argument => utils.isStringLiteral(argument) && !argument.value.includes('.'));
+}

tests/lib/rules/no-get.js

@@ -1,12 +1,21 @@
 const rule = require('../../../lib/rules/no-get');
 const RuleTester = require('eslint').RuleTester;
 
-const { makeErrorMessage } = rule;
+const { makeErrorMessageForGet, ERROR_MESSAGE_GET_PROPERTIES } = rule;
 
-const ruleTester = new RuleTester();
+const ruleTester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 2015,
+    sourceType: 'module'
+  }
+});
 
 ruleTester.run('no-get', rule, {
   valid: [
+    // **************************
+    // get
+    // **************************
+
     // Nested property path.
     "this.get('foo.bar');",
     "get(this, 'foo.bar');",
@@ -33,24 +42,91 @@ ruleTester.run('no-get', rule, {
     "this.get('foo', 'bar');",
     "get(this, 'foo', 'bar');",
 
-    // Unexpected argument type.
+    // Non-string parameter.
     'this.get(5);',
+    'this.get(MY_PROP);',
     'get(this, 5);',
+    'get(this, MY_PROP);',
 
     // Unknown sub-function call:
     "this.get.foo('bar');",
     "get.foo(this, 'bar');",
+
+    // **************************
+    // getProperties
+    // **************************
+
+    // Nested property path.
+    "this.getProperties('foo', 'bar.baz');",
+    "this.getProperties(['foo', 'bar.baz']);", // With parameters in array.
+    "getProperties(this, 'foo', 'bar.baz');",
+    "getProperties(this, ['foo', 'bar.baz']);", // With parameters in array.
+
+    // Template literals.
+    'this.getProperties(`prop1`, `prop2`);',
+    'getProperties(this, `prop1`, `prop2`);',
+
+    // Not `this`.
+    "myObject.getProperties('prop1', 'prop2');",
+
+    // Not `getProperties`.
+    "this.foo('prop1', 'prop2');",
+
+    // Non-string parameter.
+    'this.getProperties(MY_PROP);',
+    'this.getProperties(...MY_PROPS);',
+    'this.getProperties([MY_PROP]);',
+    'getProperties(this, MY_PROP);',
+    'getProperties(this, ...MY_PROPS);',
+    'getProperties(this, [MY_PROP]);',
+
+    // Unknown sub-function call:
+    "this.getProperties.foo('prop1', 'prop2');",
   ],
   invalid: [
+    // **************************
+    // get
+    // **************************
+
     {
       code: "this.get('foo');",
       output: null,
-      errors: [{ message: makeErrorMessage('foo', false) }]
+      errors: [{ message: makeErrorMessageForGet('foo', false) }]
     },
     {
       code: "get(this, 'foo');",
       output: null,
-      errors: [{ message: makeErrorMessage('foo', true) }]
+      errors: [{ message: makeErrorMessageForGet('foo', true) }]
+    },
+    {
+      code: "this.get('foo').someFunction();",
+      output: null,
+      errors: [{ message: makeErrorMessageForGet('foo', false) }]
+    },
+
+    // **************************
+    // getProperties
+    // **************************
+
+    {
+      code: "this.getProperties('prop1', 'prop2');",
+      output: null,
+      errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }]
+    },
+    {
+      code: "this.getProperties(['prop1', 'prop2']);", // With parameters in array.
+      output: null,
+      errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }]
+    },
+    {
+      code: "getProperties(this, 'prop1', 'prop2');",
+      output: null,
+      errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }]
+    },
+    {
+      code: "getProperties(this, ['prop1', 'prop2']);", // With parameters in array.
+      output: null,
+      errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }]
     }
   ]
 });