Fixed no-use-v-if-with-v-for error when using destructuring for itera…

…tion or using iteration of expression (#1071)
vuejs · Mar 8, 2020 · e77081d · e77081d
1 parent ab3bf36
commit e77081d
Show file tree

Hide file tree

Showing 2 changed files with 124 additions and 4 deletions.
diff --git a/lib/rules/no-use-v-if-with-v-for.js b/lib/rules/no-use-v-if-with-v-for.js
@@ -37,7 +37,7 @@ function getVForUsingIterationVar (vIf) {
       variable.kind === 'v-for'
     )
     if (targetVFor) {
-      return targetVFor.id.parent
+      return targetVFor
     }
   }
   return undefined
@@ -76,13 +76,20 @@ module.exports = {
         if (utils.hasDirective(element, 'for')) {
           if (isUsingIterationVar(node)) {
             if (!allowUsingIterationVar) {
-              const vFor = getVForUsingIterationVar(node)
+              const vForVar = getVForUsingIterationVar(node)
+
+              let targetVForExpr = vForVar.id.parent
+              while (targetVForExpr.type !== 'VForExpression') {
+                targetVForExpr = targetVForExpr.parent
+              }
+              const iteratorNode = targetVForExpr.right
               context.report({
                 node,
                 loc: node.loc,
-                message: "The '{{iteratorName}}' variable inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
+                message: "The '{{iteratorName}}' {{kind}} inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
                 data: {
-                  iteratorName: vFor.right.name
+                  iteratorName: iteratorNode.type === 'Identifier' ? iteratorNode.name : context.getSourceCode().getText(iteratorNode),
+                  kind: iteratorNode.type === 'Identifier' ? 'variable' : 'expression'
                 }
               })
             }

diff --git a/tests/lib/rules/no-use-v-if-with-v-for.js b/tests/lib/rules/no-use-v-if-with-v-for.js
@@ -73,6 +73,55 @@ tester.run('no-use-v-if-with-v-for', rule, {
           </ul>
         </template>
       `
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="{x} in list" v-if="x"></div></div></template>',
+      options: [{ allowUsingIterationVar: true }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="{x,y,z} in list" v-if="y.foo"></div></div></template>',
+      options: [{ allowUsingIterationVar: true }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="({x,y,z},i) in list" v-if="i%2==0"></div></div></template>',
+      options: [{ allowUsingIterationVar: true }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div v-if="shown"><div v-for="({x,y,z},i) in list"></div></div></template>'
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <ul>
+            <li
+              v-for="{user} in activeUsers"
+              :key="user.id"
+            >
+              {{ user.name }}
+            <li>
+          </ul>
+        </template>
+      `
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <ul v-if="shouldShowUsers">
+            <li
+              v-for="{user} in users"
+              :key="user.id"
+            >
+              {{ user.name }}
+            <li>
+          </ul>
+        </template>
+      `
     }
   ],
   invalid: [
@@ -139,6 +188,70 @@ tester.run('no-use-v-if-with-v-for', rule, {
         message: "This 'v-if' should be moved to the wrapper element.",
         line: 6
       }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="{x,y,z} in list" v-if="z.isActive"></div></div></template>',
+      errors: [{
+        message: "The 'list' variable inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
+        line: 1
+      }]
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <ul>
+            <li
+              v-for="{foo, bar, user} in users"
+              v-if="user.isActive"
+              :key="user.id"
+            >
+              {{ user.name }}
+            <li>
+          </ul>
+        </template>
+      `,
+      errors: [{
+        message: "The 'users' variable inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
+        line: 6
+      }]
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <ul>
+            <li
+              v-for="{foo, bar, user} in users"
+              v-if="shouldShowUsers"
+              :key="user.id"
+            >
+              {{ user.name }}
+            <li>
+          </ul>
+        </template>
+      `,
+      errors: [{
+        message: "This 'v-if' should be moved to the wrapper element.",
+        line: 6
+      }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="{x} in list()" v-if="x.isActive"></div></div></template>',
+      errors: [{
+        message: "The 'list()' expression inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
+        line: 1
+      }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="i in 5" v-if="i"></div></div></template>',
+      errors: [{
+        message: "The '5' expression inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
+        line: 1
+      }]
     }
   ]
 })