Skip to content

Commit 9a56de8

Browse files
authored
Fix false negatives and false positives in vue/require-valid-default-prop rule (#2586)
1 parent 86300c4 commit 9a56de8

File tree

2 files changed

+168
-27
lines changed

2 files changed

+168
-27
lines changed

lib/rules/require-valid-default-prop.js

+66-27
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ module.exports = {
230230
}
231231

232232
/**
233-
* @param {*} node
233+
* @param {Expression} node
234234
* @param {ComponentObjectProp | ComponentTypeProp | ComponentInferTypeProp} prop
235235
* @param {Iterable<string>} expectedTypeNames
236236
*/
@@ -249,17 +249,22 @@ module.exports = {
249249
})
250250
}
251251

252+
/**
253+
* @typedef {object} DefaultDefine
254+
* @property {Expression} expression
255+
* @property {'assignment'|'withDefaults'|'defaultProperty'} src
256+
*/
252257
/**
253258
* @param {(ComponentObjectProp | ComponentTypeProp | ComponentInferTypeProp)[]} props
254-
* @param {(propName: string) => Expression[]} otherDefaultProvider
259+
* @param {(propName: string) => Iterable<DefaultDefine>} otherDefaultProvider
255260
*/
256261
function processPropDefs(props, otherDefaultProvider) {
257262
/** @type {PropDefaultFunctionContext[]} */
258263
const propContexts = []
259264
for (const prop of props) {
260265
let typeList
261-
/** @type {Expression[]} */
262-
const defExprList = []
266+
/** @type {DefaultDefine[]} */
267+
const defaultList = []
263268
if (prop.type === 'object') {
264269
if (prop.value.type === 'ObjectExpression') {
265270
const type = getPropertyNode(prop.value, 'type')
@@ -268,36 +273,44 @@ module.exports = {
268273
typeList = getTypes(type.value)
269274

270275
const def = getPropertyNode(prop.value, 'default')
271-
if (!def) continue
272-
273-
defExprList.push(def.value)
276+
if (def) {
277+
defaultList.push({
278+
src: 'defaultProperty',
279+
expression: def.value
280+
})
281+
}
274282
} else {
275283
typeList = getTypes(prop.value)
276284
}
277285
} else {
278286
typeList = prop.types
279287
}
280288
if (prop.propName != null) {
281-
defExprList.push(...otherDefaultProvider(prop.propName))
289+
defaultList.push(...otherDefaultProvider(prop.propName))
282290
}
283291

284-
if (defExprList.length === 0) continue
292+
if (defaultList.length === 0) continue
285293

286294
const typeNames = new Set(
287295
typeList.filter((item) => NATIVE_TYPES.has(item))
288296
)
289297
// There is no native types detected
290298
if (typeNames.size === 0) continue
291299

292-
for (const defExpr of defExprList) {
293-
const defType = getValueType(defExpr)
300+
for (const defaultDef of defaultList) {
301+
const defType = getValueType(defaultDef.expression)
294302

295303
if (!defType) continue
296304

297305
if (defType.function) {
298306
if (typeNames.has('Function')) {
299307
continue
300308
}
309+
if (defaultDef.src === 'assignment') {
310+
// Factory functions cannot be used in default definitions with initial value assignments.
311+
report(defaultDef.expression, prop, typeNames)
312+
continue
313+
}
301314
if (defType.expression) {
302315
if (!defType.returnType || typeNames.has(defType.returnType)) {
303316
continue
@@ -311,18 +324,23 @@ module.exports = {
311324
})
312325
}
313326
} else {
314-
if (
315-
typeNames.has(defType.type) &&
316-
!FUNCTION_VALUE_TYPES.has(defType.type)
317-
) {
318-
continue
327+
if (typeNames.has(defType.type)) {
328+
if (defaultDef.src === 'assignment') {
329+
continue
330+
}
331+
if (!FUNCTION_VALUE_TYPES.has(defType.type)) {
332+
// For Array and Object, defaults must be defined in the factory function.
333+
continue
334+
}
319335
}
320336
report(
321-
defExpr,
337+
defaultDef.expression,
322338
prop,
323-
[...typeNames].map((type) =>
324-
FUNCTION_VALUE_TYPES.has(type) ? 'Function' : type
325-
)
339+
defaultDef.src === 'assignment'
340+
? typeNames
341+
: [...typeNames].map((type) =>
342+
FUNCTION_VALUE_TYPES.has(type) ? 'Function' : type
343+
)
326344
)
327345
}
328346
}
@@ -425,12 +443,19 @@ module.exports = {
425443
utils.getWithDefaultsPropExpressions(node)
426444
const defaultsByAssignmentPatterns =
427445
utils.getDefaultPropExpressionsForPropsDestructure(node)
428-
const propContexts = processPropDefs(props, (propName) =>
429-
[
430-
defaultsByWithDefaults[propName],
431-
defaultsByAssignmentPatterns[propName]?.expression
432-
].filter(utils.isDef)
433-
)
446+
const propContexts = processPropDefs(props, function* (propName) {
447+
const withDefaults = defaultsByWithDefaults[propName]
448+
if (withDefaults) {
449+
yield { src: 'withDefaults', expression: withDefaults }
450+
}
451+
const assignmentPattern = defaultsByAssignmentPatterns[propName]
452+
if (assignmentPattern) {
453+
yield {
454+
src: 'assignment',
455+
expression: assignmentPattern.expression
456+
}
457+
}
458+
})
434459
scriptSetupPropsContexts.push({ node, props: propContexts })
435460
},
436461
/**
@@ -450,7 +475,21 @@ module.exports = {
450475
}
451476
},
452477
onDefinePropsExit() {
453-
scriptSetupPropsContexts.pop()
478+
const data = scriptSetupPropsContexts.pop()
479+
if (!data) {
480+
return
481+
}
482+
for (const {
483+
prop,
484+
types: typeNames,
485+
default: defType
486+
} of data.props) {
487+
for (const returnType of defType.returnTypes) {
488+
if (typeNames.has(returnType.type)) continue
489+
490+
report(returnType.node, prop, typeNames)
491+
}
492+
}
454493
}
455494
})
456495
)

tests/lib/rules/require-valid-default-prop.js

+102
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,21 @@ ruleTester.run('require-valid-default-prop', rule, {
316316
languageOptions: {
317317
parser: require('vue-eslint-parser')
318318
}
319+
},
320+
{
321+
filename: 'test.vue',
322+
code: `
323+
<script setup lang="ts">
324+
const { foo = [] } = defineProps({
325+
foo: {
326+
type: Array,
327+
}
328+
})
329+
</script>
330+
`,
331+
languageOptions: {
332+
parser: require('vue-eslint-parser')
333+
}
319334
}
320335
],
321336

@@ -1098,6 +1113,93 @@ ruleTester.run('require-valid-default-prop', rule, {
10981113
line: 6
10991114
}
11001115
]
1116+
},
1117+
{
1118+
filename: 'test.vue',
1119+
code: `
1120+
<script setup>
1121+
const { foo = [] } = defineProps({
1122+
foo: {
1123+
type: Number,
1124+
}
1125+
})
1126+
</script>
1127+
`,
1128+
languageOptions: {
1129+
parser: require('vue-eslint-parser')
1130+
},
1131+
errors: [
1132+
{
1133+
message: "Type of the default value for 'foo' prop must be a number.",
1134+
line: 3
1135+
}
1136+
]
1137+
},
1138+
{
1139+
filename: 'test.vue',
1140+
code: `
1141+
<script setup>
1142+
const { foo = 42 } = defineProps({
1143+
foo: {
1144+
type: Array,
1145+
}
1146+
})
1147+
</script>
1148+
`,
1149+
languageOptions: {
1150+
parser: require('vue-eslint-parser')
1151+
},
1152+
errors: [
1153+
{
1154+
message: "Type of the default value for 'foo' prop must be a array.",
1155+
line: 3
1156+
}
1157+
]
1158+
},
1159+
{
1160+
filename: 'test.vue',
1161+
code: `
1162+
<script setup>
1163+
const { foo = [] } = defineProps({
1164+
foo: {
1165+
type: Array,
1166+
default: () => {
1167+
return 42
1168+
}
1169+
}
1170+
})
1171+
</script>
1172+
`,
1173+
languageOptions: {
1174+
parser: require('vue-eslint-parser')
1175+
},
1176+
errors: [
1177+
{
1178+
message: "Type of the default value for 'foo' prop must be a array.",
1179+
line: 7
1180+
}
1181+
]
1182+
},
1183+
{
1184+
filename: 'test.vue',
1185+
code: `
1186+
<script setup>
1187+
const { foo = (()=>[]) } = defineProps({
1188+
foo: {
1189+
type: Array,
1190+
}
1191+
})
1192+
</script>
1193+
`,
1194+
languageOptions: {
1195+
parser: require('vue-eslint-parser')
1196+
},
1197+
errors: [
1198+
{
1199+
message: "Type of the default value for 'foo' prop must be a array.",
1200+
line: 3
1201+
}
1202+
]
11011203
}
11021204
]
11031205
})

0 commit comments

Comments
 (0)