Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new no-get rule #404

Merged
merged 1 commit into from Apr 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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 |
Expand Down
44 changes: 44 additions & 0 deletions 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
1 change: 1 addition & 0 deletions lib/index.js
Expand Up @@ -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'),
Expand Down
1 change: 1 addition & 0 deletions lib/recommended-rules.js
Expand Up @@ -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",
Expand Down
51 changes: 51 additions & 0 deletions 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));
}
}
};
}
};
46 changes: 46 additions & 0 deletions 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) }]
}
]
});