Skip to content

apply 'noImplicitReturns' rule for functions that don't have type an… #5733

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

Merged
merged 2 commits into from
Nov 25, 2015
Merged
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
54 changes: 27 additions & 27 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9811,17 +9811,25 @@ namespace ts {
return aggregatedTypes;
}

// TypeScript Specification 1.0 (6.3) - July 2014
// An explicitly typed function whose return type isn't the Void or the Any type
// must have at least one return statement somewhere in its body.
// An exception to this rule is if the function implementation consists of a single 'throw' statement.
/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void or the Any type
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
* @param returnType - return type of the function, can be undefined if return type is not explicitly specified
*/
function checkAllCodePathsInNonVoidFunctionReturnOrThrow(func: FunctionLikeDeclaration, returnType: Type): void {
if (!produceDiagnostics) {
return;
}

// Functions that return 'void' or 'any' don't need any return expressions.
if (returnType === voidType || isTypeAny(returnType)) {
// Functions with with an explicitly specified 'void' or 'any' return type don't need any return expressions.
Copy link
Member

Choose a reason for hiding this comment

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

duplicate 'with with'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (returnType && (returnType === voidType || isTypeAny(returnType))) {
Copy link
Member

Choose a reason for hiding this comment

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

What about a Promise<void> in an async function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for async functions returnType is contain promised type so in case of Promise<void> return type will be just void

return;
}

// if return type is not specified then we'll do the check only if 'noImplicitReturns' option is set
if (!returnType && !compilerOptions.noImplicitReturns) {
return;
}

Expand All @@ -9831,13 +9839,14 @@ namespace ts {
return;
}

if (func.flags & NodeFlags.HasExplicitReturn) {
if (!returnType || func.flags & NodeFlags.HasExplicitReturn) {
if (compilerOptions.noImplicitReturns) {
error(func.type, Diagnostics.Not_all_code_paths_return_a_value);
error(func.type || func, Diagnostics.Not_all_code_paths_return_a_value);
}
}
else {
// This function does not conform to the specification.
// NOTE: having returnType !== undefined is a precondition for entering this branch so func.type will always be present
error(func.type, Diagnostics.A_function_whose_declared_type_is_neither_void_nor_any_must_return_a_value);
}
}
Expand Down Expand Up @@ -9912,14 +9921,10 @@ namespace ts {
emitAwaiter = true;
}

const returnType = node.type && getTypeFromTypeNode(node.type);
let promisedType: Type;
if (returnType && isAsync) {
promisedType = checkAsyncFunctionReturnType(node);
}

if (returnType && !node.asteriskToken) {
checkAllCodePathsInNonVoidFunctionReturnOrThrow(node, isAsync ? promisedType : returnType);
const returnOrPromisedType = node.type && (isAsync ? checkAsyncFunctionReturnType(node) : getTypeFromTypeNode(node.type));
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a couple of places where we have to choose between checkAsyncFunctionReturnType and getTypeFromTypeNode. I think it would be a good idea to have a getFunctionReturnType that just wraps the decision. (And it's quite possible that additional calls to getTypeFromTypeNode need to switch.)

Copy link
Member

Choose a reason for hiding this comment

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

Currently, it looks like these are the only two calls to checkAsyncFunctionReturnType, so perhaps it should just have a name change.

if (!node.asteriskToken) {
// return is not necessary in the body of generators
checkAllCodePathsInNonVoidFunctionReturnOrThrow(node, returnOrPromisedType);
}

if (node.body) {
Expand All @@ -9942,13 +9947,13 @@ namespace ts {
// check assignability of the awaited type of the expression body against the promised type of
// its return type annotation.
const exprType = checkExpression(<Expression>node.body);
if (returnType) {
if (returnOrPromisedType) {
if (isAsync) {
const awaitedType = checkAwaitedType(exprType, node.body, Diagnostics.Expression_body_for_async_arrow_function_does_not_have_a_valid_callable_then_member);
checkTypeAssignableTo(awaitedType, promisedType, node.body);
checkTypeAssignableTo(awaitedType, returnOrPromisedType, node.body);
}
else {
checkTypeAssignableTo(exprType, returnType, node.body);
checkTypeAssignableTo(exprType, returnOrPromisedType, node.body);
}
}

Expand Down Expand Up @@ -12088,14 +12093,9 @@ namespace ts {
}

checkSourceElement(node.body);
if (node.type && !isAccessor(node.kind) && !node.asteriskToken) {
const returnType = getTypeFromTypeNode(node.type);
let promisedType: Type;
if (isAsync) {
promisedType = checkAsyncFunctionReturnType(node);
}

checkAllCodePathsInNonVoidFunctionReturnOrThrow(node, isAsync ? promisedType : returnType);
if (!isAccessor(node.kind) && !node.asteriskToken) {
const returnOrPromisedType = node.type && (isAsync ? checkAsyncFunctionReturnType(node) : getTypeFromTypeNode(node.type));
checkAllCodePathsInNonVoidFunctionReturnOrThrow(node, returnOrPromisedType);
}

if (produceDiagnostics && !node.type) {
Expand Down
162 changes: 162 additions & 0 deletions tests/baselines/reference/reachabilityChecks6.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
tests/cases/compiler/reachabilityChecks6.ts(6,10): error TS7030: Not all code paths return a value.
tests/cases/compiler/reachabilityChecks6.ts(19,10): error TS7030: Not all code paths return a value.
tests/cases/compiler/reachabilityChecks6.ts(31,10): error TS7030: Not all code paths return a value.
tests/cases/compiler/reachabilityChecks6.ts(41,10): error TS7030: Not all code paths return a value.
tests/cases/compiler/reachabilityChecks6.ts(52,10): error TS7030: Not all code paths return a value.
tests/cases/compiler/reachabilityChecks6.ts(80,10): error TS7030: Not all code paths return a value.
tests/cases/compiler/reachabilityChecks6.ts(86,13): error TS7027: Unreachable code detected.
tests/cases/compiler/reachabilityChecks6.ts(94,10): error TS7030: Not all code paths return a value.
tests/cases/compiler/reachabilityChecks6.ts(116,10): error TS7030: Not all code paths return a value.
tests/cases/compiler/reachabilityChecks6.ts(123,13): error TS7027: Unreachable code detected.


==== tests/cases/compiler/reachabilityChecks6.ts (10 errors) ====

function f0(x) {
while (true);
}

function f1(x) {
~~
!!! error TS7030: Not all code paths return a value.
if (x) {
return 1
}
}

function f2(x) {
while (x) {
throw new Error();
}
return 1;
}

function f3(x) {
~~
!!! error TS7030: Not all code paths return a value.
while (x) {
throw new Error();
}
}

function f3_1 (x) {
while (x) {
}
throw new Error();
}

function f4(x) {
~~
!!! error TS7030: Not all code paths return a value.
try {
if (x) {
return 1;
}
}
catch (e) {
}
}

function f5(x) {
~~
!!! error TS7030: Not all code paths return a value.
try {
if (x) {
return 1;
}
}
catch (e) {
return 2;
}
}

function f6(x) {
~~
!!! error TS7030: Not all code paths return a value.
try {
if (x) {
return 1;
}
else
{
throw new Error();
}
}
catch (e) {
}
}

function f7(x) {
try {
if (x) {
return 1;
}
else {
throw new Error();
}
}
catch (e) {
return 1;
}
}

function f8(x) {
~~
!!! error TS7030: Not all code paths return a value.
try {
if (true) {
x++;
}
else {
return 1;
~~~~~~
!!! error TS7027: Unreachable code detected.
}
}
catch (e) {
return 1;
}
}

function f9(x) {
~~
!!! error TS7030: Not all code paths return a value.
try {
while (false) {
return 1;
}
}
catch (e) {
return 1;
}
}

function f10(x) {
try {
do {
x++;
} while (true);
}
catch (e) {
return 1;
}
}

function f11(x) {
~~~
!!! error TS7030: Not all code paths return a value.
test:
try {
do {
do {
break test;
} while (true);
x++;
~
!!! error TS7027: Unreachable code detected.
} while (true);
}
catch (e) {
return 1;
}
}
Loading