Add no-side-effects-in-computed-properties rule

docs/rules/no-side-effects-in-computed-properties.md

@@ -0,0 +1,41 @@
+# Do not introduce side effects in computed properties (no-side-effects-in-computed-properties)
+
+It is considered a very bad practice to introduce side effects inside computed properties. It makes the code not predictable and hard to understand.
+
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```js
+
+export default {
+  computed: {
+    fullName() {
+      this.firstName = 'lorem' // <- side effect
+      return `${this.firstName} ${this.lastName}`
+    },
+    somethingReversed() {
+      return this.something.reverse() // <- side effect
+    }
+  }
+}
+
+```
+
+Examples of **correct** code for this rule:
+
+```js
+
+export default {
+  computed: {
+    fullName() {
+      return `${this.firstName} ${this.lastName}`
+    },
+    somethingReversed() {
+      return this.something.slice(0).reverse()
+    }
+  }
+}
+
+```

lib/rules/no-side-effects-in-computed-properties.js

@@ -0,0 +1,127 @@
+/**
+ * @fileoverview Don't introduce side effects in computed properties
+ * @author Michał Sajnóg
+ */
+'use strict'
+
+const utils = require('../utils')
+
+function parseMemberExpression (node) {
+  const members = []
+  let memberExpression
+
+  if (node.type === 'MemberExpression') {
+    memberExpression = node
+
+    while (memberExpression.type === 'MemberExpression') {
+      if (memberExpression.property.type === 'Identifier') {
+        members.push(memberExpression.property.name)
+      }
+      memberExpression = memberExpression.object
+    }
+
+    if (memberExpression.type === 'ThisExpression') {
+      members.push('this')
+    } else if (memberExpression.type === 'Identifier') {
+      members.push(memberExpression.name)
+    }
+  }
+
+  return members.reverse()
+}
+
+function getComputedProperties (componentProperties) {
+  const computedPropertiesNode = componentProperties
+    .filter(p =>
+      p.key.type === 'Identifier' &&
+      p.key.name === 'computed' &&
+      p.value.type === 'ObjectExpression'
+    )[0]
+
+  if (!computedPropertiesNode) { return [] }
+
+  const computedProperties = computedPropertiesNode.value.properties
+
+  return computedProperties.map(cp => {
+    const key = cp.key.name
+    let value
+
+    if (cp.value.type === 'FunctionExpression') {
+      value = cp.value.body
+    } else if (cp.value.type === 'ObjectExpression') {
+      value = cp.value.properties
+        .filter(p =>
+          p.key.type === 'Identifier' &&
+          p.key.name === 'get' &&
+          p.value.type === 'FunctionExpression'
+        )
+        .map(p => p.value.body)[0]
+    }
+
+    return { key, value }
+  })
+}
+
+function create (context) {
+  const forbiddenNodes = []
+
+  return Object.assign({},
+    {
+      // this.xxx <=|+=|-=>
+      'AssignmentExpression > MemberExpression' (node) {
+        if (parseMemberExpression(node)[0] === 'this') {
+          forbiddenNodes.push(node)
+        }
+      },
+      // this.xxx <++|-->
+      'UpdateExpression > MemberExpression' (node) {
+        if (parseMemberExpression(node)[0] === 'this') {
+          forbiddenNodes.push(node)
+        }
+      },
+      // this.xxx.func()
+      'CallExpression' (node) {
+        const code = context.getSourceCode().getText(node)
+        const MUTATION_REGEX = /(this.)((?!(concat|slice|map|filter)\().)*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g
+
+        if (MUTATION_REGEX.test(code)) {
+          forbiddenNodes.push(node)
+        }
+      }
+    },
+    utils.executeOnVueComponent(context, (properties) => {
+      const computedProperties = getComputedProperties(properties)
+
+      computedProperties.forEach(cp => {
+        forbiddenNodes.forEach(node => {
+          if (
+            node.loc.start.line >= cp.value.loc.start.line &&
+            node.loc.end.line <= cp.value.loc.end.line
+          ) {
+            context.report({
+              node: node,
+              message: `Unexpected side effect in "${cp.key}" computed property.`
+            })
+          }
+        })
+      })
+    })
+  )
+}
+
+// ------------------------------------------------------------------------------
+// Rule Definition
+// ------------------------------------------------------------------------------
+
+module.exports = {
+  create,
+  meta: {
+    docs: {
+      description: 'Don\'t introduce side effects in computed properties',
+      category: 'Best Practices',
+      recommended: false
+    },
+    fixable: null,
+    schema: []
+  }
+}

lib/rules/order-in-components.js

@@ -4,6 +4,8 @@
  */
 'use strict'
 
+const utils = require('../utils')
+
 const defaultOrder = [
   ['name', 'delimiters', 'functional', 'model'],
   ['components', 'directives', 'filters'],
@@ -36,38 +38,6 @@ const groups = {
   ]
 }
 
-function isComponentFile (node, path) {
-  const isVueFile = path.endsWith('.vue') || path.endsWith('.jsx')
-  return isVueFile && node.declaration.type === 'ObjectExpression'
-}
-
-function isVueComponent (node) {
-  const callee = node.callee
-
-  const isFullVueComponent = node.type === 'CallExpression' &&
-    callee.type === 'MemberExpression' &&
-    callee.object.type === 'Identifier' &&
-    callee.object.name === 'Vue' &&
-    callee.property.type === 'Identifier' &&
-    callee.property.name === 'component' &&
-    node.arguments.length &&
-    node.arguments.slice(-1)[0].type === 'ObjectExpression'
-
-  const isDestructedVueComponent = callee.type === 'Identifier' &&
-    callee.name === 'component'
-
-  return isFullVueComponent || isDestructedVueComponent
-}
-
-function isVueInstance (node) {
-  const callee = node.callee
-  return node.type === 'NewExpression' &&
-    callee.type === 'Identifier' &&
-    callee.name === 'Vue' &&
-    node.arguments.length &&
-    node.arguments[0].type === 'ObjectExpression'
-}
-
 function getOrderMap (order) {
   const orderMap = new Map()
 
@@ -106,28 +76,13 @@ function checkOrder (propertiesNodes, orderMap, context) {
 function create (context) {
   const options = context.options[0] || {}
   const order = options.order || defaultOrder
-  const filePath = context.getFilename()
 
   const extendedOrder = order.map(property => groups[property] || property)
   const orderMap = getOrderMap(extendedOrder)
 
-  return {
-    ExportDefaultDeclaration (node) {
-      // export default {} in .vue || .jsx
-      if (!isComponentFile(node, filePath)) return
-      checkOrder(node.declaration.properties, orderMap, context)
-    },
-    CallExpression (node) {
-      // Vue.component('xxx', {}) || component('xxx', {})
-      if (!isVueComponent(node)) return
-      checkOrder(node.arguments.slice(-1)[0].properties, orderMap, context)
-    },
-    NewExpression (node) {
-      // new Vue({})
-      if (!isVueInstance(node)) return
-      checkOrder(node.arguments[0].properties, orderMap, context)
-    }
-  }
+  return utils.executeOnVueComponent(context, (properties) => {
+    checkOrder(properties, orderMap, context)
+  })
 }
 
 // ------------------------------------------------------------------------------

lib/utils/index.js

@@ -241,5 +241,82 @@ module.exports = {
     assert(typeof name === 'string')
 
     return VOID_ELEMENT_NAMES.has(name.toLowerCase())
+  },
+
+    /**
+     * Check whether the given node is a Vue component based
+     * on the filename and default export type
+     * export default {} in .vue || .jsx
+     * @param {ASTNode} node Node to check
+     * @param {string} path File name with extension
+     * @returns {boolean}
+     */
+  isVueComponentFile (node, path) {
+    const isVueFile = path.endsWith('.vue') || path.endsWith('.jsx')
+    return isVueFile &&
+      node.type === 'ExportDefaultDeclaration' &&
+      node.declaration.type === 'ObjectExpression'
+  },
+
+    /**
+     * Check whether given node is Vue component
+     * Vue.component('xxx', {}) || component('xxx', {})
+     * @param {ASTNode} node Node to check
+     * @returns {boolean}
+     */
+  isVueComponent (node) {
+    const callee = node.callee
+
+    const isFullVueComponent = node.type === 'CallExpression' &&
+      callee.type === 'MemberExpression' &&
+      callee.object.type === 'Identifier' &&
+      callee.object.name === 'Vue' &&
+      callee.property.type === 'Identifier' &&
+      callee.property.name === 'component' &&
+      node.arguments.length &&
+      node.arguments.slice(-1)[0].type === 'ObjectExpression'
+
+    const isDestructedVueComponent = callee.type === 'Identifier' &&
+      callee.name === 'component'
+
+    return isFullVueComponent || isDestructedVueComponent
+  },
+
+    /**
+     * Check whether given node is new Vue instance
+     * new Vue({})
+     * @param {ASTNode} node Node to check
+     * @returns {boolean}
+     */
+  isVueInstance (node) {
+    const callee = node.callee
+    return node.type === 'NewExpression' &&
+      callee.type === 'Identifier' &&
+      callee.name === 'Vue' &&
+      node.arguments.length &&
+      node.arguments[0].type === 'ObjectExpression'
+  },
+
+  executeOnVueComponent (context, cb) {
+    const filePath = context.getFilename()
+    const _this = this
+
+    return {
+      'ExportDefaultDeclaration:exit' (node) {
+        // export default {} in .vue || .jsx
+        if (!_this.isVueComponentFile(node, filePath)) return
+        cb(node.declaration.properties)
+      },
+      'CallExpression:exit' (node) {
+        // Vue.component('xxx', {}) || component('xxx', {})
+        if (!_this.isVueComponent(node)) return
+        cb(node.arguments.slice(-1)[0].properties)
+      },
+      'NewExpression:exit' (node) {
+        // new Vue({})
+        if (!_this.isVueInstance(node)) return
+        cb(node.arguments[0].properties)
+      }
+    }
   }
 }