fix #25794, infer parameter type from default value

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 5561 matching lines @@
    * A comment before a function should be resolved in the context of the
    * function. But when we incrementally resolve a comment, we don't want to
    * resolve the whole function.
    *
    * So, this flag is set to `true`, when just context of the function should
    * be built and the comment resolved.
    */
   bool resolveOnlyCommentInFunctionBody = false;
 
   /**
+   * True if we're analyzing in strong mode.
+   */
+  bool _strongMode;
+
+  /**
    * Body of the function currently being analyzed, if any.
    */
   FunctionBody _currentFunctionBody;
 
   /**
    * Initialize a newly created visitor to resolve the nodes in an AST node.
    *
    * The [definingLibrary] is the element for the library containing the node
    * being visited. The [source] is the source representing the compilation unit
    * containing the node being visited. The [typeProvider] is the object used to
@@ skipping 10 matching lines @@
    */
   ResolverVisitor(LibraryElement definingLibrary, Source source,
       TypeProvider typeProvider, AnalysisErrorListener errorListener,
       {Scope nameScope})
       : super(definingLibrary, source, typeProvider, errorListener,
             nameScope: nameScope) {
     this.elementResolver = new ElementResolver(this);
     this.typeSystem = definingLibrary.context.typeSystem;
     bool strongModeHints = false;
     AnalysisOptions options = definingLibrary.context.analysisOptions;
+    _strongMode = options.strongMode;
     if (options is AnalysisOptionsImpl) {
       strongModeHints = options.strongModeHints;
     }
     this.inferenceContext = new InferenceContext._(
         errorReporter, typeProvider, typeSystem, strongModeHints);
     this.typeAnalyzer = new StaticTypeAnalyzer(this);
   }
 
   /**
    * Return the element representing the function containing the current node, or `null` if
@@ skipping 670 matching lines @@
     // We do not visit the label because it needs to be visited in the context
     // of the statement.
     //
     node.accept(elementResolver);
     node.accept(typeAnalyzer);
     return null;
   }
 
   @override
   Object visitDefaultFormalParameter(DefaultFormalParameter node) {
-    InferenceContext.setType(node.defaultValue, node.parameter.element?.type);
+    ParameterElement element = node.element;
+    InferenceContext.setType(node.defaultValue, element.type);

[Jennifer Messerly, 2016/06/23 20:14:46]

I verified with an assert that `node.parameter.element == node.element` for all
of Analyzer's tests.
(which matched my understanding of the element/AST structure)

     super.visitDefaultFormalParameter(node);
-    ParameterElement element = node.element;
     if (element.initializer != null && node.defaultValue != null) {
       (element.initializer as FunctionElementImpl).returnType =
           node.defaultValue.staticType;
     }
+    if (_strongMode &&
+        node.defaultValue != null &&
+        element.hasImplicitType &&
+        element is! FieldFormalParameterElement) {

[Jennifer Messerly, 2016/06/23 20:14:46]

I experimented with allowing field formal parameter to be inferred to have a
tighter type than its field -- there is another error code that will catch
incompatible types (between the field and parameter) for us if that happens. But
it seemed like mostly an unhelpful feature (why would you want your parameter to
be more restricted than its backing field? seems to only work for if you plan to
mutate that field later to a wider type.), so removed it.

+
+      DartType type = node.defaultValue.staticType;
+      if (!type.isBottom && !type.isDynamic) {

[Leaf, 2016/06/23 22:21:17]

Are we quite sure that type can't be null here?

[Jennifer Messerly, 2016/06/24 18:33:59]

I think so? :) after resolving an expression, the invariant is supposed to be
that staticType is non-null. We visit the default value right above.

That said.... I wouldn't say I'm quite sure that any field in Analyzer
AST/Element model isn't null :)

I'm fine adding a check if you think it would be a good idea. Or we can leave it
off and see if it ever gets hit.

[Leaf, 2016/06/24 21:48:53]

 I'm ok leaving it.  Just wanted to check since we didn't used to inspect the
type.

+        (element as ParameterElementImpl).type = type;

[Leaf, 2016/06/23 22:21:17]

I'm been trying to record all inference events, even though we're not really
using them yet except for testing.  Probably worth doing so here.

[Jennifer Messerly, 2016/06/24 18:33:59]

Yeah that's a good idea ... the only strange part is recordInference is
currently typed to require Expressions, and DefaultFormalParameter isn't an
Expression. So I widened it to accept any AstNode. It looks like that works, but
not sure if this is breaking some invariant that was supposed to be there.

+      }
+    }
     // Clone the ASTs for default formal parameters, so that we can use them
     // during constant evaluation.
     if (!LibraryElementImpl.hasResolutionCapability(
         definingLibrary, LibraryResolutionCapability.constantExpressions)) {
       (element as ConstVariableElement).constantInitializer =
           new ConstantAstCloner().cloneNode(node.defaultValue);
     }
     return null;
   }
 
@@ skipping 4647 matching lines @@
       return null;
     }
     if (identical(node.staticElement, variable)) {
       if (node.inSetterContext()) {
         result = true;
       }
     }
     return null;
   }
 }