Merge pull request #934 from bmish/fix-spread-order-in-rules

Fix spread syntax crashes in `order-in-*` rules
ember-cli · Sep 7, 2020 · ac0636f · ac0636f
2 parents 4768303 + 7a2233d
commit ac0636f
Show file tree

Hide file tree

Showing 16 changed files with 76 additions and 9 deletions.
diff --git a/docs/rules/order-in-components.md b/docs/rules/order-in-components.md
@@ -10,6 +10,7 @@ const rules = {
     2,
     {
       order: [
+        'spread',
         'service',
         'property',
         'empty-method',

diff --git a/docs/rules/order-in-controllers.md b/docs/rules/order-in-controllers.md
@@ -10,6 +10,7 @@ const rules = {
     2,
     {
       order: [
+        'spread',
         'controller',
         'service',
         'query-params',

diff --git a/docs/rules/order-in-models.md b/docs/rules/order-in-models.md
@@ -11,6 +11,7 @@ const rules = {
     {
       // eslint-disable-next-line prettier/prettier
       order: [
+        'spread',
         'attribute',
         'relationship',
         'single-line-function',

diff --git a/docs/rules/order-in-routes.md b/docs/rules/order-in-routes.md
@@ -10,6 +10,7 @@ const rules = {
     2,
     {
       order: [
+        'spread',
         'service',
         'inherited-property',
         'property',

diff --git a/lib/rules/order-in-components.js b/lib/rules/order-in-components.js
@@ -7,6 +7,7 @@ const reportUnorderedProperties = propOrder.reportUnorderedProperties;
 const addBackwardsPosition = propOrder.addBackwardsPosition;
 
 const ORDER = [
+  'spread',
   'service',
   'property',
   'empty-method',

diff --git a/lib/rules/order-in-controllers.js b/lib/rules/order-in-controllers.js
@@ -7,6 +7,7 @@ const reportUnorderedProperties = propOrder.reportUnorderedProperties;
 const addBackwardsPosition = propOrder.addBackwardsPosition;
 
 const ORDER = [
+  'spread',
   'controller',
   'service',
   'query-params',

diff --git a/lib/rules/order-in-models.js b/lib/rules/order-in-models.js
@@ -6,7 +6,13 @@ const propOrder = require('../utils/property-order');
 const reportUnorderedProperties = propOrder.reportUnorderedProperties;
 const addBackwardsPosition = propOrder.addBackwardsPosition;
 
-const ORDER = ['attribute', 'relationship', 'single-line-function', 'multi-line-function'];
+const ORDER = [
+  'spread',
+  'attribute',
+  'relationship',
+  'single-line-function',
+  'multi-line-function',
+];
 
 //------------------------------------------------------------------------------
 // Organizing - Organize your models

diff --git a/lib/rules/order-in-routes.js b/lib/rules/order-in-routes.js
@@ -7,6 +7,7 @@ const reportUnorderedProperties = propOrder.reportUnorderedProperties;
 const addBackwardsPosition = propOrder.addBackwardsPosition;
 
 const ORDER = [
+  'spread',
   'service',
   'inherited-property',
   'property',

diff --git a/lib/utils/ember.js b/lib/utils/ember.js
@@ -473,7 +473,12 @@ function isRouteProperty(name) {
 }
 
 function isRouteDefaultProp(property) {
-  return isRouteProperty(property.key.name) && property.key.name !== 'actions';
+  return (
+    types.isProperty(property) &&
+    types.isIdentifier(property.key) &&
+    isRouteProperty(property.key.name) &&
+    property.key.name !== 'actions'
+  );
 }
 
 function isControllerProperty(name) {
@@ -490,7 +495,12 @@ function isControllerProperty(name) {
 }
 
 function isControllerDefaultProp(property) {
-  return isControllerProperty(property.key.name) && property.key.name !== 'actions';
+  return (
+    types.isProperty(property) &&
+    types.isIdentifier(property.key) &&
+    isControllerProperty(property.key.name) &&
+    property.key.name !== 'actions'
+  );
 }
 
 function getModuleProperties(module) {

diff --git a/lib/utils/property-order.js b/lib/utils/property-order.js
@@ -42,6 +42,7 @@ const NAMES = {
   serialize: 'lifecycle hook',
   service: 'service injection',
   setupController: 'lifecycle hook',
+  spread: 'spread property',
   unknown: 'unknown property type',
   willClearRender: 'lifecycle hook',
   willDestroyElement: 'lifecycle hook',
@@ -60,18 +61,18 @@ function determinePropertyType(node, parentType, ORDER) {
     return 'controller';
   }
 
-  if (node.key.name === 'init') {
+  if (types.isIdentifier(node.key) && node.key.name === 'init') {
     return 'init';
   }
 
   if (parentType === 'component') {
-    if (ember.isComponentLifecycleHook(node)) {
+    if (types.isIdentifier(node.key) && ember.isComponentLifecycleHook(node)) {
       return node.key.name;
     }
   }
 
   if (parentType === 'controller') {
-    if (node.key.name === 'queryParams') {
+    if (types.isIdentifier(node.key) && node.key.name === 'queryParams') {
       return 'query-params';
     } else if (ember.isControllerDefaultProp(node)) {
       return 'inherited-property';
@@ -131,6 +132,10 @@ function determinePropertyType(node, parentType, ORDER) {
     return 'method';
   }
 
+  if (node.type === 'ExperimentalSpreadProperty') {
+    return 'spread';
+  }
+
   return 'unknown';
 }
 
@@ -166,7 +171,7 @@ function getNodeKeyName(node) {
 
 function getName(type, node) {
   let prefix;
-  if (!node.computed && type !== 'actions' && type !== 'model') {
+  if (!node.computed && type !== 'actions' && type !== 'model' && type !== 'spread') {
     prefix = getNodeKeyName(node);
   }
 

diff --git a/lib/utils/types.js b/lib/utils/types.js
@@ -110,6 +110,9 @@ function isCallExpression(node) {
  * @return {boolean} Whether or not the node is a call with a function expression as the first argument
  */
 function isCallWithFunctionExpression(node) {
+  if (!isCallExpression(node)) {
+    return false;
+  }
   const callObj = isMemberExpression(node.callee) ? node.callee.object : node;
   const firstArg = callObj.arguments ? callObj.arguments[0] : null;
   return (

diff --git a/tests/lib/rules/order-in-components.js b/tests/lib/rules/order-in-components.js
@@ -11,20 +11,24 @@ const RuleTester = require('eslint').RuleTester;
 
 const eslintTester = new RuleTester({
   parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+  parser: require.resolve('babel-eslint'),
 });
 
 eslintTester.run('order-in-components', rule, {
   valid: [
     'export default Component.extend();',
+    'export default Component.extend({ ...foo });',
     `export default Component.extend({
-        role: "sloth",
+      ...foo,
+      role: "sloth",
 
         vehicle: alias("car"),
 
         levelOfHappiness: computed("attitude", "health", () => {
         }),
 
-        actions: {}
+        actions: {},
+
       });`,
     `export default Component.extend({
         role: ${`${'sloth'}`},
@@ -1027,5 +1031,21 @@ eslintTester.run('order-in-components', rule, {
         },
       ],
     },
+    {
+      code: `export default Component.extend({
+        role: "sloth",
+        ...spread,
+      });`,
+      output: `export default Component.extend({
+        ...spread,
+              role: "sloth",
+});`,
+      errors: [
+        {
+          message: 'The spread property should be above the "role" property on line 2',
+          line: 3,
+        },
+      ],
+    },
   ],
 });
diff --git a/tests/lib/rules/order-in-controllers.js b/tests/lib/rules/order-in-controllers.js
@@ -11,11 +11,13 @@ const RuleTester = require('eslint').RuleTester;
 
 const eslintTester = new RuleTester({
   parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+  parser: require.resolve('babel-eslint'),
 });
 
 eslintTester.run('order-in-controllers', rule, {
   valid: [
     'export default Controller.extend();',
+    'export default Controller.extend({ ...foo });',
     `export default Controller.extend({
         application: controller(),
         currentUser: service(),

diff --git a/tests/lib/rules/order-in-models.js b/tests/lib/rules/order-in-models.js
@@ -11,11 +11,13 @@ const RuleTester = require('eslint').RuleTester;
 
 const eslintTester = new RuleTester({
   parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+  parser: require.resolve('babel-eslint'),
 });
 
 eslintTester.run('order-in-models', rule, {
   valid: [
     'export default Model.extend();',
+    'export default Model.extend({ ...foo });',
     `export default Model.extend({
         shape: attr("string"),
         behaviors: hasMany("behaviour"),

diff --git a/tests/lib/rules/order-in-routes.js b/tests/lib/rules/order-in-routes.js
@@ -11,11 +11,13 @@ const RuleTester = require('eslint').RuleTester;
 
 const eslintTester = new RuleTester({
   parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+  parser: require.resolve('babel-eslint'),
 });
 
 eslintTester.run('order-in-routes', rule, {
   valid: [
     'export default Route.extend();',
+    'export default Route.extend({ ...foo });',
     `export default Route.extend({
         currentUser: service(),
         queryParams: {},

diff --git a/tests/lib/utils/property-order-test.js b/tests/lib/utils/property-order-test.js
@@ -198,6 +198,16 @@ describe('determinePropertyType', () => {
       expect(propertyOrder.determinePropertyType(node, 'component', [])).toStrictEqual('property');
     });
 
+    it('should determine spread syntax as a spread property', () => {
+      const context = new FauxContext(
+        `export default Component.extend({
+          ...foo
+        });`
+      );
+      const node = context.ast.body[0].declaration.arguments[0].properties[0];
+      expect(propertyOrder.determinePropertyType(node, 'component', [])).toStrictEqual('spread');
+    });
+
     it('should determine empty methods', () => {
       const context = new FauxContext(
         `export default Component.extend({