Repository: ember-cli/eslint-plugin-ember
Pull request #108 (2017-07-26 19:35:45)

Add additional position for empty methods
This PR adds a new group for empty methods for all order-in rules.  

The idea behind this is, that some applications and addons define default values for actions in components as empty functions (examples can be found in [ember-bootstrap](https://github.com/kaliber5/ember-bootstrap/blob/master/addon/components/base/bs-form.js#L193)). Right now the order-in-components rule can't be configured in a way that allows these methods to be part of the defaults block while having the real methods at a whole other position.  

This PR introduces `empty-method` properties. These are basically methods with an empty body.  
For backwards compatibility empty methods are part of the methods block. If the user has specified an own order and `empty-method` is not part of it, it will automatically included in the `method` block (if `empty-method` is included, the order stays untouched). So everything that is currently valid, should still be valid after this PR.


t-sauer: PR is updated.

Patch:
diff --git a/docs/rules/order-in-components.md b/docs/rules/order-in-components.md
index b04c7e11..45b4958b 100644
--- a/docs/rules/order-in-components.md
+++ b/docs/rules/order-in-components.md
@@ -14,7 +14,7 @@ ember/order-in-components: [2, {
     'observer',
     'lifecycle-hook',
     'actions',
-    'method',
+    ['method', 'empty-method'],
   ]
 }]
 ```
@@ -29,7 +29,7 @@ order: [
   'observer',
   'lifecycle-hook',
   'actions',
-  'method',
+  ['method', 'empty-method'],
 ]
 ```
 
diff --git a/docs/rules/order-in-controllers.md b/docs/rules/order-in-controllers.md
index c5ada761..b0f98009 100644
--- a/docs/rules/order-in-controllers.md
+++ b/docs/rules/order-in-controllers.md
@@ -15,7 +15,7 @@ ember/order-in-controllers: [2, {
     'multi-line-function',
     'observer',
     'actions',
-    'method',
+    ['method', 'empty-method'],
   ]
 }]
 ```
diff --git a/docs/rules/order-in-routes.md b/docs/rules/order-in-routes.md
index 4a260c7a..0c79d147 100644
--- a/docs/rules/order-in-routes.md
+++ b/docs/rules/order-in-routes.md
@@ -13,7 +13,7 @@ ember/order-in-routes: [2, {
     'model',
     'lifecycle-hook',
     'actions',
-    'method',
+    ['method', 'empty-method'],
   ]
 }]
 ```
@@ -27,7 +27,7 @@ order: [
   'model',
   'lifecycle-hook',
   'actions',
-  'method',
+  ['method', 'empty-method'],
 ]
 ```
 
diff --git a/lib/rules/order-in-components.js b/lib/rules/order-in-components.js
index 78bef8a6..dcc894bf 100644
--- a/lib/rules/order-in-components.js
+++ b/lib/rules/order-in-components.js
@@ -4,6 +4,7 @@ const ember = require('../utils/ember');
 const propOrder = require('../utils/property-order');
 
 const reportUnorderedProperties = propOrder.reportUnorderedProperties;
+const addBackwardsPosition = propOrder.addBackwardsPosition;
 
 const ORDER = [
   'service',
@@ -13,7 +14,7 @@ const ORDER = [
   'observer',
   'lifecycle-hook',
   'actions',
-  'method',
+  ['method', 'empty-method'],
 ];
 
 //------------------------------------------------------------------------------
@@ -32,7 +33,8 @@ module.exports = {
 
   create(context) {
     const options = context.options[0] || {};
-    const order = options.order || ORDER;
+    const order = options.order ? addBackwardsPosition(options.order, 'empty-method', 'method') : ORDER;
+
     const filePath = context.getFilename();
 
     return {
diff --git a/lib/rules/order-in-controllers.js b/lib/rules/order-in-controllers.js
index 8e335181..b2edbc3e 100644
--- a/lib/rules/order-in-controllers.js
+++ b/lib/rules/order-in-controllers.js
@@ -4,6 +4,7 @@ const ember = require('../utils/ember');
 const propOrder = require('../utils/property-order');
 
 const reportUnorderedProperties = propOrder.reportUnorderedProperties;
+const addBackwardsPosition = propOrder.addBackwardsPosition;
 
 const ORDER = [
   'service',
@@ -14,7 +15,7 @@ const ORDER = [
   'multi-line-function',
   'observer',
   'actions',
-  'method',
+  ['method', 'empty-method'],
 ];
 
 //------------------------------------------------------------------------------
@@ -33,8 +34,8 @@ module.exports = {
 
   create(context) {
     const options = context.options[0] || {};
-    const order = options.order || ORDER;
     const filePath = context.getFilename();
+    const order = options.order ? addBackwardsPosition(options.order, 'empty-method', 'method') : ORDER;
 
     return {
       CallExpression(node) {
diff --git a/lib/rules/order-in-models.js b/lib/rules/order-in-models.js
index ce7758ab..e07fc1e4 100644
--- a/lib/rules/order-in-models.js
+++ b/lib/rules/order-in-models.js
@@ -4,6 +4,7 @@ const ember = require('../utils/ember');
 const propOrder = require('../utils/property-order');
 
 const reportUnorderedProperties = propOrder.reportUnorderedProperties;
+const addBackwardsPosition = propOrder.addBackwardsPosition;
 
 const ORDER = [
   'attribute',
@@ -29,7 +30,7 @@ module.exports = {
 
   create(context) {
     const options = context.options[0] || {};
-    const order = options.order || ORDER;
+    const order = options.order ? addBackwardsPosition(options.order, 'empty-method', 'method') : ORDER;
 
     return {
       CallExpression(node) {
diff --git a/lib/rules/order-in-routes.js b/lib/rules/order-in-routes.js
index 940842b8..68af1d1b 100644
--- a/lib/rules/order-in-routes.js
+++ b/lib/rules/order-in-routes.js
@@ -4,6 +4,7 @@ const ember = require('../utils/ember');
 const propOrder = require('../utils/property-order');
 
 const reportUnorderedProperties = propOrder.reportUnorderedProperties;
+const addBackwardsPosition = propOrder.addBackwardsPosition;
 
 const ORDER = [
   'service',
@@ -14,7 +15,7 @@ const ORDER = [
   'model',
   'lifecycle-hook',
   'actions',
-  'method',
+  ['method', 'empty-method'],
 ];
 
 //------------------------------------------------------------------------------
@@ -33,8 +34,8 @@ module.exports = {
 
   create(context) {
     const options = context.options[0] || {};
-    const order = options.order || ORDER;
     const filePath = context.getFilename();
+    const order = options.order ? addBackwardsPosition(options.order, 'empty-method', 'method') : ORDER;
 
     return {
       CallExpression(node) {
diff --git a/lib/utils/property-order.js b/lib/utils/property-order.js
index 56237dcc..164768d6 100644
--- a/lib/utils/property-order.js
+++ b/lib/utils/property-order.js
@@ -1,10 +1,12 @@
 'use strict';
 
 const ember = require('./ember');
+const utils = require('./utils');
 
 module.exports = {
   determinePropertyType,
   reportUnorderedProperties,
+  addBackwardsPosition,
 };
 
 const NAMES = {
@@ -17,6 +19,7 @@ const NAMES = {
   'lifecycle-hook': 'lifecycle hook',
   actions: 'actions hash',
   method: 'method',
+  'empty-method': 'empty method',
   unknown: 'unknown property type',
   attribute: 'attribute',
   relationship: 'relationship',
@@ -81,6 +84,10 @@ function determinePropertyType(node, parentType) {
   }
 
   if (ember.isFunctionExpression(node.value)) {
+    if (utils.isEmptyMethod(node)) {
+      return 'empty-method';
+    }
+
     return 'method';
   }
 
@@ -163,3 +170,30 @@ function reportUnorderedProperties(node, context, parentType, ORDER) {
     }
   });
 }
+
+function addBackwardsPosition(order, newPosition, targetPosition) {
+  const positionOrder = order.slice();
+
+  const containsPosition = positionOrder.some((position) => {
+    if (Array.isArray(position)) {
+      return position.indexOf(newPosition) > -1;
+    }
+
+    return position === newPosition;
+  });
+
+  if (!containsPosition) {
+    const targetIndex = positionOrder.indexOf(targetPosition);
+    if (targetIndex > -1) {
+      positionOrder[targetIndex] = [targetPosition, newPosition];
+    } else {
+      positionOrder.forEach((position) => {
+        if (Array.isArray(position) && position.indexOf(targetPosition) > -1) {
+          position.push(newPosition);
+        }
+      });
+    }
+  }
+
+  return positionOrder;
+}
diff --git a/lib/utils/utils.js b/lib/utils/utils.js
index a6e042fb..3125fe82 100644
--- a/lib/utils/utils.js
+++ b/lib/utils/utils.js
@@ -25,6 +25,7 @@ module.exports = {
   findUnorderedProperty,
   getPropertyValue,
   collectObjectPatternBindings,
+  isEmptyMethod,
 };
 
 /**
@@ -357,3 +358,15 @@ function collectObjectPatternBindings(node, initialObjToBinding) {
     .filter(props => initialObjToBinding[binding].indexOf(props.key.name) > -1)
     .map(props => props.value.name);
 }
+
+/**
+ * Check whether or not a node is a empty method.
+ *
+ * @param {Object} node The node to check.
+ * @returns {boolean} Whether or not the node is an empty method.
+ */
+function isEmptyMethod(node) {
+  return node.value.body
+    && node.value.body.body
+    && node.value.body.body.length <= 0;
+}


Test patch:
diff --git a/tests/lib/rules/order-in-components.js b/tests/lib/rules/order-in-components.js
index 4ac5eae4..0ed05eb8 100644
--- a/tests/lib/rules/order-in-components.js
+++ b/tests/lib/rules/order-in-components.js
@@ -354,6 +354,35 @@ eslintTester.run('order-in-components', rule, {
         })
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+    },
+    {
+      code: `export default Component.extend({
+        foo: computed(function() {
+        }).volatile(),
+        onFoo() {},
+        bar() { const foo = 'bar'},
+        onFoo: () => {}
+      });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
+    },
+    {
+      code: `export default Component.extend({
+        onFoo() {},
+        onFoo: () => {},
+        foo: computed(function() {
+        }).volatile(),
+        bar() { const foo = 'bar'}
+      });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      options: [{
+        order: [
+          'property',
+          'empty-method',
+          'single-line-function',
+          'multi-line-function',
+          'method',
+        ],
+      }],
     }
   ],
   invalid: [
@@ -546,13 +575,14 @@ eslintTester.run('order-in-components', rule, {
     {
       code: `export default Component.extend({
         customFunc() {
+          const foo = 'bar';
         },
         actions: {}
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'The actions hash should be above the "customFunc" method on line 2',
-        line: 4,
+        line: 5,
       }],
     },
     {
diff --git a/tests/lib/rules/order-in-controllers.js b/tests/lib/rules/order-in-controllers.js
index 746ff483..4e02c12e 100644
--- a/tests/lib/rules/order-in-controllers.js
+++ b/tests/lib/rules/order-in-controllers.js
@@ -22,8 +22,8 @@ eslintTester.run('order-in-controllers', rule, {
         queryParams: [],
         customProp: "test",
         actions: {},
-        _customAction() {},
-        _customAction2: function() {},
+        _customAction() { const foo = 'bar'; },
+        _customAction2: function() { const foo = 'bar'; },
         tSomeTask: task(function* () {})
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
@@ -57,7 +57,7 @@ eslintTester.run('order-in-controllers', rule, {
         comp2: computed("test", function() {
         }),
         actions: {},
-        _customAction() {}
+        _customAction() { const foo = 'bar'; }
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
     },
@@ -68,7 +68,7 @@ eslintTester.run('order-in-controllers', rule, {
         customProp: "test",
         comp2: computed("test", function() {
         }),
-        _customAction() {}
+        _customAction() { const foo = 'bar'; }
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       options: [{
@@ -142,7 +142,7 @@ eslintTester.run('order-in-controllers', rule, {
     {
       code: `export default Controller.extend({
         queryParams: [],
-        _customAction() {},
+        _customAction() { const foo = 'bar'; },
         actions: {}
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
diff --git a/tests/lib/rules/order-in-routes.js b/tests/lib/rules/order-in-routes.js
index ffdd8cc7..baf84de2 100644
--- a/tests/lib/rules/order-in-routes.js
+++ b/tests/lib/rules/order-in-routes.js
@@ -27,8 +27,8 @@ eslintTester.run('order-in-routes', rule, {
         model() {},
         beforeModel() {},
         actions: {},
-        _customAction() {},
-        _customAction2: function() {},
+        _customAction() { const foo = 'bar'; },
+        _customAction2: function() { const foo = 'bar'; },
         tSomeTask: task(function* () {})
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
@@ -58,7 +58,7 @@ eslintTester.run('order-in-routes', rule, {
         actions: {
           test() { return this._customAction() }
         },
-        _customAction() {}
+        _customAction() { const foo = 'bar'; }
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
     },
@@ -191,7 +191,7 @@ eslintTester.run('order-in-routes', rule, {
         vehicle: alias("car"),
         customProp: "test",
         model() {},
-        _customAction() {},
+        _customAction() { const foo = 'bar'; },
         actions: {}
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
@@ -230,7 +230,7 @@ eslintTester.run('order-in-routes', rule, {
     {
       code: `export default Route.extend({
         test: "asd",
-        _test2() {},
+        _test2() { const foo = 'bar'; },
         model() {}
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
diff --git a/tests/lib/utils/property-order.js b/tests/lib/utils/property-order.js
new file mode 100644
index 00000000..f6d75fb4
--- /dev/null
+++ b/tests/lib/utils/property-order.js
@@ -0,0 +1,30 @@
+'use-strict';
+
+const propertyOrder = require('../../../lib/utils/property-order');
+
+describe('addBackwardsPosition', () => {
+  it('should not change the order if the desired position is already included in the order', () => {
+    const newOrder = propertyOrder.addBackwardsPosition(['method', 'empty-method'], 'empty-method', 'method');
+    expect(newOrder).toEqual(['method', 'empty-method']);
+  });
+
+  it('should not change the order if the desired position is already included as part of another position group', () => {
+    const newOrder = propertyOrder.addBackwardsPosition([['method', 'empty-method'], 'foo'], 'empty-method', 'method');
+    expect(newOrder).toEqual([['method', 'empty-method'], 'foo']);
+  });
+
+  it('should not add the position, if the target position is not present', () => {
+    const newOrder = propertyOrder.addBackwardsPosition(['foo'], 'empty-method', 'bar');
+    expect(newOrder).toEqual(['foo']);
+  });
+
+  it('should add the desired position to the existing target position when the target position is on its own position', () => {
+    const newOrder = propertyOrder.addBackwardsPosition(['method', 'foo'], 'empty-method', 'method');
+    expect(newOrder).toEqual([['method', 'empty-method'], 'foo']);
+  });
+
+  it('should add the desired position to the existing target position when the target position is part of a group', () => {
+    const newOrder = propertyOrder.addBackwardsPosition([['method', 'bar'], 'foo'], 'empty-method', 'method');
+    expect(newOrder).toEqual([['method', 'bar', 'empty-method'], 'foo']);
+  });
+});