diff --git a/src/language/utils.ts b/src/language/utils.ts index ad609d32922..837436d58df 100644 --- a/src/language/utils.ts +++ b/src/language/utils.ts @@ -93,11 +93,13 @@ export function someAncestor(node: ts.Node, predicate: (n: ts.Node) => boolean): return predicate(node) || (node.parent !== undefined && someAncestor(node.parent, predicate)); } -export function ancestorWhere(node: ts.Node, predicate: (n: ts.Node) => boolean): ts.Node | undefined { +export function ancestorWhere(node: ts.Node, predicate: (n: ts.Node) => n is T): T | undefined; +export function ancestorWhere(node: ts.Node, predicate: (n: ts.Node) => boolean): ts.Node | undefined; +export function ancestorWhere(node: ts.Node, predicate: (n: ts.Node) => n is T): T | undefined { let cur: ts.Node | undefined = node; do { if (predicate(cur)) { - return cur as T; + return cur; } cur = cur.parent; } while (cur !== undefined); diff --git a/src/rules/returnUndefinedRule.ts b/src/rules/returnUndefinedRule.ts index db2b75554e7..850921be531 100644 --- a/src/rules/returnUndefinedRule.ts +++ b/src/rules/returnUndefinedRule.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { isIdentifier, isReturnStatement, isUnionType } from "tsutils"; +import { isIdentifier, isReturnStatement, isTypeReference, isUnionType } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; @@ -58,7 +58,7 @@ function walk(ctx: Lint.WalkContext, checker: ts.TypeChecker) { return; } - const functionReturningFrom = Lint.ancestorWhere(node, isFunctionLike) as FunctionLike | undefined; + const functionReturningFrom = Lint.ancestorWhere(node, isFunctionLike); if (functionReturningFrom === undefined) { // Return outside of function is invalid return; @@ -108,24 +108,32 @@ function getReturnKind(node: FunctionLike, checker: ts.TypeChecker): ReturnKind return ReturnKind.Value; } - const contextual = isFunctionExpressionLike(node) ? tryGetReturnType(checker.getContextualType(node), checker) : undefined; + const contextual = isFunctionExpressionLike(node) && node.type === undefined + ? tryGetReturnType(checker.getContextualType(node), checker) + : undefined; const returnType = contextual !== undefined ? contextual : tryGetReturnType(checker.getTypeAtLocation(node), checker); - if (returnType === undefined) { + if (returnType === undefined || Lint.isTypeFlagSet(returnType, ts.TypeFlags.Any)) { return undefined; - } else if (isEffectivelyVoid(returnType)) { + } + if ((Lint.hasModifier(node.modifiers, ts.SyntaxKind.AsyncKeyword) ? isEffectivelyVoidPromise : isEffectivelyVoid)(returnType)) { return ReturnKind.Void; - } else if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.AsyncKeyword)) { - // Would need access to `checker.getPromisedTypeOfPromise` to do this properly. - // Assume that the return type is the global Promise (since this is an async function) and get its type argument. - const typeArguments = (returnType as ts.GenericType).typeArguments; - if (typeArguments !== undefined && typeArguments.length === 1) { - return isEffectivelyVoid(typeArguments[0]) ? ReturnKind.Void : ReturnKind.Value; - } } return ReturnKind.Value; } +/** True for `void`, `undefined`, Promise, or `void | undefined | Promise`. */ +function isEffectivelyVoidPromise(type: ts.Type): boolean { + // Would need access to `checker.getPromisedTypeOfPromise` to do this properly. + // Assume that the return type is the global Promise (since this is an async function) and get its type argument. + + // tslint:disable-next-line no-bitwise + return Lint.isTypeFlagSet(type, ts.TypeFlags.Void | ts.TypeFlags.Undefined) || + isUnionType(type) && type.types.every(isEffectivelyVoidPromise) || + isTypeReference(type) && type.typeArguments !== undefined && type.typeArguments.length === 1 && + isEffectivelyVoidPromise(type.typeArguments[0]); +} + /** True for `void`, `undefined`, or `void | undefined`. */ function isEffectivelyVoid(type: ts.Type): boolean { // tslint:disable-next-line no-bitwise @@ -143,8 +151,7 @@ function tryGetReturnType(fnType: ts.Type | undefined, checker: ts.TypeChecker): return undefined; } - const ret = checker.getReturnTypeOfSignature(sigs[0]); - return Lint.isTypeFlagSet(ret, ts.TypeFlags.Any) ? undefined : ret; + return checker.getReturnTypeOfSignature(sigs[0]); } function isFunctionLike(node: ts.Node): node is FunctionLike { diff --git a/test/rules/return-undefined/test.ts.lint b/test/rules/return-undefined/test.ts.lint index 2ab5a71c20d..a14f014baf3 100644 --- a/test/rules/return-undefined/test.ts.lint +++ b/test/rules/return-undefined/test.ts.lint @@ -36,17 +36,19 @@ badContextualType(() => { ~~~~~~~ [UNDEFINED] }); -// 'any' doesn't provide a contextual type, so assumes meant to return 'void'. +// Allow anything in callback taking 'any'. function takesAnyCb(cb: () => any): void; takesAnyCb(() => { return; }); takesAnyCb(() => { return undefined; }); - ~~~~~~~~~~~~~~~~~ [VOID] takesAnyCb(() => { if (1 === 2) return; - ~~~~~~~ [UNDEFINED] else return 1; }); takesAnyCb(() => { + if (1 === 2) return; + else return undefined; +}); +takesAnyCb((): void => { if (1 === 2) return; else return undefined; ~~~~~~~~~~~~~~~~~ [VOID] @@ -68,5 +70,13 @@ declare function f(action: () => T | undefined): void; f(() => { return undefined; }); ~~~~~~~~~~~~~~~~~ [VOID] +declare function test(cb: () => Promise | void): void; + +test(async () => { + if (1 === 2) return; + else return undefined; + ~~~~~~~~~~~~~~~~~ [VOID] +}); + [VOID]: `void` function should use `return;`, not `return undefined;`. [UNDEFINED]: Value-returning function should use `return undefined;`, not just `return;`.