diff --git a/README.md b/README.md index 0bc38a0154..f63fd5b75f 100644 --- a/README.md +++ b/README.md @@ -89,6 +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](./docs/rules/no-get.md) | Disallow unnecessary usage of Ember's `get` function | | :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 | diff --git a/docs/rules/no-get.md b/docs/rules/no-get.md new file mode 100644 index 0000000000..e2c25e5fe7 --- /dev/null +++ b/docs/rules/no-get.md @@ -0,0 +1,44 @@ +# no-get + +Starting in Ember 3.1, native ES5 getters are available, which eliminates much of the need to use `get` on Ember objects. + +## Rule Details + +This rule disallows using `this.get('someProperty')` when `this.someProperty` can be used. + +**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: + +* Ember proxy objects (`ObjectProxy`, `ArrayProxy`) +* Objects implementing the `unknownProperty` method + +## Examples + +Examples of **incorrect** code for this rule: + +```js +const foo = this.get('someProperty'); +``` + +```js +import { get } from '@ember/object'; +const foo = get(this, 'someProperty'); +``` + +Examples of **correct** code for this rule: + + +```js +const foo = this.someProperty; +``` + +```js +const foo = this.get('some.nested.property'); // Allowed because of nested path. +``` + +## 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 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 diff --git a/lib/index.js b/lib/index.js index c119c01c1a..cc18a112a0 100644 --- a/lib/index.js +++ b/lib/index.js @@ -21,6 +21,7 @@ module.exports = { 'no-ember-testing-in-module-scope': require('./rules/no-ember-testing-in-module-scope'), 'no-empty-attrs': require('./rules/no-empty-attrs'), 'no-function-prototype-extensions': require('./rules/no-function-prototype-extensions'), + 'no-get': require('./rules/no-get'), 'no-global-jquery': require('./rules/no-global-jquery'), 'no-invalid-debug-function-arguments': require('./rules/no-invalid-debug-function-arguments'), 'no-jquery': require('./rules/no-jquery'), diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js index 666ffdf529..93354b7413 100644 --- a/lib/recommended-rules.js +++ b/lib/recommended-rules.js @@ -23,6 +23,7 @@ module.exports = { "ember/no-ember-testing-in-module-scope": "error", "ember/no-empty-attrs": "off", "ember/no-function-prototype-extensions": "error", + "ember/no-get": "off", "ember/no-global-jquery": "error", "ember/no-invalid-debug-function-arguments": "off", "ember/no-jquery": "off", diff --git a/lib/rules/no-get.js b/lib/rules/no-get.js new file mode 100644 index 0000000000..c8b2c10c38 --- /dev/null +++ b/lib/rules/no-get.js @@ -0,0 +1,51 @@ +'use strict'; + +const utils = require('../utils/utils'); + +function makeErrorMessage(property, isImportedGet) { + return isImportedGet + ? `Use \`this.${property}\` instead of \`get(this, '${property}')\`` + : `Use \`this.${property}\` instead of \`this.get('${property}')\``; +} + +module.exports = { + makeErrorMessage, + meta: { + docs: { + description: "Disallow unnecessary usage of Ember's `get` function", + category: 'Best Practices', + recommended: false + } + }, + create(context) { + return { + CallExpression(node) { + if ( + utils.isMemberExpression(node.callee) && + utils.isThisExpression(node.callee.object) && + node.callee.property.name === 'get' && + node.arguments.length === 1 && + utils.isLiteral(node.arguments[0]) && + typeof node.arguments[0].value === 'string' && + !node.arguments[0].value.includes('.') + ) { + // Example: this.get('foo'); + context.report(node, makeErrorMessage(node.arguments[0].value), false); + } + + if ( + utils.isIdentifier(node.callee) && + node.callee.name === 'get' && + node.arguments.length === 2 && + utils.isThisExpression(node.arguments[0]) && + utils.isLiteral(node.arguments[1]) && + typeof node.arguments[1].value === 'string' && + !node.arguments[1].value.includes('.') + ) { + // Example: get(this, 'foo'); + context.report(node, makeErrorMessage(node.arguments[1].value, true)); + } + } + }; + } +}; diff --git a/tests/lib/rules/no-get.js b/tests/lib/rules/no-get.js new file mode 100644 index 0000000000..0f3c5d9169 --- /dev/null +++ b/tests/lib/rules/no-get.js @@ -0,0 +1,46 @@ +const rule = require('../../../lib/rules/no-get'); +const RuleTester = require('eslint').RuleTester; + +const { makeErrorMessage } = rule; + +const ruleTester = new RuleTester(); + +ruleTester.run('no-get', rule, { + valid: [ + // Nested property path. + "this.get('foo.bar');", + "get(this, 'foo.bar');", + + // Not `this`. + "foo.get('bar');", + "get(foo, 'bar');", + + // Not `get`. + "this.foo('bar');", + "foo(this, 'bar');", + + // Unknown extra argument. + "this.get('foo', 'bar');", + "get(this, 'foo', 'bar');", + + // Unexpected argument type. + 'this.get(5);', + 'get(this, 5);', + + // Unknown sub-function call: + "this.get.foo('bar');", + "get.foo(this, 'bar');", + ], + invalid: [ + { + code: "this.get('foo');", + output: null, + errors: [{ message: makeErrorMessage('foo', false) }] + }, + { + code: "get(this, 'foo');", + output: null, + errors: [{ message: makeErrorMessage('foo', true) }] + } + ] +});