Merge pull request #147 from Microsoft/noReturnExpression

Complain when a non-void function lacks a return expresson.

Author: DanielRosenwasser

src/compiler/checker.ts

@@ -3972,12 +3972,10 @@ module ts {
             }
 
             // Aggregate the types of expressions within all the return statements.
-            var types: Type[] = [];
-            checkAndAggregateReturnExpressionTypes(func.body);
+            var types = checkAndAggregateReturnExpressionTypes(<Block>func.body, contextualType, contextualMapper);
 
             // Try to return the best common type if we have any return expressions.
-            if (types.length) {
-
+            if (types.length > 0) {
                 var commonType = getBestCommonType(types, /*contextualType:*/ undefined, /*candidatesOnly:*/ true);
                 if (!commonType) {
                     error(func, Diagnostics.No_best_common_type_exists_among_return_expressions);
@@ -4003,16 +4001,18 @@ module ts {
             }
 
             return voidType;
+        }
+
+        // WARNING: This has the same semantics as the forEach family of functions,
+        //          in that traversal terminates in the event that 'visitor' supplies a truthy value.
+        function forEachReturnStatement<T>(body: Block, visitor: (stmt: ReturnStatement) => T): T {
+
+            return traverse(body);
 
-            function checkAndAggregateReturnExpressionTypes(node: Node) {
+            function traverse(node: Node): T {
                 switch (node.kind) {
                     case SyntaxKind.ReturnStatement:
-                        var expr = (<ReturnStatement>node).expression;
-                        if (expr) {
-                            var type = checkAndMarkExpression(expr, contextualType, contextualMapper);
-                            if (!contains(types, type)) types.push(type);
-                        }
-                        break;
+                        return visitor(node);
                     case SyntaxKind.Block:
                     case SyntaxKind.FunctionBlock:
                     case SyntaxKind.IfStatement:
@@ -4029,15 +4029,77 @@ module ts {
                     case SyntaxKind.TryBlock:
                     case SyntaxKind.CatchBlock:
                     case SyntaxKind.FinallyBlock:
-                        forEachChild(node, checkAndAggregateReturnExpressionTypes);
-                        break;
+                        return forEachChild(node, traverse);
                 }
             }
         }
 
+        /// Returns a set of types relating to every return expression relating to a function block.
+        function checkAndAggregateReturnExpressionTypes(body: Block, contextualType?: Type, contextualMapper?: TypeMapper): Type[] {
+            var aggregatedTypes: Type[] = [];
+
+            forEachReturnStatement(body, (returnStatement) => {
+                var expr = returnStatement.expression;
+                if (expr) {
+                    var type = checkAndMarkExpression(expr, contextualType, contextualMapper);
+                    if (!contains(aggregatedTypes, type)) {
+                        aggregatedTypes.push(type);
+                    }
+                }
+            });
+
+            return aggregatedTypes;
+        }
+
+        function bodyContainsAReturnStatement(funcBody: Block) {
+            return forEachReturnStatement(funcBody, (returnStatement) => {
+                return true;
+            });
+        }
+
+        function bodyContainsSingleThrowStatement(body: Block) {
+            return (body.statements.length === 1) && (body.statements[0].kind === SyntaxKind.ThrowStatement)
+        }
+
+        // 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.
+        function checkIfNonVoidFunctionHasReturnExpressionsOrSingleThrowStatment(func: FunctionDeclaration, returnType: Type): void {
+            // Functions that return 'void' or 'any' don't need any return expressions.
+            if (returnType === voidType || returnType === anyType) {
+                return;
+            }
+
+            // If all we have is a function signature, or an arrow function with an expression body, then there is nothing to check.
+            if (!func.body || func.body.kind !== SyntaxKind.FunctionBlock) {
+                return;
+            }
+
+            var bodyBlock = <Block>func.body;
+
+            // Ensure the body has at least one return expression.
+            if (bodyContainsAReturnStatement(bodyBlock)) {
+                return;
+            }
+
+            // If there are no return expressions, then we need to check if
+            // the function body consists solely of a throw statement;
+            // this is to make an exception for unimplemented functions.
+            if (bodyContainsSingleThrowStatement(bodyBlock)) {
+                return;
+            }
+
+            // This function does not conform to the specification.
+            error(func.type, Diagnostics.A_function_whose_declared_type_is_neither_void_nor_any_must_return_a_value_or_consist_of_a_single_throw_statement);
+        }
+
         function checkFunctionExpression(node: FunctionExpression, contextualType?: Type, contextualMapper?: TypeMapper): Type {
             // The identityMapper object is used to indicate that function expressions are wildcards
-            if (contextualMapper === identityMapper) return anyFunctionType;
+            if (contextualMapper === identityMapper) {
+                return anyFunctionType;
+            }
+
             var type = getTypeOfSymbol(node.symbol);
             var links = getNodeLinks(node);
 
@@ -4055,6 +4117,9 @@ module ts {
                             signature.resolvedReturnType = returnType;
                         }
                     }
+                    else {
+                        checkIfNonVoidFunctionHasReturnExpressionsOrSingleThrowStatment(node, getTypeFromTypeNode(node.type));
+                    }
                 }
                 checkSignatureDeclaration(node);
                 if (node.body.kind === SyntaxKind.FunctionBlock) {
@@ -4627,28 +4692,9 @@ module ts {
         }
 
         function checkAccessorDeclaration(node: AccessorDeclaration) {
-            function checkGetterContainsSingleThrowStatement(node: AccessorDeclaration): boolean {
-                var block = <Block>node.body;
-                return block.statements.length === 1 && block.statements[0].kind === SyntaxKind.ThrowStatement;
-            }
-
-            function checkGetterReturnsValue(n: Node): boolean {
-                switch (n.kind) {
-                    case SyntaxKind.ReturnStatement:
-                        return true;
-                    // do not dive into function-like things - return statements there don't count
-                    case SyntaxKind.FunctionExpression:
-                    case SyntaxKind.FunctionDeclaration:
-                    case SyntaxKind.ArrowFunction:
-                    case SyntaxKind.ObjectLiteral:
-                        return false;
-                    default:
-                        return forEachChild(n, checkGetterReturnsValue);
-                }
-            }            
             if (node.kind === SyntaxKind.GetAccessor) {
-                if (!isInAmbientContext(node) && node.body && !(checkGetterContainsSingleThrowStatement(node) || checkGetterReturnsValue(node))) {
-                    error(node.name, Diagnostics.Getters_must_return_a_value);
+                if (!isInAmbientContext(node) && node.body && !(bodyContainsAReturnStatement(<Block>node.body) || bodyContainsSingleThrowStatement(<Block>node.body))) {
+                    error(node.name, Diagnostics.A_get_accessor_must_return_a_value_or_consist_of_a_single_throw_statement);
                 }
             }
 
@@ -4877,8 +4923,6 @@ module ts {
                     }
                 }
             }
-
-            // TODO: Check at least one return statement in non-void/any function (except single throw)
         }
 
         function checkFunctionDeclaration(node: FunctionDeclaration) {
@@ -4890,7 +4934,11 @@ module ts {
             if (node === firstDeclaration) {
                 checkFunctionOrConstructorSymbol(symbol);
             }
+
             checkSourceElement(node.body);
+            if (node.type) {
+                checkIfNonVoidFunctionHasReturnExpressionsOrSingleThrowStatment(node, getTypeFromTypeNode(node.type));
+            }
 
             // If there is no body and no explicit return type, then report an error.
             if (program.getCompilerOptions().noImplicitAny && !node.body && !node.type) {

src/compiler/diagnosticInformationMap.generated.ts

@@ -114,8 +114,9 @@ module ts {
         The_right_hand_side_of_a_for_in_statement_must_be_of_type_any_an_object_type_or_a_type_parameter: { code: 2117, category: DiagnosticCategory.Error, key: "The right-hand side of a 'for...in' statement must be of type 'any', an object type or a type parameter." },
         The_left_hand_side_of_an_in_expression_must_be_of_types_any_string_or_number: { code: 2118, category: DiagnosticCategory.Error, key: "The left-hand side of an 'in' expression must be of types 'any', 'string' or 'number'." },
         The_right_hand_side_of_an_in_expression_must_be_of_type_any_an_object_type_or_a_type_parameter: { code: 2119, category: DiagnosticCategory.Error, key: "The right-hand side of an 'in' expression must be of type 'any', an object type or a type parameter" },
-        Getters_must_return_a_value: { code: 2126, category: DiagnosticCategory.Error, key: "Getters must return a value." },
+        A_get_accessor_must_return_a_value_or_consist_of_a_single_throw_statement: { code: 2126, category: DiagnosticCategory.Error, key: "A 'get' accessor must return a value or consist of a single 'throw' statement." },
         Getter_and_setter_accessors_do_not_agree_in_visibility: { code: 2127, category: DiagnosticCategory.Error, key: "Getter and setter accessors do not agree in visibility." },
+        A_function_whose_declared_type_is_neither_void_nor_any_must_return_a_value_or_consist_of_a_single_throw_statement: { code: 2131, category: DiagnosticCategory.Error, key: "A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement." },
         Untyped_function_calls_may_not_accept_type_arguments: { code: 2158, category: DiagnosticCategory.Error, key: "Untyped function calls may not accept type arguments." },
         The_left_hand_side_of_an_instanceof_expression_must_be_of_type_any_an_object_type_or_a_type_parameter: { code: 2120, category: DiagnosticCategory.Error, key: "The left-hand side of an 'instanceof' expression must be of type 'any', an object type or a type parameter." },
         The_right_hand_side_of_an_instanceof_expression_must_be_of_type_any_or_of_a_type_assignable_to_the_Function_interface_type: { code: 2121, category: DiagnosticCategory.Error, key: "The right-hand side of an 'instanceof' expression must be of type 'any' or of a type assignable to the 'Function' interface type." },

src/compiler/diagnosticMessages.json

@@ -449,14 +449,18 @@
         "category": "Error",
         "code": 2119
     },
-    "Getters must return a value.": {
+    "A 'get' accessor must return a value or consist of a single 'throw' statement.": {
         "category": "Error",
         "code": 2126
     },
     "Getter and setter accessors do not agree in visibility.": {
         "category": "Error",
         "code": 2127
     },
+    "A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.": {
+        "category": "Error",
+        "code": 2131
+    },
     "Untyped function calls may not accept type arguments.": {
         "category": "Error",
         "code": 2158

src/compiler/parser.ts

@@ -1070,7 +1070,7 @@ module ts {
             return finishNode(node);
         }
 
-        function checkIndexSignature(node: SignatureDeclaration, indexerStart: number, indexerLength: number): boolean {
+        function checkIndexSignature(node: SignatureDeclaration, indexerStart: number, indexerLength: number): void {
             var parameter = node.parameters[0];
             if (node.parameters.length !== 1) {
                 var arityDiagnostic = Diagnostics.An_index_signature_must_have_exactly_one_parameter

tests/baselines/reference/ParameterList5.errors.txt

@@ -1,5 +1,7 @@
-==== tests/cases/compiler/ParameterList5.ts (2 errors) ====
+==== tests/cases/compiler/ParameterList5.ts (3 errors) ====
     function A(): (public B) => C {
+                  ~~~~~~~~~~~~~~~
+!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
                    ~~~~~~~~
 !!! A parameter property is only allowed in a constructor implementation.
                                 ~

tests/baselines/reference/ambientGetters.errors.txt

@@ -1,9 +1,11 @@
-==== tests/cases/compiler/ambientGetters.ts (2 errors) ====
+==== tests/cases/compiler/ambientGetters.ts (3 errors) ====
 
     declare class A {
         get length() : number;
                              ~
 !!! '{' expected.
+                       ~~~~~~
+!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
     }
 
     declare class B {

tests/baselines/reference/conflictingTypeAnnotatedVar.errors.txt

@@ -1,8 +1,12 @@
-==== tests/cases/compiler/conflictingTypeAnnotatedVar.ts (2 errors) ====
+==== tests/cases/compiler/conflictingTypeAnnotatedVar.ts (4 errors) ====
     var foo: string;
     function foo(): number { }
              ~~~
 !!! Duplicate identifier 'foo'.
+                    ~~~~~~
+!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
     function foo(): number { }
              ~~~
-!!! Duplicate identifier 'foo'.
+!!! Duplicate identifier 'foo'.
+                    ~~~~~~
+!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.

tests/baselines/reference/errorOnContextuallyTypedReturnType.errors.txt

@@ -1,7 +1,9 @@
-==== tests/cases/compiler/errorOnContextuallyTypedReturnType.ts (1 errors) ====
+==== tests/cases/compiler/errorOnContextuallyTypedReturnType.ts (2 errors) ====
     var n1: () => boolean = function () { }; // expect an error here
         ~~
 !!! Type '() => void' is not assignable to type '() => boolean':
 !!!   Type 'void' is not assignable to type 'boolean'.
     var n2: () => boolean = function ():boolean { }; // expect an error here
+                                        ~~~~~~~
+!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
 

tests/baselines/reference/functionImplementationErrors.errors.txt

@@ -1,4 +1,4 @@
-==== tests/cases/conformance/functions/functionImplementationErrors.ts (6 errors) ====
+==== tests/cases/conformance/functions/functionImplementationErrors.ts (7 errors) ====
     // FunctionExpression with no return type annotation with multiple return statements with unrelated types
     var f1 = function () {
              ~~~~~~~~~~~~~
@@ -47,6 +47,8 @@
 
     // Function implemetnation with non -void return type annotation with no return
     function f5(): number {
+                   ~~~~~~
+!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
     }
 
     var m;

tests/baselines/reference/functionWithThrowButNoReturn1.errors.txt

@@ -0,0 +1,8 @@
+==== tests/cases/compiler/functionWithThrowButNoReturn1.ts (1 errors) ====
+    function fn(): number {
+                   ~~~~~~
+!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
+      throw new Error('NYI');
+      var t;
+    }
+