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

In effect this fixes #62.

Also
    - Changes the error message for get accessors lacking return expressions.
    - Actually checks for return expressions instead of return statements for get-accessors.
    - Removes fancy quotes.
    - Corrects errors in the compiler caught by the new check.
    - Simplified `checkAndAggregateReturnTypes` by extracting it out to `visitReturnStatements`.

Author: DanielRosenwasser

src/compiler/checker.ts

@@ -3794,7 +3794,7 @@ module ts {
                 }
                 return checkUntypedCall(node);
             }
-            // If FuncExpr’s apparent type(section 3.8.1) is a function type, the call is a typed function call.
+            // If FuncExpr's apparent type(section 3.8.1) is a function type, the call is a typed function call.
             // TypeScript employs overload resolution in typed function calls in order to support functions
             // with multiple call signatures.
             if (!signatures.length) {
@@ -3826,7 +3826,7 @@ module ts {
                 return checkUntypedCall(node);
             }
 
-            // If ConstructExpr’s apparent type(section 3.8.1) is an object type with one or
+            // If ConstructExpr's apparent type(section 3.8.1) is an object type with one or
             // more construct signatures, the expression is processed in the same manner as a
             // function call, but using the construct signatures as the initial set of candidate
             // signatures for overload resolution.The result type of the function call becomes
@@ -3847,7 +3847,7 @@ module ts {
                 return checkCall(node, constructSignatures);
             }
 
-            // If ConstructExpr’s apparent type is an object type with no construct signatures but
+            // If ConstructExpr's apparent type is an object type with no construct signatures but
             // one or more call signatures, the expression is processed as a function call. A compile-time
             // error occurs if the result of the function call is not Void. The type of the result of the
             // operation is Any.
@@ -3931,12 +3931,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);
@@ -3962,16 +3960,18 @@ module ts {
             }
 
             return voidType;
+        }
+
+        // WARNING: This has the same semantics as forEach, in that traversal terminates
+        //          in the event that 'visitor' supplies a truthy value!
+        function visitReturnStatements<T>(body: Block, visitor: (stmt: ReturnStatement) => T): T {
+
+            return visitHelper(body);
 
-            function checkAndAggregateReturnExpressionTypes(node: Node) {
+            function visitHelper(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:
@@ -3988,15 +3988,77 @@ module ts {
                     case SyntaxKind.TryBlock:
                     case SyntaxKind.CatchBlock:
                     case SyntaxKind.FinallyBlock:
-                        forEachChild(node, checkAndAggregateReturnExpressionTypes);
-                        break;
+                        return forEachChild(node, visitHelper);
+                }
+            }
+        }
+
+        /// 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[] = [];
+
+            visitReturnStatements(body, (returnStatement) => {
+                var expr = returnStatement.expression;
+                if (expr) {
+                    var type = checkAndMarkExpression(expr, contextualType, contextualMapper);
+                    if (!contains(aggregatedTypes, type)) {
+                        aggregatedTypes.push(type);
+                    }
                 }
+            });
+
+            return aggregatedTypes;
+        }
+
+        function bodyContainsReturnExpressions(funcBody: Block) {
+            return visitReturnStatements(funcBody, (returnStatement) => {
+                return returnStatement.expression !== undefined;
+            });
+        }
+
+        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(decl: 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 (!decl.body || decl.body.kind !== SyntaxKind.FunctionBlock) {
+                return;
             }
+
+            var bodyBlock = <Block>decl.body;
+
+            // Ensure the body has at least one return expression.
+            if (bodyContainsReturnExpressions(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(decl.type, Diagnostics.A_function_whose_declared_type_is_neither_void_nor_any_must_have_a_return_expression_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);
 
@@ -4014,6 +4076,9 @@ module ts {
                             signature.resolvedReturnType = returnType;
                         }
                     }
+                    else {
+                        checkIfNonVoidFunctionHasReturnExpressionsOrSingleThrowStatment(node, getTypeFromTypeNode(node.type));
+                    }
                 }
                 checkSignatureDeclaration(node);
                 if (node.body.kind === SyntaxKind.FunctionBlock) {
@@ -4586,28 +4651,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 && !(bodyContainsReturnExpressions(<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);
                 }
             }
 
@@ -4843,7 +4889,7 @@ module ts {
                 }
             }
 
-            // TODO: Check at least one return statement in non-void/any function (except single throw)
+            // TODO (drosen): Check at least one return statement in non-void/any function (except single throw)
         }
 
         function checkFunctionDeclaration(node: FunctionDeclaration) {
@@ -4856,7 +4902,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

@@ -113,8 +113,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_have_a_return_expression_or_consist_of_a_single_throw_statement: { code: 2131, category: DiagnosticCategory.Error, key: "A function whose declared type is neither 'void' nor 'any' must have a 'return' expression 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

@@ -446,14 +446,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 have a 'return' expression 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

@@ -1029,7 +1029,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

src/compiler/scanner.ts

@@ -142,18 +142,18 @@ module ts {
         As per ECMAScript Language Specification 3th Edition, Section 7.6: Identifiers
         IdentifierStart ::
             Can contain Unicode 3.0.0  categories:
-            “Uppercase letter (Lu)”,
-            “Lowercase letter (Ll)”,
-            “Titlecase letter (Lt)”,
-            “Modifier letter (Lm)”,
-            “Other letter (Lo)”, or
-            “Letter number (Nl)”.
+            "Uppercase letter (Lu)",
+            "Lowercase letter (Ll)",
+            "Titlecase letter (Lt)",
+            "Modifier letter (Lm)",
+            "Other letter (Lo)", or
+            "Letter number (Nl)".
         IdentifierPart :: =
             Can contain IdentifierStart + Unicode 3.0.0  categories:
-            “Non-spacing mark (Mn)”,
-            “Combining spacing mark (Mc)”,
-            “Decimal number (Nd)”, or
-            “Connector punctuation (Pc)”.
+            "Non-spacing mark (Mn)",
+            "Combining spacing mark (Mc)",
+            "Decimal number (Nd)", or
+            "Connector punctuation (Pc)".
 
         Codepoint ranges for ES3 Identifiers are extracted from the Unicode 3.0.0 specification at:
         http://www.unicode.org/Public/3.0-Update/UnicodeData-3.0.0.txt
@@ -165,18 +165,18 @@ module ts {
         As per ECMAScript Language Specification 5th Edition, Section 7.6: ISyntaxToken Names and Identifiers
         IdentifierStart ::
             Can contain Unicode 6.2  categories:
-            “Uppercase letter (Lu)”,
-            “Lowercase letter (Ll)”,
-            “Titlecase letter (Lt)”,
-            “Modifier letter (Lm)”,
-            “Other letter (Lo)”, or
-            “Letter number (Nl)”.
+            "Uppercase letter (Lu)",
+            "Lowercase letter (Ll)",
+            "Titlecase letter (Lt)",
+            "Modifier letter (Lm)",
+            "Other letter (Lo)", or
+            "Letter number (Nl)".
         IdentifierPart ::
             Can contain IdentifierStart + Unicode 6.2  categories:
-            “Non-spacing mark (Mn)”,
-            “Combining spacing mark (Mc)”,
-            “Decimal number (Nd)”,
-            “Connector punctuation (Pc)”,
+            "Non-spacing mark (Mn)",
+            "Combining spacing mark (Mc)",
+            "Decimal number (Nd)",
+            "Connector punctuation (Pc)",
             <ZWNJ>, or
             <ZWJ>.
 

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 have a 'return' expression 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 have a 'return' expression 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 have a 'return' expression 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 have a 'return' expression 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 have a 'return' expression 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 have a 'return' expression or consist of a single 'throw' statement.
     }
 
     var m;