Merge pull request #1297 from raido/fix-if-else-no-shadow-route

ember-cli · Sep 20, 2021 · 863afc7 · 863afc7
2 parents fe15a4e + d37ca0e
commit 863afc7
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 7 deletions.
diff --git a/lib/rules/no-shadow-route-definition.js b/lib/rules/no-shadow-route-definition.js
@@ -71,11 +71,9 @@ module.exports = {
           return;
         }
 
-        if (
-          routeMap.has(routeInfo.route.fullPathWithGenericParams) &&
-          !isNestedRouteWithSamePath(routeInfo)
-        ) {
-          const existingRouteInfo = routeMap.get(routeInfo.route.fullPathWithGenericParams);
+        const routeLookupKey = `${routeInfo.route.blockStatementsTreePrefix}::${routeInfo.route.fullPathWithGenericParams}`;
+        if (routeMap.has(routeLookupKey) && !isNestedRouteWithSamePath(routeInfo)) {
+          const existingRouteInfo = routeMap.get(routeLookupKey);
           context.report({
             node,
             message: buildErrorMessage({
@@ -93,8 +91,8 @@ module.exports = {
           });
           return;
         }
-        if (!routeMap.has(routeInfo.route.fullPathWithGenericParams)) {
-          routeMap.set(routeInfo.route.fullPathWithGenericParams, routeInfo);
+        if (!routeMap.has(routeLookupKey)) {
+          routeMap.set(routeLookupKey, routeInfo);
         }
       },
     };
@@ -118,6 +116,10 @@ function getRouteInfo(node, scopeManager) {
 
   const routeName = getRouteName(node).stringValue;
 
+  // We gather block statement ranges as prefix for route lookup paths
+  // Example: "121-150-130-135"
+  const blockStatementsTreePrefix = lookupIfElseBlockStatementsTreePrefix(node).join('-');
+
   // We replace "////post/something" -> "/post/something".
   // As that what nesting of / configured routes means for Ember router.
   return {
@@ -128,6 +130,7 @@ function getRouteInfo(node, scopeManager) {
       fullPathWithGenericParams: trimRootLevelNestedRoutes(
         convertPathToGenericForMatching([...parentRoutes, basePath])
       ),
+      blockStatementsTreePrefix,
       source: {
         loc: node.loc,
       },
@@ -252,3 +255,22 @@ function isString(value) {
 function isValidRouteInfo(routeInfo) {
   return routeInfo !== null;
 }
+
+function lookupIfElseBlockStatementsTreePrefix(node) {
+  const inspectedNode = node.parent;
+  let stack = [];
+  if (inspectedNode) {
+    if (inspectedNode.type === 'BlockStatement') {
+      if (inspectedNode.parent.type === 'IfStatement') {
+        if (
+          inspectedNode.parent.consequent === inspectedNode ||
+          inspectedNode.parent.alternate === inspectedNode
+        ) {
+          stack.push(inspectedNode.range.join('-'));
+        }
+      }
+    }
+    stack = [...stack, ...lookupIfElseBlockStatementsTreePrefix(inspectedNode)];
+  }
+  return stack;
+}
diff --git a/tests/lib/rules/no-shadow-route-definition.js b/tests/lib/rules/no-shadow-route-definition.js
@@ -124,6 +124,37 @@ ruleTester.run('no-shadow-route-definition', rule, {
       this.route('route');
     });`,
 
+    // Test if else conditions for route definitions
+    `if (this.options.getTreatmentIsEnabled('test-key')) {
+      this.route('new-route', { path: '/test' });
+    } else {
+      this.route('old-route', { path: '/test' });
+    }`,
+
+    `if (this.options.getTreatmentIsEnabled('test-key')) {
+      this.route('new-route', { path: '/test' });
+    } else {
+      if (false) {
+        this.route('old-route', { path: '/test' });
+      } else {
+        this.route('old-route-v2', { path: '/test' });
+      }
+    }`,
+
+    `if (this.options.getTreatmentIsEnabled('test-key')) {
+      if (true) {
+        this.route('new-route', { path: '/test' });
+      } else {
+        this.route('new-route-v2', { path: '/test' });
+      }
+    } else {
+      if (false) {
+        this.route('old-route', { path: '/test' });
+      } else {
+        this.route('old-route-v2', { path: '/test' });
+      }
+    }`,
+
     // Not Ember's route function:
     'test();',
     "test('blog');",