fix #25487, infer block lambdas from return statements

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

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library analyzer.src.generated.resolver;
 
 import 'dart:collection';
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ skipping 4584 matching lines @@
    * Type provider, needed for type matching.
    */
   final TypeProvider _typeProvider;
 
   /**
    * The type system in use.
    */
   final TypeSystem _typeSystem;
 
   /**
+   * When no context type is available, this will track the least upper bound
+   * of all return statements in a lambda.
+   *
+   * This will always be kept in sync with [_returnStack].
+   */
+  final List<DartType> _inferredReturn = <DartType>[];
+
+  /**
    * A stack of return types for all of the enclosing
    * functions and methods.
    */
   // TODO(leafp) Handle the implicit union type for Futures
   // https://github.com/dart-lang/sdk/issues/25322
-  List<DartType> _returnStack = <DartType>[];
+  final List<DartType> _returnStack = <DartType>[];
 
   InferenceContext._(this._errorListener, TypeProvider typeProvider,
       this._typeSystem, this._inferenceHints)
       : _typeProvider = typeProvider;
 
   /**
    * Get the return type of the current enclosing function, if any.
    *
    * The type returned for a function is the type that is expected
    * to be used in a return or yield context.  For ordinary functions
@@ skipping 10 matching lines @@
    * of t1 of the form `K<S0, ..., Sm>` where `t2 = K<S0', ..., Sm'>`
    * If the supertype exists, use the constraints `S0 <: S0', ... Sm <: Sm'`
    * to derive a concrete instantation for I of the form `<T0, ..., Tn>`,
    * such that `I<T0, .., Tn> <: t2`
    */
   List<DartType> matchTypes(DartType t1, DartType t2) =>
       (t1 is InterfaceType && t2 is InterfaceType) ? _matchTypes(t1, t2) : null;
 
   /**
    * Pop a return type off of the return stack.
+   *
+   * Also record any inferred return type using [setType], unless this node
+   * already has a context type. This recorded type will be the least upper
+   * bound of all types added with [addReturnType].
    */
-  void popReturnContext() {
-    assert(_returnStack.isNotEmpty);
+  void popReturnContext(BlockFunctionBody node) {
+    assert(_returnStack.isNotEmpty && _inferredReturn.isNotEmpty);
     if (_returnStack.isNotEmpty) {
       _returnStack.removeLast();
     }
+    if (_inferredReturn.isNotEmpty) {
+      DartType inferred = _inferredReturn.removeLast();
+      if (!inferred.isBottom) {
+        setType(node, inferred);
+      }
+    }
   }
 
   /**
-   * Push a [returnType] onto the return stack.
+   * Push a block function body's return type onto the return stack.
    */
-  void pushReturnContext(DartType returnType) {
+  void pushReturnContext(BlockFunctionBody node) {
+    DartType returnType = getType(node);
     _returnStack.add(returnType);
+    _inferredReturn.add(BottomTypeImpl.instance);
   }
 
   /**
+   * Records the type of the expression of a return statement.
+   *
+   * This will be used for inferring a block bodied lambda, if no context
+   * type was available.
+   */
+  void addReturnType(DartType type) {
+    if (type == null || _returnStack.isEmpty) {
+      return;
+    }
+    DartType context = _returnStack.last;
+    if (context == null || context.isDynamic) {
+      DartType inferred = _inferredReturn.last;
+      inferred = _typeSystem.getLeastUpperBound(_typeProvider, type, inferred);
+      _inferredReturn[_inferredReturn.length - 1] = inferred;
+    }
+  }
+
+  /**
    * Place an info node into the error stream indicating that a
    * [type] has been inferred as the type of [node].
    */
   void recordInference(Expression node, DartType type) {
     StaticInfo info = InferredType.create(_typeSystem, node, type);
     if (!_inferenceHints || info == null) {
       return;
     }
     AnalysisError error = info.toAnalysisError();
     _errorListener.onError(error);
@@ skipping 2983 matching lines @@
     }
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitBlockFunctionBody(BlockFunctionBody node) {
     _overrideManager.enterScope();
     try {
-      inferenceContext.pushReturnContext(InferenceContext.getType(node));
+      inferenceContext.pushReturnContext(node);
       super.visitBlockFunctionBody(node);
     } finally {
       _overrideManager.exitScope();
-      inferenceContext.popReturnContext();
+      inferenceContext.popReturnContext(node);
     }
     return null;
   }
 
   @override
   Object visitBreakStatement(BreakStatement node) {
     //
     // We do not visit the label because it needs to be visited in the context
     // of the statement.
     //
@@ skipping 769 matching lines @@
     //
     InferenceContext.setType(node.argumentList, node.staticElement?.type);
     safelyVisit(node.argumentList);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitReturnStatement(ReturnStatement node) {
-    InferenceContext.setType(node.expression, inferenceContext.returnContext);
-    return super.visitReturnStatement(node);
+    Expression e = node.expression;
+    InferenceContext.setType(e, inferenceContext.returnContext);
+    super.visitReturnStatement(node);
+    if (e != null) {
+      inferenceContext.addReturnType(e.staticType);

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

As it stands, I think that this will do the right thing for async functions that
return only futures, or return only values.  For async functions returning
Future<T> and T, I think it does a safe thing (Future<dynamic>) but I think we
could handle this case and return Future<T>.  Maybe a TODO?

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

Nice catch. Yeah, it seems like we should interpret "T" here as "fatten(T)" in
an "async" body.

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

Done.

+    }
+    return null;
   }
 
   @override
   Object visitShowCombinator(ShowCombinator node) => null;
 
   @override
   Object visitSuperConstructorInvocation(SuperConstructorInvocation node) {
     //
     // We visit the argument list, but do not visit the optional identifier
     // because it needs to be visited in the context of the constructor
@@ skipping 100 matching lines @@
       _implicitLabelScope = outerImplicitScope;
     }
     // TODO(brianwilkerson) If the loop can only be exited because the condition
     // is false, then propagateFalseState(condition);
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitYieldStatement(YieldStatement node) {

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

It seems like there's an analogous inference for yields.  Maybe a TODO?

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

Good catch. Completely forgot that sync*/async* could even be used in an
anonymous function. :)

I'm tempted to just handle it correctly. I think it should be about as much code
as writing the TODO.

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

Done. This one was a little tricky, in particular we need some code similar to
flatten(T) for futures. I wasn't able to share quite as much as I would have
liked.

     DartType returnType = inferenceContext.returnContext;
     if (returnType != null && _enclosingFunction != null) {
       // If we're not in a generator ([a]sync*, then we shouldn't have a yield.
       // so don't infer
       if (_enclosingFunction.isGenerator) {
         // If this just a yield, then we just pass on the element type
         DartType type = returnType;
         if (node.star != null) {
           // If this is a yield*, then we wrap the element return type
           // If it's synchronous, we expect Iterable<T>, otherwise Stream<T>
@@ skipping 4196 matching lines @@
     nonFields.add(node);
     return null;
   }
 
   @override
   Object visitNode(AstNode node) => node.accept(TypeResolverVisitor_this);
 
   @override
   Object visitWithClause(WithClause node) => null;
 }