Skip to content

Commit

Permalink
Merge pull request #594 from steventsao/master
Browse files Browse the repository at this point in the history
Add new rule `no-get-with-default`
  • Loading branch information
bmish authored Nov 19, 2019
2 parents 7fff62e + 6fee2e4 commit 1c18fd9
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ The `--fix` option on the command line automatically fixes problems reported by
| | [named-functions-in-promises](./docs/rules/named-functions-in-promises.md) | Enforces usage of named functions in promises |
| :white_check_mark: | [new-module-imports](./docs/rules/new-module-imports.md) | Use "New Module Imports" from Ember RFC #176 |
| :white_check_mark: | [no-function-prototype-extensions](./docs/rules/no-function-prototype-extensions.md) | Prevents usage of Ember's `function` prototype extensions |
| :car: | [no-get-with-default](./docs/rules/no-get-with-default.md) | Disallows use of the Ember's `getWithDefault` function |
| :car::wrench: | [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 |
| :car: | [no-jquery](./docs/rules/no-jquery.md) | Disallow any usage of jQuery |
Expand Down
41 changes: 41 additions & 0 deletions docs/rules/no-get-with-default.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# No `getWithDefault` (no-get-with-default)

This rule attempts to catch and prevent the use of `getWithDefault`.

## Rule Details

Even though the behavior for `getWithDefault` is more defined such that it only falls back to the default value on `undefined`,
its inconsistency with the native `||` is confusing to many developers who assume otherwise. This rule encourages developers to use
the native `||` operator instead.

In addition, [Nullish Coalescing Operator `??`](https://github.com/tc39/proposal-nullish-coalescing) will land in the JavaScript language soon so developers can leverage safe property access with native support instead of using `getWithDefault`.

This rule **forbids** the following:

```js
const test = this.getWithDefault('key', []);
```

```js
const test = getWithDefault(this, 'key', []);
```

This rule **allows** the following:

```js
const test = this.key === undefined ? [] : this.key;
```

```js
// the behavior of this is different because `test` would be assigned `[]` on any falsy value instead of on only `undefined`.
const test = this.key || [];
```

## References

- [RFC](https://github.com/emberjs/rfcs/pull/554/) to deprecate `getWithDefault`
- [spec](http://api.emberjs.com/ember/3.13/functions/@ember%2Fobject/getWithDefault)

## Related Rules

- [no-get](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-get.md)
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,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-with-default': require('./rules/no-get-with-default'),
'no-get': require('./rules/no-get'),
'no-global-jquery': require('./rules/no-global-jquery'),
'no-incorrect-calls-with-inline-anonymous-functions': require('./rules/no-incorrect-calls-with-inline-anonymous-functions'),
Expand Down
1 change: 1 addition & 0 deletions lib/octane-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module.exports = {
"ember/no-classic-classes": "error",
"ember/no-classic-components": "error",
"ember/no-computed-properties-in-native-classes": "error",
"ember/no-get-with-default": "error",
"ember/no-get": "error",
"ember/no-jquery": "error",
"ember/require-tagless-components": "error"
Expand Down
1 change: 1 addition & 0 deletions lib/recommended-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,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-with-default": "off",
"ember/no-get": "off",
"ember/no-global-jquery": "error",
"ember/no-incorrect-calls-with-inline-anonymous-functions": "error",
Expand Down
46 changes: 46 additions & 0 deletions lib/rules/no-get-with-default.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

const types = require('../utils/types');

const ERROR_MESSAGE = 'Use `||` or the ternary operator instead of `getWithDefault()`';

module.exports = {
ERROR_MESSAGE,
meta: {
type: 'suggestion',
docs: {
description: "Disallows use of the Ember's `getWithDefault` function",
category: 'Best Practices',
recommended: false,
octane: true,
url:
'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-get-with-default.md',
},
},
create(context) {
return {
CallExpression(node) {
if (
types.isMemberExpression(node.callee) &&
types.isThisExpression(node.callee.object) &&
types.isIdentifier(node.callee.property) &&
node.callee.property.name === 'getWithDefault' &&
node.arguments.length === 2
) {
// Example: this.getWithDefault('foo', 'bar');
context.report(node, ERROR_MESSAGE);
}

if (
types.isIdentifier(node.callee) &&
node.callee.name === 'getWithDefault' &&
node.arguments.length === 3 &&
types.isThisExpression(node.arguments[0])
) {
// Example: getWithDefault(this, 'foo', 'bar');
context.report(node, ERROR_MESSAGE);
}
},
};
},
};
41 changes: 41 additions & 0 deletions tests/lib/rules/no-get-with-default.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
const rule = require('../../../lib/rules/no-get-with-default');
const RuleTester = require('eslint').RuleTester;

const { ERROR_MESSAGE } = rule;
const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 2015,
sourceType: 'module',
},
});

ruleTester.run('no-get-with-default', rule, {
valid: [
"const test = this.get('key') || [];",
"const test = get(this, 'target') || [];",
"testClass.getWithDefault('key', [])",
"getWithDefault.testMethod(testClass, 'key', [])",
],
invalid: [
{
code: "const test = this.getWithDefault('key', []);",
errors: [
{
message: ERROR_MESSAGE,
type: 'CallExpression',
},
],
output: null,
},
{
code: "const test = getWithDefault(this, 'key', []);",
errors: [
{
message: ERROR_MESSAGE,
type: 'CallExpression',
},
],
output: null,
},
],
});

0 comments on commit 1c18fd9

Please sign in to comment.