Fixed false positives for v-bind="object" syntax in vue/attributes-order rule (#1391)

ota-meshi · web-flow · commit 543361bc4a14 · 2020-12-27T11:56:22.000+09:00
diff --git a/docs/rules/attributes-order.md b/docs/rules/attributes-order.md
@@ -91,7 +91,33 @@ This rule aims to enforce ordering of component attributes. The default order is
 
 </eslint-code-block>
 
+Note that `v-bind="object"` syntax is considered to be the same as the next or previous attribute categories.
+
+<eslint-code-block fix :rules="{'vue/attributes-order': ['error']}">
+
+```vue
+<template>
+  <!-- ✓ GOOD (`v-bind="object"` is considered GLOBAL category) -->
+  <MyComponent
+    v-bind="object"
+    id="x"
+    v-model="x"
+    v-bind:foo="x">
+  </MyComponent>
+
+  <!-- ✗ BAD (`v-bind="object"` is considered UNIQUE category) -->
+  <MyComponent
+    key="x"
+    v-model="x"
+    v-bind="object">
+  </MyComponent>
+</template>
+```
+
+</eslint-code-block>
+
 ## :wrench: Options
+
 ```json
 {
   "vue/attributes-order": ["error", {
@@ -113,7 +139,7 @@ This rule aims to enforce ordering of component attributes. The default order is
 }
 ```
 
-### `"alphabetical": true` 
+### `"alphabetical": true`
 
 <eslint-code-block fix :rules="{'vue/attributes-order': ['error', {alphabetical: true}]}">
 
diff --git a/lib/rules/attributes-order.js b/lib/rules/attributes-order.js
@@ -8,6 +8,11 @@ const utils = require('../utils')
 // ------------------------------------------------------------------------------
 // Rule Definition
 // ------------------------------------------------------------------------------
+
+/**
+ * @typedef { VDirective & { key: VDirectiveKey & { name: VIdentifier & { name: 'bind' } } } } VBindDirective
+ */
+
 const ATTRS = {
   DEFINITION: 'DEFINITION',
   LIST_RENDERING: 'LIST_RENDERING',
@@ -22,13 +27,47 @@ const ATTRS = {
   CONTENT: 'CONTENT'
 }
 
+/**
+ * Check whether the given attribute is `v-bind` directive.
+ * @param {VAttribute | VDirective | undefined | null} node
+ * @returns { node is VBindDirective }
+ */
+function isVBind(node) {
+  return Boolean(node && node.directive && node.key.name.name === 'bind')
+}
+/**
+ * Check whether the given attribute is plain attribute.
+ * @param {VAttribute | VDirective | undefined | null} node
+ * @returns { node is VAttribute }
+ */
+function isVAttribute(node) {
+  return Boolean(node && !node.directive)
+}
+/**
+ * Check whether the given attribute is plain attribute or `v-bind` directive.
+ * @param {VAttribute | VDirective | undefined | null} node
+ * @returns { node is VAttribute }
+ */
+function isVAttributeOrVBind(node) {
+  return isVAttribute(node) || isVBind(node)
+}
+
+/**
+ * Check whether the given attribute is `v-bind="..."` directive.
+ * @param {VAttribute | VDirective | undefined | null} node
+ * @returns { node is VBindDirective }
+ */
+function isVBindObject(node) {
+  return isVBind(node) && node.key.argument == null
+}
+
 /**
  * @param {VAttribute | VDirective} attribute
  * @param {SourceCode} sourceCode
  */
 function getAttributeName(attribute, sourceCode) {
   if (attribute.directive) {
-    if (attribute.key.name.name === 'bind') {
+    if (isVBind(attribute)) {
       return attribute.key.argument
         ? sourceCode.getText(attribute.key.argument)
         : ''
@@ -62,7 +101,7 @@ function getDirectiveKeyName(directiveKey, sourceCode) {
 function getAttributeType(attribute, sourceCode) {
   let propName
   if (attribute.directive) {
-    if (attribute.key.name.name !== 'bind') {
+    if (!isVBind(attribute)) {
       const name = attribute.key.name.name
       if (name === 'for') {
         return ATTRS.LIST_RENDERING
@@ -130,24 +169,14 @@ function getPosition(attribute, attributePosition, sourceCode) {
  * @param {SourceCode} sourceCode
  */
 function isAlphabetical(prevNode, currNode, sourceCode) {
-  const isSameType =
-    getAttributeType(prevNode, sourceCode) ===
-    getAttributeType(currNode, sourceCode)
-  if (isSameType) {
-    const prevName = getAttributeName(prevNode, sourceCode)
-    const currName = getAttributeName(currNode, sourceCode)
-    if (prevName === currName) {
-      const prevIsBind = Boolean(
-        prevNode.directive && prevNode.key.name.name === 'bind'
-      )
-      const currIsBind = Boolean(
-        currNode.directive && currNode.key.name.name === 'bind'
-      )
-      return prevIsBind <= currIsBind
-    }
-    return prevName < currName
+  const prevName = getAttributeName(prevNode, sourceCode)
+  const currName = getAttributeName(currNode, sourceCode)
+  if (prevName === currName) {
+    const prevIsBind = isVBind(prevNode)
+    const currIsBind = isVBind(currNode)
+    return prevIsBind <= currIsBind
   }
-  return true
+  return prevName < currName
 }
 
 /**
@@ -186,16 +215,6 @@ function create(context) {
     } else attributePosition[item] = i
   })
 
-  /**
-   * @typedef {object} State
-   * @property {number} currentPosition
-   * @property {VAttribute | VDirective} previousNode
-   */
-  /**
-   * @type {State | null}
-   */
-  let state
-
   /**
    * @param {VAttribute | VDirective} node
    * @param {VAttribute | VDirective} previousNode
@@ -213,43 +232,112 @@ function create(context) {
 
       fix(fixer) {
         const attributes = node.parent.attributes
-        const shiftAttrs = attributes.slice(
+
+        /** @type { (node: VAttribute | VDirective | undefined) => boolean } */
+        let isMoveUp
+
+        if (isVBindObject(node)) {
+          // prev, v-bind:foo, v-bind -> v-bind:foo, v-bind, prev
+          isMoveUp = isVAttributeOrVBind
+        } else if (isVAttributeOrVBind(node)) {
+          // prev, v-bind, v-bind:foo -> v-bind, v-bind:foo, prev
+          isMoveUp = isVBindObject
+        } else {
+          isMoveUp = () => false
+        }
+
+        const previousNodes = attributes.slice(
           attributes.indexOf(previousNode),
-          attributes.indexOf(node) + 1
+          attributes.indexOf(node)
         )
+        const moveUpNodes = [node]
+        const moveDownNodes = []
+        let index = 0
+        while (previousNodes[index]) {
+          const node = previousNodes[index++]
+          if (isMoveUp(node)) {
+            moveUpNodes.unshift(node)
+          } else {
+            moveDownNodes.push(node)
+          }
+        }
+        const moveNodes = [...moveUpNodes, ...moveDownNodes]
 
-        return shiftAttrs.map((attr, i) => {
-          const text =
-            attr === previousNode
-              ? sourceCode.getText(node)
-              : sourceCode.getText(shiftAttrs[i - 1])
-          return fixer.replaceText(attr, text)
+        return moveNodes.map((moveNode, index) => {
+          const text = sourceCode.getText(moveNode)
+          return fixer.replaceText(previousNodes[index] || node, text)
         })
       }
     })
   }
 
   return utils.defineTemplateBodyVisitor(context, {
-    VStartTag() {
-      state = null
-    },
-    VAttribute(node) {
-      let inAlphaOrder = true
-      if (state && alphabetical) {
-        inAlphaOrder = isAlphabetical(state.previousNode, node, sourceCode)
+    VStartTag(node) {
+      const attributes = node.attributes.filter((node, index, attributes) => {
+        if (
+          isVBindObject(node) &&
+          (isVAttributeOrVBind(attributes[index - 1]) ||
+            isVAttributeOrVBind(attributes[index + 1]))
+        ) {
+          // In Vue 3, ignore the `v-bind:foo=" ... "` and `v-bind ="object"` syntax
+          // as they behave differently if you change the order.
+          return false
+        }
+        return true
+      })
+      if (attributes.length <= 1) {
+        return
       }
-      if (
-        !state ||
-        (state.currentPosition <=
-          getPosition(node, attributePosition, sourceCode) &&
-          inAlphaOrder)
-      ) {
-        state = {
-          currentPosition: getPosition(node, attributePosition, sourceCode),
-          previousNode: node
+
+      let previousNode = attributes[0]
+      let previousPosition = getPositionFromAttrIndex(0)
+      for (let index = 1; index < attributes.length; index++) {
+        const node = attributes[index]
+        const position = getPositionFromAttrIndex(index)
+
+        let valid = previousPosition <= position
+        if (valid && alphabetical && previousPosition === position) {
+          valid = isAlphabetical(previousNode, node, sourceCode)
         }
-      } else {
-        reportIssue(node, state.previousNode)
+        if (valid) {
+          previousNode = node
+          previousPosition = position
+        } else {
+          reportIssue(node, previousNode)
+        }
+      }
+
+      /**
+       * @param {number} index
+       * @returns {number}
+       */
+      function getPositionFromAttrIndex(index) {
+        const node = attributes[index]
+        if (isVBindObject(node)) {
+          // node is `v-bind ="object"` syntax
+
+          // In Vue 3, if change the order of `v-bind:foo=" ... "` and `v-bind ="object"`,
+          // the behavior will be different, so adjust so that there is no change in behavior.
+
+          const len = attributes.length
+          for (let nextIndex = index + 1; nextIndex < len; nextIndex++) {
+            const next = attributes[nextIndex]
+
+            if (isVAttributeOrVBind(next) && !isVBindObject(next)) {
+              // It is considered to be in the same order as the next bind prop node.
+              return getPositionFromAttrIndex(nextIndex)
+            }
+          }
+          for (let prevIndex = index - 1; prevIndex >= 0; prevIndex--) {
+            const prev = attributes[prevIndex]
+
+            if (isVAttributeOrVBind(prev) && !isVBindObject(prev)) {
+              // It is considered to be in the same order as the prev bind prop node.
+              return getPositionFromAttrIndex(prevIndex)
+            }
+          }
+        }
+        return getPosition(node, attributePosition, sourceCode)
       }
     }
   })
diff --git a/tests/lib/rules/attributes-order.js b/tests/lib/rules/attributes-order.js
