fix(vue/no-duplicate-class-names): improve non-intersecting conditions and combining parents (#2980)

Author: Yizack

.changeset/major-planes-fly.md

@@ -0,0 +1,5 @@
+---
+'eslint-plugin-vue': patch
+---
+
+Fixed false positives in non-intersecting conditions in `vue/no-duplicate-class-names` and correctly detect duplicates in combining expressions

lib/rules/no-duplicate-class-names.js

@@ -11,15 +11,21 @@ const utils = require('../utils')
  * @param {VDirective} node
  * @param {Expression} [expression]
  * @param {boolean} [unconditional=true] whether the expression is unconditional
- * @return {IterableIterator<{ node: Literal | TemplateElement, unconditional: boolean }>}
+ * @param {Expression} [parentExpr] parent expression for context
+ * @return {IterableIterator<{ node: Literal | TemplateElement, unconditional: boolean, parentExpr?: Expression }>}
  */
-function* extractClassNodes(node, expression, unconditional = true) {
+function* extractClassNodes(
+  node,
+  expression,
+  unconditional = true,
+  parentExpr
+) {
   const nodeExpression = expression ?? node.value?.expression
   if (!nodeExpression) return
 
   switch (nodeExpression.type) {
     case 'Literal': {
-      yield { node: nodeExpression, unconditional }
+      yield { node: nodeExpression, unconditional, parentExpr }
       break
     }
     case 'ObjectExpression': {
@@ -29,42 +35,76 @@ function* extractClassNodes(node, expression, unconditional = true) {
           prop.key?.type === 'Literal' &&
           typeof prop.key.value === 'string'
         ) {
-          yield { node: prop.key, unconditional: false }
+          yield {
+            node: prop.key,
+            unconditional: false,
+            parentExpr: nodeExpression
+          }
         }
       }
       break
     }
     case 'ArrayExpression': {
       for (const element of nodeExpression.elements) {
         if (!element || element.type === 'SpreadElement') continue
-        yield* extractClassNodes(node, element, unconditional)
+        yield* extractClassNodes(node, element, unconditional, nodeExpression)
       }
       break
     }
     case 'ConditionalExpression': {
-      yield* extractClassNodes(node, nodeExpression.consequent, false)
-      yield* extractClassNodes(node, nodeExpression.alternate, false)
+      yield* extractClassNodes(
+        node,
+        nodeExpression.consequent,
+        false,
+        nodeExpression
+      )
+      yield* extractClassNodes(
+        node,
+        nodeExpression.alternate,
+        false,
+        nodeExpression
+      )
       break
     }
     case 'TemplateLiteral': {
       for (const quasi of nodeExpression.quasis) {
-        yield { node: quasi, unconditional }
+        yield { node: quasi, unconditional, parentExpr: nodeExpression }
       }
       for (const expr of nodeExpression.expressions) {
-        yield* extractClassNodes(node, expr, unconditional)
+        yield* extractClassNodes(node, expr, unconditional, nodeExpression)
       }
       break
     }
     case 'BinaryExpression': {
       if (nodeExpression.operator === '+') {
-        yield* extractClassNodes(node, nodeExpression.left, unconditional)
-        yield* extractClassNodes(node, nodeExpression.right, unconditional)
+        yield* extractClassNodes(
+          node,
+          nodeExpression.left,
+          unconditional,
+          nodeExpression
+        )
+        yield* extractClassNodes(
+          node,
+          nodeExpression.right,
+          unconditional,
+          nodeExpression
+        )
       }
       break
     }
     case 'LogicalExpression': {
-      yield* extractClassNodes(node, nodeExpression.left, unconditional)
-      yield* extractClassNodes(node, nodeExpression.right, unconditional)
+      yield* extractClassNodes(
+        node,
+        nodeExpression.left,
+        unconditional,
+        nodeExpression
+      )
+      yield* extractClassNodes(
+        node,
+        nodeExpression.right,
+        false,
+        nodeExpression
+      )
       break
     }
   }
@@ -250,11 +290,15 @@ module.exports = {
         /** @type {Map<string, ASTNode>} */
         const seen = new Map()
 
-        /** @type {Map<string, {node: ASTNode, unconditional: boolean}>} */
+        /** @type {Map<string, {node: ASTNode, unconditional: boolean, parentExpr?: Expression}>} */
         const collected = new Map()
 
         const classNodes = extractClassNodes(node)
-        for (const { node: reportNode, unconditional } of classNodes) {
+        for (const {
+          node: reportNode,
+          unconditional,
+          parentExpr
+        } of classNodes) {
           // report fixable duplicates and collect reported class names
           const reportedClasses = reportDuplicateClasses(reportNode)
           if (reportedClasses) {
@@ -272,14 +316,21 @@ module.exports = {
             if (reported.has(className)) continue
             const existing = collected.get(className)
             if (existing) {
-              // only add duplicate if at least one is unconditional
-              if (existing.unconditional || unconditional) {
+              // only add duplicate if at least one is unconditional, or share the same combining parent
+              const isSameParent =
+                parentExpr &&
+                existing.parentExpr === parentExpr &&
+                (parentExpr.type === 'BinaryExpression' ||
+                  parentExpr.type === 'TemplateLiteral')
+
+              if (existing.unconditional || unconditional || isSameParent) {
                 duplicatesInExpression.add(className)
               }
             } else {
               collected.set(className, {
                 node: reportNode.parent,
-                unconditional
+                unconditional,
+                parentExpr
               })
             }
             // track unconditional duplicates separately for reporting

tests/lib/rules/no-duplicate-class-names.js

@@ -56,6 +56,18 @@ tester.run('no-duplicate-class-names', rule, {
     {
       filename: 'class-object-duplicate-value.vue',
       code: `<div :class="{ 'foo bar': isActive, 'foo': isAnotherActive }"></div>`
+    },
+    {
+      filename: 'class-non-intersecting-conditions.vue',
+      code: `<template><div :class="[isActive1 && { 'foo': isActive2, 'bar': isActive3 }, isActive4 && 'bar']"></div></template>`
+    },
+    {
+      filename: 'class-multiple-logical-non-intersecting.vue',
+      code: `<template><div :class="[isActive1 && 'foo', isActive2 && 'foo']"></div></template>`
+    },
+    {
+      filename: 'class-binary-in-logical-non-intersecting.vue',
+      code: `<template><div :class="[isActive1 && ('foo' + ' bar'), isActive2 && 'foo']"></div></template>`
     }
   ],
   invalid: [
@@ -527,6 +539,36 @@ tester.run('no-duplicate-class-names', rule, {
           endColumn: 63
         }
       ]
+    },
+    {
+      filename: 'duplicate-class-binary-in-logical-expression.vue',
+      code: `<template><div :class="isActive && 'bar' + ' bar'"></div></template>`,
+      output: null,
+      errors: [
+        {
+          message: "Duplicate class name 'bar'.",
+          type: 'BinaryExpression',
+          line: 1,
+          column: 36,
+          endLine: 1,
+          endColumn: 50
+        }
+      ]
+    },
+    {
+      filename: 'duplicate-class-template-literal-in-logical-expression.vue',
+      code: '<template><div :class="isActive && `foo ${bar} foo`"></div></template>',
+      output: null,
+      errors: [
+        {
+          message: "Duplicate class name 'foo'.",
+          type: 'TemplateLiteral',
+          line: 1,
+          column: 36,
+          endLine: 1,
+          endColumn: 52
+        }
+      ]
     }
   ]
 })