Skip to content

Commit

Permalink
Adding return-from-computed rule. Fixes ember-cli#247.
Browse files Browse the repository at this point in the history
  • Loading branch information
Garrett Murphey committed Nov 12, 2018
1 parent 390c39c commit 08c424e
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 0 deletions.
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 @@ -31,6 +31,7 @@ module.exports = {
'order-in-controllers': require('./rules/order-in-controllers'),
'order-in-models': require('./rules/order-in-models'),
'order-in-routes': require('./rules/order-in-routes'),
'return-from-computed': require('./rules/return-from-computed'),
'require-super-in-init': require('./rules/require-super-in-init'),
'routes-segments-snake-case': require('./rules/routes-segments-snake-case'),
'use-brace-expansion': require('./rules/use-brace-expansion'),
Expand Down
81 changes: 81 additions & 0 deletions lib/rules/return-from-computed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
'use strict';


const { isComputedProp } = require('../utils/ember');

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

function isReachable(segment) {
return segment.reachable;
}


module.exports = {
meta: {
docs: {
description: 'Warns about missing return statements in computed properties',
category: 'Possible Errors',
recommended: false,
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 = 'Always return a value from computed properties';

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

let funcInfo = {
upper: null,
codePath: null,
hasReturn: false,
shouldCheck: false,
node: null
};

function checkLastSegment(node) {
if (funcInfo.shouldCheck &&
funcInfo.codePath.currentSegments.some(isReachable)
) {
report(node);
}
}

return {
onCodePathStart(codePath, node) {
funcInfo = {
upper: funcInfo,
codePath,
hasReturn: false,
shouldCheck: context.getAncestors().findIndex(isComputedProp) > -1,
node
};
},

onCodePathEnd() {
funcInfo = funcInfo.upper;
},

ReturnStatement(node) {
if (funcInfo.shouldCheck) {
funcInfo.hasReturn = true;

if (!node.argument) {
report(node);
}
}
},

'FunctionExpression:exit': function (node) {
if (isComputedProp(node.parent) || isComputedProp(node.parent.parent.parent)) {
checkLastSegment(node);
}
}
};
}
};
78 changes: 78 additions & 0 deletions tests/lib/rules/return-from-computed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// ------------------------------------------------------------------------------
// 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", function() { return ""; })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'let foo = computed("test", { get() { return true; }, set() { return true; } })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'let foo = computed("test", function() { if (true) { return ""; } return ""; })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'let foo = computed("test", { get() { data.forEach(function() { }); return true; }, set() { return true; } })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'let foo = computed("test", function() { data.forEach(function() { }); return ""; })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
}
],
invalid: [
{
code: 'let foo = computed("test", function() { })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{
message: 'Always return a value from computed properties'
}]
},
{
code: 'let foo = computed("test", function() { return; })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{
message: 'Always return a value from computed properties'
}]
},
{
code: 'let foo = computed("test", function() { if (true) { return ""; } })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{
message: 'Always return a value from computed properties'
}]
},
{
code: 'let foo = computed("test", { get() {}, set() {} })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [
{
message: 'Always return a value from computed properties'
},
{
message: 'Always return a value from computed properties'
}
]
},
{
code: 'let foo = computed("test", function() { })',
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{
message: 'Always return a value from computed properties'
}]
}
],
});

0 comments on commit 08c424e

Please sign in to comment.