fix: find duplicates within unconditional expression nodes + add comments

related to #2932 discussion

Author: Yizack

docs/rules/no-duplicate-class-names.md

@@ -39,6 +39,8 @@ This rule prevents the same class name from appearing multiple times within the
   <div :class="isActive ? 'foo foo' : 'bar'"></div>
   <div :class="'foo foo ' + 'bar'"></div>
   <div class="foo" :class="'foo'"></div>
+  <div :class="['foo', 'foo']"></div>
+  <div :class="'foo ' + 'foo'"></div>
 </template>
 ```
 

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

@@ -10,15 +10,16 @@ const utils = require('../utils')
 /**
  * @param {VDirective} node
  * @param {Expression} [expression]
- * @return {IterableIterator<{ node: Literal | TemplateElement }>}
+ * @param {boolean} [unconditional=true] whether the expression is unconditional
+ * @return {IterableIterator<{ node: Literal | TemplateElement, unconditional: boolean }>}
  */
-function* extractClassNodes(node, expression) {
+function* extractClassNodes(node, expression, unconditional = true) {
   const nodeExpression = expression ?? node.value?.expression
   if (!nodeExpression) return
 
   switch (nodeExpression.type) {
     case 'Literal': {
-      yield { node: nodeExpression }
+      yield { node: nodeExpression, unconditional }
       break
     }
     case 'ObjectExpression': {
@@ -28,37 +29,36 @@ function* extractClassNodes(node, expression) {
           prop.key?.type === 'Literal' &&
           typeof prop.key.value === 'string'
         ) {
-          yield { node: prop.key }
+          yield { node: prop.key, unconditional: false }
         }
       }
       break
     }
     case 'ArrayExpression': {
       for (const element of nodeExpression.elements) {
         if (!element || element.type === 'SpreadElement') continue
-
-        yield* extractClassNodes(node, element)
+        yield* extractClassNodes(node, element, unconditional)
       }
       break
     }
     case 'ConditionalExpression': {
-      yield* extractClassNodes(node, nodeExpression.consequent)
-      yield* extractClassNodes(node, nodeExpression.alternate)
+      yield* extractClassNodes(node, nodeExpression.consequent, false)
+      yield* extractClassNodes(node, nodeExpression.alternate, false)
       break
     }
     case 'TemplateLiteral': {
       for (const quasi of nodeExpression.quasis) {
-        yield { node: quasi }
+        yield { node: quasi, unconditional }
       }
       for (const expr of nodeExpression.expressions) {
-        yield* extractClassNodes(node, expr)
+        yield* extractClassNodes(node, expr, unconditional)
       }
       break
     }
     case 'BinaryExpression': {
       if (nodeExpression.operator === '+') {
-        yield* extractClassNodes(node, nodeExpression.left)
-        yield* extractClassNodes(node, nodeExpression.right)
+        yield* extractClassNodes(node, nodeExpression.left, unconditional)
+        yield* extractClassNodes(node, nodeExpression.right, unconditional)
       }
       break
     }
@@ -138,6 +138,14 @@ function removeDuplicateClassNames(raw) {
   return quote + kept.join('') + quote
 }
 
+/** @param {VLiteral | Literal | TemplateElement | null} node */
+function getRawValue(node) {
+  if (!node?.value) return null
+  return typeof node.value === 'object' && 'raw' in node.value
+    ? node.value.raw
+    : node.value
+}
+
 module.exports = {
   meta: {
     type: 'suggestion',
@@ -160,11 +168,7 @@ module.exports = {
     function reportDuplicateClasses(node) {
       if (!node?.value) return
 
-      const classList =
-        typeof node.value === 'object' && 'raw' in node.value
-          ? node.value.raw
-          : node.value
-
+      const classList = getRawValue(node)
       if (typeof classList !== 'string') return
 
       const classNames = getClassNames(classList)
@@ -193,6 +197,8 @@ module.exports = {
           return fixer.replaceText(node, removeDuplicateClassNames(raw))
         }
       })
+
+      return duplicates
     }
 
     return utils.defineTemplateBodyVisitor(context, {
@@ -208,7 +214,6 @@ module.exports = {
       ) {
         const parent = node.parent
         const attrs = parent.attributes || []
-
         const staticAttr = attrs.find(
           (attr) =>
             attr.key &&
@@ -217,9 +222,9 @@ module.exports = {
             attr.value.type === 'VLiteral'
         )
 
+        // get static classes
         /** @type {Set<string> | null} */
         let staticClasses = null
-
         if (
           staticAttr &&
           staticAttr.value &&
@@ -228,30 +233,65 @@ module.exports = {
           staticClasses = new Set(getClassNames(String(staticAttr.value.value)))
         }
 
-        for (const { node: reportNode } of extractClassNodes(node)) {
-          reportDuplicateClasses(reportNode)
+        const reported = new Set()
+        const duplicatesInExpression = new Set()
+        /** @type {Map<string, ASTNode>} */
+        const seen = new Map()
 
-          if (staticClasses) {
-            const classList =
-              reportNode.value &&
-              typeof reportNode.value === 'object' &&
-              'raw' in reportNode.value
-                ? reportNode.value.raw
-                : reportNode.value
+        const classNodes = extractClassNodes(node)
+        for (const { node: reportNode, unconditional } of classNodes) {
+          // report fixable duplicates and collect reported class names
+          const reportedClasses = reportDuplicateClasses(reportNode)
+          if (reportedClasses) {
+            for (const classes of reportedClasses) reported.add(classes)
+          }
 
-            if (typeof classList !== 'string') continue
+          // collect duplicates within the expression nodes
+          if (unconditional) {
+            const classList = getRawValue(reportNode)
+            if (typeof classList === 'string') {
+              const classNames = getClassNames(classList)
+              for (const className of classNames) {
+                if (seen.has(className)) {
+                  duplicatesInExpression.add(className)
+                } else {
+                  seen.set(className, reportNode.parent)
+                }
+              }
+            }
+          }
 
-            const classNames = getClassNames(classList)
-            const intersection = classNames.filter((n) => staticClasses.has(n))
-            if (intersection.length > 0 && parent) {
-              context.report({
-                node: parent,
-                messageId: 'duplicateClassName',
-                data: { name: intersection.join(', ') }
-              })
+          // report duplicates between static and dynamic class attributes
+          if (staticClasses) {
+            const classList = getRawValue(reportNode)
+            if (typeof classList === 'string') {
+              const classNames = getClassNames(classList)
+              const intersection = classNames.filter((n) =>
+                staticClasses.has(n)
+              )
+              if (intersection.length > 0 && parent) {
+                context.report({
+                  node: parent,
+                  messageId: 'duplicateClassName',
+                  data: { name: intersection.join(', ') }
+                })
+              }
             }
           }
         }
+
+        // report duplicates between dynamic class nodes excluding already reported
+        for (const r of reported) duplicatesInExpression.delete(r)
+        for (const className of duplicatesInExpression) {
+          const reportNode = seen.get(className)
+          if (reportNode) {
+            context.report({
+              node: reportNode,
+              messageId: 'duplicateClassName',
+              data: { name: className }
+            })
+          }
+        }
       }
     })
   }

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

@@ -48,6 +48,14 @@ tester.run('no-duplicate-class-names', rule, {
     {
       filename: 'class-conditional-expression.vue',
       code: `<template><div :class="isActive ? 'foo' : 'bar'"></div></template>`
+    },
+    {
+      filename: 'class-conditional-duplicate-expression-value.vue',
+      code: `<template><div :class="isActive ? 'foo' : 'foo'"></div></template>`
+    },
+    {
+      filename: 'class-object-duplicate-value.vue',
+      code: `<div :class="{ 'foo bar': isActive, 'foo': isAnotherActive }"></div>`
     }
   ],
   invalid: [
@@ -315,6 +323,28 @@ tester.run('no-duplicate-class-names', rule, {
           type: 'VStartTag'
         }
       ]
+    },
+    {
+      filename: 'duplicate-class-cross-node-array.vue',
+      code: `<template><div :class="['foo', 'foo']"></div></template>`,
+      output: null,
+      errors: [
+        {
+          message: "Duplicate class name 'foo'.",
+          type: 'ArrayExpression'
+        }
+      ]
+    },
+    {
+      filename: 'duplicate-class-cross-node-binary.vue',
+      code: `<template><div :class="'foo ' + 'foo'"></div></template>`,
+      output: null,
+      errors: [
+        {
+          message: "Duplicate class name 'foo'.",
+          type: 'BinaryExpression'
+        }
+      ]
     }
   ]
 })