Prevent Lonely Setters

A setter should never be present when there are no getters no matter what other options are selected.
ember-cli · May 7, 2019 · 1a77d94 · 1a77d94
1 parent 1dac868
commit 1a77d94
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 46 deletions.
diff --git a/lib/rules/computed-property-getters.js b/lib/rules/computed-property-getters.js
@@ -31,7 +31,7 @@ module.exports = {
       context.report(node, message);
     };
 
-    const requireGetterWithSetterInComputedProperty = function (node) {
+    const requireGetterOnlyWithASetterInComputedProperty = function (node) {
       const objectExpressions = node.arguments.filter(arg => utils.isObjectExpression(arg));
       if (
         objectExpressions.length
@@ -40,8 +40,8 @@ module.exports = {
         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)
+          (setters.length > 0 && getters.length === 0) ||
+          (getters.length > 0 && setters.length === 0)
         ) {
           report(node);
         }
@@ -53,15 +53,26 @@ module.exports = {
       if (
         objectExpressions.length
       ) {
-        report(node);
+        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 (getters.length > 0 || setters.length > 0) {
+          report(node);
+        }
       }
     };
 
     const requireGetterInComputedProperty = function (node) {
-      const functionExpressions = node.arguments.filter(arg => utils.isFunctionExpression(arg));
+      const objectExpressions = node.arguments.filter(arg => utils.isObjectExpression(arg));
       if (
-        functionExpressions.length
+        objectExpressions.length
       ) {
+        const { properties } = objectExpressions[0];
+        const getters = properties.filter(prop => prop.key && prop.key.name && prop.key.name === 'get');
+        if (getters.length === 0) {
+          report(node);
+        }
+      } else {
         report(node);
       }
     };
@@ -73,7 +84,7 @@ module.exports = {
           node.arguments.length
         ) {
           if (requireGetters === 'always-with-setter') {
-            requireGetterWithSetterInComputedProperty(node);
+            requireGetterOnlyWithASetterInComputedProperty(node);
           }
           if (requireGetters === 'always') {
             requireGetterInComputedProperty(node);

diff --git a/tests/lib/rules/computed-property-getters.js b/tests/lib/rules/computed-property-getters.js
@@ -14,8 +14,12 @@ const RuleTester = require('eslint').RuleTester;
 
 const ruleTester = new RuleTester();
 const parserOptions = { ecmaVersion: 2018, sourceType: 'module' };
+const errors = [{
+  message: rule.meta.message,
+}];
+const output = null;
 
-const codeWithoutGetters = [
+const codeWithoutGettersOrSetters = [
   `
   {
     foo: computed('model', function() {})
@@ -34,7 +38,7 @@ const codeWithoutGetters = [
     }`
 ];
 
-const codeWithGetters = [
+const codeWithOnlyGetters = [
   `{
       foo: computed({
         get() {
@@ -70,6 +74,42 @@ const codeWithGetters = [
     }`
 ];
 
+const codeWithOnlySetters = [
+  `{
+      foo: computed({
+        set() {
+          return true;
+        }
+      }).readonly()
+    }`,
+  `{
+      foo: computed({
+        set() {
+          return true;
+        }
+      })
+    }`,
+  `{
+      foo: computed('model.foo', {
+        set() {
+          return true;
+        }
+      }).readonly()
+    }`,
+  `{
+      foo: computed('model.foo', {
+        set() {
+          return true;
+        }
+      })
+    }`,
+  `{
+      foo: computed('model.foo', {
+        set() {}
+      })
+    }`
+];
+
 const codeWithSettersAndGetters = [
   `{
       foo: computed({
@@ -121,11 +161,11 @@ const codeWithSettersAndGetters = [
 
 
 const validWithDefaultOptions = [];
-validWithDefaultOptions.push(...codeWithoutGetters.map(code => ({ code, parserOptions })));
+validWithDefaultOptions.push(...codeWithoutGettersOrSetters.map(code => ({ code, parserOptions })));
 validWithDefaultOptions.push(...codeWithSettersAndGetters.map(code => ({ code, parserOptions })));
 
 const validWithAlwaysWithSetterOptions = [];
-validWithAlwaysWithSetterOptions.push(...codeWithoutGetters.map((code) => {
+validWithAlwaysWithSetterOptions.push(...codeWithoutGettersOrSetters.map((code) => {
   const options = ['always-with-setter'];
   return { code, parserOptions, options };
 }));
@@ -134,52 +174,52 @@ validWithAlwaysWithSetterOptions.push(...codeWithSettersAndGetters.map((code) =>
   return { code, parserOptions, options };
 }));
 
-const validWithNeverOption = codeWithoutGetters.map((code) => {
+const validWithNeverOption = codeWithoutGettersOrSetters.map((code) => {
   const options = ['never'];
   return { code, parserOptions, options };
 });
 
-const validWithAlwaysOption = codeWithGetters.map((code) => {
+const validWithAlwaysOption = [];
+validWithAlwaysOption.push(...codeWithOnlyGetters.map((code) => {
   const options = ['always'];
   return { code, parserOptions, options };
-});
+}));
+validWithAlwaysOption.push(...codeWithSettersAndGetters.map((code) => {
+  const options = ['always'];
+  return { code, parserOptions, options };
+}));
 
-const inValidWithDefaultOptions = codeWithGetters.map(code => (
-  {
-    code,
-    parserOptions,
-    output: null,
-    errors: [{
-      message: rule.meta.message,
-    }]
-  }
+const inValidWithDefaultOptions = [];
+inValidWithDefaultOptions.push(...codeWithOnlyGetters.map(code =>
+  ({ code, parserOptions, output, errors })
+));
+inValidWithDefaultOptions.push(...codeWithOnlySetters.map(code =>
+  ({ code, parserOptions, output, errors })
 ));
 
-const inValidWithNeverOption = codeWithGetters.map((code) => {
+const inValidWithNeverOption = [];
+inValidWithNeverOption.push(...codeWithOnlyGetters.map((code) => {
   const options = ['never'];
-  return {
-    code,
-    parserOptions,
-    options,
-    output: null,
-    errors: [{
-      message: rule.meta.message,
-    }]
-  };
-});
+  return { code, parserOptions, options, output, errors };
+}));
+inValidWithNeverOption.push(...codeWithOnlySetters.map((code) => {
+  const options = ['never'];
+  return { code, parserOptions, options, output, errors };
+}));
+inValidWithNeverOption.push(...codeWithSettersAndGetters.map((code) => {
+  const options = ['never'];
+  return { code, parserOptions, options, output, errors };
+}));
 
-const inValidWithAlwaysOption = codeWithoutGetters.map((code) => {
+const inValidWithAlwaysOption = [];
+inValidWithAlwaysOption.push(...codeWithoutGettersOrSetters.map((code) => {
   const options = ['always'];
-  return {
-    code,
-    parserOptions,
-    options,
-    output: null,
-    errors: [{
-      message: rule.meta.message,
-    }]
-  };
-});
+  return { code, parserOptions, options, output, errors };
+}));
+inValidWithAlwaysOption.push(...codeWithOnlySetters.map((code) => {
+  const options = ['always'];
+  return { code, parserOptions, options, output, errors };
+}));
 
 ruleTester.run('computed-property-getters', rule, {
   valid: [
@@ -188,5 +228,9 @@ ruleTester.run('computed-property-getters', rule, {
     ...validWithNeverOption,
     ...validWithAlwaysOption
   ],
-  invalid: [...inValidWithDefaultOptions, ...inValidWithNeverOption, ...inValidWithAlwaysOption],
+  invalid: [
+    ...inValidWithDefaultOptions,
+    ...inValidWithNeverOption,
+    ...inValidWithAlwaysOption
+  ],
 });