From 9a56de8959dea8728aa3020324cd00990ae477be Mon Sep 17 00:00:00 2001 From: Yosuke Ota Date: Mon, 28 Oct 2024 19:01:29 +0900 Subject: [PATCH] Fix false negatives and false positives in `vue/require-valid-default-prop` rule (#2586) --- lib/rules/require-valid-default-prop.js | 93 +++++++++++----- tests/lib/rules/require-valid-default-prop.js | 102 ++++++++++++++++++ 2 files changed, 168 insertions(+), 27 deletions(-) diff --git a/lib/rules/require-valid-default-prop.js b/lib/rules/require-valid-default-prop.js index ca834d620..9ddef9216 100644 --- a/lib/rules/require-valid-default-prop.js +++ b/lib/rules/require-valid-default-prop.js @@ -230,7 +230,7 @@ module.exports = { } /** - * @param {*} node + * @param {Expression} node * @param {ComponentObjectProp | ComponentTypeProp | ComponentInferTypeProp} prop * @param {Iterable} expectedTypeNames */ @@ -249,17 +249,22 @@ module.exports = { }) } + /** + * @typedef {object} DefaultDefine + * @property {Expression} expression + * @property {'assignment'|'withDefaults'|'defaultProperty'} src + */ /** * @param {(ComponentObjectProp | ComponentTypeProp | ComponentInferTypeProp)[]} props - * @param {(propName: string) => Expression[]} otherDefaultProvider + * @param {(propName: string) => Iterable} otherDefaultProvider */ function processPropDefs(props, otherDefaultProvider) { /** @type {PropDefaultFunctionContext[]} */ const propContexts = [] for (const prop of props) { let typeList - /** @type {Expression[]} */ - const defExprList = [] + /** @type {DefaultDefine[]} */ + const defaultList = [] if (prop.type === 'object') { if (prop.value.type === 'ObjectExpression') { const type = getPropertyNode(prop.value, 'type') @@ -268,9 +273,12 @@ module.exports = { typeList = getTypes(type.value) const def = getPropertyNode(prop.value, 'default') - if (!def) continue - - defExprList.push(def.value) + if (def) { + defaultList.push({ + src: 'defaultProperty', + expression: def.value + }) + } } else { typeList = getTypes(prop.value) } @@ -278,10 +286,10 @@ module.exports = { typeList = prop.types } if (prop.propName != null) { - defExprList.push(...otherDefaultProvider(prop.propName)) + defaultList.push(...otherDefaultProvider(prop.propName)) } - if (defExprList.length === 0) continue + if (defaultList.length === 0) continue const typeNames = new Set( typeList.filter((item) => NATIVE_TYPES.has(item)) @@ -289,8 +297,8 @@ module.exports = { // There is no native types detected if (typeNames.size === 0) continue - for (const defExpr of defExprList) { - const defType = getValueType(defExpr) + for (const defaultDef of defaultList) { + const defType = getValueType(defaultDef.expression) if (!defType) continue @@ -298,6 +306,11 @@ module.exports = { if (typeNames.has('Function')) { continue } + if (defaultDef.src === 'assignment') { + // Factory functions cannot be used in default definitions with initial value assignments. + report(defaultDef.expression, prop, typeNames) + continue + } if (defType.expression) { if (!defType.returnType || typeNames.has(defType.returnType)) { continue @@ -311,18 +324,23 @@ module.exports = { }) } } else { - if ( - typeNames.has(defType.type) && - !FUNCTION_VALUE_TYPES.has(defType.type) - ) { - continue + if (typeNames.has(defType.type)) { + if (defaultDef.src === 'assignment') { + continue + } + if (!FUNCTION_VALUE_TYPES.has(defType.type)) { + // For Array and Object, defaults must be defined in the factory function. + continue + } } report( - defExpr, + defaultDef.expression, prop, - [...typeNames].map((type) => - FUNCTION_VALUE_TYPES.has(type) ? 'Function' : type - ) + defaultDef.src === 'assignment' + ? typeNames + : [...typeNames].map((type) => + FUNCTION_VALUE_TYPES.has(type) ? 'Function' : type + ) ) } } @@ -425,12 +443,19 @@ module.exports = { utils.getWithDefaultsPropExpressions(node) const defaultsByAssignmentPatterns = utils.getDefaultPropExpressionsForPropsDestructure(node) - const propContexts = processPropDefs(props, (propName) => - [ - defaultsByWithDefaults[propName], - defaultsByAssignmentPatterns[propName]?.expression - ].filter(utils.isDef) - ) + const propContexts = processPropDefs(props, function* (propName) { + const withDefaults = defaultsByWithDefaults[propName] + if (withDefaults) { + yield { src: 'withDefaults', expression: withDefaults } + } + const assignmentPattern = defaultsByAssignmentPatterns[propName] + if (assignmentPattern) { + yield { + src: 'assignment', + expression: assignmentPattern.expression + } + } + }) scriptSetupPropsContexts.push({ node, props: propContexts }) }, /** @@ -450,7 +475,21 @@ module.exports = { } }, onDefinePropsExit() { - scriptSetupPropsContexts.pop() + const data = scriptSetupPropsContexts.pop() + if (!data) { + return + } + for (const { + prop, + types: typeNames, + default: defType + } of data.props) { + for (const returnType of defType.returnTypes) { + if (typeNames.has(returnType.type)) continue + + report(returnType.node, prop, typeNames) + } + } } }) ) diff --git a/tests/lib/rules/require-valid-default-prop.js b/tests/lib/rules/require-valid-default-prop.js index 0f4fd1902..238460876 100644 --- a/tests/lib/rules/require-valid-default-prop.js +++ b/tests/lib/rules/require-valid-default-prop.js @@ -316,6 +316,21 @@ ruleTester.run('require-valid-default-prop', rule, { languageOptions: { parser: require('vue-eslint-parser') } + }, + { + filename: 'test.vue', + code: ` + + `, + languageOptions: { + parser: require('vue-eslint-parser') + } } ], @@ -1098,6 +1113,93 @@ ruleTester.run('require-valid-default-prop', rule, { line: 6 } ] + }, + { + filename: 'test.vue', + code: ` + + `, + languageOptions: { + parser: require('vue-eslint-parser') + }, + errors: [ + { + message: "Type of the default value for 'foo' prop must be a number.", + line: 3 + } + ] + }, + { + filename: 'test.vue', + code: ` + + `, + languageOptions: { + parser: require('vue-eslint-parser') + }, + errors: [ + { + message: "Type of the default value for 'foo' prop must be a array.", + line: 3 + } + ] + }, + { + filename: 'test.vue', + code: ` + + `, + languageOptions: { + parser: require('vue-eslint-parser') + }, + errors: [ + { + message: "Type of the default value for 'foo' prop must be a array.", + line: 7 + } + ] + }, + { + filename: 'test.vue', + code: ` + + `, + languageOptions: { + parser: require('vue-eslint-parser') + }, + errors: [ + { + message: "Type of the default value for 'foo' prop must be a array.", + line: 3 + } + ] } ] })