Merge pull request #694 from nlfurniss/no-private-routing-service

Add new rule `no-private-routing-service`

Author: bmish

README.md

@@ -78,6 +78,7 @@ Each rule has emojis denoting what configuration it belongs to and/or a :wrench:
 | :white_check_mark: | [no-observers](./docs/rules/no-observers.md) | disallow usage of observers |
 | :white_check_mark::wrench: | [no-old-shims](./docs/rules/no-old-shims.md) | disallow usage of old shims for modules |
 | :white_check_mark: | [no-on-calls-in-components](./docs/rules/no-on-calls-in-components.md) | disallow usage of `on` to call lifecycle hooks in components |
+|  | [no-private-routing-service](./docs/rules/no-private-routing-service.md) | disallow injecting the private routing service |
 | :white_check_mark: | [no-restricted-resolver-tests](./docs/rules/no-restricted-resolver-tests.md) | disallow the use of patterns that use the restricted resolver in tests |
 |  | [no-unnecessary-index-route](./docs/rules/no-unnecessary-index-route.md) | disallow unnecessary `index` route definition |
 | :white_check_mark::wrench: | [no-unnecessary-route-path-option](./docs/rules/no-unnecessary-route-path-option.md) | disallow unnecessary usage of the route `path` option |

docs/rules/no-private-routing-service.md

@@ -0,0 +1,50 @@
+# no-private-routing-service
+
+Disallow the use of the private `-routing` service.
+
+There has been a public `router` service since Ember 2.16 and using the private routing service should be unnecessary.
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+import Component from '@ember/component';
+import { inject as service } from '@ember/service';
+
+export default Component.extend({
+  routing: service('-routing')
+});
+```
+
+```javascript
+import Component from '@ember/component';
+
+export default class MyComponent extends Component {
+  @service('-routing') routing;
+}
+```
+
+Examples of **correct** code for this rule:
+
+```javascript
+import Component from '@ember/component';
+import { inject as service } from '@ember/service';
+
+export default Component.extend({
+  router: service('router')
+});
+```
+
+```javascript
+import Component from '@ember/component';
+
+export default class MyComponent extends Component {
+  @service
+  router;
+}
+```
+
+## References
+
+[Router RFC](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md)

lib/index.js

@@ -41,6 +41,7 @@ module.exports = {
     'no-old-shims': require('./rules/no-old-shims'),
     'no-on-calls-in-components': require('./rules/no-on-calls-in-components'),
     'no-pause-test': require('./rules/no-pause-test'),
+    'no-private-routing-service': require('./rules/no-private-routing-service'),
     'no-proxies': require('./rules/no-proxies'),
     'no-restricted-resolver-tests': require('./rules/no-restricted-resolver-tests'),
     'no-side-effects': require('./rules/no-side-effects'),

lib/recommended-rules.js

@@ -44,6 +44,7 @@ module.exports = {
   "ember/no-old-shims": "error",
   "ember/no-on-calls-in-components": "error",
   "ember/no-pause-test": "off",
+  "ember/no-private-routing-service": "off",
   "ember/no-proxies": "off",
   "ember/no-restricted-resolver-tests": "error",
   "ember/no-side-effects": "error",

lib/rules/no-private-routing-service.js

@@ -0,0 +1,61 @@
+'use strict';
+
+const emberUtils = require('../utils/ember');
+const types = require('../utils/types');
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+const ERROR_MESSAGE =
+  "Don't inject the private '-routing' service. Instead use the public 'router' service.";
+
+module.exports = {
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: 'disallow injecting the private routing service',
+      category: 'Best Practices',
+      recommended: false,
+      url:
+        'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-private-routing-service.md',
+    },
+    fixable: null,
+    schema: [],
+  },
+
+  ERROR_MESSAGE,
+
+  create(context) {
+    const ROUTING_SERVICE_NAME = '-routing';
+
+    return {
+      Property(node) {
+        if (
+          emberUtils.isInjectedServiceProp(node) &&
+          node.value.arguments[0].value === ROUTING_SERVICE_NAME
+        ) {
+          context.report({ node, message: ERROR_MESSAGE });
+        }
+      },
+      ClassProperty(node) {
+        if (!node.decorators || !types.isClassPropertyWithDecorator(node, 'service')) {
+          return;
+        }
+
+        const hasRoutingServiceDecorator = node.decorators.some(decorator => {
+          const expression = decorator.expression;
+          return (
+            expression &&
+            expression.arguments &&
+            expression.arguments[0].value === ROUTING_SERVICE_NAME
+          );
+        });
+
+        if (hasRoutingServiceDecorator) {
+          context.report({ node, message: ERROR_MESSAGE });
+        }
+      },
+    };
+  },
+};

tests/lib/rules/no-private-routing-service.js

@@ -0,0 +1,57 @@
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-private-routing-service');
+const RuleTester = require('eslint').RuleTester;
+
+const { ERROR_MESSAGE } = rule;
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('babel-eslint'),
+  parserOptions: {
+    ecmaVersion: 2015,
+    sourceType: 'module',
+  },
+});
+
+ruleTester.run('no-private-routing-service', rule, {
+  valid: [
+    "export default Component.extend({ someService: service('routing') });",
+    "export default Component.extend({ someService: service('-router') });",
+    "export default Component.extend({ '-routing': service('routing') });",
+    "export default Component.extend({ '-routing': service('-router') });",
+    "Component.extend({ routing: someOtherFunction('-routing') });",
+    'export default class MyComponent extends Component { @service router; }',
+    "export default class MyComponent extends Component { @service('router') routing; }",
+    'export default class MyComponent extends Component { @service routing; }',
+    "export default class MyComponent extends Component { @service('routing') routing; }",
+    `
+    export default class MyComponent extends Component {
+      @computed('-routing', 'lastName')
+        get fullName() {
+        return \`${this.firstName} ${this.lastName}\`;
+      }
+    }
+    `,
+  ],
+  invalid: [
+    // Classic
+    {
+      code: "export default Component.extend({ routing: service('-routing') });",
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'Property' }],
+    },
+
+    // Octane
+    {
+      code: "export default class MyComponent extends Component { @service('-routing') routing; }",
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'ClassProperty' }],
+    },
+  ],
+});