Repository: vuejs/eslint-plugin-vue
Pull request #480 (2018-05-26 16:41:35)

Update no-side-effects-in-computed-properties.md
Provide more details on working with arrays inside computed properties.

SketchBookkeeper: Vue newbie here, It just took me a minute to figure out how the correct example compared to the incorrect one. Thought some clarification here could be helpful.

Patch:
diff --git a/README.md b/README.md
index 015e2922..81f1486c 100644
--- a/README.md
+++ b/README.md
@@ -45,6 +45,17 @@ module.exports = {
 }
 ```
 
+### Single File Components
+
+ESLint only targets `.js` files by default. You must include the `.vue` extension using [the `--ext` option](https://eslint.org/docs/user-guide/configuring#specifying-file-extensions-to-lint) or a glob pattern.
+
+Examples:
+
+```bash
+eslint --ext .js,.vue src
+eslint src/**/*.{js,vue}
+```
+
 ### Attention
 
 All component-related rules are being applied to code that passes any of the following checks:
@@ -140,6 +151,7 @@ Enforce all the rules in this category, as well as all higher priority rules, wi
 |  | [vue/no-template-key](./docs/rules/no-template-key.md) | disallow `key` attribute on `<template>` |
 |  | [vue/no-textarea-mustache](./docs/rules/no-textarea-mustache.md) | disallow mustaches in `<textarea>` |
 |  | [vue/no-unused-vars](./docs/rules/no-unused-vars.md) | disallow unused variable definitions of v-for directives or scope attributes |
+|  | [vue/no-use-v-if-with-v-for](./docs/rules/no-use-v-if-with-v-for.md) | disallow use v-if on the same element as v-for. |
 |  | [vue/require-component-is](./docs/rules/require-component-is.md) | require `v-bind:is` of `<component>` elements |
 |  | [vue/require-render-return](./docs/rules/require-render-return.md) | enforce render function to always return value |
 |  | [vue/require-v-for-key](./docs/rules/require-v-for-key.md) | require `v-bind:key` with `v-for` directives |
diff --git a/docs/rules/no-side-effects-in-computed-properties.md b/docs/rules/no-side-effects-in-computed-properties.md
index 36066d82..e8966625 100644
--- a/docs/rules/no-side-effects-in-computed-properties.md
+++ b/docs/rules/no-side-effects-in-computed-properties.md
@@ -16,7 +16,7 @@ computed: {
     return `${this.firstName} ${this.lastName}`
   },
   reversedArray () {
-    return this.array.reverse() // <- side effect
+    return this.array.reverse() // <- side effect - orginal array is being mutated
   }
 }
 ```
@@ -29,7 +29,7 @@ computed: {
     return `${this.firstName} ${this.lastName}`
   },
   reversedArray () {
-    return this.array.slice(0).reverse()
+    return this.array.slice(0).reverse() // .slice makes a copy of the array, instead of mutating the orginal
   }
 }
 ```
diff --git a/docs/rules/no-use-v-if-with-v-for.md b/docs/rules/no-use-v-if-with-v-for.md
new file mode 100644
index 00000000..cbe33005
--- /dev/null
+++ b/docs/rules/no-use-v-if-with-v-for.md
@@ -0,0 +1,98 @@
+# disallow use v-if on the same element as v-for. (vue/no-use-v-if-with-v-for)
+
+> Never use `v-if` on the same element as `v-for`.
+>
+> There are two common cases where this can be tempting:
+>
+> * To filter items in a list (e.g. `v-for="user in users" v-if="user.isActive"`). In these cases, replace `users` with a new computed property that returns your filtered list (e.g. `activeUsers`).
+>
+> * To avoid rendering a list if it should be hidden (e.g. `v-for="user in users" v-if="shouldShowUsers"`). In these cases, move the `v-if` to a container element (e.g. `ul`, `ol`).
+>
+> https://vuejs.org/v2/style-guide/#Avoid-v-if-with-v-for-essential
+
+
+## :book: Rule Details
+
+:-1: Examples of **incorrect** code for this rule:
+
+```html
+<TodoItem
+  v-if="complete"
+  v-for="todo in todos"
+  :todo="todo"
+/>
+```
+
+In this case, the `v-if` should be written on the wrapper element.
+
+
+```html
+<TodoItem
+  v-for="todo in todos"
+  v-if="todo.shown"
+  :todo="todo"
+/>
+```
+
+In this case, the `v-for` list variable should be replace with a computed property that returns your filtered list.
+
+
+:+1: Examples of **correct** code for this rule:
+
+
+```html
+<ul v-if="complete">
+  <TodoItem
+    v-for="todo in todos"
+    :todo="todo"
+  />
+</ul>
+```
+
+
+
+```html
+<TodoItem
+  v-for="todo in shownTodos"
+  :todo="todo"
+/>
+```
+
+```js
+computed: {
+  shownTodos() {
+    return this.todos.filter((todo) => todo.shown)
+  }
+}
+```
+
+## :wrench: Options
+
+`allowUsingIterationVar` - Enables The `v-if` directive use the reference which is to the variables which are defined by the `v-for` directives.
+
+```js
+'vue/no-use-v-if-with-v-for': ['error', {
+  allowUsingIterationVar: true // default: false
+}]
+```
+
+:+1: Examples of additional **correct** code for this rule with sample `"allowUsingIterationVar": true` options:
+
+```html
+<TodoItem
+  v-for="todo in todos"
+  v-if="todo.shown"
+  :todo="todo"
+/>
+```
+
+:-1: Examples of additional **incorrect** code for this rule with sample `"allowUsingIterationVar": true` options:
+
+```html
+<TodoItem
+  v-if="complete"
+  v-for="todo in todos"
+  :todo="todo"
+/>
+```
+
diff --git a/lib/index.js b/lib/index.js
index d96fd38e..008a3c4d 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -32,6 +32,7 @@ module.exports = {
     'no-template-key': require('./rules/no-template-key'),
     'no-textarea-mustache': require('./rules/no-textarea-mustache'),
     'no-unused-vars': require('./rules/no-unused-vars'),
+    'no-use-v-if-with-v-for': require('./rules/no-use-v-if-with-v-for'),
     'order-in-components': require('./rules/order-in-components'),
     'prop-name-casing': require('./rules/prop-name-casing'),
     'require-component-is': require('./rules/require-component-is'),
diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js
index 149abac5..9ef2d8b7 100644
--- a/lib/rules/no-side-effects-in-computed-properties.js
+++ b/lib/rules/no-side-effects-in-computed-properties.js
@@ -41,7 +41,7 @@ module.exports = {
         },
         // this.xxx.func()
         'CallExpression' (node) {
-          const code = context.getSourceCode().getText(node)
+          const code = utils.parseMemberOrCallExpression(node)
           const MUTATION_REGEX = /(this.)((?!(concat|slice|map|filter)\().)[^\)]*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g
 
           if (MUTATION_REGEX.test(code)) {
diff --git a/lib/rules/no-use-v-if-with-v-for.js b/lib/rules/no-use-v-if-with-v-for.js
new file mode 100644
index 00000000..e76bf824
--- /dev/null
+++ b/lib/rules/no-use-v-if-with-v-for.js
@@ -0,0 +1,99 @@
+/**
+ * @author Yosuke Ota
+ *
+ * issue        https://github.com/vuejs/eslint-plugin-vue/issues/403
+ * Style guide: https://vuejs.org/v2/style-guide/#Avoid-v-if-with-v-for-essential
+ *
+ * I implemented it with reference to `no-confusing-v-for-v-if`
+ */
+'use strict'
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+const utils = require('../utils')
+
+// ------------------------------------------------------------------------------
+// Helpers
+// ------------------------------------------------------------------------------
+
+/**
+ * Check whether the given `v-if` node is using the variable which is defined by the `v-for` directive.
+ * @param {ASTNode} vIf The `v-if` attribute node to check.
+ * @returns {boolean} `true` if the `v-if` is using the variable which is defined by the `v-for` directive.
+ */
+function isUsingIterationVar (vIf) {
+  return !!getVForUsingIterationVar(vIf)
+}
+
+function getVForUsingIterationVar (vIf) {
+  const element = vIf.parent.parent
+  for (var i = 0; i < vIf.value.references.length; i++) {
+    const reference = vIf.value.references[i]
+
+    const targetVFor = element.variables.find(variable =>
+      variable.id.name === reference.id.name &&
+      variable.kind === 'v-for'
+    )
+    if (targetVFor) {
+      return targetVFor.id.parent
+    }
+  }
+  return undefined
+}
+
+// ------------------------------------------------------------------------------
+// Rule Definition
+// ------------------------------------------------------------------------------
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'disallow use v-if on the same element as v-for',
+      category: undefined,
+      url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v4.3.0/docs/rules/no-use-v-if-with-v-for.md'
+    },
+    fixable: null,
+    schema: [{
+      type: 'object',
+      properties: {
+        allowUsingIterationVar: {
+          type: 'boolean'
+        }
+      }
+    }]
+  },
+
+  create (context) {
+    const options = context.options[0] || {}
+    const allowUsingIterationVar = options.allowUsingIterationVar === true // default false
+    return utils.defineTemplateBodyVisitor(context, {
+      "VAttribute[directive=true][key.name='if']" (node) {
+        const element = node.parent.parent
+
+        if (utils.hasDirective(element, 'for')) {
+          if (isUsingIterationVar(node)) {
+            if (!allowUsingIterationVar) {
+              const vFor = getVForUsingIterationVar(node)
+              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'.",
+                data: {
+                  iteratorName: vFor.right.name
+                }
+              })
+            }
+          } else {
+            context.report({
+              node,
+              loc: node.loc,
+              message: "This 'v-if' should be moved to the wrapper element."
+            })
+          }
+        }
+      }
+    })
+  }
+}
diff --git a/lib/rules/require-default-prop.js b/lib/rules/require-default-prop.js
index 7dfc8f20..339ae08c 100644
--- a/lib/rules/require-default-prop.js
+++ b/lib/rules/require-default-prop.js
@@ -50,7 +50,10 @@ module.exports = {
      */
     function propHasDefault (prop) {
       const propDefaultNode = prop.value.properties
-        .find(p => p.key && p.key.name === 'default')
+        .find(p =>
+          p.key &&
+          (p.key.name === 'default' || p.key.value === 'default')
+        )
 
       return Boolean(propDefaultNode)
     }
diff --git a/lib/utils/index.js b/lib/utils/index.js
index 677fff6b..f45950fc 100644
--- a/lib/utils/index.js
+++ b/lib/utils/index.js
@@ -344,6 +344,7 @@ module.exports = {
         } else if (cp.value.type === 'ObjectExpression') {
           value = cp.value.properties
             .filter(p =>
+              p.type === 'Property' &&
               p.key.type === 'Identifier' &&
               p.key.name === 'get' &&
               p.value.type === 'FunctionExpression'
@@ -617,5 +618,42 @@ module.exports = {
       return
     }
     return body.errors.some(error => typeof error.code === 'string' && error.code.startsWith('eof-'))
+  },
+
+  /**
+   * Parse CallExpression or MemberExpression to get simplified version without arguments
+   *
+   * @param  {Object} node The node to parse (MemberExpression | CallExpression)
+   * @return {String} eg. 'this.asd.qwe().map().filter().test.reduce()'
+   */
+  parseMemberOrCallExpression (node) {
+    const parsedCallee = []
+    let n = node
+    let isFunc
+
+    while (n.type === 'MemberExpression' || n.type === 'CallExpression') {
+      if (n.type === 'CallExpression') {
+        n = n.callee
+        isFunc = true
+      } else {
+        if (n.computed) {
+          parsedCallee.push('[]')
+        } else if (n.property.type === 'Identifier') {
+          parsedCallee.push(n.property.name + (isFunc ? '()' : ''))
+        }
+        isFunc = false
+        n = n.object
+      }
+    }
+
+    if (n.type === 'Identifier') {
+      parsedCallee.push(n.name)
+    }
+
+    if (n.type === 'ThisExpression') {
+      parsedCallee.push('this')
+    }
+
+    return parsedCallee.reverse().join('.').replace(/\.\[/g, '[')
   }
 }


Test patch:
diff --git a/tests/lib/rules/no-async-in-computed-properties.js b/tests/lib/rules/no-async-in-computed-properties.js
index 746ba319..2e7c2788 100644
--- a/tests/lib/rules/no-async-in-computed-properties.js
+++ b/tests/lib/rules/no-async-in-computed-properties.js
@@ -51,10 +51,14 @@ ruleTester.run('no-async-in-computed-properties', rule, {
                 return bar
               }
             },
-            foo2: {
+            bar: {
               set () {
                 new Promise((resolve, reject) => {})
               }
+            },
+            baz: {
+              ...mapGetters({ get: 'getBaz' }),
+              ...mapActions({ set: 'setBaz' })
             }
           }
         }
diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js
index cfd71c7d..3eadc45c 100644
--- a/tests/lib/rules/no-side-effects-in-computed-properties.js
+++ b/tests/lib/rules/no-side-effects-in-computed-properties.js
@@ -81,6 +81,25 @@ ruleTester.run('no-side-effects-in-computed-properties', rule, {
             get() {
               return Object.keys(this.a).sort()
             }
+          },
+          test11() {
+            const categories = {}
+
+            this.types.forEach(c => {
+              categories[c.category] = categories[c.category] || []
+              categories[c.category].push(c)
+            })
+
+            return categories
+          },
+          test12() {
+            return this.types.map(t => {
+              // [].push('xxx')
+              return t
+            })
+          },
+          test13 () {
+            this.someArray.forEach(arr => console.log(arr))
           }
         }
       })`,
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
new file mode 100644
index 00000000..4ee3bb8a
--- /dev/null
+++ b/tests/lib/rules/no-use-v-if-with-v-for.js
@@ -0,0 +1,144 @@
+/**
+ * @author Yosuke Ota
+ */
+'use strict'
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+const RuleTester = require('eslint').RuleTester
+const rule = require('../../../lib/rules/no-use-v-if-with-v-for')
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+const tester = new RuleTester({
+  parser: 'vue-eslint-parser',
+  parserOptions: { ecmaVersion: 2015 }
+})
+
+tester.run('no-use-v-if-with-v-for', rule, {
+  valid: [
+    {
+      filename: 'test.vue',
+      code: ''
+    },
+    {
+      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 in list" v-if="x.foo"></div></div></template>',
+      options: [{ allowUsingIterationVar: true }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="(x,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,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: [
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="x in list" v-if="shown"></div></div></template>',
+      errors: [{
+        message: "This 'v-if' should be moved to the wrapper element.",
+        line: 1
+      }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="x in list" v-if="list.length&gt;0"></div></div></template>',
+      errors: [{
+        message: "This 'v-if' should be moved to the wrapper element.",
+        line: 1
+      }]
+    },
+    {
+      filename: 'test.vue',
+      code: '<template><div><div v-for="x in list" v-if="x.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="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="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
+      }]
+    }
+  ]
+})
diff --git a/tests/lib/rules/require-default-prop.js b/tests/lib/rules/require-default-prop.js
index 5aa7d71a..df5fc0af 100644
--- a/tests/lib/rules/require-default-prop.js
+++ b/tests/lib/rules/require-default-prop.js
@@ -39,11 +39,16 @@ ruleTester.run('require-default-prop', rule, {
             },
             c: {
               type: Number,
-              default: 0,
-              required: false
+              required: false,
+              default: 0
+            },
+            d: {
+              type: String,
+              required: false,
+              'default': 'lorem'
             },
             // eslint-disable-next-line require-default-prop
-            d: Number
+            e: Number
           }
         }
       `,
diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js
index 2189f896..09e39ef2 100644
--- a/tests/lib/utils/index.js
+++ b/tests/lib/utils/index.js
@@ -120,6 +120,28 @@ describe('getComputedProperties', () => {
 
     assert.ok(computedProperties[0].value)
   })
+
+  it('should not collide with object spread operator inside CP', () => {
+    node = parse(`const test = {
+      name: 'test',
+      computed: {
+        foo: {
+          ...mapGetters({ get: 'getFoo' }),
+          ...mapActions({ set: 'setFoo' })
+        }
+      }
+    }`)
+
+    const computedProperties = utils.getComputedProperties(node)
+
+    assert.equal(
+      computedProperties.length,
+      1,
+      'it detects all computed properties'
+    )
+
+    assert.notOk(computedProperties[0].value)
+  })
 })
 
 describe('getStaticPropertyName', () => {
@@ -166,3 +188,23 @@ describe('getStaticPropertyName', () => {
     assert.ok(parsed === 'computed')
   })
 })
+
+describe('parseMemberOrCallExpression', () => {
+  let node
+
+  const parse = function (code) {
+    return babelEslint.parse(code).body[0].declarations[0].init
+  }
+
+  it('should parse CallExpression', () => {
+    node = parse(`const test = this.lorem['ipsum'].map(d => d.id).filter((a, b) => a > b).reduce((acc, d) => acc + d, 0)`)
+    const parsed = utils.parseMemberOrCallExpression(node)
+    assert.equal(parsed, 'this.lorem[].map().filter().reduce()')
+  })
+
+  it('should parse MemberExpression', () => {
+    node = parse(`const test = this.lorem['ipsum'][0].map(d => d.id).dolor.reduce((acc, d) => acc + d, 0).sit`)
+    const parsed = utils.parseMemberOrCallExpression(node)
+    assert.equal(parsed, 'this.lorem[][].map().dolor.reduce().sit')
+  })
+})