Repository: ember-cli/eslint-plugin-ember
Pull request #978 (2020-10-15 18:46:58)

Add new rule `no-shadow-route-definition`
This PR adds new rule to avoid route path definition collisions for Router.

Example message: `Route "blog" (/blog, 3L:8C) is shadowing route "blog" (/blog, 2L:1C)`

Closes: #951 


Patch:
diff --git a/README.md b/README.md
index c23950b9..dcc46a81 100644
--- a/README.md
+++ b/README.md
@@ -163,6 +163,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
 | :white_check_mark: | [no-capital-letters-in-routes](./docs/rules/no-capital-letters-in-routes.md) | disallow routes with uppercased letters in router.js |
 | :white_check_mark: | [no-controller-access-in-routes](./docs/rules/no-controller-access-in-routes.md) | disallow routes from accessing the controller outside of setupController/resetController |
 | :white_check_mark: | [no-private-routing-service](./docs/rules/no-private-routing-service.md) | disallow injecting the private routing service |
+|  | [no-shadow-route-definition](./docs/rules/no-shadow-route-definition.md) | enforce no route path definition shadowing |
 |  | [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 |
 |  | [route-path-style](./docs/rules/route-path-style.md) | enforce usage of kebab-case (instead of snake_case or camelCase) in route paths |
diff --git a/docs/rules/no-shadow-route-definition.md b/docs/rules/no-shadow-route-definition.md
new file mode 100644
index 00000000..fbb637bc
--- /dev/null
+++ b/docs/rules/no-shadow-route-definition.md
@@ -0,0 +1,20 @@
+# no-shadow-route-definition
+
+Enforce no route path definition shadowing in Router.
+
+## Rule Details
+
+This rule disallows defining shadowing route definitions.
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```js
+this.route('main', { path: '/' }, function () {
+  this.route('nested');
+});
+this.route('nested');
+```
+
+In this example from Router perspective both `nested` routes are on URL `/nested`.
diff --git a/lib/index.js b/lib/index.js
index 92d62331..b9c974ba 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -52,6 +52,7 @@ module.exports = {
     'no-replace-test-comments': require('./rules/no-replace-test-comments'),
     'no-restricted-resolver-tests': require('./rules/no-restricted-resolver-tests'),
     'no-restricted-service-injections': require('./rules/no-restricted-service-injections'),
+    'no-shadow-route-definition': require('./rules/no-shadow-route-definition'),
     'no-settled-after-test-helper': require('./rules/no-settled-after-test-helper'),
     'no-side-effects': require('./rules/no-side-effects'),
     'no-string-prototype-extensions': require('./rules/no-string-prototype-extensions'),
diff --git a/lib/rules/no-shadow-route-definition.js b/lib/rules/no-shadow-route-definition.js
new file mode 100644
index 00000000..f489329a
--- /dev/null
+++ b/lib/rules/no-shadow-route-definition.js
@@ -0,0 +1,180 @@
+'use strict';
+
+const ember = require('../utils/ember');
+const types = require('../utils/types');
+
+const ROOT_PATH_TRIM_REGEX = /\/{2,}/g;
+
+function buildErrorMessage(opts) {
+  const { leftRoute, rightRoute } = opts;
+  return `Route ${buildRouteErrorInfo(leftRoute)} is shadowing route ${buildRouteErrorInfo(
+    rightRoute
+  )}`;
+}
+
+function buildSourceLocationString(routeInfo) {
+  return `${routeInfo.source.loc.start.line}L:${routeInfo.source.loc.start.column}C`;
+}
+
+function buildRouteErrorInfo(routeInfo) {
+  return `"${routeInfo.name}" (${routeInfo.fullPath}, ${buildSourceLocationString(routeInfo)})`;
+}
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'enforce no route path definition shadowing',
+      category: 'Routes',
+      recommended: false,
+      url:
+        'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-shadow-route-definition.md',
+    },
+    fixable: null,
+    schema: [],
+  },
+
+  buildErrorMessage,
+
+  create(context) {
+    if (ember.isTestFile(context.getFilename())) {
+      // This rule does not apply to test files.
+      return {};
+    }
+
+    const routeMap = new Map();
+
+    return {
+      CallExpression(node) {
+        if (!ember.isRoute(node)) {
+          return;
+        }
+        const routeInfo = getRouteInfo(node);
+        if (
+          routeMap.has(routeInfo.route.fullPathWithGenericParams) &&
+          !routeMap.has(routeInfo.parent.fullPathWithGenericParams)
+        ) {
+          const existingRouteInfo = routeMap.get(routeInfo.route.fullPathWithGenericParams);
+          context.report({
+            node,
+            message: buildErrorMessage({
+              leftRoute: {
+                name: routeInfo.route.name,
+                fullPath: routeInfo.route.fullPath,
+                source: routeInfo.route.source,
+              },
+              rightRoute: {
+                name: existingRouteInfo.route.name,
+                fullPath: existingRouteInfo.route.fullPath,
+                source: existingRouteInfo.route.source,
+              },
+            }),
+          });
+          return;
+        }
+
+        routeMap.set(routeInfo.route.fullPathWithGenericParams, routeInfo);
+      },
+    };
+  },
+};
+
+function getRouteInfo(node) {
+  const routeName = node.arguments[0].value;
+  let basePath = getRouteBasePath(node);
+  const parentRoutes = getParentRoutesPaths(node);
+
+  // If top-level route, prepend "/" to indicate that.
+  if (parentRoutes.length === 0 && !basePath.startsWith('/')) {
+    basePath = `/${basePath}`;
+  }
+
+  // We replace "////post/something" -> "/post/something".
+  // As that what nesting of / configured routes means for Ember router.
+  const parentRoutesFullPath = trimRootLevelNestedRoutes(parentRoutes.join(''));
+  const fullPath = `${parentRoutesFullPath}${basePath}`;
+
+  return {
+    route: {
+      name: routeName,
+      basePath,
+      fullPath,
+      fullPathWithGenericParams: convertPathToGenericForMatching(fullPath),
+      source: {
+        loc: node.loc,
+      },
+    },
+    parent: {
+      fullPath: parentRoutesFullPath,
+      fullPathWithGenericParams: convertPathToGenericForMatching(parentRoutesFullPath),
+    },
+  };
+}
+
+function getRouteBasePath(node) {
+  let routePath = node.arguments[0].value;
+
+  if (hasPathProperty(node)) {
+    const pathOptionNode = getPropertyByKeyName(node.arguments[1], 'path');
+    const pathValue = pathOptionNode.value.value;
+    if (pathValue) {
+      routePath = pathValue;
+    }
+  }
+  return routePath;
+}
+
+function getParentRoutesPaths(node) {
+  const parentNode = node.parent;
+  let stack = [];
+  if (parentNode) {
+    if (ember.isRoute(parentNode)) {
+      stack.push(getRouteBasePath(parentNode));
+    }
+    stack = [...stack, ...getParentRoutesPaths(parentNode)];
+  }
+  return stack;
+}
+
+function hasPathProperty(node) {
+  return (
+    types.isObjectExpression(node.arguments[1]) && hasPropertyWithKeyName(node.arguments[1], 'path')
+  );
+}
+
+function hasPropertyWithKeyName(objectExpression, keyName) {
+  return getPropertyByKeyName(objectExpression, keyName) !== undefined;
+}
+
+function getPropertyByKeyName(objectExpression, keyName) {
+  return objectExpression.properties.find(
+    (property) =>
+      types.isProperty(property) &&
+      types.isIdentifier(property.key) &&
+      property.key.name === keyName
+  );
+}
+
+function trimRootLevelNestedRoutes(routesPath) {
+  return routesPath.replace(ROOT_PATH_TRIM_REGEX, '/');
+}
+
+function convertPathToGenericForMatching(sourcePath) {
+  return sourcePath
+    .split('/')
+    .map((chunk) => {
+      const mappedValue = chunk.trim();
+      if (mappedValue.startsWith(':')) {
+        return ':generic-param-for-matching';
+      }
+      if (mappedValue.startsWith('*')) {
+        return '*generic-wildcard-for-matching';
+      }
+      return mappedValue;
+    })
+    .join('/');
+}


Test patch:
diff --git a/tests/lib/rules/no-shadow-route-definition.js b/tests/lib/rules/no-shadow-route-definition.js
new file mode 100644
index 00000000..233ce68d
--- /dev/null
+++ b/tests/lib/rules/no-shadow-route-definition.js
@@ -0,0 +1,714 @@
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-shadow-route-definition');
+const RuleTester = require('eslint').RuleTester;
+
+const { buildErrorMessage } = rule;
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('babel-eslint'),
+  parserOptions: {
+    ecmaVersion: 6,
+    sourceType: 'module',
+  },
+});
+
+ruleTester.run('no-shadow-route-definition', rule, {
+  valid: [
+    'this.route("blog");',
+    'this.route("blog", function() {});',
+    'this.route("blog", { ...foo });',
+    'this.route("blog", { path: undefined });',
+    'this.route("blog", { path: "" });',
+    'this.route("blog", { path: "/" });',
+    'this.route("blog", { path: "blog-posts" });',
+    'this.route("blog", { path: "blog-posts" }, function() {});',
+    'this.route("blog", { path: "/blog-posts" });',
+    'this.route("blog", { path: "blog-posts", otherOption: true });',
+    `this.route("blog", { path: "/" }, function() {
+      this.route("sub-blog", { path: "/" });
+    });`,
+    `this.route("blog", function() {
+      this.route("post");
+    });`,
+    `this.route("blog", { path: "/" }, function() {
+      this.route("post");
+    });`,
+    `this.route("blog", { path: "/" }, function() {
+      this.route("post", { path: "hero" });
+    });`,
+    `this.route("blog", function() {
+      this.route("post", { path: "hero" });
+    });`,
+    `this.route("blog", { path: "/blog/post" }, function() {
+      this.route("post", { path: "/" });
+    });
+    this.route("post")`,
+    `this.route("blog", function() {
+      this.route("post", { path: "/" });
+    });
+    this.route("post")`,
+
+    // With dynamic segment:
+    'this.route("blog", { path: ":blog" });',
+    'this.route("blog", { path: "/:blog" });',
+    'this.route("blog", { path: "blog/:blog_id" });',
+
+    // With dynamic segment, nested:
+    `this.route("blog", { path: "blog/:blog_id" }, function() {
+      this.route("post");
+    });
+    this.route("post");`,
+    `this.route("blog", { path: "/:blog" }, function() {
+      this.route("post");
+    });
+    this.route("post");`,
+    `this.route("blog", { path: ":blog" }, function() {
+      this.route("post");
+    });
+    this.route("post");`,
+
+    // With wildcard segment:
+    'this.route("blog", { path: "*blog" });',
+    'this.route("blog", { path: "/*blog" });',
+    'this.route("blog", { path: "blog/*blog" });',
+
+    // With wildcard segment, nested:
+    `this.route("blog", { path: "*blog" }, function() {
+      this.route("post");
+    });
+    this.route("post");`,
+    `this.route("blog", { path: "/*blog" }, function() {
+      this.route("post");
+    });
+    this.route("post");`,
+    `this.route("blog", { path: "blog/*blog" }, function() {
+      this.route("post");
+    });
+    this.route("post");`,
+
+    // Not Ember's route function:
+    'test();',
+    "test('blog');",
+    "test('blog', { path: 'blog' });",
+    "test('blog', { path: '/blog' });",
+    "this.test('blog');",
+    "this.test('blog', { path: 'blog' });",
+    "this.test('blog', { path: '/blog' });",
+    "MyClass.route('blog');",
+    "MyClass.route('blog', { path: 'blog' });",
+    "MyClass.route('blog', { path: '/blog' });",
+    "route.unrelatedFunction('blog', { path: 'blog' });",
+    "this.route.unrelatedFunction('blog', { path: 'blog' });",
+    {
+      filename: 'my-test.js',
+      code: `
+        this.route("blog");
+        this.route("blog");
+      `,
+    },
+  ],
+  invalid: [
+    {
+      code: `
+        this.route("blog");
+        this.route("blog");
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'blog',
+              fullPath: '/blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 2,
+                    column: 8,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("blog", { path: "/" }, function() {
+          this.route("post");
+        });
+        this.route("post");
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'post',
+              fullPath: '/post',
+              source: {
+                loc: {
+                  start: {
+                    line: 5,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'post',
+              fullPath: '/post',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 10,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("blog", { path: "/" }, function() {
+          this.route("post", function() {
+            this.route("author");
+          });
+        });
+        this.route("post");
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'post',
+              fullPath: '/post',
+              source: {
+                loc: {
+                  start: {
+                    line: 7,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'post',
+              fullPath: '/post',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 10,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("blog", { path: "/" }, function() {
+          this.route("sub-blog", { path: "/" }, function() {
+            this.route("post");
+          });
+        });
+        this.route("post");
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'post',
+              fullPath: '/post',
+              source: {
+                loc: {
+                  start: {
+                    line: 7,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'post',
+              fullPath: '/post',
+              source: {
+                loc: {
+                  start: {
+                    line: 4,
+                    column: 12,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("blog", { path: "/" }, function() {
+          this.route("sub-blog", { path: "/" }, function() {
+            this.route("post");
+          });
+        });
+        this.route("sub-blog", { path: "/" });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'sub-blog',
+              fullPath: '/',
+              source: {
+                loc: {
+                  start: {
+                    line: 7,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/',
+              source: {
+                loc: {
+                  start: {
+                    line: 2,
+                    column: 8,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("blog", { path: "/" }, function() {
+          this.route("sub-blog", { path: "/" }, function() {
+            this.route("post");
+          });
+        });
+        this.route("sub-blog", { path: "/" }, function() {
+          this.route("post");
+        });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'sub-blog',
+              fullPath: '/',
+              source: {
+                loc: {
+                  start: {
+                    line: 7,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/',
+              source: {
+                loc: {
+                  start: {
+                    line: 2,
+                    column: 8,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("blog", { path: "*blog" });
+        this.route("blog", { path: "/*blog" });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'blog',
+              fullPath: '/*blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/*blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 2,
+                    column: 8,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("main", { path: "/" }, function() {
+          this.route("blog", { path: "*blog" });
+        });
+        this.route("blog", { path: "/*blog" });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'blog',
+              fullPath: '/*blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 5,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/*blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 10,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("main", { path: "/" }, function() {
+          this.route("blog", { path: "*blog" });
+        });
+        this.route("blog", { path: "/*anotherWildcard" });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'blog',
+              fullPath: '/*anotherWildcard',
+              source: {
+                loc: {
+                  start: {
+                    line: 5,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/*blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 10,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("blog", { path: ":blog" });
+        this.route("blog", { path: "/:blog" });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'blog',
+              fullPath: '/:blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/:blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 2,
+                    column: 8,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("blog", { path: ":blog" });
+        this.route("blog", { path: "/:anotherParamName" });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'blog',
+              fullPath: '/:anotherParamName',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/:blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 2,
+                    column: 8,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("main", { path: "/" }, function() {
+          this.route("blog", { path: ":blog" });
+        });
+        this.route("blog", { path: "/:blog" });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'blog',
+              fullPath: '/:blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 5,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'blog',
+              fullPath: '/:blog',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 10,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("first", { path: "/" });
+        this.route("second", { path: "/" });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'second',
+              fullPath: '/',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 8,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'first',
+              fullPath: '/',
+              source: {
+                loc: {
+                  start: {
+                    line: 2,
+                    column: 8,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+    {
+      code: `
+        this.route("main", function() {
+          this.route("first", { path: "/" });
+          this.route("second", { path: "/" });
+        });
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'second',
+              fullPath: 'main/',
+              source: {
+                loc: {
+                  start: {
+                    line: 4,
+                    column: 10,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'first',
+              fullPath: 'main/',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 10,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
+  ],
+});
+
+describe('no-shadow-route-definition', () => {
+  it('builds error message string', () => {
+    const message = buildErrorMessage({
+      leftRoute: {
+        name: 'second',
+        fullPath: 'main/',
+        source: {
+          loc: {
+            start: {
+              line: 4,
+              column: 10,
+            },
+          },
+        },
+      },
+      rightRoute: {
+        name: 'first',
+        fullPath: 'main/',
+        source: {
+          loc: {
+            start: {
+              line: 3,
+              column: 10,
+            },
+          },
+        },
+      },
+    });
+
+    expect(message).toStrictEqual(
+      'Route "second" (main/, 4L:10C) is shadowing route "first" (main/, 3L:10C)'
+    );
+  });
+});