Repository: ember-cli/eslint-plugin-ember
Pull request #421 (2019-05-15 20:19:35)

Update the `no-get` rule to handle `getProperties` and mark `no-get-properties` as deprecated
Based on the discussion in #415 and #419, we decided to make the `no-get-properties` rule more generic, but then it becomes almost identical to the `no-get` rule, so then we should just consolidate it into the `no-get` rule (which can handle both `this.get` and `this.getProperties`). Both rules are intended to enforce the new ES5 getter syntax, and there's no need to have two separate rules for enforcing the same exact best practice.

Turbo87: > Note that the `no-get-properties` rule is new and has only been around for one short release so I think it's best to simply delete it now.

if it's released we can't remove it anymore unfortunately :-/
bmish: @Turbo87 good point, I updated the PR to leave `no-get-properties` in place but mark it as deprecated instead.
bmish: @pavlikmarek would you like to review since you worked on the related PR? I'll get this released once it's approved.
bmish: @Turbo87 will plan to merge this in the next few days unless you have more feedback.
Turbo87: 👍 

Patch:
diff --git a/README.md b/README.md
index 3068815a..57c7bfed 100644
--- a/README.md
+++ b/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-->
 
diff --git a/docs/rules/no-get-properties.md b/docs/rules/no-get-properties.md
index e88875f9..2d87ef6b 100644
--- a/docs/rules/no-get-properties.md
+++ b/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
diff --git a/docs/rules/no-get.md b/docs/rules/no-get.md
index a7262585..6a7d0a6a 100644
--- a/docs/rules/no-get.md
+++ b/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)
diff --git a/lib/rules/no-get-properties.js b/lib/rules/no-get-properties.js
index 1926550e..f5365dff 100644
--- a/lib/rules/no-get-properties.js
+++ b/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
   },
 
diff --git a/lib/rules/no-get.js b/lib/rules/no-get.js
index 3ab1f091..2ffb9530 100644
--- a/lib/rules/no-get.js
+++ b/lib/rules/no-get.js
@@ -2,17 +2,20 @@
 
 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
     }
@@ -20,16 +23,21 @@ module.exports = {
   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('.'));
+}


Test patch:
diff --git a/tests/lib/rules/no-get.js b/tests/lib/rules/no-get.js
index 555288be..4299a94a 100644
--- a/tests/lib/rules/no-get.js
+++ b/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' }]
     }
   ]
 });