Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/soft-boats-repair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'eslint-plugin-vue': patch
---

Fixed false positive in `vue/return-in-computed-property` and `vue/require-render-return` for exhaustive switch statements when TypeScript type information is available
2 changes: 1 addition & 1 deletion lib/rules/require-render-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ module.exports = {
renderFunctions.set(node, node.parent.key)
}
}),
utils.executeOnFunctionsWithoutReturn(true, (node) => {
utils.executeOnFunctionsWithoutReturn(context, true, (node) => {
const key = renderFunctions.get(node)
if (key) {
context.report({
Expand Down
1 change: 1 addition & 0 deletions lib/rules/return-in-computed-property.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ module.exports = {
}
}),
utils.executeOnFunctionsWithoutReturn(
context,
treatUndefinedAsUnspecified,
(node) => {
for (const cp of computedProperties) {
Expand Down
9 changes: 7 additions & 2 deletions lib/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const {
getComponentPropsFromTypeDefine,
getComponentEmitsFromTypeDefine,
getComponentSlotsFromTypeDefine,
hasExhaustiveSwitchReturn,
isTypeNode
} = require('./ts-utils')

Expand Down Expand Up @@ -1815,11 +1816,12 @@ module.exports = {

/**
* Find all functions which do not always return values
* @param {RuleContext} context The ESLint rule context object.
* @param {boolean} treatUndefinedAsUnspecified
* @param { (node: ArrowFunctionExpression | FunctionExpression | FunctionDeclaration) => void } cb Callback function
* @returns {RuleListener}
*/
executeOnFunctionsWithoutReturn(treatUndefinedAsUnspecified, cb) {
executeOnFunctionsWithoutReturn(context, treatUndefinedAsUnspecified, cb) {
/**
* @typedef {object} FuncInfo
* @property {FuncInfo | null} funcInfo
Expand All @@ -1837,7 +1839,10 @@ module.exports = {
if (!funcInfo) {
return true
}
if (funcInfo.currentSegments.some((segment) => segment.reachable)) {
if (
funcInfo.currentSegments.some((segment) => segment.reachable) &&
!hasExhaustiveSwitchReturn(context, funcInfo.node)
) {
return false
}
return !treatUndefinedAsUnspecified || funcInfo.hasReturnValue
Expand Down
4 changes: 3 additions & 1 deletion lib/utils/ts-utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const {
const {
getComponentPropsFromTypeDefineTypes,
getComponentEmitsFromTypeDefineTypes,
getComponentSlotsFromTypeDefineTypes
getComponentSlotsFromTypeDefineTypes,
hasExhaustiveSwitchReturn
} = require('./ts-types')

/**
Expand All @@ -31,6 +32,7 @@ const {

module.exports = {
isTypeNode,
hasExhaustiveSwitchReturn,
getComponentPropsFromTypeDefine,
getComponentEmitsFromTypeDefine,
getComponentSlotsFromTypeDefine
Expand Down
130 changes: 129 additions & 1 deletion lib/utils/ts-utils/ts-types.js
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

question
Most of the work was done here because it's TS related, but I wasn't sure if this is actually the most appropriate location.
Is there a more suitable place or is it ok keep the exhaustive switch-related logic here?

Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ module.exports = {
getComponentPropsFromTypeDefineTypes,
getComponentEmitsFromTypeDefineTypes,
getComponentSlotsFromTypeDefineTypes,
inferRuntimeTypeFromTypeNode
inferRuntimeTypeFromTypeNode,
hasExhaustiveSwitchReturn
}

/**
Expand Down Expand Up @@ -334,3 +335,130 @@ function* iterateTypes(type) {
yield type
}
}

/**
* @param {Type} type
* @returns {Type[]}
*/
function unionConstituents(type) {
return type.isUnion() ? type.types : [type]
}

/**
* @param {Type} type
* @returns {Type[]}
*/
function intersectionConstituents(type) {
return type.isIntersection() ? type.types : [type]
}
Comment on lines +339 to +353
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Inlined TS utility functions from ts-api-utils


/**
* @param {Type} type
* @param {typeof import('typescript')} ts
* @returns {boolean}
*/
function isTypeLiteralLike(type, ts) {
return (
(type.flags &
(ts.TypeFlags.Literal |
ts.TypeFlags.Undefined |
ts.TypeFlags.Null |
ts.TypeFlags.UniqueESSymbol)) !==
0
)
}
Comment on lines +355 to +369
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Basically copied from switch-exhaustiveness-check.ts#L415


/**
* @param {ESNode} stmt
* @returns {boolean}
*/
function hasTerminalReturn(stmt) {
if (stmt.type === 'ReturnStatement' || stmt.type === 'ThrowStatement') {
return true
}
if (stmt.type === 'BlockStatement') {
const stmts = stmt.body
if (stmts.length === 0) return false
return hasTerminalReturn(/** @type {ESNode} */ (stmts.at(-1)))
}
return false
}

/**
* @param {RuleContext} context
* @param {SwitchStatement} switchNode
* @returns {boolean}
*/
function isExhaustiveSwitch(context, switchNode) {
Comment on lines +387 to +392
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

const services = getTSParserServices(context)
if (!services) return false

const { ts, tsNodeMap, checker } = services

const discriminantTSNode = tsNodeMap.get(switchNode.discriminant)
if (!discriminantTSNode) return false

let discriminantType = checker.getTypeAtLocation(discriminantTSNode)
if (!discriminantType) return false
discriminantType =
checker.getBaseConstraintOfType(discriminantType) ?? discriminantType

/** @type {Set<Type>} */
const caseTypes = new Set()
for (const switchCase of switchNode.cases) {
if (switchCase.test === null) return true
const caseTSNode = tsNodeMap.get(switchCase.test)
if (!caseTSNode) return false
let caseType = checker.getTypeAtLocation(caseTSNode)
if (!caseType) return false
caseType = checker.getBaseConstraintOfType(caseType) ?? caseType
caseTypes.add(caseType)
}

let hasLiteralConstituent = false
const caseHasUndefined = [...caseTypes].some(
(t) => (t.flags & ts.TypeFlags.Undefined) !== 0
)
for (const unionPart of unionConstituents(discriminantType)) {
for (const intersectionPart of intersectionConstituents(unionPart)) {
if (!isTypeLiteralLike(intersectionPart, ts)) continue
hasLiteralConstituent = true
if (caseTypes.has(intersectionPart)) continue
// multiple TS objects can represent undefined
if (
caseHasUndefined &&
(intersectionPart.flags & ts.TypeFlags.Undefined) !== 0
) {
continue
}
return false
}
}
if (!hasLiteralConstituent) return false

for (const switchCase of switchNode.cases) {
if (switchCase.consequent.length === 0) continue
const last = switchCase.consequent.at(-1)
if (!last || !hasTerminalReturn(last)) return false
}
return true
}

/**
* Check if a function body's last statement is a type-exhaustive switch
* where every non-fallthrough case returns or throws.
* @param {RuleContext} context The ESLint rule context object.
* @param {ArrowFunctionExpression | FunctionExpression | FunctionDeclaration} node
* @returns {boolean}
*/
function hasExhaustiveSwitchReturn(context, node) {
const body = node.body
const stmts = body && body.type === 'BlockStatement' ? body.body : null
if (!stmts || stmts.length === 0) return false
const lastStmt = stmts.at(-1)
return (
lastStmt != null &&
lastStmt.type === 'SwitchStatement' &&
isExhaustiveSwitch(context, lastStmt)
)
}
25 changes: 25 additions & 0 deletions tests/fixtures/typescript/src/test01.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,28 @@ export type Slots1 = {
default(props: { msg: string }): any
foo(props: { msg: string }): any
}

export type Status = 'active' | 'inactive' | 'pending'
export enum Color {
Red,
Green,
Blue
}
export type NullableKind = 'a' | 'b' | null
export type UndefinedKind = 'a' | 'b' | undefined
export type FullyNullable = 'x' | 'y' | null | undefined
export type NumericUnion = 1 | 2 | 3
export enum StringStatus {
Active = 'active',
Inactive = 'inactive'
}

type Brand<T, B> = T & { __brand: B }
export type BrandedStatus = Brand<'active', 'status'> | Brand<'inactive', 'status'>
export type BigIntUnion = 1n | 2n | 3n
export type MixedLiterals = 0 | 1 | 'two' | true

export type ClickEvent = { type: 'click'; x: number; y: number }
export type HoverEvent = { type: 'hover'; target: string }
export type KeyEvent = { type: 'key'; code: number }
export type AppEvent = ClickEvent | HoverEvent | KeyEvent
106 changes: 106 additions & 0 deletions tests/lib/rules/require-render-return.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,20 @@ ruleTester.run('require-render-return', rule, {
}
}`,
languageOptions
},
// Switch with all cases returning AND a default
{
filename: 'test.vue',
code: `export default {
render() {
switch (this.type) {
case 'a': return h('div')
case 'b': return h('span')
default: return h('p')
}
}
}`,
languageOptions
}
],

Expand Down Expand Up @@ -243,6 +257,98 @@ ruleTester.run('require-render-return', rule, {
endColumn: 15
}
]
},
// JS: Switch with all cases returning but no default — no type info, must error
{
filename: 'test.vue',
code: `export default {
render() {
switch (this.type) {
case 'a': return h('div')
case 'b': return h('span')
}
}
}`,
languageOptions,
errors: [
{
message: 'Expected to return a value in render function.',
line: 2
}
]
},
// JS: Vue.component switch without default — must error
{
code: `Vue.component('test', {
render() {
switch (this.type) {
case 'a': return h('div')
case 'b': return h('span')
}
}
})`,
languageOptions,
errors: [
{
message: 'Expected to return a value in render function.',
line: 2
}
]
},
// Switch where one case uses break
{
filename: 'test.vue',
code: `export default {
render() {
switch (this.type) {
case 'a': return h('div')
case 'b': break
}
}
}`,
languageOptions,
errors: [
{
message: 'Expected to return a value in render function.',
line: 2
}
]
},
// Empty switch
{
filename: 'test.vue',
code: `export default {
render() {
switch (this.type) {
}
}
}`,
languageOptions,
errors: [
{
message: 'Expected to return a value in render function.',
line: 2
}
]
},
// Switch with only fallthrough cases
{
filename: 'test.vue',
code: `export default {
render() {
switch (this.type) {
case 'a':
case 'b':
}
}
}`,
languageOptions,
errors: [
{
message: 'Expected to return a value in render function.',
line: 2
}
]
}
]
})
Loading