fix #25487, infer block lambdas from return statements

pkg/analyzer/lib/src/generated/static_type_analyzer.dart

Index: pkg/analyzer/lib/src/generated/static_type_analyzer.dart
diff --git a/pkg/analyzer/lib/src/generated/static_type_analyzer.dart b/pkg/analyzer/lib/src/generated/static_type_analyzer.dart
index cc8e2f06e5ca9c4f409d14c684c20f332974cef1..82b3796c645d51ad12bfca232075775245660309 100644
--- a/pkg/analyzer/lib/src/generated/static_type_analyzer.dart
+++ b/pkg/analyzer/lib/src/generated/static_type_analyzer.dart
@@ -461,10 +461,21 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
       // node.
       return null;
     }
+    bool recordInference = false;
     ExecutableElementImpl functionElement =
         node.element as ExecutableElementImpl;
     DartType computedType = _computeStaticReturnTypeOfFunctionExpression(node);
     if (_strongMode) {
+      // Strong mode can infer the type of a block function body.
+      FunctionBody body = node.body;
+      if (body is BlockFunctionBody) {
+        DartType inferredType = InferenceContext.getType(body);

[Leaf, 2016/02/23 02:17:47]

It bothers me a little bit that this segment of code partially overlaps with the
original downwards inference code below.  I think (but could be wrong) that
these two can be folded together.  

This code path here will fire if we were doing downwards inference on a block
function body, or if we successfully did upwards inference.

The code path below will fire if we were doing downwards inference (for either
kind of body).  I believe that the second code path is entirely redundant in the
downwards inference case.  This seems like an infelicity in the existing code -
it basically redoes _resolver.matchFunctionTypeParameters to compute the
function type.  This should, I think, succeed iff the call to it in
resolver.visitFunctionExpression succeeded, and if that call succeeds, then we
should have that returnType == _computeReturnTypeOfFunction(body,
getType(body)).

So it really seems like we ought to be able to just fold these two together, and
do something uniform whenever we there is a type recorded on the body.  Does
this make sense?

[Jennifer Messerly, 2016/02/23 17:41:46]

Yeah, makes sense. I was having the same thoughts but wasn't sure how much
refactoring to get into. It's funny that Resolver.visitFunctionExpression will
do almost the same logic as below, then call
"super.visitFunctionExpression(node);" which takes us right back into this
method (although we did visit children in the mean time, so it's not completely
useless). But yeah, there's got to be a way to simplify it. I'll play around
with it. I like your idea of just removing the redundant code below, that just
might work. :)

[Jennifer Messerly, 2016/02/23 22:16:21]

Ah, I remembered one issue. This code isn't integrated with
_computeStaticReturnTypeOfFunctionExpression so we can record that inference
happened. I'll see about restructuring it.

[Jennifer Messerly, 2016/02/24 00:20:41]

Done.

+        if (inferredType != null) {
+          computedType = _computeReturnTypeOfFunction(body, inferredType);
+          recordInference = true;
+        }
+      }
+
       // In strong mode, we don't want to allow the function's return type to
       // be bottom. If the surrounding context has a more precise type, we
       // will push it down with inference, below. If not we want to use dynamic.
@@ -484,14 +495,17 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
           if (computedType.isDynamic &&
               !(returnType.isDynamic || returnType.isBottom)) {
             computedType = returnType;
-            _resolver.inferenceContext.recordInference(node, functionType);
+            recordInference = true;
           }
         }
       }
     }
     functionElement.returnType = computedType;
     _recordPropagatedTypeOfFunction(functionElement, node.body);
-    _recordStaticType(node, node.element.type);
+    _recordStaticType(node, functionElement.type);
+    if (recordInference) {
+      _resolver.inferenceContext.recordInference(node, functionElement.type);
+    }
     return null;
   }
 
@@ -1473,6 +1487,27 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
   }
 
   /**
+   * Given a function body and its return type, compute the return type of
+   * the entire function, taking into account whether the function body
+   * is `sync*`, `async` or `async*`.
+   *
+   * See also [FunctionBody.isAsynchronous], [FunctionBody.isGenerator].
+   */
+  DartType _computeReturnTypeOfFunction(FunctionBody body, DartType type) {
+    if (body.isGenerator) {
+      InterfaceType genericType = body.isAsynchronous
+          ? _typeProvider.streamType
+          : _typeProvider.iterableType;
+      return genericType.substitute4(<DartType>[type]);
+    } else if (body.isAsynchronous) {
+      return _typeProvider.futureType
+          .substitute4(<DartType>[type.flattenFutures(_typeSystem)]);
+    } else {
+      return type;
+    }
+  }
+
+  /**
    * Compute the static return type of the method or function represented by the given element.
    *
    * @param element the element representing the method or function invoked by the given node
@@ -1525,25 +1560,14 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
   DartType _computeStaticReturnTypeOfFunctionExpression(
       FunctionExpression node) {
     FunctionBody body = node.body;
-    if (body.isGenerator) {
-      if (body.isAsynchronous) {
-        return _typeProvider.streamDynamicType;
-      } else {
-        return _typeProvider.iterableDynamicType;
-      }
-    }
+
     DartType type;
     if (body is ExpressionFunctionBody) {
       type = _getStaticType(body.expression);
     } else {
       type = _dynamicType;
     }
-    if (body.isAsynchronous) {
-      return _typeProvider.futureType
-          .substitute4(<DartType>[type.flattenFutures(_typeSystem)]);
-    } else {
-      return type;
-    }
+    return _computeReturnTypeOfFunction(body, type);
   }
 
   DartType _findIteratedType(DartType type, DartType targetType) {