Repository: vuejs/eslint-plugin-vue
Pull request #632 (2018-11-06 21:30:51)

Fix issues with props
With those changes, its going to be easier to add support for classes in feature 

* `vue/prop-name-casing`: is working now with array props [literals]
* `vue/prop-name-casing`: reports all errors if there are non Literal keys in it
* `vue/require-default-prop`: allow to use shorthand, properties and function calls
* `vue/require-prop-types`: reports names for types different than literals
* `vue/require-prop-types`: fix false positive error in when is set to empty array
* `vue/require-prop-types`: will return now errors about each prop from ArrayExpression
* add new `getComponentProps` helper to easily deal with props 
* fix spread operator issues across all `prop` related rules

fix issue: #596 
fix issue #595

armano2: @michalsnik all requested changes are applied, 
additionally i made interface of `getComponentProps` more like `getComputedProperties`
armano2: @michalsnik requested changes applied

Patch:
diff --git a/.gitignore b/.gitignore
index 1828a214..c07de3ba 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,5 @@
 .idea
+*.iml
 /.nyc_output
 /coverage
 /tests/integrations/*/node_modules
diff --git a/lib/rules/prop-name-casing.js b/lib/rules/prop-name-casing.js
index cf2fa3d6..fffbdeff 100644
--- a/lib/rules/prop-name-casing.js
+++ b/lib/rules/prop-name-casing.js
@@ -8,12 +8,12 @@ const utils = require('../utils')
 const casing = require('../utils/casing')
 const allowedCaseOptions = ['camelCase', 'snake_case']
 
-function canFixPropertyName (node, originalName) {
+function canFixPropertyName (node, key, originalName) {
   // Can not fix of computed property names & shorthand
   if (node.computed || node.shorthand) {
     return false
   }
-  const key = node.key
+
   // Can not fix of unknown types
   if (key.type !== 'Literal' && key.type !== 'Identifier') {
     return false
@@ -36,42 +36,25 @@ function create (context) {
   // ----------------------------------------------------------------------
 
   return utils.executeOnVue(context, (obj) => {
-    const node = obj.properties.find(p =>
-      p.type === 'Property' &&
-      p.key.type === 'Identifier' &&
-      p.key.name === 'props' &&
-      (p.value.type === 'ObjectExpression' || p.value.type === 'ArrayExpression')
-    )
-
-    if (!node) return
-
-    const items = node.value.type === 'ObjectExpression' ? node.value.properties : node.value.elements
-    for (const item of items) {
-      if (item.type !== 'Property') {
-        return
-      }
-      if (item.computed) {
-        if (item.key.type !== 'Literal') {
-          // TemplateLiteral | Identifier(variable) | Expression(s)
-          return
-        }
-        if (typeof item.key.value !== 'string') {
-          // (boolean | null | number | RegExp) Literal
-          return
-        }
-      }
+    const props = utils.getComponentProps(obj)
+      .filter(prop => prop.key && prop.key.type === 'Literal' || (prop.key.type === 'Identifier' && !prop.node.computed))
 
+    for (const item of props) {
       const propName = item.key.type === 'Literal' ? item.key.value : item.key.name
+      if (typeof propName !== 'string') {
+        // (boolean | null | number | RegExp) Literal
+        continue
+      }
       const convertedName = converter(propName)
       if (convertedName !== propName) {
         context.report({
-          node: item,
+          node: item.node,
           message: 'Prop "{{name}}" is not in {{caseType}}.',
           data: {
             name: propName,
             caseType: caseType
           },
-          fix: canFixPropertyName(item, propName) ? fixer => {
+          fix: canFixPropertyName(item.node, item.key, propName) ? fixer => {
             return item.key.type === 'Literal'
               ? fixer.replaceText(item.key, item.key.raw.replace(item.key.value, convertedName))
               : fixer.replaceText(item.key, convertedName)
diff --git a/lib/rules/require-default-prop.js b/lib/rules/require-default-prop.js
index 3d75387b..1e1995de 100644
--- a/lib/rules/require-default-prop.js
+++ b/lib/rules/require-default-prop.js
@@ -6,6 +6,16 @@
 
 const utils = require('../utils')
 
+const NATIVE_TYPES = new Set([
+  'String',
+  'Number',
+  'Boolean',
+  'Function',
+  'Object',
+  'Array',
+  'Symbol'
+])
+
 // ------------------------------------------------------------------------------
 // Rule Definition
 // ------------------------------------------------------------------------------
@@ -21,7 +31,7 @@ module.exports = {
     schema: []
   },
 
-  create: function (context) {
+  create (context) {
     // ----------------------------------------------------------------------
     // Helpers
     // ----------------------------------------------------------------------
@@ -32,7 +42,7 @@ module.exports = {
      * @return {boolean}
      */
     function propIsRequired (prop) {
-      const propRequiredNode = utils.unwrapTypes(prop.value).properties
+      const propRequiredNode = prop.value.properties
         .find(p =>
           p.type === 'Property' &&
           p.key.name === 'required' &&
@@ -49,7 +59,7 @@ module.exports = {
      * @return {boolean}
      */
     function propHasDefault (prop) {
-      const propDefaultNode = utils.unwrapTypes(prop.value).properties
+      const propDefaultNode = prop.value.properties
         .find(p =>
           p.key &&
           (p.key.name === 'default' || p.key.value === 'default')
@@ -60,15 +70,14 @@ module.exports = {
 
     /**
      * Finds all props that don't have a default value set
-     * @param {Property} propsNode - Vue component's "props" node
+     * @param {Array} props - Vue component's "props" node
      * @return {Array} Array of props without "default" value
      */
-    function findPropsWithoutDefaultValue (propsNode) {
-      return propsNode.value.properties
-        .filter(prop => prop.type === 'Property')
+    function findPropsWithoutDefaultValue (props) {
+      return props
         .filter(prop => {
-          if (utils.unwrapTypes(prop.value).type !== 'ObjectExpression') {
-            return true
+          if (prop.value.type !== 'ObjectExpression') {
+            return (prop.value.type !== 'CallExpression' && prop.value.type !== 'Identifier') || NATIVE_TYPES.has(prop.value.name)
           }
 
           return !propIsRequired(prop) && !propHasDefault(prop)
@@ -124,28 +133,21 @@ module.exports = {
     // ----------------------------------------------------------------------
 
     return utils.executeOnVue(context, (obj) => {
-      const propsNode = obj.properties
-        .find(p =>
-          p.type === 'Property' &&
-          p.key.type === 'Identifier' &&
-          p.key.name === 'props' &&
-          p.value.type === 'ObjectExpression'
-        )
-
-      if (!propsNode) return
+      const props = utils.getComponentProps(obj)
+        .filter(prop => prop.key && prop.value && !prop.node.shorthand)
 
-      const propsWithoutDefault = findPropsWithoutDefaultValue(propsNode)
+      const propsWithoutDefault = findPropsWithoutDefaultValue(props)
       const propsToReport = excludeBooleanProps(propsWithoutDefault)
 
-      propsToReport.forEach(prop => {
+      for (const prop of propsToReport) {
         context.report({
-          node: prop,
+          node: prop.node,
           message: `Prop '{{propName}}' requires default value to be set.`,
           data: {
             propName: prop.key.name
           }
         })
-      })
+      }
     })
   }
 }
diff --git a/lib/rules/require-prop-type-constructor.js b/lib/rules/require-prop-type-constructor.js
index abc76848..b79ecf29 100644
--- a/lib/rules/require-prop-type-constructor.js
+++ b/lib/rules/require-prop-type-constructor.js
@@ -70,31 +70,23 @@ module.exports = {
     }
 
     return utils.executeOnVueComponent(context, (obj) => {
-      const node = obj.properties.find(p =>
-        p.type === 'Property' &&
-        p.key.type === 'Identifier' &&
-        p.key.name === 'props' &&
-        p.value.type === 'ObjectExpression'
-      )
+      const props = utils.getComponentProps(obj)
+        .filter(prop => prop.key && prop.value)
 
-      if (!node) return
+      for (const prop of props) {
+        if (isForbiddenType(prop.value) || prop.value.type === 'ArrayExpression') {
+          checkPropertyNode(prop.key, prop.value)
+        } else if (prop.value.type === 'ObjectExpression') {
+          const typeProperty = prop.value.properties.find(property =>
+            property.type === 'Property' &&
+            property.key.name === 'type'
+          )
 
-      node.value.properties
-        .forEach(p => {
-          const pValue = utils.unwrapTypes(p.value)
-          if (isForbiddenType(pValue) || pValue.type === 'ArrayExpression') {
-            checkPropertyNode(p.key, pValue)
-          } else if (pValue.type === 'ObjectExpression') {
-            const typeProperty = pValue.properties.find(prop =>
-              prop.type === 'Property' &&
-              prop.key.name === 'type'
-            )
+          if (!typeProperty) continue
 
-            if (!typeProperty) return
-
-            checkPropertyNode(p.key, utils.unwrapTypes(typeProperty.value))
-          }
-        })
+          checkPropertyNode(prop.key, typeProperty.value)
+        }
+      }
     })
   }
 }
diff --git a/lib/rules/require-prop-types.js b/lib/rules/require-prop-types.js
index 0899426b..b3b84035 100644
--- a/lib/rules/require-prop-types.js
+++ b/lib/rules/require-prop-types.js
@@ -42,30 +42,26 @@ module.exports = {
       return Boolean(typeProperty || validatorProperty)
     }
 
-    function checkProperties (items) {
-      for (const cp of items) {
-        if (cp.type !== 'Property') {
-          return
-        }
-        let hasType = true
-        const cpValue = utils.unwrapTypes(cp.value)
+    function checkProperty (key, value, node) {
+      let hasType = true
 
-        if (cpValue.type === 'ObjectExpression') { // foo: {
-          hasType = objectHasType(cpValue)
-        } else if (cpValue.type === 'ArrayExpression') { // foo: [
-          hasType = cpValue.elements.length > 0
-        } else if (cpValue.type === 'FunctionExpression' || cpValue.type === 'ArrowFunctionExpression') {
-          hasType = false
-        }
-        if (!hasType) {
-          context.report({
-            node: cp,
-            message: 'Prop "{{name}}" should define at least its type.',
-            data: {
-              name: cp.key.name
-            }
-          })
-        }
+      if (!value) {
+        hasType = false
+      } else if (value.type === 'ObjectExpression') { // foo: {
+        hasType = objectHasType(value)
+      } else if (value.type === 'ArrayExpression') { // foo: [
+        hasType = value.elements.length > 0
+      } else if (value.type === 'FunctionExpression' || value.type === 'ArrowFunctionExpression') {
+        hasType = false
+      }
+      if (!hasType) {
+        context.report({
+          node,
+          message: 'Prop "{{name}}" should define at least its type.',
+          data: {
+            name: utils.getStaticPropertyName(key || node) || 'Unknown prop'
+          }
+        })
       }
     }
 
@@ -74,24 +70,10 @@ module.exports = {
     // ----------------------------------------------------------------------
 
     return utils.executeOnVue(context, (obj) => {
-      const node = obj.properties
-        .find(p =>
-          p.type === 'Property' &&
-          p.key.type === 'Identifier' &&
-          p.key.name === 'props'
-        )
+      const props = utils.getComponentProps(obj)
 
-      if (!node) return
-
-      if (node.value.type === 'ObjectExpression') {
-        checkProperties(node.value.properties)
-      }
-
-      if (node.value.type === 'ArrayExpression') {
-        context.report({
-          node,
-          message: 'Props should at least define their types.'
-        })
+      for (const prop of props) {
+        checkProperty(prop.key, prop.value, prop.node)
       }
     })
   }
diff --git a/lib/rules/require-valid-default-prop.js b/lib/rules/require-valid-default-prop.js
index 21e1f253..04db9393 100644
--- a/lib/rules/require-valid-default-prop.js
+++ b/lib/rules/require-valid-default-prop.js
@@ -88,20 +88,11 @@ module.exports = {
     // ----------------------------------------------------------------------
 
     return utils.executeOnVue(context, obj => {
-      const props = obj.properties.find(p =>
-        isPropertyIdentifier(p) &&
-        p.key.name === 'props' &&
-        p.value.type === 'ObjectExpression'
-      )
-      if (!props) return
-
-      const properties = props.value.properties.filter(p =>
-        isPropertyIdentifier(p) &&
-        utils.unwrapTypes(p.value).type === 'ObjectExpression'
-      )
+      const props = utils.getComponentProps(obj)
+        .filter(prop => prop.key && prop.value && prop.value.type === 'ObjectExpression')
 
-      for (const prop of properties) {
-        const type = getPropertyNode(utils.unwrapTypes(prop.value), 'type')
+      for (const prop of props) {
+        const type = getPropertyNode(prop.value, 'type')
         if (!type) continue
 
         const typeNames = new Set(getTypes(type.value)
@@ -111,7 +102,7 @@ module.exports = {
         // There is no native types detected
         if (typeNames.size === 0) continue
 
-        const def = getPropertyNode(utils.unwrapTypes(prop.value), 'default')
+        const def = getPropertyNode(prop.value, 'default')
         if (!def) continue
 
         const defType = getValueType(def.value)
diff --git a/lib/utils/index.js b/lib/utils/index.js
index 4a8a8cb1..28caa444 100644
--- a/lib/utils/index.js
+++ b/lib/utils/index.js
@@ -365,9 +365,46 @@ module.exports = {
     return null
   },
 
+  /**
+   * Get all props by looking at all component's properties
+   * @param {ObjectExpression} componentObject Object with component definition
+   * @return {Array} Array of component props in format: [{key?: String, value?: ASTNode, node: ASTNod}]
+   */
+  getComponentProps (componentObject) {
+    const propsNode = componentObject.properties
+      .find(p =>
+        p.type === 'Property' &&
+        p.key.type === 'Identifier' &&
+        p.key.name === 'props' &&
+        (p.value.type === 'ObjectExpression' || p.value.type === 'ArrayExpression')
+      )
+
+    if (!propsNode) {
+      return []
+    }
+
+    let props
+
+    if (propsNode.value.type === 'ObjectExpression') {
+      props = propsNode.value.properties
+        .filter(prop => prop.type === 'Property')
+        .map(prop => {
+          return { key: prop.key, value: this.unwrapTypes(prop.value), node: prop }
+        })
+    } else {
+      props = propsNode.value.elements
+        .map(prop => {
+          const key = prop.type === 'Literal' && typeof prop.value === 'string' ? prop : null
+          return { key, value: null, node: prop }
+        })
+    }
+
+    return props
+  },
+
   /**
    * Get all computed properties by looking at all component's properties
-   * @param {ObjectExpression} Object with component definition
+   * @param {ObjectExpression} componentObject Object with component definition
    * @return {Array} Array of computed properties in format: [{key: String, value: ASTNode}]
    */
   getComputedProperties (componentObject) {


Test patch:
diff --git a/tests/lib/rules/prop-name-casing.js b/tests/lib/rules/prop-name-casing.js
index d9a23a46..842e07ab 100644
--- a/tests/lib/rules/prop-name-casing.js
+++ b/tests/lib/rules/prop-name-casing.js
@@ -67,7 +67,7 @@ ruleTester.run('prop-name-casing', rule, {
       filename: 'test.vue',
       code: `
         export default {
-          props: ['greetingText']
+          props: ['greeting_text']
         }
       `,
       options: ['snake_case'],
@@ -338,6 +338,26 @@ ruleTester.run('prop-name-casing', rule, {
         line: 4
       }]
     },
+    {
+      filename: 'test.vue',
+      code: `
+        export default {
+          props: ['greeting_text']
+        }
+      `,
+      options: ['camelCase'],
+      output: `
+        export default {
+          props: ['greetingText']
+        }
+      `,
+      parserOptions,
+      errors: [{
+        message: 'Prop "greeting_text" is not in camelCase.',
+        type: 'Literal',
+        line: 3
+      }]
+    },
     {
       filename: 'test.vue',
       code: `
diff --git a/tests/lib/rules/require-default-prop.js b/tests/lib/rules/require-default-prop.js
index cdbecaca..7eb689a3 100644
--- a/tests/lib/rules/require-default-prop.js
+++ b/tests/lib/rules/require-default-prop.js
@@ -141,6 +141,18 @@ ruleTester.run('require-default-prop', rule, {
         });
       `,
       parser: 'typescript-eslint-parser'
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        export default {
+          props: {
+            bar,
+            baz: prop,
+            bar1: foo()
+          }
+        }
+      `
     }
   ],
 
diff --git a/tests/lib/rules/require-prop-type-constructor.js b/tests/lib/rules/require-prop-type-constructor.js
index 81a189ef..fd86930d 100644
--- a/tests/lib/rules/require-prop-type-constructor.js
+++ b/tests/lib/rules/require-prop-type-constructor.js
@@ -15,9 +15,9 @@ const RuleTester = require('eslint').RuleTester
 // Tests
 // ------------------------------------------------------------------------------
 
-var ruleTester = new RuleTester({
+const ruleTester = new RuleTester({
   parserOptions: {
-    ecmaVersion: 7,
+    ecmaVersion: 2018,
     sourceType: 'module'
   }
 })
@@ -29,6 +29,7 @@ ruleTester.run('require-prop-type-constructor', rule, {
       code: `
         export default {
           props: {
+            ...props,
             myProp: Number,
             anotherType: [Number, String],
             extraProp: {
diff --git a/tests/lib/rules/require-prop-types.js b/tests/lib/rules/require-prop-types.js
index 3cd2adcd..aac85634 100644
--- a/tests/lib/rules/require-prop-types.js
+++ b/tests/lib/rules/require-prop-types.js
@@ -114,6 +114,24 @@ ruleTester.run('require-prop-types', rule, {
       `,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' }
     },
+    {
+      filename: 'test.vue',
+      code: `
+        export default {
+          props: []
+        }
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        export default {
+          props: {}
+        }
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
+    },
     {
       filename: 'test.vue',
       code: `
@@ -149,12 +167,21 @@ ruleTester.run('require-prop-types', rule, {
       filename: 'test.vue',
       code: `
         export default {
-          props: ['foo']
+          props: ['foo', bar, \`baz\`, foo()]
         }
       `,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
-        message: 'Props should at least define their types.',
+        message: 'Prop "foo" should define at least its type.',
+        line: 3
+      }, {
+        message: 'Prop "bar" should define at least its type.',
+        line: 3
+      }, {
+        message: 'Prop "baz" should define at least its type.',
+        line: 3
+      }, {
+        message: 'Prop "Unknown prop" should define at least its type.',
         line: 3
       }]
     },
@@ -162,12 +189,21 @@ ruleTester.run('require-prop-types', rule, {
       filename: 'test.js',
       code: `
         new Vue({
-          props: ['foo']
+          props: ['foo', bar, \`baz\`, foo()]
         })
       `,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
-        message: 'Props should at least define their types.',
+        message: 'Prop "foo" should define at least its type.',
+        line: 3
+      }, {
+        message: 'Prop "bar" should define at least its type.',
+        line: 3
+      }, {
+        message: 'Prop "baz" should define at least its type.',
+        line: 3
+      }, {
+        message: 'Prop "Unknown prop" should define at least its type.',
         line: 3
       }]
     },
diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js
index 067ddc59..0b8c07e2 100644
--- a/tests/lib/utils/index.js
+++ b/tests/lib/utils/index.js
@@ -247,3 +247,104 @@ describe('getRegisteredComponents', () => {
     )
   })
 })
+
+describe('getComponentProps', () => {
+  let props
+
+  const parse = function (code) {
+    const data = babelEslint.parse(code).body[0].declarations[0].init
+    return utils.getComponentProps(data)
+  }
+
+  it('should return empty array when there is no component props', () => {
+    props = parse(`const test = {
+      name: 'test',
+      data() {
+        return {}
+      }
+    }`)
+
+    assert.equal(props.length, 0)
+  })
+
+  it('should return empty array when component props is empty array', () => {
+    props = parse(`const test = {
+      name: 'test',
+      props: []
+    }`)
+
+    assert.equal(props.length, 0)
+  })
+
+  it('should return empty array when component props is empty object', () => {
+    props = parse(`const test = {
+      name: 'test',
+      props: {}
+    }`)
+
+    assert.equal(props.length, 0)
+  })
+
+  it('should return computed props', () => {
+    props = parse(`const test = {
+      name: 'test',
+      ...test,
+      data() {
+        return {}
+      },
+      props: {
+        ...foo,
+        a: String,
+        b: {},
+        c: [String],
+        d
+      }
+    }`)
+
+    assert.equal(props.length, 4, 'it detects all props')
+
+    assert.ok(props[0].key.type === 'Identifier')
+    assert.ok(props[0].node.type === 'Property')
+    assert.ok(props[0].value.type === 'Identifier')
+
+    assert.ok(props[1].key.type === 'Identifier')
+    assert.ok(props[1].node.type === 'Property')
+    assert.ok(props[1].value.type === 'ObjectExpression')
+
+    assert.ok(props[2].key.type === 'Identifier')
+    assert.ok(props[2].node.type === 'Property')
+    assert.ok(props[2].value.type === 'ArrayExpression')
+
+    assert.deepEqual(props[3].key, props[3].value)
+    assert.ok(props[3].node.type === 'Property')
+    assert.ok(props[3].value.type === 'Identifier')
+  })
+
+  it('should return computed from array props', () => {
+    props = parse(`const test = {
+      name: 'test',
+      data() {
+        return {}
+      },
+      props: ['a', b, \`c\`, null]
+    }`)
+
+    assert.equal(props.length, 4, 'it detects all props')
+
+    assert.ok(props[0].node.type === 'Literal')
+    assert.deepEqual(props[0].key, props[0].node)
+    assert.notOk(props[0].value)
+
+    assert.ok(props[1].node.type === 'Identifier')
+    assert.notOk(props[1].key)
+    assert.notOk(props[1].value)
+
+    assert.ok(props[2].node.type === 'TemplateLiteral')
+    assert.notOk(props[2].key)
+    assert.notOk(props[2].value)
+
+    assert.ok(props[3].node.type === 'Literal')
+    assert.notOk(props[3].key)
+    assert.notOk(props[3].value)
+  })
+})