From 48446121c47f8a4725bb4cbff70c40f5e7e58726 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 27 May 2024 14:58:13 +0200 Subject: [PATCH 01/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) handle native types (String, Boolean etc.) --- docs/rules/define-props-declaration.md | 2 + lib/rules/define-props-declaration.js | 102 ++++++++++++- tests/lib/rules/define-props-declaration.js | 150 ++++++++++++++++++-- 3 files changed, 239 insertions(+), 15 deletions(-) diff --git a/docs/rules/define-props-declaration.md b/docs/rules/define-props-declaration.md index 5c72d538a..554777fbc 100644 --- a/docs/rules/define-props-declaration.md +++ b/docs/rules/define-props-declaration.md @@ -10,6 +10,8 @@ since: v9.5.0 > enforce declaration style of `defineProps` +- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule. + ## :book: Rule Details This rule enforces `defineProps` typing style which you should use `type-based` or `runtime` declaration. diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 3862bb228..52c908ac2 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -6,6 +6,40 @@ const utils = require('../utils') +const mapNativeType = (/** @type {string} */ nativeType) => { + switch (nativeType) { + case 'String': { + return 'string' + } + case 'Number': { + return 'number' + } + case 'Boolean': + case 'BigInt': { + return 'boolean' + } + case 'Object': { + return 'Record' + } + case 'Array': { + return 'any[]' + } + case 'Function': { + return '() => void' + } + case 'Symbol': { + return 'symbol' + } + default: { + return nativeType + } + } +} + +/** + * @typedef {import('../utils').ComponentProp} ComponentProp + */ + module.exports = { meta: { type: 'suggestion', @@ -14,7 +48,7 @@ module.exports = { categories: undefined, url: 'https://eslint.vuejs.org/rules/define-props-declaration.html' }, - fixable: null, + fixable: 'code', schema: [ { enum: ['type-based', 'runtime'] @@ -27,6 +61,40 @@ module.exports = { }, /** @param {RuleContext} context */ create(context) { + /** + * @param {Expression} node + * @returns {string | null} + */ + function optionGetType(node) { + switch (node.type) { + case 'Identifier': { + return node.name + } + case 'ObjectExpression': { + // foo: { + const typeProperty = utils.findProperty(node, 'type') + if (typeProperty == null) { + return null + } + return optionGetType(typeProperty.value) + } + case 'ArrayExpression': { + // foo: [ + return null + // return node.elements.map((arrayElement) => + // optionGetType(arrayElement) + // ) + } + case 'FunctionExpression': + case 'ArrowFunctionExpression': { + return null + } + } + + // Unknown + return null + } + const scriptSetup = utils.getScriptSetupElement(context) if (!scriptSetup || !utils.hasAttribute(scriptSetup, 'lang', 'ts')) { return {} @@ -34,13 +102,41 @@ module.exports = { const defineType = context.options[0] || 'type-based' return utils.defineScriptSetupVisitor(context, { - onDefinePropsEnter(node) { + onDefinePropsEnter(node, props) { switch (defineType) { case 'type-based': { if (node.arguments.length > 0) { context.report({ node, - messageId: 'hasArg' + messageId: 'hasArg', + *fix(fixer) { + const propTypes = props.map((prop) => { + const unknownType = { name: prop.propName, type: 'unknown' } + + if (prop.type !== 'object') { + return unknownType + } + const type = optionGetType(prop.value) + if (type === null) { + return unknownType + } + + return { + name: prop.propName, + type: mapNativeType(type) + } + }) + + const definePropsType = `{ ${propTypes + .map(({ name, type }) => `${name}: ${type}`) + .join(', ')} }` + + yield fixer.insertTextAfter( + node.callee, + `<${definePropsType}>` + ) + yield fixer.replaceText(node.arguments[0], '') + } }) } break diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 3af4513ea..9190496f4 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -10,8 +10,11 @@ const rule = require('../../../lib/rules/define-props-declaration') const tester = new RuleTester({ languageOptions: { parser: require('vue-eslint-parser'), - ecmaVersion: 2020, - sourceType: 'module' + ecmaVersion: 'latest', + sourceType: 'module', + parserOptions: { + parser: require.resolve('@typescript-eslint/parser') + } } }) @@ -108,6 +111,7 @@ tester.run('define-props-declaration', rule, { } ], invalid: [ + // default { filename: 'test.vue', code: ` @@ -117,6 +121,34 @@ tester.run('define-props-declaration', rule, { }) `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, + /* TYPE-BASED */ + // shorthand syntax + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, errors: [ { message: 'Use type-based declaration instead of runtime declaration.', @@ -124,6 +156,7 @@ tester.run('define-props-declaration', rule, { } ] }, + // String { filename: 'test.vue', code: ` @@ -133,6 +166,11 @@ tester.run('define-props-declaration', rule, { }) `, + output: ` + + `, options: ['type-based'], errors: [ { @@ -141,27 +179,115 @@ tester.run('define-props-declaration', rule, { } ] }, + // Number { filename: 'test.vue', code: ` + `, + output: ` + `, - options: ['runtime'], errors: [ { - message: 'Use runtime declaration instead of type-based declaration.', + message: 'Use type-based declaration instead of runtime declaration.', line: 3 } - ], - languageOptions: { - parserOptions: { - parser: require.resolve('@typescript-eslint/parser') + ] + }, + // Boolean + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 } - } + ] + }, + // Object + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, + // Array + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, + // Function + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] } ] }) From b770232a69f3039af2ddf189261b9fc29e92a15c Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 27 May 2024 15:17:47 +0200 Subject: [PATCH 02/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) handle PropTypes (e.g. String as PropType<'test'>) --- lib/rules/define-props-declaration.js | 18 ++++ tests/lib/rules/define-props-declaration.js | 98 +++++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 52c908ac2..fe55bfd43 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -61,6 +61,7 @@ module.exports = { }, /** @param {RuleContext} context */ create(context) { + const sourceCode = context.getSourceCode() /** * @param {Expression} node * @returns {string | null} @@ -76,6 +77,23 @@ module.exports = { if (typeProperty == null) { return null } + if (typeProperty.value.type === 'TSAsExpression') { + if ( + typeProperty.value.typeAnnotation.typeName.name !== 'PropType' + ) { + return null + } + + const typeArgument = + typeProperty.value.typeAnnotation.typeArguments.params[0] + if (typeArgument === undefined) { + return null + } + + const text = sourceCode.getText(typeArgument) + + return text + } return optionGetType(typeProperty.value) } case 'ArrayExpression': { diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 9190496f4..b80ecdff8 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -288,6 +288,104 @@ tester.run('define-props-declaration', rule, { line: 3 } ] + }, + // Native Type with PropType + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, + // Object with PropType + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, + // Array with PropType + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, + // Function with PropType + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] } ] }) From 99e2f3ef01e55ee08f868aa5f9be5268de0e1911 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 27 May 2024 15:28:22 +0200 Subject: [PATCH 03/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) handle required option --- lib/rules/define-props-declaration.js | 37 ++++++++++++--- tests/lib/rules/define-props-declaration.js | 50 +++++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index fe55bfd43..a821b5135 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -89,10 +89,8 @@ module.exports = { if (typeArgument === undefined) { return null } - - const text = sourceCode.getText(typeArgument) - - return text + + return sourceCode.getText(typeArgument) } return optionGetType(typeProperty.value) } @@ -113,6 +111,26 @@ module.exports = { return null } + /** + * @param {Expression} node + * @returns {boolean | undefined } + */ + function optionGetRequired(node) { + if (node.type === 'ObjectExpression') { + const requiredProperty = utils.findProperty(node, 'required') + if (requiredProperty == null) { + return undefined + } + + if (requiredProperty.value.type === 'Literal') { + return Boolean(requiredProperty.value.value) + } + } + + // Unknown + return undefined + } + const scriptSetup = utils.getScriptSetupElement(context) if (!scriptSetup || !utils.hasAttribute(scriptSetup, 'lang', 'ts')) { return {} @@ -129,7 +147,7 @@ module.exports = { messageId: 'hasArg', *fix(fixer) { const propTypes = props.map((prop) => { - const unknownType = { name: prop.propName, type: 'unknown' } + const unknownType = { name: prop.propName, type: 'unknown', required: undefined } if (prop.type !== 'object') { return unknownType @@ -138,15 +156,20 @@ module.exports = { if (type === null) { return unknownType } + const required = optionGetRequired(prop.value) return { name: prop.propName, - type: mapNativeType(type) + type: mapNativeType(type), + required } }) const definePropsType = `{ ${propTypes - .map(({ name, type }) => `${name}: ${type}`) + .map( + ({ name, type, required }) => + `${name}${required === false ? '?' : ''}: ${type}` + ) .join(', ')} }` yield fixer.insertTextAfter( diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index b80ecdff8..3b83576d8 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -386,6 +386,56 @@ tester.run('define-props-declaration', rule, { line: 3 } ] + }, + // required + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, + // not required + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] } ] }) From f0294e80538601d6175ad0b94aa75382c21ec474 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 27 May 2024 15:41:43 +0200 Subject: [PATCH 04/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) handle default option --- lib/rules/define-props-declaration.js | 50 ++++++++++++++++++--- tests/lib/rules/define-props-declaration.js | 28 +++++++++++- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index a821b5135..f4f145c91 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -62,6 +62,7 @@ module.exports = { /** @param {RuleContext} context */ create(context) { const sourceCode = context.getSourceCode() + /** * @param {Expression} node * @returns {string | null} @@ -131,6 +132,24 @@ module.exports = { return undefined } + /** + * @param {Expression} node + * @returns {Expression | undefined } + */ + function optionGetDefault(node) { + if (node.type === 'ObjectExpression') { + const defaultProperty = utils.findProperty(node, 'default') + if (defaultProperty == null) { + return undefined + } + + return defaultProperty.value + } + + // Unknown + return undefined + } + const scriptSetup = utils.getScriptSetupElement(context) if (!scriptSetup || !utils.hasAttribute(scriptSetup, 'lang', 'ts')) { return {} @@ -147,7 +166,12 @@ module.exports = { messageId: 'hasArg', *fix(fixer) { const propTypes = props.map((prop) => { - const unknownType = { name: prop.propName, type: 'unknown', required: undefined } + const unknownType = { + name: prop.propName, + type: 'unknown', + required: undefined, + defaultValue: undefined + } if (prop.type !== 'object') { return unknownType @@ -157,26 +181,42 @@ module.exports = { return unknownType } const required = optionGetRequired(prop.value) + const defaultValue = optionGetDefault(prop.value) return { name: prop.propName, type: mapNativeType(type), - required + required, + defaultValue } }) const definePropsType = `{ ${propTypes .map( - ({ name, type, required }) => - `${name}${required === false ? '?' : ''}: ${type}` + ({ name, type, required, defaultValue }) => + `${name}${(required === false || defaultValue) ? '?' : ''}: ${type}` ) .join(', ')} }` + yield fixer.replaceText(node.arguments[0], '') yield fixer.insertTextAfter( node.callee, `<${definePropsType}>` ) - yield fixer.replaceText(node.arguments[0], '') + const defaults = propTypes.filter( + ({ defaultValue }) => defaultValue + ) + if (defaults.length > 0) { + const defaultsCode = defaults + .map( + ({ name, defaultValue }) => + `${name}: ${sourceCode.getText(defaultValue)}` + ) + .join(', ') + + yield fixer.insertTextBefore(node, `withDefaults(`) + yield fixer.insertTextAfter(node, `, { ${defaultsCode} })`) + } } }) } diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 3b83576d8..3c7cf0785 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -344,8 +344,7 @@ tester.run('define-props-declaration', rule, { @@ -436,6 +435,31 @@ tester.run('define-props-declaration', rule, { line: 3 } ] + }, + // default value + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] } ] }) From 5a4a15e2d57dfc0c29f746cd6fc1ddea99eb27df Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 27 May 2024 16:01:47 +0200 Subject: [PATCH 05/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) handle separateInterface rule option --- lib/rules/define-props-declaration.js | 34 ++++++++++++++++++--- tests/lib/rules/define-props-declaration.js | 25 +++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index f4f145c91..f05e6da3c 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -52,6 +52,15 @@ module.exports = { schema: [ { enum: ['type-based', 'runtime'] + }, + { + type: 'object', + properties: { + separateInterface: { + type: 'boolean', + default: false + } + } } ], messages: { @@ -156,6 +165,8 @@ module.exports = { } const defineType = context.options[0] || 'type-based' + const separateInterface = context.options[1]?.separateInterface || false + return utils.defineScriptSetupVisitor(context, { onDefinePropsEnter(node, props) { switch (defineType) { @@ -194,15 +205,28 @@ module.exports = { const definePropsType = `{ ${propTypes .map( ({ name, type, required, defaultValue }) => - `${name}${(required === false || defaultValue) ? '?' : ''}: ${type}` + `${name}${required === false || defaultValue ? '?' : ''}: ${type}` ) .join(', ')} }` yield fixer.replaceText(node.arguments[0], '') - yield fixer.insertTextAfter( - node.callee, - `<${definePropsType}>` - ) + + if (separateInterface) { + const variableDeclarationNode = node.parent.parent + if (!variableDeclarationNode) return + + yield fixer.insertTextBefore( + variableDeclarationNode, + `interface Props ${definePropsType.replaceAll(';', ',')}; ` + ) + yield fixer.insertTextAfter(node.callee, ``) + } else { + yield fixer.insertTextAfter( + node.callee, + `<${definePropsType}>` + ) + } + const defaults = propTypes.filter( ({ defaultValue }) => defaultValue ) diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 3c7cf0785..6f4925480 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -460,6 +460,31 @@ tester.run('define-props-declaration', rule, { line: 3 } ] + }, + // separate interface + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + options: ['type-based', { separateInterface: true }], + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] } ] }) From 583c0dbdfc0563fa6a9814cf7ee03e5321e977cb Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 27 May 2024 17:35:57 +0200 Subject: [PATCH 06/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) bring back the runtime check --- tests/lib/rules/define-props-declaration.js | 24 +++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 6f4925480..87c6633ca 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -485,6 +485,30 @@ tester.run('define-props-declaration', rule, { line: 3 } ] + }, + // runtime + { + filename: 'test.vue', + code: ` + + `, + output: null, + options: ['runtime'], + errors: [ + { + message: 'Use runtime declaration instead of type-based declaration.', + line: 3 + } + ], + languageOptions: { + parserOptions: { + parser: require.resolve('@typescript-eslint/parser') + } + } } ] }) From 4499597db133d15755e3a6dfbe3a43d9dc8c3743 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 27 May 2024 17:36:23 +0200 Subject: [PATCH 07/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) additional tests and refactoring --- lib/rules/define-props-declaration.js | 239 +++++++++++--------- tests/lib/rules/define-props-declaration.js | 80 ++++++- 2 files changed, 206 insertions(+), 113 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index f05e6da3c..17dfb8830 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -36,6 +36,127 @@ const mapNativeType = (/** @type {string} */ nativeType) => { } } +/** + * @param {ComponentProp} prop + * @param {SourceCode} sourceCode + */ +function getComponentPropData(prop, sourceCode) { + const unknownType = { + name: prop.propName, + type: 'unknown', + required: undefined, + defaultValue: undefined + } + + if (prop.type !== 'object') { + return unknownType + } + const type = optionGetType(prop.value, sourceCode) + if (type === null) { + return unknownType + } + const required = optionGetRequired(prop.value) + const defaultValue = optionGetDefault(prop.value) + + return { + name: prop.propName, + type: mapNativeType(type), + required, + defaultValue + } +} + +/** + * @param {Expression} node + * @param {SourceCode} sourceCode + * @returns {string | null} + */ +function optionGetType(node, sourceCode) { + switch (node.type) { + case 'Identifier': { + return node.name + } + case 'ObjectExpression': { + // foo: { + const typeProperty = utils.findProperty(node, 'type') + if (typeProperty == null) { + return null + } + if (typeProperty.value.type === 'TSAsExpression') { + const typeAnnotation = typeProperty.value.typeAnnotation + if (typeAnnotation.typeName.name !== 'PropType') { + return null + } + + // in some project configuration parser populates deprecated field `typeParameters` instead of `typeArguments` + const typeArguments = + 'typeArguments' in typeProperty.value + ? typeAnnotation.typeArguments + : typeAnnotation.typeParameters + + const typeArgument = Array.isArray(typeArguments) + ? typeArguments[0].params[0] + : typeArguments.params[0] + + if (typeArgument === undefined) { + return null + } + + return sourceCode.getText(typeArgument) + } + return optionGetType(typeProperty.value, sourceCode) + } + case 'ArrayExpression': { + return null + } + case 'FunctionExpression': + case 'ArrowFunctionExpression': { + return null + } + } + + // Unknown + return null +} + +/** + * @param {Expression} node + * @returns {boolean | undefined } + */ +function optionGetRequired(node) { + if (node.type === 'ObjectExpression') { + const requiredProperty = utils.findProperty(node, 'required') + if (requiredProperty == null) { + return undefined + } + + if (requiredProperty.value.type === 'Literal') { + return Boolean(requiredProperty.value.value) + } + } + + // Unknown + return undefined +} + +/** + * @param {Expression} node + * @returns {Expression | undefined } + */ +function optionGetDefault(node) { + if (node.type === 'ObjectExpression') { + const defaultProperty = utils.findProperty(node, 'default') + if (defaultProperty == null) { + return undefined + } + + return defaultProperty.value + } + + // Unknown + return undefined +} + /** * @typedef {import('../utils').ComponentProp} ComponentProp */ @@ -72,93 +193,6 @@ module.exports = { create(context) { const sourceCode = context.getSourceCode() - /** - * @param {Expression} node - * @returns {string | null} - */ - function optionGetType(node) { - switch (node.type) { - case 'Identifier': { - return node.name - } - case 'ObjectExpression': { - // foo: { - const typeProperty = utils.findProperty(node, 'type') - if (typeProperty == null) { - return null - } - if (typeProperty.value.type === 'TSAsExpression') { - if ( - typeProperty.value.typeAnnotation.typeName.name !== 'PropType' - ) { - return null - } - - const typeArgument = - typeProperty.value.typeAnnotation.typeArguments.params[0] - if (typeArgument === undefined) { - return null - } - - return sourceCode.getText(typeArgument) - } - return optionGetType(typeProperty.value) - } - case 'ArrayExpression': { - // foo: [ - return null - // return node.elements.map((arrayElement) => - // optionGetType(arrayElement) - // ) - } - case 'FunctionExpression': - case 'ArrowFunctionExpression': { - return null - } - } - - // Unknown - return null - } - - /** - * @param {Expression} node - * @returns {boolean | undefined } - */ - function optionGetRequired(node) { - if (node.type === 'ObjectExpression') { - const requiredProperty = utils.findProperty(node, 'required') - if (requiredProperty == null) { - return undefined - } - - if (requiredProperty.value.type === 'Literal') { - return Boolean(requiredProperty.value.value) - } - } - - // Unknown - return undefined - } - - /** - * @param {Expression} node - * @returns {Expression | undefined } - */ - function optionGetDefault(node) { - if (node.type === 'ObjectExpression') { - const defaultProperty = utils.findProperty(node, 'default') - if (defaultProperty == null) { - return undefined - } - - return defaultProperty.value - } - - // Unknown - return undefined - } - const scriptSetup = utils.getScriptSetupElement(context) if (!scriptSetup || !utils.hasAttribute(scriptSetup, 'lang', 'ts')) { return {} @@ -176,31 +210,9 @@ module.exports = { node, messageId: 'hasArg', *fix(fixer) { - const propTypes = props.map((prop) => { - const unknownType = { - name: prop.propName, - type: 'unknown', - required: undefined, - defaultValue: undefined - } - - if (prop.type !== 'object') { - return unknownType - } - const type = optionGetType(prop.value) - if (type === null) { - return unknownType - } - const required = optionGetRequired(prop.value) - const defaultValue = optionGetDefault(prop.value) - - return { - name: prop.propName, - type: mapNativeType(type), - required, - defaultValue - } - }) + const propTypes = props.map((prop) => + getComponentPropData(prop, sourceCode) + ) const definePropsType = `{ ${propTypes .map( @@ -209,8 +221,10 @@ module.exports = { ) .join(', ')} }` + // remove defineProps function parameters yield fixer.replaceText(node.arguments[0], '') + // add type annotation if (separateInterface) { const variableDeclarationNode = node.parent.parent if (!variableDeclarationNode) return @@ -227,6 +241,7 @@ module.exports = { ) } + // add defaults if needed const defaults = propTypes.filter( ({ defaultValue }) => defaultValue ) diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 87c6633ca..dd98c3a2f 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -10,7 +10,7 @@ const rule = require('../../../lib/rules/define-props-declaration') const tester = new RuleTester({ languageOptions: { parser: require('vue-eslint-parser'), - ecmaVersion: 'latest', + ecmaVersion: '2020', sourceType: 'module', parserOptions: { parser: require.resolve('@typescript-eslint/parser') @@ -289,6 +289,28 @@ tester.run('define-props-declaration', rule, { } ] }, + // Custom type + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, // Native Type with PropType { filename: 'test.vue', @@ -337,6 +359,62 @@ tester.run('define-props-declaration', rule, { } ] }, + // Object with PropType with separate type + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 5 + } + ] + }, + // Object with PropType with separate imported type + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 5 + } + ] + }, // Array with PropType { filename: 'test.vue', From 17ac982a05b0d7810d3bff5a0d80a3ad5366dd53 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 27 May 2024 17:39:13 +0200 Subject: [PATCH 08/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) documentation update --- docs/rules/define-props-declaration.md | 4 ++-- docs/rules/index.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rules/define-props-declaration.md b/docs/rules/define-props-declaration.md index 554777fbc..bd4281a02 100644 --- a/docs/rules/define-props-declaration.md +++ b/docs/rules/define-props-declaration.md @@ -18,7 +18,7 @@ This rule enforces `defineProps` typing style which you should use `type-based` This rule only works in setup script and `lang="ts"`. - + ```vue + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, // runtime { filename: 'test.vue', From f3015464b0f98fc818da6cb29bead90086d43b1f Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Tue, 28 May 2024 11:11:14 +0200 Subject: [PATCH 11/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) refactoring --- lib/rules/define-props-declaration.js | 51 +++++++++++---------- tests/lib/rules/define-props-declaration.js | 1 - 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 47d927dde..233560500 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -40,12 +40,15 @@ const mapNativeType = (/** @type {string} */ nativeType) => { * @param {SourceCode} sourceCode */ function getComponentPropData(prop, sourceCode) { + if (prop.propName === null) { + throw new Error('Unexpected prop with null name.') + } if (prop.type !== 'object') { throw new Error(`Unexpected prop type: ${prop.type}.`) } const type = optionGetType(prop.value, sourceCode) if (type === null) { - throw new Error(`Unexpected prop type: ${prop.type}.`) + throw new Error(`Unable to read prop type`) } const required = optionGetRequired(prop.value) const defaultValue = optionGetDefault(prop.value) @@ -69,33 +72,10 @@ function optionGetType(node, sourceCode) { return mapNativeType(node.name) } case 'ObjectExpression': { - // foo: { const typeProperty = utils.findProperty(node, 'type') if (typeProperty == null) { return null } - if (typeProperty.value.type === 'TSAsExpression') { - const typeAnnotation = typeProperty.value.typeAnnotation - if (typeAnnotation.typeName.name !== 'PropType') { - return null - } - - // in some project configuration parser populates deprecated field `typeParameters` instead of `typeArguments` - const typeArguments = - 'typeArguments' in typeProperty.value - ? typeAnnotation.typeArguments - : typeAnnotation.typeParameters - - const typeArgument = Array.isArray(typeArguments) - ? typeArguments[0].params[0] - : typeArguments.params[0] - - if (typeArgument === undefined) { - return null - } - - return sourceCode.getText(typeArgument) - } return optionGetType(typeProperty.value, sourceCode) } case 'ArrayExpression': { @@ -111,6 +91,29 @@ function optionGetType(node, sourceCode) { .filter(Boolean) .join(' | ') } + case 'TSAsExpression': { + const typeAnnotation = node.typeAnnotation + if (typeAnnotation.typeName.name !== 'PropType') { + return null + } + + // in some project configuration parser populates deprecated field `typeParameters` instead of `typeArguments` + const typeArguments = + 'typeArguments' in node + ? typeAnnotation.typeArguments + : typeAnnotation.typeParameters + + const typeArgument = Array.isArray(typeArguments) + ? typeArguments[0].params[0] + : typeArguments.params[0] + + if (typeArgument === undefined) { + return null + } + + return sourceCode.getText(typeArgument) + } + case 'FunctionExpression': case 'ArrowFunctionExpression': { return null diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index caf822193..4b0318cd2 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -566,7 +566,6 @@ tester.run('define-props-declaration', rule, { }, // Array of types { - only: true, filename: 'test.vue', code: ` + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, // runtime { filename: 'test.vue', From d08bed1b6d799334013c7f19cddc609c0267c7e7 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Tue, 28 May 2024 12:33:30 +0200 Subject: [PATCH 13/27] feat: autofix in `define-props-declaration`: runtime syntax to type-based syntax (#2465) handle union type --- lib/rules/define-props-declaration.js | 10 +++++++++ tests/lib/rules/define-props-declaration.js | 24 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index c59477a22..638e034ce 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -110,6 +110,16 @@ function optionGetType(node, sourceCode) { return sourceCode.getText(typeArgument) } + case 'LogicalExpression': { + if (node.operator === '||') { + const left = optionGetType(node.left, sourceCode) + const right = optionGetType(node.right, sourceCode) + if (left && right) { + return `${left} | ${right}` + } + } + return sourceCode.getText(node) + } default: { return sourceCode.getText(node) } diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 4c58861dd..3bbc882ba 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -588,6 +588,30 @@ tester.run('define-props-declaration', rule, { } ] }, + // Union type (Number || String) + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] + }, // Some unhandled expression type { filename: 'test.vue', From 6cb715371e94e37f75f8b25698124a864b5baaa6 Mon Sep 17 00:00:00 2001 From: Marcin Date: Fri, 21 Jun 2024 14:07:21 +0200 Subject: [PATCH 14/27] Update lib/rules/define-props-declaration.js Co-authored-by: Flo Edelmann --- lib/rules/define-props-declaration.js | 2 +- tests/lib/rules/define-props-declaration.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 638e034ce..03270f763 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -24,7 +24,7 @@ const mapNativeType = (/** @type {string} */ nativeType) => { return 'any[]' } case 'Function': { - return '() => void' + return '(...args: any[]) => any' } case 'Symbol': { return 'symbol' diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 3bbc882ba..9bdfac78b 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -279,7 +279,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ From f6c205f6573be4861d4f433b9b989e50fe2aec75 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Fri, 21 Jun 2024 14:22:50 +0200 Subject: [PATCH 15/27] fix: required default value = false --- lib/rules/define-props-declaration.js | 4 +- tests/lib/rules/define-props-declaration.js | 41 ++++++++++----------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 03270f763..616d33df9 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -134,7 +134,7 @@ function optionGetRequired(node) { if (node.type === 'ObjectExpression') { const requiredProperty = utils.findProperty(node, 'required') if (requiredProperty == null) { - return undefined + return false } if (requiredProperty.value.type === 'Literal') { @@ -143,7 +143,7 @@ function optionGetRequired(node) { } // Unknown - return undefined + return false } /** diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 9bdfac78b..2ad6178a4 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -123,7 +123,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -146,7 +146,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -168,7 +168,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, options: ['type-based'], @@ -191,7 +191,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -207,13 +207,13 @@ tester.run('define-props-declaration', rule, { code: ` `, output: ` `, errors: [ @@ -235,7 +235,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -257,7 +257,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -279,7 +279,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -301,7 +301,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -325,7 +325,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -349,7 +349,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -377,7 +377,7 @@ tester.run('define-props-declaration', rule, { `, errors: [ @@ -405,7 +405,7 @@ tester.run('define-props-declaration', rule, { `, errors: [ @@ -429,7 +429,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -447,14 +447,13 @@ tester.run('define-props-declaration', rule, { const props = defineProps({ kind: { type: Function as PropType<(a: number, b: string) => boolean>, - required: true } }) `, output: ` `, errors: [ @@ -553,7 +552,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, options: ['type-based', { separateInterface: true }], @@ -578,7 +577,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -602,7 +601,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ @@ -626,7 +625,7 @@ tester.run('define-props-declaration', rule, { `, output: ` `, errors: [ From 536c6a140eaa3e83b8ea1b49ab55c6d0c3f0331c Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Fri, 21 Jun 2024 14:39:45 +0200 Subject: [PATCH 16/27] feature: rename autoFixToSeparateInterface option and describe it in the docs --- docs/rules/define-props-declaration.md | 10 +++++++++- lib/rules/define-props-declaration.js | 6 +++--- tests/lib/rules/define-props-declaration.js | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/rules/define-props-declaration.md b/docs/rules/define-props-declaration.md index bd4281a02..db23e13d2 100644 --- a/docs/rules/define-props-declaration.md +++ b/docs/rules/define-props-declaration.md @@ -41,11 +41,19 @@ const props = defineProps({ ## :wrench: Options ```json - "vue/define-props-declaration": ["error", "type-based" | "runtime"] +{ + "vue/define-props-declaration": ["error", + "type-based" | "runtime", + { + "autoFixToSeparateInterface": false + } + ] +} ``` - `type-based` (default) enforces type-based declaration - `runtime` enforces runtime declaration +- `autoFixToSeparateInterface` (`boolean`) define `interface Props` used for type-based declaration instead of providing types inline ### `"runtime"` diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 616d33df9..ed08eb948 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -184,7 +184,7 @@ module.exports = { { type: 'object', properties: { - separateInterface: { + autoFixToSeparateInterface: { type: 'boolean', default: false } @@ -206,7 +206,7 @@ module.exports = { } const defineType = context.options[0] || 'type-based' - const separateInterface = context.options[1]?.separateInterface || false + const autoFixToSeparateInterface = context.options[1]?.autoFixToSeparateInterface || false return utils.defineScriptSetupVisitor(context, { onDefinePropsEnter(node, props) { @@ -235,7 +235,7 @@ module.exports = { yield fixer.replaceText(node.arguments[0], '') // add type annotation - if (separateInterface) { + if (autoFixToSeparateInterface) { const variableDeclarationNode = node.parent.parent if (!variableDeclarationNode) { return diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 2ad6178a4..0242a848e 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -555,7 +555,7 @@ tester.run('define-props-declaration', rule, { interface Props { kind?: { id: number, name: string } }; const props = defineProps() `, - options: ['type-based', { separateInterface: true }], + options: ['type-based', { autoFixToSeparateInterface: true }], errors: [ { message: 'Use type-based declaration instead of runtime declaration.', From 100065d7e74a6115806f7faeff60670e02ddc15b Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Fri, 21 Jun 2024 14:55:51 +0200 Subject: [PATCH 17/27] chore: extract fixTypeBased function --- lib/rules/define-props-declaration.js | 127 +++++++++++++------------- 1 file changed, 63 insertions(+), 64 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index ed08eb948..96c6074b3 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -6,6 +6,68 @@ const utils = require('../utils') +const PROPS_SEPARATOR = ', ' + +/** + * @param {RuleFixer} fixer + * @param {CallExpression} node + * @param {ComponentProp[]} props + * @param {RuleContext} context + */ +function* fixTypeBased(fixer, node, props, context) { + try { + const sourceCode = context.getSourceCode() + const autoFixToSeparateInterface = + context.options[1]?.autoFixToSeparateInterface || false + + const propTypes = props.map((prop) => + getComponentPropData(prop, sourceCode) + ) + + const definePropsType = `{ ${propTypes + .map( + ({ name, type, required, defaultValue }) => + `${name}${required === false || defaultValue ? '?' : ''}: ${type}` + ) + .join(PROPS_SEPARATOR)} }` + + // remove defineProps function parameters + yield fixer.replaceText(node.arguments[0], '') + + // add type annotation + if (autoFixToSeparateInterface) { + const variableDeclarationNode = node.parent.parent + if (!variableDeclarationNode) { + return + } + + yield fixer.insertTextBefore( + variableDeclarationNode, + `interface Props ${definePropsType.replace(/;/g, ',')}; ` + ) + yield fixer.insertTextAfter(node.callee, ``) + } else { + yield fixer.insertTextAfter(node.callee, `<${definePropsType}>`) + } + + // add defaults if needed + const defaults = propTypes.filter(({ defaultValue }) => defaultValue) + if (defaults.length > 0) { + const defaultsCode = defaults + .map( + ({ name, defaultValue }) => + `${name}: ${sourceCode.getText(defaultValue)}` + ) + .join(PROPS_SEPARATOR) + + yield fixer.insertTextBefore(node, `withDefaults(`) + yield fixer.insertTextAfter(node, `, { ${defaultsCode} })`) + } + return null + } catch (error) { + return null + } +} const mapNativeType = (/** @type {string} */ nativeType) => { switch (nativeType) { case 'String': { @@ -198,15 +260,12 @@ module.exports = { }, /** @param {RuleContext} context */ create(context) { - const sourceCode = context.getSourceCode() - const scriptSetup = utils.getScriptSetupElement(context) if (!scriptSetup || !utils.hasAttribute(scriptSetup, 'lang', 'ts')) { return {} } const defineType = context.options[0] || 'type-based' - const autoFixToSeparateInterface = context.options[1]?.autoFixToSeparateInterface || false return utils.defineScriptSetupVisitor(context, { onDefinePropsEnter(node, props) { @@ -217,67 +276,7 @@ module.exports = { node, messageId: 'hasArg', *fix(fixer) { - try { - const propTypes = props.map((prop) => - getComponentPropData(prop, sourceCode) - ) - - const definePropsType = `{ ${propTypes - .map( - ({ name, type, required, defaultValue }) => - `${name}${ - required === false || defaultValue ? '?' : '' - }: ${type}` - ) - .join(', ')} }` - - // remove defineProps function parameters - yield fixer.replaceText(node.arguments[0], '') - - // add type annotation - if (autoFixToSeparateInterface) { - const variableDeclarationNode = node.parent.parent - if (!variableDeclarationNode) { - return - } - - yield fixer.insertTextBefore( - variableDeclarationNode, - `interface Props ${definePropsType.replace( - /;/g, - ',' - )}; ` - ) - yield fixer.insertTextAfter(node.callee, ``) - } else { - yield fixer.insertTextAfter( - node.callee, - `<${definePropsType}>` - ) - } - - // add defaults if needed - const defaults = propTypes.filter( - ({ defaultValue }) => defaultValue - ) - if (defaults.length > 0) { - const defaultsCode = defaults - .map( - ({ name, defaultValue }) => - `${name}: ${sourceCode.getText(defaultValue)}` - ) - .join(', ') - - yield fixer.insertTextBefore(node, `withDefaults(`) - yield fixer.insertTextAfter( - node, - `, { ${defaultsCode} })` - ) - } - return null - } catch (error) { - return null - } + yield* fixTypeBased(fixer, node, props, context) } }) } From 7d9e731d51b0243db479dd76eb7e606d9e950466 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Fri, 21 Jun 2024 15:07:11 +0200 Subject: [PATCH 18/27] chore: refactor fixTypeBased function --- lib/rules/define-props-declaration.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 96c6074b3..ba0c1c6d0 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -20,16 +20,16 @@ function* fixTypeBased(fixer, node, props, context) { const autoFixToSeparateInterface = context.options[1]?.autoFixToSeparateInterface || false - const propTypes = props.map((prop) => + const componentPropsData = props.map((prop) => getComponentPropData(prop, sourceCode) ) - const definePropsType = `{ ${propTypes - .map( - ({ name, type, required, defaultValue }) => - `${name}${required === false || defaultValue ? '?' : ''}: ${type}` - ) - .join(PROPS_SEPARATOR)} }` + const componentPropsTypeCode = `{${componentPropsData + .map(({ name, type, required, defaultValue }) => { + const isOptional = required === false || defaultValue + return `${name}${isOptional ? '?' : ''}: ${type}` + }) + .join(PROPS_SEPARATOR)}}` // remove defineProps function parameters yield fixer.replaceText(node.arguments[0], '') @@ -43,17 +43,19 @@ function* fixTypeBased(fixer, node, props, context) { yield fixer.insertTextBefore( variableDeclarationNode, - `interface Props ${definePropsType.replace(/;/g, ',')}; ` + `interface Props ${componentPropsTypeCode.replace(/;/g, ',')}; ` ) yield fixer.insertTextAfter(node.callee, ``) } else { - yield fixer.insertTextAfter(node.callee, `<${definePropsType}>`) + yield fixer.insertTextAfter(node.callee, `<${componentPropsTypeCode}>`) } // add defaults if needed - const defaults = propTypes.filter(({ defaultValue }) => defaultValue) - if (defaults.length > 0) { - const defaultsCode = defaults + const propTypesDataWithDefaultValue = componentPropsData.filter( + ({ defaultValue }) => defaultValue + ) + if (propTypesDataWithDefaultValue.length > 0) { + const defaultsCode = propTypesDataWithDefaultValue .map( ({ name, defaultValue }) => `${name}: ${sourceCode.getText(defaultValue)}` From 071594396140f8b0a3b3dda65f72cb5485ad6623 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 24 Jun 2024 10:18:48 +0200 Subject: [PATCH 19/27] chore: refactor componentPropsTypeCode creation --- lib/rules/define-props-declaration.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index ba0c1c6d0..c41fc5d99 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -24,12 +24,14 @@ function* fixTypeBased(fixer, node, props, context) { getComponentPropData(prop, sourceCode) ) - const componentPropsTypeCode = `{${componentPropsData - .map(({ name, type, required, defaultValue }) => { + const componentPropsTypes = componentPropsData.map( + ({ name, type, required, defaultValue }) => { const isOptional = required === false || defaultValue return `${name}${isOptional ? '?' : ''}: ${type}` - }) - .join(PROPS_SEPARATOR)}}` + } + ) + + const componentPropsTypeCode = `{${componentPropsTypes.join(PROPS_SEPARATOR)}}` // remove defineProps function parameters yield fixer.replaceText(node.arguments[0], '') From bbdc1342b84007fec62cdf768dbd7c383faf4ac1 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 1 Jul 2024 17:51:06 +0200 Subject: [PATCH 20/27] fix: fix tests failing --- lib/rules/define-props-declaration.js | 88 +++++++++++++-------------- 1 file changed, 42 insertions(+), 46 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index c41fc5d99..705165663 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -15,62 +15,58 @@ const PROPS_SEPARATOR = ', ' * @param {RuleContext} context */ function* fixTypeBased(fixer, node, props, context) { - try { - const sourceCode = context.getSourceCode() - const autoFixToSeparateInterface = - context.options[1]?.autoFixToSeparateInterface || false + const sourceCode = context.getSourceCode() + const autoFixToSeparateInterface = + context.options[1]?.autoFixToSeparateInterface || false - const componentPropsData = props.map((prop) => - getComponentPropData(prop, sourceCode) - ) - - const componentPropsTypes = componentPropsData.map( - ({ name, type, required, defaultValue }) => { - const isOptional = required === false || defaultValue - return `${name}${isOptional ? '?' : ''}: ${type}` - } - ) + const componentPropsData = props.map((prop) => + getComponentPropData(prop, sourceCode) + ) - const componentPropsTypeCode = `{${componentPropsTypes.join(PROPS_SEPARATOR)}}` + const componentPropsTypes = componentPropsData.map( + ({ name, type, required, defaultValue }) => { + const isOptional = required === false || defaultValue + return `${name}${isOptional ? '?' : ''}: ${type}` + } + ) - // remove defineProps function parameters - yield fixer.replaceText(node.arguments[0], '') + const componentPropsTypeCode = `{ ${componentPropsTypes.join(PROPS_SEPARATOR)} }` - // add type annotation - if (autoFixToSeparateInterface) { - const variableDeclarationNode = node.parent.parent - if (!variableDeclarationNode) { - return - } + // remove defineProps function parameters + yield fixer.replaceText(node.arguments[0], '') - yield fixer.insertTextBefore( - variableDeclarationNode, - `interface Props ${componentPropsTypeCode.replace(/;/g, ',')}; ` - ) - yield fixer.insertTextAfter(node.callee, ``) - } else { - yield fixer.insertTextAfter(node.callee, `<${componentPropsTypeCode}>`) + // add type annotation + if (autoFixToSeparateInterface) { + const variableDeclarationNode = node.parent.parent + if (!variableDeclarationNode) { + return } - // add defaults if needed - const propTypesDataWithDefaultValue = componentPropsData.filter( - ({ defaultValue }) => defaultValue + yield fixer.insertTextBefore( + variableDeclarationNode, + `interface Props ${componentPropsTypeCode.replace(/;/g, ',')}; ` ) - if (propTypesDataWithDefaultValue.length > 0) { - const defaultsCode = propTypesDataWithDefaultValue - .map( - ({ name, defaultValue }) => - `${name}: ${sourceCode.getText(defaultValue)}` - ) - .join(PROPS_SEPARATOR) + yield fixer.insertTextAfter(node.callee, ``) + } else { + yield fixer.insertTextAfter(node.callee, `<${componentPropsTypeCode}>`) + } - yield fixer.insertTextBefore(node, `withDefaults(`) - yield fixer.insertTextAfter(node, `, { ${defaultsCode} })`) - } - return null - } catch (error) { - return null + // add defaults if needed + const propTypesDataWithDefaultValue = componentPropsData.filter( + ({ defaultValue }) => defaultValue + ) + if (propTypesDataWithDefaultValue.length > 0) { + const defaultsCode = propTypesDataWithDefaultValue + .map( + ({ name, defaultValue }) => + `${name}: ${sourceCode.getText(defaultValue)}` + ) + .join(PROPS_SEPARATOR) + + yield fixer.insertTextBefore(node, `withDefaults(`) + yield fixer.insertTextAfter(node, `, { ${defaultsCode} })`) } + return null } const mapNativeType = (/** @type {string} */ nativeType) => { switch (nativeType) { From 2a1c654c6ccbf3d8125e328f67aad4e38afaf742 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 1 Jul 2024 17:52:10 +0200 Subject: [PATCH 21/27] feature: remove autoFixToSeparateInterface option --- lib/rules/define-props-declaration.js | 26 +-------------------- tests/lib/rules/define-props-declaration.js | 25 -------------------- 2 files changed, 1 insertion(+), 50 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 705165663..db4b0a1a2 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -16,8 +16,6 @@ const PROPS_SEPARATOR = ', ' */ function* fixTypeBased(fixer, node, props, context) { const sourceCode = context.getSourceCode() - const autoFixToSeparateInterface = - context.options[1]?.autoFixToSeparateInterface || false const componentPropsData = props.map((prop) => getComponentPropData(prop, sourceCode) @@ -36,20 +34,7 @@ function* fixTypeBased(fixer, node, props, context) { yield fixer.replaceText(node.arguments[0], '') // add type annotation - if (autoFixToSeparateInterface) { - const variableDeclarationNode = node.parent.parent - if (!variableDeclarationNode) { - return - } - - yield fixer.insertTextBefore( - variableDeclarationNode, - `interface Props ${componentPropsTypeCode.replace(/;/g, ',')}; ` - ) - yield fixer.insertTextAfter(node.callee, ``) - } else { - yield fixer.insertTextAfter(node.callee, `<${componentPropsTypeCode}>`) - } + yield fixer.insertTextAfter(node.callee, `<${componentPropsTypeCode}>`) // add defaults if needed const propTypesDataWithDefaultValue = componentPropsData.filter( @@ -242,15 +227,6 @@ module.exports = { schema: [ { enum: ['type-based', 'runtime'] - }, - { - type: 'object', - properties: { - autoFixToSeparateInterface: { - type: 'boolean', - default: false - } - } } ], messages: { diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 0242a848e..fd55543a2 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -538,31 +538,6 @@ tester.run('define-props-declaration', rule, { } ] }, - // separate interface - { - filename: 'test.vue', - code: ` - - `, - output: ` - - `, - options: ['type-based', { autoFixToSeparateInterface: true }], - errors: [ - { - message: 'Use type-based declaration instead of runtime declaration.', - line: 3 - } - ] - }, // Array of types { filename: 'test.vue', From 0e6ea56d94dadead32efaba3263e7d7fb2f85c87 Mon Sep 17 00:00:00 2001 From: Flo Edelmann Date: Tue, 2 Jul 2024 15:39:23 +0200 Subject: [PATCH 22/27] Fix tests --- tests/lib/rules/define-props-declaration.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index b93d627bf..4285f087d 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -10,7 +10,7 @@ const rule = require('../../../lib/rules/define-props-declaration') const tester = new RuleTester({ languageOptions: { parser: require('vue-eslint-parser'), - ecmaVersion: '2020', + ecmaVersion: 2020, sourceType: 'module', parserOptions: { parser: require.resolve('@typescript-eslint/parser') From e9d44003fbed3e9ae29d6f81d0a58c4091a72dc4 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Wed, 3 Jul 2024 18:18:41 +0200 Subject: [PATCH 23/27] feature: code cleanup --- docs/rules/define-props-declaration.md | 6 +----- lib/rules/define-props-declaration.js | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/docs/rules/define-props-declaration.md b/docs/rules/define-props-declaration.md index db23e13d2..51a2864e7 100644 --- a/docs/rules/define-props-declaration.md +++ b/docs/rules/define-props-declaration.md @@ -43,17 +43,13 @@ const props = defineProps({ ```json { "vue/define-props-declaration": ["error", - "type-based" | "runtime", - { - "autoFixToSeparateInterface": false - } + "type-based" | "runtime" ] } ``` - `type-based` (default) enforces type-based declaration - `runtime` enforces runtime declaration -- `autoFixToSeparateInterface` (`boolean`) define `interface Props` used for type-based declaration instead of providing types inline ### `"runtime"` diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index db4b0a1a2..22e4f751b 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -6,6 +6,10 @@ const utils = require('../utils') +/** + * @typedef {import('../utils').ComponentProp} ComponentProp + */ + const PROPS_SEPARATOR = ', ' /** @@ -53,8 +57,11 @@ function* fixTypeBased(fixer, node, props, context) { } return null } -const mapNativeType = (/** @type {string} */ nativeType) => { - switch (nativeType) { + /** + * @param {string} nativeType + * @returns {string} + */ +const mapNativeType = (nativeType) => {switch (nativeType) { case 'String': { return 'string' } @@ -125,7 +132,6 @@ function optionGetType(node, sourceCode) { case 'ArrayExpression': { return node.elements .map((element) => { - // TODO handle SpreadElement if (element === null || element.type === 'SpreadElement') { return sourceCode.getText(node) } @@ -175,7 +181,7 @@ function optionGetType(node, sourceCode) { /** * @param {Expression} node - * @returns {boolean | undefined } + * @returns {boolean} */ function optionGetRequired(node) { if (node.type === 'ObjectExpression') { @@ -195,7 +201,7 @@ function optionGetRequired(node) { /** * @param {Expression} node - * @returns {Expression | undefined } + * @returns {Expression | undefined} */ function optionGetDefault(node) { if (node.type === 'ObjectExpression') { @@ -211,10 +217,6 @@ function optionGetDefault(node) { return undefined } -/** - * @typedef {import('../utils').ComponentProp} ComponentProp - */ - module.exports = { meta: { type: 'suggestion', From 0bd915bec38157804ffba9e2a378704b15f7abf1 Mon Sep 17 00:00:00 2001 From: Flo Edelmann Date: Thu, 4 Jul 2024 09:41:46 +0200 Subject: [PATCH 24/27] Lint --- lib/rules/define-props-declaration.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 22e4f751b..7f4e6348a 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -57,11 +57,13 @@ function* fixTypeBased(fixer, node, props, context) { } return null } - /** + +/** * @param {string} nativeType * @returns {string} */ -const mapNativeType = (nativeType) => {switch (nativeType) { +const mapNativeType = (nativeType) => { + switch (nativeType) { case 'String': { return 'string' } From 1d58a2bf1716a7c5dd2bad68c15a7b04ba5a720f Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Mon, 15 Jul 2024 19:03:09 +0200 Subject: [PATCH 25/27] feature: handle array as props list (#2465) --- lib/rules/define-props-declaration.js | 46 ++++++++++++++------- tests/lib/rules/define-props-declaration.js | 28 +++++++++++-- 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 7f4e6348a..949b355c5 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -32,7 +32,9 @@ function* fixTypeBased(fixer, node, props, context) { } ) - const componentPropsTypeCode = `{ ${componentPropsTypes.join(PROPS_SEPARATOR)} }` + const componentPropsTypeCode = `{ ${componentPropsTypes.join( + PROPS_SEPARATOR + )} }` // remove defineProps function parameters yield fixer.replaceText(node.arguments[0], '') @@ -96,22 +98,34 @@ const mapNativeType = (nativeType) => { * @param {SourceCode} sourceCode */ function getComponentPropData(prop, sourceCode) { - if (prop.propName === null) { - throw new Error('Unexpected prop with null name.') - } - if (prop.type !== 'object') { - throw new Error(`Unexpected prop type: ${prop.type}.`) - } - const type = optionGetType(prop.value, sourceCode) - const required = optionGetRequired(prop.value) - const defaultValue = optionGetDefault(prop.value) - - return { - name: prop.propName, - type, - required, - defaultValue + if (prop.type === 'array') { + if (prop.node.type !== 'Identifier') { + throw new Error(`Unexpected prop type inside array: ${prop.node.type}`) + } + + return { + name: prop.node.name, + type: 'string', + required: false, + defaultValue: undefined + } + } else if (prop.type === 'object') { + if (prop.propName === null) { + throw new Error('Unexpected prop with null name.') + } + + const type = optionGetType(prop.value, sourceCode) + const required = optionGetRequired(prop.value) + const defaultValue = optionGetDefault(prop.value) + + return { + name: prop.propName, + type, + required, + defaultValue + } } + throw new Error(`Unexpected prop type: ${prop.type}.`) } /** diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 4285f087d..38e700e65 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -365,7 +365,7 @@ tester.run('define-props-declaration', rule, { code: ` `, @@ -393,7 +393,7 @@ tester.run('define-props-declaration', rule, { code: ` `, @@ -633,6 +633,26 @@ tester.run('define-props-declaration', rule, { line: 3 } ] + }, + // array + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: 'Use type-based declaration instead of runtime declaration.', + line: 3 + } + ] } ] }) From da17d74c8ef953b579efc111f5044b0f07f511d7 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Fri, 26 Jul 2024 21:45:36 +0200 Subject: [PATCH 26/27] feature: catch errors and ignore them (#2465) --- lib/rules/define-props-declaration.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 949b355c5..7f58b87cd 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -270,7 +270,11 @@ module.exports = { node, messageId: 'hasArg', *fix(fixer) { - yield* fixTypeBased(fixer, node, props, context) + try { + yield* fixTypeBased(fixer, node, props, context) + } catch (error) { + // ignore + } } }) } From 6634c2dc06b9821f7fcbc4156c97235929b82983 Mon Sep 17 00:00:00 2001 From: "marcin.piniarski" Date: Fri, 26 Jul 2024 21:48:23 +0200 Subject: [PATCH 27/27] feature: do not handle array prop declaration (#2465) --- lib/rules/define-props-declaration.js | 11 +---------- tests/lib/rules/define-props-declaration.js | 5 ----- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/lib/rules/define-props-declaration.js b/lib/rules/define-props-declaration.js index 7f58b87cd..8112054db 100644 --- a/lib/rules/define-props-declaration.js +++ b/lib/rules/define-props-declaration.js @@ -99,16 +99,7 @@ const mapNativeType = (nativeType) => { */ function getComponentPropData(prop, sourceCode) { if (prop.type === 'array') { - if (prop.node.type !== 'Identifier') { - throw new Error(`Unexpected prop type inside array: ${prop.node.type}`) - } - - return { - name: prop.node.name, - type: 'string', - required: false, - defaultValue: undefined - } + throw new Error(`Unable to resolve types based on array prop declaration.`) } else if (prop.type === 'object') { if (prop.propName === null) { throw new Error('Unexpected prop with null name.') diff --git a/tests/lib/rules/define-props-declaration.js b/tests/lib/rules/define-props-declaration.js index 38e700e65..116e48754 100644 --- a/tests/lib/rules/define-props-declaration.js +++ b/tests/lib/rules/define-props-declaration.js @@ -642,11 +642,6 @@ tester.run('define-props-declaration', rule, { const props = defineProps([kind]) `, - output: ` - - `, errors: [ { message: 'Use type-based declaration instead of runtime declaration.',