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

feat(eslint-plugin): [no-misused-promises] check more places for checksVoidReturn #4541

Merged
merged 22 commits into from Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9598b4d
refactor(eslint-plugin): create isVoidReturningFunctionType
uhyo Feb 12, 2022
2d0f9d8
feat(eslint-plugin): add checkAssignments and checkVariableDeclaration
uhyo Feb 12, 2022
96ae88d
test(eslint-plugin): add valid test cases
uhyo Feb 12, 2022
5d39edd
feat(eslint-plugin): add object property checks
uhyo Feb 12, 2022
18811b2
feat(eslint-plugin): add checkReturnStatements
uhyo Feb 12, 2022
733ad0f
feat(eslint-plugin): add checkJSXAttributes
uhyo Feb 12, 2022
7de026d
Merge branch 'main' into fix/4523
uhyo Feb 12, 2022
3585e63
fix(website): resolve newly introduced lint errors
uhyo Feb 12, 2022
88fff21
test(eslint-plugin): worship code coverage
uhyo Feb 12, 2022
c9f3fc9
Merge branch 'main' into fix/4523
uhyo Feb 13, 2022
99997da
Merge branch 'main' into fix/4523
uhyo Feb 14, 2022
e017c70
Merge branch 'main' into fix/4523
uhyo Feb 16, 2022
6ff0ea4
Merge branch 'main' into fix/4523
uhyo Feb 24, 2022
e82b964
Apply suggestions from code review
uhyo Feb 24, 2022
9c4bcfd
fix(eslint-plugin): rename checker functions to singular
uhyo Feb 24, 2022
dc452e3
fix(eslint-plugin): align error messages
uhyo Feb 24, 2022
b59abc4
refactor(eslint-plugin): change MessageId
uhyo Feb 24, 2022
deae755
refactor(eslint-plugin): fix if statements style
uhyo Feb 24, 2022
691cc2c
refactor(eslint-plugin): no need of getTypeAtLocation
uhyo Feb 24, 2022
f277f83
refactor(eslint-plugin): make coverage 100%
uhyo Feb 24, 2022
eadaaa8
refactor(eslint-plugin): update comment
uhyo Feb 24, 2022
42701f5
Merge branch 'main' into fix/4523
JoshuaKGoldberg Feb 24, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
228 changes: 204 additions & 24 deletions packages/eslint-plugin/src/rules/no-misused-promises.ts
Expand Up @@ -11,7 +11,15 @@ type Options = [
},
];

export default util.createRule<Options, 'conditional' | 'voidReturn'>({
type MessageId =
| 'conditional'
| 'voidReturnArgument'
| 'voidReturnVariable'
| 'voidReturnProperty'
| 'voidReturnReturnValue'
| 'voidReturnAttribute';

export default util.createRule<Options, MessageId>({
name: 'no-misused-promises',
meta: {
docs: {
Expand All @@ -20,8 +28,16 @@ export default util.createRule<Options, 'conditional' | 'voidReturn'>({
requiresTypeChecking: true,
},
messages: {
voidReturn:
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.',
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.',
conditional: 'Expected non-Promise value in a boolean conditional.',
},
schema: [
Expand Down Expand Up @@ -67,6 +83,11 @@ export default util.createRule<Options, 'conditional' | 'voidReturn'>({
const voidReturnChecks: TSESLint.RuleListener = {
CallExpression: checkArguments,
NewExpression: checkArguments,
AssignmentExpression: checkAssignment,
VariableDeclarator: checkVariableDeclaration,
Property: checkProperty,
ReturnStatement: checkReturnStatement,
JSXAttribute: checkJSXAttribute,
};

function checkTestConditional(node: {
Expand Down Expand Up @@ -130,13 +151,168 @@ export default util.createRule<Options, 'conditional' | 'voidReturn'>({
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(argument);
if (returnsThenable(checker, tsNode as ts.Expression)) {
context.report({
messageId: 'voidReturn',
messageId: 'voidReturnArgument',
node: argument,
});
}
}
}

function checkAssignment(node: TSESTree.AssignmentExpression): void {
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
const varType = checker.getTypeAtLocation(tsNode.left);
if (!isVoidReturningFunctionType(checker, tsNode.left, varType)) {
return;
}

if (returnsThenable(checker, tsNode.right)) {
context.report({
messageId: 'voidReturnVariable',
node: node.right,
});
}
}

function checkVariableDeclaration(node: TSESTree.VariableDeclarator): void {
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
if (tsNode.initializer === undefined || node.init === null) {
return;
}
const varType = checker.getTypeAtLocation(tsNode.name);
if (!isVoidReturningFunctionType(checker, tsNode.initializer, varType)) {
return;
}

if (returnsThenable(checker, tsNode.initializer)) {
context.report({
messageId: 'voidReturnVariable',
node: node.init,
});
}
}

function checkProperty(node: TSESTree.Property): void {
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
if (ts.isPropertyAssignment(tsNode)) {
const contextualType = checker.getContextualType(tsNode.initializer);
if (
contextualType !== undefined &&
isVoidReturningFunctionType(
checker,
tsNode.initializer,
contextualType,
) &&
returnsThenable(checker, tsNode.initializer)
) {
context.report({
messageId: 'voidReturnProperty',
node: node.value,
});
}
} else if (ts.isShorthandPropertyAssignment(tsNode)) {
const contextualType = checker.getContextualType(tsNode.name);
if (
contextualType !== undefined &&
isVoidReturningFunctionType(checker, tsNode.name, contextualType) &&
returnsThenable(checker, tsNode.name)
) {
context.report({
messageId: 'voidReturnProperty',
node: node.value,
});
}
} else if (ts.isMethodDeclaration(tsNode)) {
if (ts.isComputedPropertyName(tsNode.name)) {
return;
}
const obj = tsNode.parent;

// Below condition isn't satisfied unless something goes wrong,
// but is needed for type checking.
// 'node' does not include class method declaration so 'obj' is
// always an object literal expression, but after converting 'node'
// to TypeScript AST, its type includes MethodDeclaration which
// does include the case of class method declaration.
if (!ts.isObjectLiteralExpression(obj)) {
return;
}

const objType = checker.getContextualType(obj);
if (objType === undefined) {
return;
}
const propertySymbol = checker.getPropertyOfType(
objType,
tsNode.name.text,
);
if (propertySymbol === undefined) {
return;
}

const contextualType = checker.getTypeOfSymbolAtLocation(
propertySymbol,
tsNode.name,
);

if (
isVoidReturningFunctionType(checker, tsNode.name, contextualType) &&
returnsThenable(checker, tsNode)
) {
context.report({
messageId: 'voidReturnProperty',
node: node.value,
});
}
return;
}
}

function checkReturnStatement(node: TSESTree.ReturnStatement): void {
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
if (tsNode.expression === undefined || node.argument === null) {
return;
}
const contextualType = checker.getContextualType(tsNode.expression);
if (
contextualType !== undefined &&
isVoidReturningFunctionType(
checker,
tsNode.expression,
contextualType,
) &&
returnsThenable(checker, tsNode.expression)
) {
context.report({
messageId: 'voidReturnReturnValue',
node: node.argument,
});
}
}

function checkJSXAttribute(node: TSESTree.JSXAttribute): void {
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
const value = tsNode.initializer;
if (
node.value === null ||
value === undefined ||
!ts.isJsxExpression(value) ||
value.expression === undefined
) {
return;
}
const contextualType = checker.getContextualType(value);
if (
contextualType !== undefined &&
isVoidReturningFunctionType(checker, value, contextualType) &&
returnsThenable(checker, value.expression)
) {
context.report({
messageId: 'voidReturnAttribute',
node: node.value,
});
}
}

return {
...(checksConditionals ? conditionalChecks : {}),
...(checksVoidReturn ? voidReturnChecks : {}),
Expand Down Expand Up @@ -219,7 +395,6 @@ function voidFunctionParams(
node: ts.CallExpression | ts.NewExpression,
): Set<number> {
const voidReturnIndices = new Set<number>();
const thenableReturnIndices = new Set<number>();
const type = checker.getTypeAtLocation(node.expression);

for (const subType of tsutils.unionTypeParts(type)) {
Expand All @@ -233,36 +408,41 @@ function voidFunctionParams(
parameter,
node.expression,
);
for (const subType of tsutils.unionTypeParts(type)) {
for (const signature of subType.getCallSignatures()) {
const returnType = signature.getReturnType();
if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) {
voidReturnIndices.add(index);
} else if (
tsutils.isThenableType(checker, node.expression, returnType)
) {
thenableReturnIndices.add(index);
}
}
if (isVoidReturningFunctionType(checker, node.expression, type)) {
voidReturnIndices.add(index);
}
}
}
}

// If a certain positional argument accepts both thenable and void returns,
// a promise-returning function is valid
for (const thenable of thenableReturnIndices) {
voidReturnIndices.delete(thenable);
}

return voidReturnIndices;
}

// Returns true if the expression is a function that returns a thenable
function returnsThenable(
// Returns true if given type is a void-returning function.
function isVoidReturningFunctionType(
checker: ts.TypeChecker,
node: ts.Expression,
node: ts.Node,
type: ts.Type,
): boolean {
let hasVoidReturningFunction = false;
let hasThenableReturningFunction = false;
for (const subType of tsutils.unionTypeParts(type)) {
for (const signature of subType.getCallSignatures()) {
const returnType = signature.getReturnType();
if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) {
hasVoidReturningFunction = true;
} else if (tsutils.isThenableType(checker, node, returnType)) {
hasThenableReturningFunction = true;
}
}
}
// If a certain positional argument accepts both thenable and void returns,
// a promise-returning function is valid
return hasVoidReturningFunction && !hasThenableReturningFunction;
}

// Returns true if the expression is a function that returns a thenable
function returnsThenable(checker: ts.TypeChecker, node: ts.Node): boolean {
const type = checker.getApparentType(checker.getTypeAtLocation(node));

for (const subType of tsutils.unionTypeParts(type)) {
Expand Down