Fix siblings and interleaving issues reported in #2342

lib/rules/v-if-else-key.js

@@ -25,6 +25,33 @@ const casing = require('../utils/casing')
  * @property {VElement | null} else - The node associated with the 'v-else' directive, or null if there isn't one.
  */
 
+/**
+ * Checks if a given node has sibling nodes of the same type that are also conditionally rendered.
+ * This is used to determine if multiple instances of the same component are being conditionally
+ * rendered within the same parent scope.
+ *
+ * @param {VElement} node - The Vue component node to check for conditional rendering siblings.
+ * @param {string} componentName - The name of the component to check for sibling instances.
+ * @returns {boolean} True if there are sibling nodes of the same type and conditionally rendered, false otherwise.
+ */
+const hasConditionalRenderedSiblings = (node, componentName) => {
+  if (node.parent && node.parent.type === 'VElement') {
+    const siblings = node.parent.children.filter(
+      (child) => child.type === 'VElement'
+    )
+
+    return siblings.some(
+      (sibling) =>
+        sibling !== node &&
+        sibling.type === 'VElement' &&
+        sibling.rawName === componentName &&
+        hasConditionalDirective(sibling)
+    )
+  }
+
+  return false
+}
+
 /**
  * Checks for the presence of a 'key' attribute in the given node. If the 'key' attribute is missing
  * and the node is part of a conditional family a report is generated.
@@ -44,35 +71,39 @@ const checkForKey = (
   uniqueKey,
   conditionalFamilies
 ) => {
-  if (node.parent && node.parent.type === 'VElement') {
+  if (
+    node.parent &&
+    node.parent.type === 'VElement' &&
+    hasConditionalRenderedSiblings(node, componentName)
+  ) {
     const conditionalFamily = conditionalFamilies.get(node.parent)
 
-    if (
-      conditionalFamily &&
-      (utils.hasDirective(node, 'bind', 'key') ||
-        utils.hasAttribute(node, 'key') ||
-        !hasConditionalDirective(node) ||
-        !(conditionalFamily.else || conditionalFamily.elseIf.length > 0))
-    ) {
-      return
-    }
+    if (conditionalFamily && !utils.hasAttribute(node, 'key')) {
+      let needsKey = false
 
-    context.report({
-      node: node.startTag,
-      loc: node.startTag.loc,
-      messageId: 'requireKey',
-      data: {
-        componentName
-      },
-      fix(fixer) {
-        const afterComponentNamePosition =
-          node.startTag.range[0] + componentName.length + 1
-        return fixer.insertTextBeforeRange(
-          [afterComponentNamePosition, afterComponentNamePosition],
-          ` key="${uniqueKey}"`
-        )
+      if (node === conditionalFamily.if || node === conditionalFamily.else) {
+        needsKey = true
+      } else if (conditionalFamily.elseIf.includes(node)) {
+        needsKey = true
       }
-    })
+
+      if (needsKey) {
+        context.report({
+          node: node.startTag,
+          loc: node.startTag.loc,
+          messageId: 'requireKey',
+          data: { componentName },
+          fix(fixer) {
+            const afterComponentNamePosition =
+              node.startTag.range[0] + componentName.length + 1
+            return fixer.insertTextBeforeRange(
+              [afterComponentNamePosition, afterComponentNamePosition],
+              ` key="${uniqueKey}"`
+            )
+          }
+        })
+      }
+    }
   }
 }
 
@@ -189,7 +220,7 @@ module.exports = {
           if (node.parent && node.parent.type === 'VElement') {
             let conditionalFamily = conditionalFamilies.get(node.parent)
 
-            if (conditionType === 'if' && !conditionalFamily) {
+            if (conditionType === 'if') {
               conditionalFamily = createConditionalFamily(node)
               conditionalFamilies.set(node.parent, conditionalFamily)
             }
@@ -222,44 +253,55 @@ module.exports = {
         const currentScope = componentUsageStack[componentUsageStack.length - 1]
         const usageInfo = currentScope.get(componentName) || {
           count: 0,
-          firstNode: null
+          firstNode: null,
+          hasIf: false
         }
 
-        if (hasConditionalDirective(node)) {
-          // Store the first node if this is the first occurrence
-          if (usageInfo.count === 0) {
-            usageInfo.firstNode = node
-          }
+        const isIfDirective = utils.getDirective(node, 'if') !== null
+        const isConditional =
+          isIfDirective ||
+          utils.getDirective(node, 'else-if') !== null ||
+          utils.getDirective(node, 'else') !== null
 
-          if (usageInfo.count > 0) {
-            const uniqueKey = `${casing.kebabCase(componentName)}-${
-              usageInfo.count + 1
-            }`
-            checkForKey(
-              node,
-              context,
-              componentName,
-              uniqueKey,
-              conditionalFamilies
-            )
-
-            // If this is the second occurrence, also apply a fix to the first occurrence
-            if (usageInfo.count === 1) {
-              const uniqueKeyForFirstInstance = `${casing.kebabCase(
-                componentName
-              )}-1`
+        if (isConditional) {
+          if (isIfDirective && usageInfo.hasIf) {
+            // Reset if family already has an 'if' directive
+            usageInfo.count = 1
+            usageInfo.firstNode = node
+          } else {
+            usageInfo.hasIf = true
+            if (usageInfo.count === 0) {
+              usageInfo.firstNode = node
+            } else if (usageInfo.count > 0 && usageInfo.firstNode !== node) {
+              const uniqueKey = `${casing.kebabCase(componentName)}-${
+                usageInfo.count + 1
+              }`
               checkForKey(
-                usageInfo.firstNode,
+                node,
                 context,
                 componentName,
-                uniqueKeyForFirstInstance,
+                uniqueKey,
                 conditionalFamilies
               )
+
+              if (usageInfo.count === 1) {
+                const uniqueKeyForFirstInstance = `${casing.kebabCase(
+                  componentName
+                )}-1`
+                checkForKey(
+                  usageInfo.firstNode,
+                  context,
+                  componentName,
+                  uniqueKeyForFirstInstance,
+                  conditionalFamilies
+                )
+              }
             }
+            usageInfo.count++
           }
-          usageInfo.count += 1
           currentScope.set(componentName, usageInfo)
         }
+
         componentUsageStack.push(new Map())
         pushedNodes.add(node)
       },

tests/lib/rules/v-if-else-key.js

@@ -127,6 +127,90 @@ tester.run('v-if-else-key', rule, {
         }
         </script>
         `
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <div>
+            <div>
+              <ComponentA v-if="foo" /> 
+              <ComponentB v-else-if="bar" /> 
+            </div>
+            <div>
+              <div>
+                <ComponentA v-if="foo" />
+                <ComponentB v-else-if="baz" /> 
+              </div>
+              <div>
+                <ComponentA v-if="foo" />
+                <ComponentB v-else-if="baz" />
+              </div>
+            </div>
+            <div>
+              <ComponentA v-if="foo" />
+              <ComponentB v-else /> 
+            </div>
+            <div>
+              <div v-if="foo" />
+              <ComponentB v-else />
+            </div>
+          </div>
+        </template>
+        <script>
+        export default {
+          components: {
+            ComponentA,
+            ComponentB
+          }
+        }
+        </script>
+      `
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <div>
+            <div>
+              <CustomComponent v-if="foo" />
+              <div v-else />
+            </div>
+
+            <CustomComponent v-if="bar" />
+          </div>
+        </template>
+        <script>
+        export default {
+            components: {
+                CustomComponent
+            }
+        }
+        </script>
+        `
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <tile>
+            <template v-if="state === 'foo'">
+                <ComponentA>…</ComponentA>
+                <ComponentB>…</ComponentB>
+            </template>
+            <ComponentA v-else-if="state === 'bar'" key="a">…</ComponentA>
+            <ComponentA v-else-if="state === 'bar'" key="b">…</ComponentA>
+          </tile>
+        </template>
+        <script>
+        export default {
+            components: {
+                ComponentA,
+                ComponentB
+            }
+        }
+        </script>
+        `
     }
   ],
   invalid: [
@@ -424,6 +508,62 @@ tester.run('v-if-else-key', rule, {
           line: 6
         }
       ]
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <div>
+            <ComponentA v-if="foo" />
+
+            <ComponentA v-if="bar" />
+            <ComponentA v-else-if="baz" />
+            <ComponentA v-else />
+          </div>
+        </template>
+        <script>
+        export default {
+          components: {
+            ComponentA
+          }
+        }
+        </script>
+        `,
+      output: `
+        <template>
+          <div>
+            <ComponentA v-if="foo" />
+
+            <ComponentA key="component-a-1" v-if="bar" />
+            <ComponentA key="component-a-2" v-else-if="baz" />
+            <ComponentA key="component-a-3" v-else />
+          </div>
+        </template>
+        <script>
+        export default {
+          components: {
+            ComponentA
+          }
+        }
+        </script>
+        `,
+      errors: [
+        {
+          message:
+            "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.",
+          line: 6
+        },
+        {
+          message:
+            "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.",
+          line: 7
+        },
+        {
+          message:
+            "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.",
+          line: 8
+        }
+      ]
     }
   ]
 })