Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implements autofix in define-props-declaration (#2465) #2466

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4844612
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
b770232
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
99e2f3e
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
f0294e8
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
5a4a15e
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
583c0db
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
4499597
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
17ac982
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
5e1d3b1
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 27, 2024
bc506f9
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 28, 2024
f301546
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 28, 2024
207477e
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 28, 2024
d08bed1
feat: autofix in `define-props-declaration`: runtime syntax to type-b…
mpiniarski May 28, 2024
6cb7153
Update lib/rules/define-props-declaration.js
mpiniarski Jun 21, 2024
f6c205f
fix: required default value = false
mpiniarski Jun 21, 2024
536c6a1
feature: rename autoFixToSeparateInterface option and describe it in …
mpiniarski Jun 21, 2024
100065d
chore: extract fixTypeBased function
mpiniarski Jun 21, 2024
7d9e731
chore: refactor fixTypeBased function
mpiniarski Jun 21, 2024
0715943
chore: refactor componentPropsTypeCode creation
mpiniarski Jun 24, 2024
bbdc134
fix: fix tests failing
mpiniarski Jul 1, 2024
2a1c654
feature: remove autoFixToSeparateInterface option
mpiniarski Jul 1, 2024
7cdf3ff
Merge branch 'master' into feature/#2465_autofix_in_define-props-decl…
mpiniarski Jul 2, 2024
0e6ea56
Fix tests
FloEdelmann Jul 2, 2024
e9d4400
feature: code cleanup
mpiniarski Jul 3, 2024
0bd915b
Lint
FloEdelmann Jul 4, 2024
1d58a2b
feature: handle array as props list (#2465)
mpiniarski Jul 15, 2024
da17d74
feature: catch errors and ignore them (#2465)
mpiniarski Jul 26, 2024
6634c2d
feature: do not handle array prop declaration (#2465)
mpiniarski Jul 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/rules/define-props-declaration.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ 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.

This rule only works in setup script and `lang="ts"`.

<eslint-code-block :rules="{'vue/define-props-declaration': ['error']}">
<eslint-code-block fix :rules="{'vue/define-props-declaration': ['error']}">

```vue
<script setup lang="ts">
Expand Down Expand Up @@ -47,7 +49,7 @@ const props = defineProps({

### `"runtime"`

<eslint-code-block :rules="{'vue/define-emits-declaration': ['error', 'runtime']}">
<eslint-code-block fix :rules="{'vue/define-emits-declaration': ['error', 'runtime']}">

```vue
<script setup lang="ts">
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ For example:
| [vue/custom-event-name-casing](./custom-event-name-casing.md) | enforce specific casing for custom event name | | :hammer: |
| [vue/define-emits-declaration](./define-emits-declaration.md) | enforce declaration style of `defineEmits` | | :hammer: |
| [vue/define-macros-order](./define-macros-order.md) | enforce order of `defineEmits` and `defineProps` compiler macros | :wrench::bulb: | :lipstick: |
| [vue/define-props-declaration](./define-props-declaration.md) | enforce declaration style of `defineProps` | | :hammer: |
| [vue/define-props-declaration](./define-props-declaration.md) | enforce declaration style of `defineProps` | :wrench: | :hammer: |
| [vue/enforce-style-attribute](./enforce-style-attribute.md) | enforce or forbid the use of the `scoped` and `module` attributes in SFC top level style tags | | :hammer: |
| [vue/html-button-has-type](./html-button-has-type.md) | disallow usage of button without an explicit type attribute | | :hammer: |
| [vue/html-comment-content-newline](./html-comment-content-newline.md) | enforce unified line brake in HTML comments | :wrench: | :lipstick: |
Expand Down
244 changes: 241 additions & 3 deletions lib/rules/define-props-declaration.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,168 @@

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

const mapNativeType = (/** @type {string} */ nativeType) => {
mpiniarski marked this conversation as resolved.
Show resolved Hide resolved
switch (nativeType) {
case 'String': {
return 'string'
}
case 'Number': {
return 'number'
}
case 'Boolean': {
return 'boolean'
}
case 'Object': {
return 'Record<string, any>'
}
case 'Array': {
return 'any[]'
}
case 'Function': {
return '() => void'
mpiniarski marked this conversation as resolved.
Show resolved Hide resolved
}
case 'Symbol': {
return 'symbol'
}
default: {
return nativeType
}
}
}

/**
* @param {ComponentProp} prop
* @param {SourceCode} sourceCode
*/
function getComponentPropData(prop, sourceCode) {
if (prop.propName === null) {
throw new Error('Unexpected prop with null name.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it need to throw an error here?
Can't we make it guard against those before processing the autofix?

Copy link
Author

@mpiniarski mpiniarski Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?
I think it's more clear if we throw an error here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that if we throw an error, the user's eslint command will fail.
Did you throw the error with that intention? If so, why is that?
If auto-fix is not possible, I think it should be handled so that auto-fix is not performed, rather than throwing an error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Let me catch errors and ignore them.

I prefer to leave a sign of an error somewhere, maybe in console.debug(), but it is up to you.

}
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
}
}

/**
* @param {Expression} node
* @param {SourceCode} sourceCode
* @returns {string}
*/
function optionGetType(node, sourceCode) {
switch (node.type) {
case 'Identifier': {
return mapNativeType(node.name)
}
case 'ObjectExpression': {
const typeProperty = utils.findProperty(node, 'type')
if (typeProperty == null) {
return sourceCode.getText(node)
}
return optionGetType(typeProperty.value, sourceCode)
}
case 'ArrayExpression': {
return node.elements
.map((element) => {
// TODO handle SpreadElement

Check warning on line 81 in lib/rules/define-props-declaration.js

View workflow job for this annotation

GitHub Actions / Lint

Unexpected 'todo' comment: 'TODO handle SpreadElement'
FloEdelmann marked this conversation as resolved.
Show resolved Hide resolved
if (element === null || element.type === 'SpreadElement') {
return sourceCode.getText(node)
}

return optionGetType(element, sourceCode)
})
.filter(Boolean)
.join(' | ')
}
case 'TSAsExpression': {
const typeAnnotation = node.typeAnnotation
if (typeAnnotation.typeName.name !== 'PropType') {
return sourceCode.getText(node)
}

// 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 sourceCode.getText(node)
}

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)
}
}
}

/**
* @param {Expression} node
* @returns {boolean | undefined }
mpiniarski marked this conversation as resolved.
Show resolved Hide resolved
*/
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 }
mpiniarski marked this conversation as resolved.
Show resolved Hide resolved
*/
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
FloEdelmann marked this conversation as resolved.
Show resolved Hide resolved
*/

module.exports = {
meta: {
type: 'suggestion',
Expand All @@ -14,10 +176,19 @@
categories: undefined,
url: 'https://eslint.vuejs.org/rules/define-props-declaration.html'
},
fixable: null,
fixable: 'code',
schema: [
{
enum: ['type-based', 'runtime']
},
{
type: 'object',
properties: {
separateInterface: {
FloEdelmann marked this conversation as resolved.
Show resolved Hide resolved
type: 'boolean',
default: false
}
}
}
],
messages: {
Expand All @@ -27,20 +198,87 @@
},
/** @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 separateInterface = context.options[1]?.separateInterface || false

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) {
FloEdelmann marked this conversation as resolved.
Show resolved Hide resolved
try {
const propTypes = props.map((prop) =>
getComponentPropData(prop, sourceCode)
)

const definePropsType = `{ ${propTypes
.map(
({ name, type, required, defaultValue }) =>
`${name}${
required === false || defaultValue ? '?' : ''
}: ${type}`
)
.join(', ')} }`
FloEdelmann marked this conversation as resolved.
Show resolved Hide resolved

// remove defineProps function parameters
yield fixer.replaceText(node.arguments[0], '')

// add type annotation
if (separateInterface) {
const variableDeclarationNode = node.parent.parent
if (!variableDeclarationNode) {
return
}

yield fixer.insertTextBefore(
variableDeclarationNode,
`interface Props ${definePropsType.replace(
/;/g,
','
)}; `
)
yield fixer.insertTextAfter(node.callee, `<Props>`)
} 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
}
}
})
}
break
Expand Down
Loading
Loading