Skip to content

Commit

Permalink
Fixed an error when using spread elements in `vue/require-default-pro…
Browse files Browse the repository at this point in the history
…p`. (#1046)

* Fixed an error when using spread elements in `vue/require-default-prop`.

* Revert vscode/settings
  • Loading branch information
ota-meshi authored Feb 16, 2020
1 parent a4e3f0f commit 5980cdc
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 20 deletions.
43 changes: 27 additions & 16 deletions lib/rules/require-default-prop.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@
*/
'use strict'

/**
* @typedef {import('vue-eslint-parser').AST.ESLintExpression} Expression
* @typedef {import('vue-eslint-parser').AST.ESLintObjectExpression} ObjectExpression
* @typedef {import('vue-eslint-parser').AST.ESLintPattern} Pattern
*/
/**
* @typedef {import('../utils').ComponentObjectProp} ComponentObjectProp
* @typedef {ComponentObjectProp & { value: ObjectExpression} } ComponentObjectPropObject
*/

const utils = require('../utils')

const NATIVE_TYPES = new Set([
Expand Down Expand Up @@ -39,14 +49,14 @@ module.exports = {

/**
* Checks if the passed prop is required
* @param {Property} prop - Property AST node for a single prop
* @param {ComponentObjectPropObject} prop - Property AST node for a single prop
* @return {boolean}
*/
function propIsRequired (prop) {
const propRequiredNode = prop.value.properties
.find(p =>
p.type === 'Property' &&
p.key.name === 'required' &&
utils.getStaticPropertyName(p) === 'required' &&
p.value.type === 'Literal' &&
p.value.value === true
)
Expand All @@ -56,38 +66,38 @@ module.exports = {

/**
* Checks if the passed prop has a default value
* @param {Property} prop - Property AST node for a single prop
* @param {ComponentObjectPropObject} prop - Property AST node for a single prop
* @return {boolean}
*/
function propHasDefault (prop) {
const propDefaultNode = prop.value.properties
.find(p =>
p.key &&
(p.key.name === 'default' || p.key.value === 'default')
p.type === 'Property' && utils.getStaticPropertyName(p) === 'default'
)

return Boolean(propDefaultNode)
}

/**
* Finds all props that don't have a default value set
* @param {Array} props - Vue component's "props" node
* @return {Array} Array of props without "default" value
* @param {ComponentObjectProp[]} props - Vue component's "props" node
* @return {ComponentObjectProp[]} Array of props without "default" value
*/
function findPropsWithoutDefaultValue (props) {
return props
.filter(prop => {
if (prop.value.type !== 'ObjectExpression') {
return (prop.value.type !== 'CallExpression' && prop.value.type !== 'Identifier') || NATIVE_TYPES.has(prop.value.name)
return (prop.value.type !== 'CallExpression' && prop.value.type !== 'Identifier') ||
(prop.value.type === 'Identifier' && NATIVE_TYPES.has(prop.value.name))
}

return !propIsRequired(prop) && !propHasDefault(prop)
return !propIsRequired(/** @type {ComponentObjectPropObject} */(prop)) && !propHasDefault(/** @type {ComponentObjectPropObject} */(prop))
})
}

/**
* Detects whether given value node is a Boolean type
* @param {Node} value
* @param {Expression | Pattern} value
* @return {Boolean}
*/
function isValueNodeOfBooleanType (value) {
Expand All @@ -104,15 +114,16 @@ module.exports = {

/**
* Detects whether given prop node is a Boolean
* @param {Node} prop
* @param {ComponentObjectProp} prop
* @return {Boolean}
*/
function isBooleanProp (prop) {
const value = utils.unwrapTypes(prop.value)

return isValueNodeOfBooleanType(value) || (
value.type === 'ObjectExpression' &&
value.properties.find(p =>
value.properties.some(p =>
p.type === 'Property' &&
p.key.type === 'Identifier' &&
p.key.name === 'type' &&
isValueNodeOfBooleanType(p.value)
Expand All @@ -122,8 +133,8 @@ module.exports = {

/**
* Excludes purely Boolean props from the Array
* @param {Array} props - Array with props
* @return {Array}
* @param {ComponentObjectProp[]} props - Array with props
* @return {ComponentObjectProp[]}
*/
function excludeBooleanProps (props) {
return props.filter(prop => !isBooleanProp(prop))
Expand All @@ -135,9 +146,9 @@ module.exports = {

return utils.executeOnVue(context, (obj) => {
const props = utils.getComponentProps(obj)
.filter(prop => prop.key && prop.value && !prop.node.shorthand)
.filter(prop => prop.key && prop.value && !(prop.node.type === 'Property' && prop.node.shorthand))

const propsWithoutDefault = findPropsWithoutDefaultValue(props)
const propsWithoutDefault = findPropsWithoutDefaultValue(/** @type {ComponentObjectProp[]} */(props))
const propsToReport = excludeBooleanProps(propsWithoutDefault)

for (const prop of propsToReport) {
Expand Down
26 changes: 22 additions & 4 deletions lib/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,23 @@
*/
'use strict'

/**
* @typedef {import('vue-eslint-parser').AST.ESLintArrayExpression} ArrayExpression
* @typedef {import('vue-eslint-parser').AST.ESLintExpression} Expression
* @typedef {import('vue-eslint-parser').AST.ESLintIdentifier} Identifier
* @typedef {import('vue-eslint-parser').AST.ESLintLiteral} Literal
* @typedef {import('vue-eslint-parser').AST.ESLintMemberExpression} MemberExpression
* @typedef {import('vue-eslint-parser').AST.ESLintMethodDefinition} MethodDefinition
* @typedef {import('vue-eslint-parser').AST.ESLintObjectExpression} ObjectExpression
* @typedef {import('vue-eslint-parser').AST.ESLintProperty} Property
* @typedef {import('vue-eslint-parser').AST.ESLintTemplateLiteral} TemplateLiteral
*/

/**
* @typedef { {key: Literal | null, value: null, node: ArrayExpression['elements'][0], propName: string} } ComponentArrayProp
* @typedef { {key: Property['key'], value: Property['value'], node: Property, propName: string} } ComponentObjectProp
*/

// ------------------------------------------------------------------------------
// Helpers
// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -382,7 +399,7 @@ module.exports = {

/**
* Gets the property name of a given node.
* @param {ASTNode} node - The node to get.
* @param {Property|MethodDefinition|MemberExpression|Literal|TemplateLiteral|Identifier} node - The node to get.
* @return {string|null} The property name if static. Otherwise, null.
*/
getStaticPropertyName (node) {
Expand Down Expand Up @@ -425,7 +442,7 @@ module.exports = {
/**
* 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}]
* @return {(ComponentArrayProp | ComponentObjectProp)[]} Array of component props in format: [{key?: String, value?: ASTNode, node: ASTNod}]
*/
getComponentProps (componentObject) {
const propsNode = componentObject.properties
Expand Down Expand Up @@ -844,8 +861,9 @@ module.exports = {

/**
* Unwrap typescript types like "X as F"
* @param {ASTNode} node
* @return {ASTNode}
* @template T
* @param {T} node
* @return {T}
*/
unwrapTypes (node) {
return node.type === 'TSAsExpression' ? node.expression : node
Expand Down
54 changes: 54 additions & 0 deletions tests/lib/rules/require-default-prop.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,33 @@ ruleTester.run('require-default-prop', rule, {
}
}
`
},
{
// https://github.com/vuejs/eslint-plugin-vue/issues/1040
filename: 'destructuring-test.vue',
code: `
export default {
props: {
foo: {
...foo,
default: 0
},
}
}
`
},
{
filename: 'unknown-prop-details-test.vue',
code: `
export default {
props: {
foo: {
[bar]: true,
default: 0
},
}
}
`
}
],

Expand Down Expand Up @@ -282,6 +309,33 @@ ruleTester.run('require-default-prop', rule, {
message: `Prop '[baz.baz]' requires default value to be set.`,
line: 6
}]
},
{
// https://github.com/vuejs/eslint-plugin-vue/issues/1040
filename: 'destructuring-test.vue',
code: `
export default {
props: {
foo: {
...foo
},
}
}
`,
errors: ['Prop \'foo\' requires default value to be set.']
},
{
filename: 'unknown-prop-details-test.vue',
code: `
export default {
props: {
foo: {
[bar]: true
},
}
}
`,
errors: ['Prop \'foo\' requires default value to be set.']
}
]
})

0 comments on commit 5980cdc

Please sign in to comment.