-
-
Notifications
You must be signed in to change notification settings - Fork 706
fix: false positive for returns in exhaustive switch #3067
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
base: master
Are you sure you want to change the base?
Changes from all commits
5ee70d0
e867508
fe54871
09c7b2e
421d9f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,8 @@ module.exports = { | |
| getComponentPropsFromTypeDefineTypes, | ||
| getComponentEmitsFromTypeDefineTypes, | ||
| getComponentSlotsFromTypeDefineTypes, | ||
| inferRuntimeTypeFromTypeNode | ||
| inferRuntimeTypeFromTypeNode, | ||
| hasExhaustiveSwitchReturn | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inlined TS utility functions from |
||
|
|
||
| /** | ||
| * @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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is derived from getSwitchMetadata in switch-exhaustiveness-check.ts |
||
| 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) | ||
| ) | ||
| } | ||
There was a problem hiding this comment.
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?