Repository: ember-cli/eslint-plugin-ember
Pull request #1167 (2021-04-21 22:19:17)

Update route rules to handle route path option passed as object variable


Patch:
diff --git a/lib/rules/no-shadow-route-definition.js b/lib/rules/no-shadow-route-definition.js
index 7f330c1d..3ad00dff 100644
--- a/lib/rules/no-shadow-route-definition.js
+++ b/lib/rules/no-shadow-route-definition.js
@@ -2,6 +2,7 @@
 
 const ember = require('../utils/ember');
 const types = require('../utils/types');
+const { getNodeOrNodeFromVariable } = require('../utils/utils');
 
 const ROOT_PATH_TRIM_REGEX = /\/{2,}/g;
 const SOURCE_PATH_VALUE_TYPE = {
@@ -57,13 +58,16 @@ module.exports = {
 
     const routeMap = new Map();
 
+    const sourceCode = context.getSourceCode();
+    const { scopeManager } = sourceCode;
+
     return {
       CallExpression(node) {
         if (!ember.isRoute(node)) {
           return;
         }
 
-        const routeInfo = getRouteInfo(node);
+        const routeInfo = getRouteInfo(node, scopeManager);
         if (!isValidRouteInfo(routeInfo)) {
           return;
         }
@@ -98,13 +102,13 @@ module.exports = {
   },
 };
 
-function getRouteInfo(node) {
-  const basePath = getRouteBasePath(node);
+function getRouteInfo(node, scopeManager) {
+  const basePath = getRouteBasePath(node, scopeManager);
   if (basePath.normalizedPath === null) {
     return null;
   }
 
-  const parentRoutes = getParentRoutesPaths(node);
+  const parentRoutes = getParentRoutesPaths(node, scopeManager);
   const notSupportedParentRoutePathArguments = parentRoutes.find((routePathInfo) => {
     return routePathInfo.normalizedPath === null;
   });
@@ -138,10 +142,15 @@ function getRouteInfo(node) {
   };
 }
 
-function getRouteBasePath(node) {
+function getRouteBasePath(node, scopeManager) {
   let routePathInfo = getRouteName(node);
-  if (hasPathProperty(node)) {
-    const pathOptionNode = getPropertyByKeyName(node.arguments[1], 'path');
+  const optionsNode =
+    node.arguments.length >= 2 && getNodeOrNodeFromVariable(node.arguments[1], scopeManager);
+  const pathOptionNode =
+    optionsNode &&
+    optionsNode.type === 'ObjectExpression' &&
+    getPropertyByKeyName(optionsNode, 'path');
+  if (pathOptionNode) {
     routePathInfo = getNodeValue(pathOptionNode.value);
   }
 
@@ -156,28 +165,18 @@ function getRouteBasePath(node) {
   };
 }
 
-function getParentRoutesPaths(node) {
+function getParentRoutesPaths(node, scopeManager) {
   const parentNode = node.parent;
   let stack = [];
   if (parentNode) {
     if (ember.isRoute(parentNode)) {
-      stack.push(getRouteBasePath(parentNode));
+      stack.push(getRouteBasePath(parentNode, scopeManager));
     }
     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) =>
diff --git a/lib/rules/no-unnecessary-route-path-option.js b/lib/rules/no-unnecessary-route-path-option.js
index 9245a29a..43a5da50 100644
--- a/lib/rules/no-unnecessary-route-path-option.js
+++ b/lib/rules/no-unnecessary-route-path-option.js
@@ -3,6 +3,7 @@
 const types = require('../utils/types');
 const ember = require('../utils/ember');
 const fixerUtils = require('../utils/fixer');
+const { getNodeOrNodeFromVariable } = require('../utils/utils');
 
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -27,20 +28,26 @@ module.exports = {
   ERROR_MESSAGE,
 
   create(context) {
+    const sourceCode = context.getSourceCode();
+    const { scopeManager } = sourceCode;
+
     return {
       CallExpression(node) {
         if (!ember.isRoute(node)) {
           return;
         }
 
+        let optionsNode;
         const hasExplicitPathOption =
-          types.isObjectExpression(node.arguments[1]) &&
-          hasPropertyWithKeyName(node.arguments[1], 'path');
+          node.arguments.length >= 2 &&
+          (optionsNode = getNodeOrNodeFromVariable(node.arguments[1], scopeManager)) &&
+          optionsNode.type === 'ObjectExpression' &&
+          hasPropertyWithKeyName(optionsNode, 'path');
         if (!hasExplicitPathOption) {
           return;
         }
 
-        const pathOptionNode = getPropertyByKeyName(node.arguments[1], 'path');
+        const pathOptionNode = getPropertyByKeyName(optionsNode, 'path');
         const pathOptionValue = pathOptionNode.value.value;
         const routeName = node.arguments[0].value;
 
@@ -51,9 +58,16 @@ module.exports = {
             fix(fixer) {
               const sourceCode = context.getSourceCode();
 
+              if (optionsNode.parent.type === 'VariableDeclarator') {
+                // When the options object is a separate variable.
+                return optionsNode.properties.length === 1
+                  ? fixer.remove(pathOptionNode)
+                  : fixerUtils.removeCommaSeparatedNode(pathOptionNode, sourceCode, fixer);
+              }
+
               // If the `path` option is the only property in the object, remove the entire object.
-              const shouldRemoveObject = node.arguments[1].properties.length === 1;
-              const nodeToRemove = shouldRemoveObject ? node.arguments[1] : pathOptionNode;
+              const shouldRemoveObject = optionsNode.properties.length === 1;
+              const nodeToRemove = shouldRemoveObject ? optionsNode : pathOptionNode;
 
               return fixerUtils.removeCommaSeparatedNode(nodeToRemove, sourceCode, fixer);
             },
diff --git a/lib/rules/route-path-style.js b/lib/rules/route-path-style.js
index de8608ed..22adb3fe 100644
--- a/lib/rules/route-path-style.js
+++ b/lib/rules/route-path-style.js
@@ -3,6 +3,7 @@
 const types = require('../utils/types');
 const ember = require('../utils/ember');
 const kebabCase = require('lodash.kebabcase');
+const { getNodeOrNodeFromVariable } = require('../utils/utils');
 
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -32,6 +33,9 @@ module.exports = {
   ERROR_MESSAGE,
 
   create(context) {
+    const sourceCode = context.getSourceCode();
+    const { scopeManager } = sourceCode;
+
     return {
       CallExpression(node) {
         if (!ember.isRoute(node) || node.arguments.length === 0) {
@@ -42,12 +46,14 @@ module.exports = {
         // 1. this.route('route-and-path');
         // 2. this.route('route', { path: 'path' });
 
+        let optionsNode;
         const hasExplicitPathOption =
           node.arguments.length >= 2 &&
-          types.isObjectExpression(node.arguments[1]) &&
-          hasPropertyWithKeyName(node.arguments[1], 'path');
+          (optionsNode = getNodeOrNodeFromVariable(node.arguments[1], scopeManager)) &&
+          optionsNode.type === 'ObjectExpression' &&
+          hasPropertyWithKeyName(optionsNode, 'path');
         const pathValueNode = hasExplicitPathOption
-          ? getPropertyByKeyName(node.arguments[1], 'path').value
+          ? getPropertyByKeyName(optionsNode, 'path').value
           : node.arguments[0];
 
         if (!types.isStringLiteral(pathValueNode)) {
diff --git a/lib/rules/routes-segments-snake-case.js b/lib/rules/routes-segments-snake-case.js
index 51addca5..a9232d34 100644
--- a/lib/rules/routes-segments-snake-case.js
+++ b/lib/rules/routes-segments-snake-case.js
@@ -3,6 +3,7 @@
 const types = require('../utils/types');
 const ember = require('../utils/ember');
 const { snakeCase } = require('snake-case');
+const { getNodeOrNodeFromVariable } = require('../utils/utils');
 
 //------------------------------------------------------------------------------
 // Routing - Snake case in dynamic segments of routes
@@ -51,15 +52,22 @@ module.exports = {
       return property.value.value.match(routeSegmentRegex).map((segment) => segment.slice(1));
     };
 
+    const sourceCode = context.getSourceCode();
+    const { scopeManager } = sourceCode;
+
     return {
       CallExpression(node) {
         if (!ember.isRoute(node)) {
           return;
         }
 
-        const routeOptions = types.isObjectExpression(node.arguments[1])
-          ? node.arguments[1]
-          : false;
+        let optionsNode;
+        const routeOptions =
+          node.arguments.length >= 2 &&
+          (optionsNode = getNodeOrNodeFromVariable(node.arguments[1], scopeManager)) &&
+          optionsNode.type === 'ObjectExpression'
+            ? optionsNode
+            : false;
 
         if (routeOptions) {
           for (const property of routeOptions.properties) {


Test patch:
diff --git a/tests/lib/rules/no-shadow-route-definition.js b/tests/lib/rules/no-shadow-route-definition.js
index c16eed5f..a2030d98 100644
--- a/tests/lib/rules/no-shadow-route-definition.js
+++ b/tests/lib/rules/no-shadow-route-definition.js
@@ -1005,6 +1005,45 @@ ruleTester.run('no-shadow-route-definition', rule, {
         },
       ],
     },
+
+    {
+      code: `
+        const options1 = { path: '/' }; this.route('post', options1);
+        const options2 = { path: '/' }; this.route('edit', options2);
+      `,
+      output: null,
+      errors: [
+        {
+          message: buildErrorMessage({
+            leftRoute: {
+              name: 'edit',
+              fullPath: '/',
+              source: {
+                loc: {
+                  start: {
+                    line: 3,
+                    column: 40,
+                  },
+                },
+              },
+            },
+            rightRoute: {
+              name: 'post',
+              fullPath: '/',
+              source: {
+                loc: {
+                  start: {
+                    line: 2,
+                    column: 40,
+                  },
+                },
+              },
+            },
+          }),
+          type: 'CallExpression',
+        },
+      ],
+    },
   ],
 });
 
diff --git a/tests/lib/rules/no-unnecessary-route-path-option.js b/tests/lib/rules/no-unnecessary-route-path-option.js
index 204af738..097a64b7 100644
--- a/tests/lib/rules/no-unnecessary-route-path-option.js
+++ b/tests/lib/rules/no-unnecessary-route-path-option.js
@@ -75,5 +75,37 @@ ruleTester.run('no-unnecessary-route-path-option', rule, {
       output: 'this.route("blog", { otherOption: true  });',
       errors: [{ message: ERROR_MESSAGE, type: 'Property' }],
     },
+    {
+      code: 'this.route("blog", { firstOption: true, path: "/blog", lastOption: true });',
+      output: 'this.route("blog", { firstOption: true,  lastOption: true });',
+      errors: [{ message: ERROR_MESSAGE, type: 'Property' }],
+    },
+
+    {
+      // With object variable: single option.
+      code: 'const options = { path: "blog" }; this.route("blog", options);',
+      output: 'const options = {  }; this.route("blog", options);',
+      errors: [{ message: ERROR_MESSAGE, type: 'Property' }],
+    },
+    {
+      // With object variable: first option.
+      code: 'const options = { path: "blog", somethingElse: true }; this.route("blog", options);',
+      output: 'const options = {  somethingElse: true }; this.route("blog", options);',
+      errors: [{ message: ERROR_MESSAGE, type: 'Property' }],
+    },
+    {
+      // With object variable: last option.
+      code: 'const options = { somethingElse: true, path: "blog" }; this.route("blog", options);',
+      output: 'const options = { somethingElse: true  }; this.route("blog", options);',
+      errors: [{ message: ERROR_MESSAGE, type: 'Property' }],
+    },
+    {
+      // With object variable: middle option.
+      code:
+        'const options = { firstOption: true, path: "blog", lastOption: true }; this.route("blog", options);',
+      output:
+        'const options = { firstOption: true,  lastOption: true }; this.route("blog", options);',
+      errors: [{ message: ERROR_MESSAGE, type: 'Property' }],
+    },
   ],
 });
diff --git a/tests/lib/rules/route-path-style.js b/tests/lib/rules/route-path-style.js
index 357855c7..59d32bc4 100644
--- a/tests/lib/rules/route-path-style.js
+++ b/tests/lib/rules/route-path-style.js
@@ -158,6 +158,23 @@ ruleTester.run('route-path-style', rule, {
         },
       ],
     },
+    {
+      // With object variable.
+      code: 'const options = { path: "/blogPosts" }; this.route("blog-posts", options);',
+      output: null,
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'Literal',
+          suggestions: [
+            {
+              messageId: 'convertToKebabCase',
+              output: 'const options = { path: "/blog-posts" }; this.route("blog-posts", options);',
+            },
+          ],
+        },
+      ],
+    },
     {
       code: 'this.route("blog-posts", { path: "/blogPosts/:blog_id" });',
       output: null,
diff --git a/tests/lib/rules/routes-segments-snake-case.js b/tests/lib/rules/routes-segments-snake-case.js
index 321b796f..1deb871a 100644
--- a/tests/lib/rules/routes-segments-snake-case.js
+++ b/tests/lib/rules/routes-segments-snake-case.js
@@ -38,6 +38,12 @@ eslintTester.run('routes-segments-snake-case', rule, {
         },
       ],
     },
+    {
+      // With object variable.
+      code: 'const options = { path: ":treeId"}; this.route("tree", options);',
+      output: null,
+      errors: [{ message: 'Use snake case in dynamic segments of routes', type: 'Literal' }],
+    },
     {
       code: 'this.route("tree", { path: ":tree-id" });',
       output: null,