Merge pull request #871 from bmish/no-private-routing-service-router-main

Add `catchRouterMain` option to `no-private-routing-service` rule

Author: bmish

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

@@ -6,6 +6,7 @@ Disallow the use of:
 
 * The private `-routing` service
 * The private `_routerMicrolib` property (when the `catchRouterMicrolib` option is enabled)
+* The private `router:main` property (when the `catchRouterMain` option is enabled)
 
 There has been a public `router` service since Ember 2.16 and using the private routing service should be unnecessary.
 
@@ -44,6 +45,20 @@ export default class MyComponent extends Component {
 }
 ```
 
+```javascript
+// When `catchRouterMain` option is enabled.
+
+import Component from '@ember/component';
+import { getOwner } from '@ember/application';
+
+export default class MyComponent extends Component {
+  someFunction() {
+    const router = getOwner(this).lookup('router:main');
+    // ...
+  }
+}
+```
+
 Examples of **correct** code for this rule:
 
 ```javascript
@@ -69,6 +84,7 @@ export default class MyComponent extends Component {
 This rule takes an optional object containing:
 
 * `boolean` -- `catchRouterMicrolib` -- whether the rule should catch usages of the private property `_routerMicrolib` (default `false`) (TODO: enable this by default in the next major release)
+* `boolean` -- `catchRouterMain` -- whether the rule should catch usages of the private property `router:main` (default `false`) (TODO: enable this by default in the next major release)
 
 ## References
 

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

@@ -2,6 +2,7 @@
 
 const emberUtils = require('../utils/ember');
 const decoratorUtils = require('../utils/decorators');
+const types = require('../utils/types');
 
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -10,7 +11,11 @@ const decoratorUtils = require('../utils/decorators');
 const PRIVATE_ROUTING_SERVICE_ERROR_MESSAGE =
   "Don't inject the private '-routing' service. Instead use the public 'router' service.";
 
-const ROUTER_MICROLIB_ERROR_MESSAGE = "Don't access the `_routerMicrolib` as it is private API";
+const ROUTER_MICROLIB_ERROR_MESSAGE =
+  "Don't access the `_routerMicrolib` as it is a private API. Instead use the public 'router' service.";
+
+const ROUTER_MAIN_ERROR_MESSAGE =
+  "Don't access `router:main` as it is a private API. Instead use the public 'router' service.";
 
 module.exports = {
   meta: {
@@ -31,6 +36,10 @@ module.exports = {
             type: 'boolean',
             default: false,
           },
+          catchRouterMain: {
+            type: 'boolean',
+            default: false,
+          },
         },
         additionalProperties: false,
       },
@@ -39,13 +48,15 @@ module.exports = {
 
   PRIVATE_ROUTING_SERVICE_ERROR_MESSAGE,
   ROUTER_MICROLIB_ERROR_MESSAGE,
+  ROUTER_MAIN_ERROR_MESSAGE,
 
   create(context) {
     const ROUTING_SERVICE_NAME = '-routing';
     const ROUTER_MICROLIB_NAME = '_routerMicrolib';
 
     const options = context.options[0] || {};
     const catchRouterMicrolib = options.catchRouterMicrolib || false;
+    const catchRouterMain = options.catchRouterMain || false;
 
     return {
       Property(node) {
@@ -93,6 +104,33 @@ module.exports = {
           context.report({ node, message: ROUTER_MICROLIB_ERROR_MESSAGE });
         }
       },
+
+      CallExpression(node) {
+        checkForRouterMain(node);
+      },
+
+      OptionalCallExpression(node) {
+        checkForRouterMain(node);
+      },
     };
+
+    function checkForRouterMain(node) {
+      // Looks for expressions like these:
+      // x.lookup('router:main')
+      // x.y.lookup('router:main')
+      // x?.lookup('router:main')
+      if (
+        catchRouterMain &&
+        (types.isMemberExpression(node.callee) || types.isOptionalMemberExpression(node.callee)) &&
+        types.isIdentifier(node.callee.property) &&
+        node.callee.property.name === 'lookup' &&
+        node.arguments.length === 1 &&
+        types.isLiteral(node.arguments[0]) &&
+        typeof node.arguments[0].value === 'string' &&
+        node.arguments[0].value === 'router:main'
+      ) {
+        context.report({ node, message: ROUTER_MAIN_ERROR_MESSAGE });
+      }
+    }
   },
 };

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

@@ -5,7 +5,11 @@
 const rule = require('../../../lib/rules/no-private-routing-service');
 const RuleTester = require('eslint').RuleTester;
 
-const { PRIVATE_ROUTING_SERVICE_ERROR_MESSAGE, ROUTER_MICROLIB_ERROR_MESSAGE } = rule;
+const {
+  PRIVATE_ROUTING_SERVICE_ERROR_MESSAGE,
+  ROUTER_MICROLIB_ERROR_MESSAGE,
+  ROUTER_MAIN_ERROR_MESSAGE,
+} = rule;
 
 //------------------------------------------------------------------------------
 // Tests
@@ -51,9 +55,28 @@ ruleTester.run('no-private-routing-service', rule, {
     'class MyComponent extends Component { aProp="another value"; }',
     'class MyComponent extends Component { anIntProp=25; }',
 
-    // _routerMicrolib (`catchRouterMicrolib` option off)
+    // _routerMicrolib
     "get(this, 'router._routerMicrolib');",
     'this.router._routerMicrolib;',
+    { code: "get(this, 'router.somethingElse');", options: [{ catchRouterMicrolib: true }] },
+    { code: 'this.router.somethingElse;', options: [{ catchRouterMicrolib: true }] },
+
+    // router:main
+    "getOwner(this).lookup('router:main');",
+    "owner.lookup('router:main');",
+    "this.owner.lookup('router:main');",
+    {
+      code: "getOwner(this).lookup('router:somethingElse');",
+      options: [{ catchRouterMicrolib: true }],
+    },
+    {
+      code: "owner.lookup('router:somethingElse');",
+      options: [{ catchRouterMicrolib: true }],
+    },
+    {
+      code: "this.owner.lookup('router:somethingElse');",
+      options: [{ catchRouterMicrolib: true }],
+    },
   ],
   invalid: [
     // Classic
@@ -95,5 +118,32 @@ ruleTester.run('no-private-routing-service', rule, {
       options: [{ catchRouterMicrolib: true }],
       errors: [{ message: ROUTER_MICROLIB_ERROR_MESSAGE, type: 'Identifier' }],
     },
+
+    // router:main (`catchRouterMain` option on)
+    {
+      code: "getOwner(this).lookup('router:main');",
+      output: null,
+      options: [{ catchRouterMain: true }],
+      errors: [{ message: ROUTER_MAIN_ERROR_MESSAGE, type: 'CallExpression' }],
+    },
+    {
+      code: "owner.lookup('router:main');",
+      output: null,
+      options: [{ catchRouterMain: true }],
+      errors: [{ message: ROUTER_MAIN_ERROR_MESSAGE, type: 'CallExpression' }],
+    },
+    {
+      code: "this.owner.lookup('router:main');",
+      output: null,
+      options: [{ catchRouterMain: true }],
+      errors: [{ message: ROUTER_MAIN_ERROR_MESSAGE, type: 'CallExpression' }],
+    },
+    {
+      // Optional chaining.
+      code: "this?.owner?.lookup?.('router:main');",
+      output: null,
+      options: [{ catchRouterMain: true }],
+      errors: [{ message: ROUTER_MAIN_ERROR_MESSAGE, type: 'OptionalCallExpression' }],
+    },
   ],
 });