Skip to content

Commit

Permalink
Adding return-from-computed recommended rule. Fixes ember-cli#247.
Browse files Browse the repository at this point in the history
  • Loading branch information
Garrett Murphey committed Jun 12, 2018
1 parent 39259f3 commit 50737f1
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 0 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ The `--fix` option on the command line automatically fixes problems reported by
| :white_check_mark: | [no-function-prototype-extensions](./docs/rules/no-function-prototype-extensions.md) | Prevents usage of Ember's `function` prototype extensions |
| :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 |
| | [no-observers](./docs/rules/no-observers.md) | Prevents usage of observers |
| :white_check_mark::wrench: | [no-old-shims](./docs/rules/no-old-shims.md) | Prevents usage of old shims for modules |
| :white_check_mark: | [no-on-calls-in-components](./docs/rules/no-on-calls-in-components.md) | Prevents usage of `on` calls in components |
Expand All @@ -107,6 +108,7 @@ The `--fix` option on the command line automatically fixes problems reported by
| :white_check_mark: | [no-duplicate-dependent-keys](./docs/rules/no-duplicate-dependent-keys.md) | Disallow repeating dependent keys |
| :white_check_mark: | [no-side-effects](./docs/rules/no-side-effects.md) | Warns about unexpected side effects in computed properties |
| :white_check_mark: | [require-super-in-init](./docs/rules/require-super-in-init.md) | Enforces super calls in init hooks |
| :white_check_mark: | [return-from-computed](./docs/rules/return-from-computed.md) | Warns about missing return statements in computed properties |
| :white_check_mark: | [routes-segments-snake-case](./docs/rules/routes-segments-snake-case.md) | Enforces usage of snake_cased dynamic segments in routes |


Expand Down
44 changes: 44 additions & 0 deletions docs/rules/return-from-computed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
## Always return a value from a computed property function

### Rule name: `return-from-computed`

When using computed properties always return a value.

```javascript
import Ember from 'ember';

const {
Component,
computed
} = Ember;

export default Component.extend({
firstName: null,
lastName: null,

// GOOD
fullName: computed('firstName', 'lastName', {
get(key) {
return `${this.get('firstName')} ${this.get('lastName')}`;
},
set(key, value) {
let [firstName, lastName] = value.split(/\s+/);
this.set('firstName', firstName);
this.set('lastName', lastName);
return value;
}
}),

// BAD
fullName: computed('firstName', 'lastName', {
get(key) {
return `${this.get('firstName')} ${this.get('lastName')}`;
},
set(key, value) {
let [firstName, lastName] = value.split(/\s+/);
this.set('firstName', firstName);
this.set('lastName', lastName);
}
})
});
```
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ module.exports = {
'order-in-models': require('./rules/order-in-models'),
'order-in-routes': require('./rules/order-in-routes'),
'require-super-in-init': require('./rules/require-super-in-init'),
'return-from-computed': require('./rules/return-from-computed'),
'routes-segments-snake-case': require('./rules/routes-segments-snake-case'),
'use-brace-expansion': require('./rules/use-brace-expansion'),
'use-ember-get-and-set': require('./rules/use-ember-get-and-set'),
Expand Down
2 changes: 2 additions & 0 deletions lib/recommended-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module.exports = {
"ember/no-function-prototype-extensions": "error",
"ember/no-global-jquery": "error",
"ember/no-jquery": "off",
"ember/no-new-mixins": "off",
"ember/no-observers": "off",
"ember/no-old-shims": "error",
"ember/no-on-calls-in-components": "error",
Expand All @@ -30,6 +31,7 @@ module.exports = {
"ember/order-in-models": "off",
"ember/order-in-routes": "off",
"ember/require-super-in-init": "error",
"ember/return-from-computed": "error",
"ember/routes-segments-snake-case": "error",
"ember/use-brace-expansion": "error",
"ember/use-ember-get-and-set": "off"
Expand Down
53 changes: 53 additions & 0 deletions lib/rules/return-from-computed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';

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

//------------------------------------------------------------------------------
// General rule - Don't introduce side-effects in computed properties
//------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Warns about missing return statements in computed properties',
category: 'Possible Errors',
recommended: true,
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-side-effects.md'
},
fixable: null, // or "code" or "whitespace"
},

create(context) {
const message = 'Return a value from computed property';

const report = function (node) {
context.report(node, message);
};

return {
'CallExpression[callee.name="computed"]': function (node) {
const objExps = utils.findNodes(node.arguments, 'ObjectExpression');
const fnExps = utils.findNodes(node.arguments, 'FunctionExpression');

if (objExps.length) {
const computedObj = objExps[0];
const props = computedObj.properties;

props.forEach((prop) => {
if (utils.isFunctionExpression(prop.value) && !utils.findNodes(prop.value.body.body, 'ReturnStatement').length) {
report(prop.value);
}
});
}

if (fnExps.length) {
const computedFn = fnExps[0];

if (!utils.findNodes(computedFn.body.body, 'ReturnStatement').length) {
report(computedFn);
}
}
}
};
}
};
1 change: 1 addition & 0 deletions tests/__snapshots__/recommended.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Array [
"no-on-calls-in-components",
"no-side-effects",
"require-super-in-init",
"return-from-computed",
"routes-segments-snake-case",
"use-brace-expansion",
]
Expand Down
59 changes: 59 additions & 0 deletions tests/lib/rules/return-from-computed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

const rule = require('../../../lib/rules/return-from-computed');
const RuleTester = require('eslint').RuleTester;

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------

const eslintTester = new RuleTester();
eslintTester.run('return-from-computed', rule, {
valid: [
{
code: 'let foo = computed("test.length", { get() { return ""; }, set() { return ""; }})',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'let foo = computed("test", function() { return ""; })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
}
],
invalid: [
{
code: 'let foo = computed("test", function() { })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{
message: 'Return a value from computed property'
}]
},
{
code: 'let foo = computed("test", function() { if (true) { return ""; } })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{
message: 'Return a value from computed property'
}]
},
{
code: 'let foo = computed("test", { get() {}, set() {} })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [
{
message: 'Return a value from computed property'
},
{
message: 'Return a value from computed property'
}
]
},
{
code: 'let foo = computed("test", function() { })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{
message: 'Return a value from computed property'
}]
}
],
});

0 comments on commit 50737f1

Please sign in to comment.