Skip to content

Commit

Permalink
Add rule to enforce consistent computed property style
Browse files Browse the repository at this point in the history
Defaults to `never` allow getters in computed properties, has the option
to `always` require them.
  • Loading branch information
jrjohnson committed Mar 23, 2019
1 parent afc8bd3 commit 468661f
Show file tree
Hide file tree
Showing 4 changed files with 251 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ The `--fix` option on the command line automatically fixes problems reported by
| | Rule ID | Description |
|:---|:--------|:------------|
| :white_check_mark: | [avoid-leaking-state-in-ember-objects](./docs/rules/avoid-leaking-state-in-ember-objects.md) | Avoids state leakage |
| | [computed-property-getters](./docs/rules/computed-property-getters.md) | Enforce the consistent use of getters in computed properties |


### Testing
Expand Down
57 changes: 57 additions & 0 deletions docs/rules/computed-property-getters.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Rule name: `computed-property-getters`

## Enforce the consistent use of getters in computed properties

Computed properties may be created with or without a `get` method. This rule ensures that the choice
is consistent.

## Options

This rule takes a single string option

String option:

* `"never"` (default) getters are *never allowed* in computed properties
* `"always"` getters are *required* in computed properties

### never

```javascript
/// GOOD
Ember.Object.extend({
fullName: computed('firstName', 'lastName', function() {
//...
})
});

// BAD
Ember.Object.extend({
fullName: computed('firstName', 'lastName', {
get() {
//...
}
})
});
```

### always

```javascript
/// GOOD
Ember.Object.extend({
fullName: computed('firstName', 'lastName', {
get() {
//...
}
})
});

// BAD
Ember.Object.extend({
fullName: computed('firstName', 'lastName', function() {
//...
})
});
```


67 changes: 67 additions & 0 deletions lib/rules/computed-property-getters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
'use strict';

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

//------------------------------------------------------------------------------
// General rule - Prevent using a getter inside computed properties.
//------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Warns about getters in computed properties',
category: 'Possible Errors',
recommended: false,
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/computed-property-getters.md'
},
fixable: null, // or "code" or "whitespace"
schema: [
{
enum: ['always', 'never']
},
],
},

create(context) {
const requireGetters = context.options[0] || 'never';
const message = 'Do not use a getter inside computed properties.';

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

const preventGetterInComputedProperty = function (node) {
const objectExpressions = node.arguments.filter(arg => utils.isObjectExpression(arg));
if (
objectExpressions.length
) {
report(node);
}
};

const requireGetterInComputedProperty = function (node) {
const functionExpressions = node.arguments.filter(arg => utils.isFunctionExpression(arg));
if (
functionExpressions.length
) {
report(node);
}
};

return {
CallExpression(node) {
if (
ember.isComputedProp(node) &&
node.arguments.length
) {
if (requireGetters === 'always') {
requireGetterInComputedProperty(node);
} else {
preventGetterInComputedProperty(node);
}
}
}
};
}
};
126 changes: 126 additions & 0 deletions tests/lib/rules/computed-property-getters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
'use strict';

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

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


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

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

const codeWithoutGetters = [
`
{
foo: computed('model', function() {})
}`,
`{
foo: computed('model', function() {}).volatile()
}`,
`{
foo: computed(function() {})
}`,
`{
foo: computed(async function() {})
}`,
`{
foo: computed('model', async function() {})
}`
];

const codeWithGetters = [
`{
foo: computed({
get() {
return true;
}
}).readonly()
}`,
`{
foo: computed({
get() {
return true;
}
})
}`,
`{
foo: computed('model.foo', {
get() {
return true;
}
}).readonly()
}`,
`{
foo: computed('model.foo', {
get() {
return true;
}
})
}`,
`{
foo: computed('model.foo', {
get() {}
})
}`
];


const validWithDefaultOptions = codeWithoutGetters.map(code => ({ code, parserOptions }));

const validWithNeverOption = codeWithoutGetters.map((code) => {
const options = ['never'];
return { code, parserOptions, options };
});

const validWithAlwaysOption = codeWithGetters.map((code) => {
const options = ['always'];
return { code, parserOptions, options };
});

const inValidWithDefaultOptions = codeWithGetters.map(code => (
{
code,
parserOptions,
output: null,
errors: [{
message: rule.meta.message,
}]
}
));

const inValidWithNeverOption = codeWithGetters.map((code) => {
const options = ['never'];
return {
code,
parserOptions,
options,
output: null,
errors: [{
message: rule.meta.message,
}]
};
});

const inValidWithAlwaysOption = codeWithoutGetters.map((code) => {
const options = ['always'];
return {
code,
parserOptions,
options,
output: null,
errors: [{
message: rule.meta.message,
}]
};
});

ruleTester.run('computed-property-getters', rule, {
valid: [...validWithDefaultOptions, ...validWithNeverOption, ...validWithAlwaysOption],
invalid: [...inValidWithDefaultOptions, ...inValidWithNeverOption, ...inValidWithAlwaysOption],
});

0 comments on commit 468661f

Please sign in to comment.