diff --git a/README.md b/README.md index 3068815aff..57c7bfedd8 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) | diff --git a/docs/rules/no-get-properties.md b/docs/rules/no-get-properties.md index e88875f9fd..2d87ef6ba6 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 a72625851c..6a7d0a6a42 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 1926550ea1..f5365dff58 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 3ab1f09191..2ffb953091 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('.')); +} diff --git a/tests/lib/rules/no-get.js b/tests/lib/rules/no-get.js index 555288be21..4299a94a4b 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' }] } ] });