Merge pull request #2561 from RobbieTheWagner/feat/native-route-class-order-support

Add native class support for order-in-routes

Author: NullVoxPopuli

docs/rules/order-in-routes.md

@@ -150,8 +150,4 @@ export default Route.extend({
 });
 ```
 
-## Help Wanted
-
-| Issue                                      | Link                                                                |
-| :----------------------------------------- | :------------------------------------------------------------------ |
-| ❌ Missing native JavaScript class support | [#560](https://github.com/ember-cli/eslint-plugin-ember/issues/560) |
+This rule checks ordering only; it does not enforce indentation or other whitespace formatting.

lib/rules/order-in-routes.js

@@ -125,6 +125,40 @@ module.exports = {
           scopeManager
         );
       },
+      ClassDeclaration(node) {
+        if (!ember.isEmberRoute(context, node)) {
+          return;
+        }
+
+        reportUnorderedProperties(
+          node,
+          context,
+          'route',
+          order,
+          importedEmberName,
+          importedInjectName,
+          importedObserverName,
+          importedControllerName,
+          scopeManager
+        );
+      },
+      ClassExpression(node) {
+        if (!ember.isEmberRoute(context, node)) {
+          return;
+        }
+
+        reportUnorderedProperties(
+          node,
+          context,
+          'route',
+          order,
+          importedEmberName,
+          importedInjectName,
+          importedObserverName,
+          importedControllerName,
+          scopeManager
+        );
+      },
     };
   },
 };

lib/utils/ember.js

@@ -631,11 +631,18 @@ function isRouteProperty(name) {
 }
 
 function isRouteDefaultProp(property) {
-  return (
-    types.isProperty(property) &&
+  const isRouteClassField =
+    types.isClassPropertyOrPropertyDefinition(property) &&
     types.isIdentifier(property.key) &&
     isRouteProperty(property.key.name) &&
-    property.key.name !== 'actions'
+    property.key.name !== 'actions';
+
+  return (
+    isRouteClassField ||
+    (types.isProperty(property) &&
+      types.isIdentifier(property.key) &&
+      isRouteProperty(property.key.name) &&
+      property.key.name !== 'actions')
   );
 }
 
@@ -711,6 +718,10 @@ function isMultiLineFn(property, importedEmberName, importedObserverName) {
 }
 
 function isFunctionExpression(property) {
+  if (!property) {
+    return false;
+  }
+
   return (
     types.isFunctionExpression(property) ||
     types.isArrowFunctionExpression(property) ||

tests/lib/rules/order-in-routes.js

@@ -10,7 +10,13 @@ const RuleTester = require('eslint').RuleTester;
 // ------------------------------------------------------------------------------
 
 const eslintTester = new RuleTester({
-  parserOptions: { ecmaVersion: 2022, sourceType: 'module' },
+  parserOptions: {
+    ecmaVersion: 2022,
+    sourceType: 'module',
+    babelOptions: {
+      configFile: require.resolve('../../../.babelrc'),
+    },
+  },
   parser: require.resolve('@babel/eslint-parser'),
 });
 
@@ -170,6 +176,56 @@ eslintTester.run('order-in-routes', rule, {
       ],
       parserOptions: { ecmaVersion: 2022, sourceType: 'module' },
     },
+    `import Route from '@ember/routing/route';
+      import { inject as service } from '@ember/service';
+      export default class UserRoute extends Route {
+        @service currentUser;
+        queryParams = {};
+        customProp = 'test';
+        beforeModel() {}
+        model() {}
+      }`,
+    {
+      code: `import Route from '@ember/routing/route';
+        import { inject as service } from '@ember/service';
+        export default class UserRoute extends Route {
+          model() {}
+          beforeModel() {}
+          @service currentUser;
+        }`,
+      options: [
+        {
+          order: ['model', 'lifecycle-hook', 'service'],
+        },
+      ],
+    },
+    {
+      code: `import Route from '@ember/routing/route';
+        import { inject as service } from '@ember/service';
+        export default class UserRoute extends Route {
+          deactivate() {}
+          setupController() {}
+          beforeModel() {}
+          @service currentUser;
+          model() {}
+        }`,
+      options: [
+        {
+          order: [['deactivate', 'setupController', 'beforeModel'], 'service', 'model'],
+        },
+      ],
+    },
+    // spacing/indentation is intentionally not validated by this rule;
+    // only member ordering should matter.
+    `import Route from '@ember/routing/route';
+      import { inject as service } from '@ember/service';
+      export default class UserRoute extends Route {
+            @service currentUser;
+        queryParams = {};
+                 customProp = 'test';
+
+           beforeModel() {}
+      }`,
   ],
   invalid: [
     {
@@ -793,5 +849,108 @@ eslintTester.run('order-in-routes', rule, {
         },
       ],
     },
+    {
+      code: `import Route from '@ember/routing/route';
+        import { inject as service } from '@ember/service';
+        export default class UserRoute extends Route {
+          queryParams = {};
+          @service currentUser;
+          customProp = 'test';
+          beforeModel() {}
+          model() {}
+        }`,
+      output: `import Route from '@ember/routing/route';
+        import { inject as service } from '@ember/service';
+        export default class UserRoute extends Route {
+          @service currentUser;
+          queryParams = {};
+          customProp = 'test';
+          beforeModel() {}
+          model() {}
+        }`,
+      errors: [
+        {
+          message:
+            'The "currentUser" service injection should be above the inherited "queryParams" property on line 4',
+          line: 5,
+        },
+      ],
+    },
+    {
+      code: `import Route from '@ember/routing/route';
+        export default class UserRoute extends Route {
+          customProp = 'test';
+          queryParams = {};
+          model() {}
+          beforeModel() {}
+        }`,
+      output: `import Route from '@ember/routing/route';
+        export default class UserRoute extends Route {
+          queryParams = {};
+          customProp = 'test';
+          model() {}
+          beforeModel() {}
+        }`,
+      errors: [
+        {
+          message:
+            'The inherited "queryParams" property should be above the "customProp" property on line 3',
+          line: 4,
+        },
+        {
+          message: 'The "beforeModel" lifecycle hook should be above the "model" hook on line 5',
+          line: 6,
+        },
+      ],
+    },
+    {
+      code: `import Route from '@ember/routing/route';
+        export default class UserRoute extends Route {
+          customProp = 'test';
+          model() {}
+          beforeModel() {}
+        }`,
+      output: `import Route from '@ember/routing/route';
+        export default class UserRoute extends Route {
+          customProp = 'test';
+          beforeModel() {}
+        model() {}
+          }`,
+      errors: [
+        {
+          message: 'The "beforeModel" lifecycle hook should be above the "model" hook on line 4',
+          line: 5,
+        },
+      ],
+    },
+    {
+      code: `import Route from '@ember/routing/route';
+        import { inject as service } from '@ember/service';
+        export default class UserRoute extends Route {
+          model() {}
+          beforeModel() {}
+          @service currentUser;
+          queryParams = {};
+        }`,
+      output: `import Route from '@ember/routing/route';
+        import { inject as service } from '@ember/service';
+        export default class UserRoute extends Route {
+          @service currentUser;
+          model() {}
+          beforeModel() {}
+          queryParams = {};
+        }`,
+      options: [
+        {
+          order: ['service', 'model', 'lifecycle-hook', 'inherited-property'],
+        },
+      ],
+      errors: [
+        {
+          message: 'The "currentUser" service injection should be above the "model" hook on line 4',
+          line: 6,
+        },
+      ],
+    },
   ],
 });

tests/lib/utils/ember-test.js

@@ -1545,6 +1545,16 @@ describe('isRouteLifecycleHook', () => {
     node = getProperty('test = { deactivate() {} }');
     expect(emberUtils.isRouteLifecycleHook(node)).toBeTruthy();
   });
+
+  it('should not crash when class field has no initializer', () => {
+    const classField = {
+      key: { name: 'beforeModel' },
+      value: null,
+    };
+
+    expect(() => emberUtils.isRouteLifecycleHook(classField)).not.toThrow();
+    expect(emberUtils.isRouteLifecycleHook(classField)).toBeFalsy();
+  });
 });
 
 describe('isActionsProp', () => {