Skip to content

Commit

Permalink
Change default for computed-property-getters
Browse files Browse the repository at this point in the history
This defaults to requiring getters if there is a setter. It preserves
the option to prevent getters entirely as a proxy for preventing setters
as well.
  • Loading branch information
jrjohnson committed Apr 15, 2019
1 parent 468661f commit bf2cb10
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 9 deletions.
53 changes: 49 additions & 4 deletions docs/rules/computed-property-getters.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,44 @@ This rule takes a single string option

String option:

* `"never"` (default) getters are *never allowed* in computed properties
* `"always-with-setter"` (default) getters are *required* when computed property has a setter
* `"always"` getters are *required* in computed properties
* `"never"` getters are *never allowed* in computed properties

### never

### always-with-setter

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

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

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


### always

```javascript
Expand All @@ -54,4 +69,34 @@ Ember.Object.extend({
});
```

### never

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

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

// BAD
Ember.Object.extend({
fullName: computed('firstName', 'lastName', {
get() {
//...
},
set() {
//...
}
})
});
```
27 changes: 24 additions & 3 deletions lib/rules/computed-property-getters.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,36 @@ module.exports = {
fixable: null, // or "code" or "whitespace"
schema: [
{
enum: ['always', 'never']
enum: ['always-with-setter', 'always', 'never']
},
],
},

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

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

const requireGetterWithSetterInComputedProperty = function (node) {
const objectExpressions = node.arguments.filter(arg => utils.isObjectExpression(arg));
if (
objectExpressions.length
) {
const { properties } = objectExpressions[0];
const getters = properties.filter(prop => prop.key && prop.key.name && prop.key.name === 'get');
const setters = properties.filter(prop => prop.key && prop.key.name && prop.key.name === 'set');
if (
setters.length === 0 ||
setters.length > 0 && getters.length === 0
) {
report(node);
}
}
};

const preventGetterInComputedProperty = function (node) {
const objectExpressions = node.arguments.filter(arg => utils.isObjectExpression(arg));
if (
Expand All @@ -55,9 +72,13 @@ module.exports = {
ember.isComputedProp(node) &&
node.arguments.length
) {
if (requireGetters === 'always-with-setter') {
requireGetterWithSetterInComputedProperty(node);
}
if (requireGetters === 'always') {
requireGetterInComputedProperty(node);
} else {
}
if (requireGetters === 'never') {
preventGetterInComputedProperty(node);
}
}
Expand Down
70 changes: 68 additions & 2 deletions tests/lib/rules/computed-property-getters.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,69 @@ const codeWithGetters = [
}`
];

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


const validWithDefaultOptions = [];
validWithDefaultOptions.push(...codeWithoutGetters.map(code => ({ code, parserOptions })));
validWithDefaultOptions.push(...codeWithSettersAndGetters.map(code => ({ code, parserOptions })));

const validWithDefaultOptions = codeWithoutGetters.map(code => ({ code, parserOptions }));
const validWithAlwaysWithSetterOptions = [];
validWithAlwaysWithSetterOptions.push(...codeWithoutGetters.map((code) => {
const options = ['always-with-setter'];
return { code, parserOptions, options };
}));
validWithAlwaysWithSetterOptions.push(...codeWithSettersAndGetters.map((code) => {
const options = ['always-with-setter'];
return { code, parserOptions, options };
}));

const validWithNeverOption = codeWithoutGetters.map((code) => {
const options = ['never'];
Expand Down Expand Up @@ -121,6 +182,11 @@ const inValidWithAlwaysOption = codeWithoutGetters.map((code) => {
});

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

0 comments on commit bf2cb10

Please sign in to comment.