From bf2cb10f148a14ae45ff5b1ce6a4703db1fc2e31 Mon Sep 17 00:00:00 2001
From: Jonathan Johnson <jon.johnson@ucsf.edu>
Date: Mon, 15 Apr 2019 15:34:30 -0700
Subject: [PATCH] Change default for computed-property-getters

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.
---
 docs/rules/computed-property-getters.md      | 53 +++++++++++++--
 lib/rules/computed-property-getters.js       | 27 +++++++-
 tests/lib/rules/computed-property-getters.js | 70 +++++++++++++++++++-
 3 files changed, 141 insertions(+), 9 deletions(-)

diff --git a/docs/rules/computed-property-getters.md b/docs/rules/computed-property-getters.md
index b79fc48931..de6c5d98a4 100644
--- a/docs/rules/computed-property-getters.md
+++ b/docs/rules/computed-property-getters.md
@@ -11,22 +11,36 @@ 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() {
             //...
         }
@@ -34,6 +48,7 @@ Ember.Object.extend({
 });
 ```
 
+
 ### always
 
 ```javascript
@@ -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() {
+            //...
+        }
+    })
+});
+```
diff --git a/lib/rules/computed-property-getters.js b/lib/rules/computed-property-getters.js
index 78e0828019..88dcb19dcf 100644
--- a/lib/rules/computed-property-getters.js
+++ b/lib/rules/computed-property-getters.js
@@ -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 (
@@ -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);
           }
         }
diff --git a/tests/lib/rules/computed-property-getters.js b/tests/lib/rules/computed-property-getters.js
index facee06124..29a1748de0 100644
--- a/tests/lib/rules/computed-property-getters.js
+++ b/tests/lib/rules/computed-property-getters.js
@@ -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'];
@@ -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],
 });