-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(eslint-plugin): [no-misused-promises] check subtype methods against heritage type methods #8765
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
feat(eslint-plugin): [no-misused-promises] check subtype methods against heritage type methods #8765
Changes from all commits
5bc1c5f
8a75efa
24283e0
0936f3b
3cc1c0d
7146bce
87c6947
b3c477e
714fffe
1f09c9b
32dd7f7
36b460c
a7b4e1f
3c9c945
c71dfca
62cc008
330b8d4
ce240c7
ab6174c
93ce983
a175db7
74d421d
d6cfd24
eca836f
fbcc229
f10b62d
6944e66
8304cef
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 |
---|---|---|
|
@@ -23,6 +23,7 @@ type Options = [ | |
interface ChecksVoidReturnOptions { | ||
arguments?: boolean; | ||
attributes?: boolean; | ||
inheritedMethods?: boolean; | ||
properties?: boolean; | ||
returns?: boolean; | ||
variables?: boolean; | ||
|
@@ -33,6 +34,7 @@ type MessageId = | |
| 'spread' | ||
| 'voidReturnArgument' | ||
| 'voidReturnAttribute' | ||
| 'voidReturnInheritedMethod' | ||
| 'voidReturnProperty' | ||
| 'voidReturnReturnValue' | ||
| 'voidReturnVariable'; | ||
|
@@ -49,6 +51,7 @@ function parseChecksVoidReturn( | |
return { | ||
arguments: true, | ||
attributes: true, | ||
inheritedMethods: true, | ||
properties: true, | ||
returns: true, | ||
variables: true, | ||
|
@@ -58,6 +61,7 @@ function parseChecksVoidReturn( | |
return { | ||
arguments: checksVoidReturn.arguments ?? true, | ||
attributes: checksVoidReturn.attributes ?? true, | ||
inheritedMethods: checksVoidReturn.inheritedMethods ?? true, | ||
properties: checksVoidReturn.properties ?? true, | ||
returns: checksVoidReturn.returns ?? true, | ||
variables: checksVoidReturn.variables ?? true, | ||
|
@@ -76,14 +80,16 @@ export default createRule<Options, MessageId>({ | |
messages: { | ||
voidReturnArgument: | ||
'Promise returned in function argument where a void return was expected.', | ||
voidReturnVariable: | ||
'Promise-returning function provided to variable where a void return was expected.', | ||
voidReturnAttribute: | ||
'Promise-returning function provided to attribute where a void return was expected.', | ||
voidReturnInheritedMethod: | ||
"Promise-returning method provided where a void return was expected by extended/implemented type '{{ heritageTypeName }}'.", | ||
voidReturnProperty: | ||
'Promise-returning function provided to property where a void return was expected.', | ||
voidReturnReturnValue: | ||
'Promise-returning function provided to return value where a void return was expected.', | ||
voidReturnAttribute: | ||
'Promise-returning function provided to attribute where a void return was expected.', | ||
voidReturnVariable: | ||
'Promise-returning function provided to variable where a void return was expected.', | ||
alythobani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
conditional: 'Expected non-Promise value in a boolean conditional.', | ||
spread: 'Expected a non-Promise value to be spreaded in an object.', | ||
}, | ||
|
@@ -103,6 +109,7 @@ export default createRule<Options, MessageId>({ | |
properties: { | ||
arguments: { type: 'boolean' }, | ||
attributes: { type: 'boolean' }, | ||
inheritedMethods: { type: 'boolean' }, | ||
properties: { type: 'boolean' }, | ||
returns: { type: 'boolean' }, | ||
variables: { type: 'boolean' }, | ||
|
@@ -156,6 +163,11 @@ export default createRule<Options, MessageId>({ | |
...(checksVoidReturn.attributes && { | ||
JSXAttribute: checkJSXAttribute, | ||
}), | ||
...(checksVoidReturn.inheritedMethods && { | ||
ClassDeclaration: checkClassLikeOrInterfaceNode, | ||
ClassExpression: checkClassLikeOrInterfaceNode, | ||
TSInterfaceDeclaration: checkClassLikeOrInterfaceNode, | ||
}), | ||
...(checksVoidReturn.properties && { | ||
Property: checkProperty, | ||
}), | ||
|
@@ -466,6 +478,71 @@ export default createRule<Options, MessageId>({ | |
} | ||
} | ||
|
||
function checkClassLikeOrInterfaceNode( | ||
node: | ||
| TSESTree.ClassDeclaration | ||
| TSESTree.ClassExpression | ||
| TSESTree.TSInterfaceDeclaration, | ||
): void { | ||
const tsNode = services.esTreeNodeToTSNodeMap.get(node); | ||
|
||
const heritageTypes = getHeritageTypes(checker, tsNode); | ||
if (!heritageTypes?.length) { | ||
return; | ||
} | ||
|
||
for (const nodeMember of tsNode.members) { | ||
const memberName = nodeMember.name?.getText(); | ||
if (memberName === undefined) { | ||
// Call/construct/index signatures don't have names. TS allows call signatures to mismatch, | ||
// and construct signatures can't be async. | ||
// TODO - Once we're able to use `checker.isTypeAssignableTo` (v8), we can check an index | ||
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. v8 is upon us! Though of course this PR has been in the works since long before that. @alythobani, let's merge as-is, but would you mind filing a followup (that we now know will not be blocked 🙂) 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. Sweet, yeah definitely! Do you mean filing an issue for this? And I can start looking into a proper implementation too if you'd like |
||
// signature here against its compatible index signatures in `heritageTypes` | ||
continue; | ||
} | ||
JoshuaKGoldberg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!returnsThenable(checker, nodeMember)) { | ||
continue; | ||
} | ||
for (const heritageType of heritageTypes) { | ||
checkHeritageTypeForMemberReturningVoid( | ||
nodeMember, | ||
heritageType, | ||
memberName, | ||
); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Checks `heritageType` for a member named `memberName` that returns void; reports the | ||
* 'voidReturnInheritedMethod' message if found. | ||
* @param nodeMember Node member that returns a Promise | ||
* @param heritageType Heritage type to check against | ||
* @param memberName Name of the member to check for | ||
*/ | ||
function checkHeritageTypeForMemberReturningVoid( | ||
nodeMember: ts.Node, | ||
heritageType: ts.Type, | ||
memberName: string, | ||
): void { | ||
const heritageMember = getMemberIfExists(heritageType, memberName); | ||
if (heritageMember === undefined) { | ||
return; | ||
} | ||
const memberType = checker.getTypeOfSymbolAtLocation( | ||
heritageMember, | ||
nodeMember, | ||
); | ||
if (!isVoidReturningFunctionType(checker, nodeMember, memberType)) { | ||
return; | ||
} | ||
context.report({ | ||
node: services.tsNodeToESTreeNodeMap.get(nodeMember), | ||
messageId: 'voidReturnInheritedMethod', | ||
data: { heritageTypeName: checker.typeToString(heritageType) }, | ||
}); | ||
} | ||
|
||
function checkJSXAttribute(node: TSESTree.JSXAttribute): void { | ||
if ( | ||
node.value == null || | ||
|
@@ -777,3 +854,26 @@ function returnsThenable(checker: ts.TypeChecker, node: ts.Node): boolean { | |
.unionTypeParts(type) | ||
.some(t => anySignatureIsThenableType(checker, node, t)); | ||
} | ||
|
||
function getHeritageTypes( | ||
checker: ts.TypeChecker, | ||
tsNode: ts.ClassDeclaration | ts.ClassExpression | ts.InterfaceDeclaration, | ||
): ts.Type[] | undefined { | ||
return tsNode.heritageClauses | ||
?.flatMap(clause => clause.types) | ||
.map(typeExpression => checker.getTypeAtLocation(typeExpression)); | ||
} | ||
|
||
/** | ||
* @returns The member with the given name in `type`, if it exists. | ||
*/ | ||
function getMemberIfExists( | ||
type: ts.Type, | ||
memberName: string, | ||
): ts.Symbol | undefined { | ||
const escapedMemberName = ts.escapeLeadingUnderscores(memberName); | ||
const symbolMemberMatch = type.getSymbol()?.members?.get(escapedMemberName); | ||
return ( | ||
symbolMemberMatch ?? tsutils.getPropertyOfType(type, escapedMemberName) | ||
); | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.