fix #25573, add option to disable implicit dynamic

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

Index: pkg/analyzer/lib/src/generated/error_verifier.dart
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
index eca427d5390ccb170d4b1cab66e96d115e3eae17..583f26cfe1e725394d86d618b54499975c5c6126 100644
--- a/pkg/analyzer/lib/src/generated/error_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -79,7 +79,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   /**
    * The options for verification.
    */
-  AnalysisOptions _options;
+  AnalysisOptionsImpl _options;
 
   /**
    * The object providing access to the types defined by the language.
@@ -676,6 +676,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   }
 
   @override
+  Object visitExtendsClause(ExtendsClause node) {
+    _checkForImplicitDynamicType(node.superclass);
+    return super.visitExtendsClause(node);
+  }
+
+  @override
   Object visitFieldDeclaration(FieldDeclaration node) {
     _isInStaticVariableDeclaration = node.isStatic;
     _isInInstanceVariableDeclaration = !_isInStaticVariableDeclaration;
@@ -747,6 +753,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       }
       _checkForTypeAnnotationDeferredClass(returnType);
       _checkForIllegalReturnType(returnType);
+      _checkForImplicitDynamicReturn(node, node.element);

[Leaf, 2016/06/24 21:40:25]

Will this fire on things that are setters only?  If so, is that what we want?

[Jennifer Messerly, 2016/06/27 18:38:17]

There's a check inside `_checkForImplicitDynamicReturn` that it's not a setter.
It looks like there's a test case for it as well.

       return super.visitFunctionDeclaration(node);
     } finally {
       _enclosingFunction = outerFunction;
@@ -761,6 +768,9 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       ExecutableElement outerFunction = _enclosingFunction;
       try {
         _enclosingFunction = node.element;
+        // TODO(jmesserly): there's no way to actually write this `dynamic`
+        // explicitly. Maybe this shouldn't be an error?
+        _checkForImplicitDynamicReturn(node, node.element);

[Leaf, 2016/06/24 21:40:25]

Good question.  :)  I think probably?  But this might be something we want to
get feedback on.

[Jennifer Messerly, 2016/06/27 18:38:17]

Sounds good. Based on our chat I'm going to leave this out for now. Keeps the
flag a bit more friendly, and those lambdas should usually be okay regardless,
given that we have errors in other places (generic methods, variable
declarations). 

         return super.visitFunctionExpression(node);
       } finally {
         _enclosingFunction = outerFunction;
@@ -781,6 +791,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     } else if (expressionType is FunctionType) {
       _checkTypeArguments(expressionType.element, node.typeArguments);
     }
+    _checkForImplicitDynamicInvoke(node);
     return super.visitFunctionExpressionInvocation(node);
   }
 
@@ -799,6 +810,18 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     _isInFunctionTypedFormalParameter = true;
     try {
       _checkForTypeAnnotationDeferredClass(node.returnType);
+
+      // TODO(jmesserly): ideally we'd use _checkForImplicitDynamicReturn, and
+      // we can get the function element via `node?.element?.type?.element` but
+      // it doesn't have hasImplicitReturnType set correctly.
+      if (!_options.implicitDynamic && node.returnType == null) {
+        DartType parameterType = node.element.type;
+        if (parameterType is FunctionType &&
+            parameterType.returnType.isDynamic) {
+          _errorReporter.reportErrorForNode(
+              StrongModeCode.IMPLICIT_DYNAMIC_RETURN, node, [node.identifier]);
+        }
+      }
       return super.visitFunctionTypedFormalParameter(node);
     } finally {
       _isInFunctionTypedFormalParameter = old;
@@ -812,6 +835,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   }
 
   @override
+  Object visitImplementsClause(ImplementsClause node) {
+    node.interfaces.forEach(_checkForImplicitDynamicType);
+    return super.visitImplementsClause(node);
+  }
+
+  @override
   Object visitImportDirective(ImportDirective node) {
     ImportElement importElement = node.element;
     if (importElement != null) {
@@ -848,6 +877,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
           _checkForNewWithUndefinedConstructor(node, constructorName, typeName);
         }
       }
+      _checkForImplicitDynamicType(typeName);
       return super.visitInstanceCreationExpression(node);
     } finally {
       _isInConstInstanceCreation = wasInConstInstanceCreation;
@@ -873,7 +903,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       }
       _checkForExpectedOneListTypeArgument(node, typeArguments);
     }
-
+    _checkForImplicitDynamicTypedLiteral(node);
     _checkForListElementTypeNotAssignable(node);
     return super.visitListLiteral(node);
   }
@@ -891,7 +921,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       }
       _checkExpectedTwoMapTypeArguments(typeArguments);
     }
-
+    _checkForImplicitDynamicTypedLiteral(node);
     _checkForMapTypeNotAssignable(node);
     _checkForNonConstMapAsExpressionStatement(node);
     return super.visitMapLiteral(node);
@@ -930,6 +960,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       _checkForAllInvalidOverrideErrorCodesForMethod(node);
       _checkForTypeAnnotationDeferredClass(returnTypeName);
       _checkForIllegalReturnType(returnTypeName);
+      _checkForImplicitDynamicReturn(node, node.element);
       _checkForMustCallSuper(node);
       return super.visitMethodDeclaration(node);
     } finally {
@@ -951,6 +982,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     }
     _checkTypeArguments(
         node.methodName.staticElement, node.typeArguments, target?.staticType);
+    _checkForImplicitDynamicInvoke(node);
     return super.visitMethodInvocation(node);
   }
 
@@ -1046,6 +1078,15 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     _checkForConstFormalParameter(node);
     _checkForPrivateOptionalParameter(node);
     _checkForTypeAnnotationDeferredClass(node.type);
+
+    // Checks for an implicit dynamic parameter type.
+    //
+    // We can skip other parameter kinds besides simple formal, because:
+    // - DefaultFormalParameter contains a simple one, so it gets here,
+    // - FieldFormalParameter error should be reported on the field,
+    // - FunctionTypedFormalParameter is a function type, not dynamic.
+    _checkForImplicitDynamicIdentifier(node, node.identifier);
+
     return super.visitSimpleFormalParameter(node);
   }
 
@@ -1116,6 +1157,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
         CompileTimeErrorCode.BUILT_IN_IDENTIFIER_AS_TYPE_PARAMETER_NAME);
     _checkForTypeParameterSupertypeOfItsBound(node);
     _checkForTypeAnnotationDeferredClass(node.bound);
+    _checkForImplicitDynamicType(node.bound);
     return super.visitTypeParameter(node);
   }
 
@@ -1125,6 +1167,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     Expression initializerNode = node.initializer;
     // do checks
     _checkForInvalidAssignment(nameNode, initializerNode);
+    _checkForImplicitDynamicIdentifier(node, nameNode);
     // visit name
     nameNode.accept(this);
     // visit initializer
@@ -1163,6 +1206,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   }
 
   @override
+  Object visitWithClause(WithClause node) {
+    node.mixinTypes.forEach(_checkForImplicitDynamicType);
+    return super.visitWithClause(node);
+  }
+
+  @override
   Object visitYieldStatement(YieldStatement node) {
     if (_inGenerator) {
       _checkForYieldOfInvalidType(node.expression, node.star != null);
@@ -3538,6 +3587,113 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     return foundError;
   }
 
+  void _checkForImplicitDynamicIdentifier(AstNode node, Identifier id) {
+    if (_options.implicitDynamic) {
+      return;
+    }
+    VariableElement variable = getVariableElement(id);
+    if (variable != null &&
+        variable.hasImplicitType &&
+        variable.type.isDynamic) {

[Leaf, 2016/06/24 21:40:26]

Can we trust that type is non-null here?

[Jennifer Messerly, 2016/06/27 18:38:17]

As near as I can tell, yes. An Element should always have a non-null type after
resolution. Inside this file, it doesn't seem to check for an element's type
being `null`.

(Definitely AST child nodes can be null, due to errors during parse. Also within
the Resolver visitors things are still being filled in, so more care is needed.)

+      ErrorCode errorCode;
+      if (variable is FieldElement) {
+        errorCode = StrongModeCode.IMPLICIT_DYNAMIC_FIELD;
+      } else if (variable is ParameterElement) {
+        errorCode = StrongModeCode.IMPLICIT_DYNAMIC_PARAMETER;
+      } else {
+        errorCode = StrongModeCode.IMPLICIT_DYNAMIC_VARIABLE;
+      }
+      _errorReporter.reportErrorForNode(errorCode, node, [id]);
+    }
+  }
+
+  void _checkForImplicitDynamicInvoke(InvocationExpression node) {
+    if (_options.implicitDynamic ||
+        node == null ||
+        node.typeArguments != null) {
+      return;
+    }
+    DartType invokeType = node.staticInvokeType;
+    DartType declaredType = node.function.staticType;
+    if (invokeType is FunctionType && declaredType is FunctionType) {
+      Iterable<DartType> typeArgs =
+          FunctionTypeImpl.recoverTypeArguments(declaredType, invokeType);
+      if (typeArgs.any((t) => t.isDynamic)) {
+        // Issue an error depending on what we're trying to call.
+        Expression function = node.function;
+        if (function is Identifier) {
+          Element element = function.staticElement;
+          if (element is MethodElement) {
+            _errorReporter.reportErrorForNode(
+                StrongModeCode.IMPLICIT_DYNAMIC_METHOD,
+                node.function,
+                [element.displayName, element.typeParameters.join(', ')]);
+            return;
+          }
+
+          if (element is FunctionElement) {
+            _errorReporter.reportErrorForNode(
+                StrongModeCode.IMPLICIT_DYNAMIC_FUNCTION,
+                node.function,
+                [element.displayName, element.typeParameters.join(', ')]);
+            return;
+          }
+        }
+
+        // The catch all case if neither of those matched.
+        // For example, invoking a function expression.
+        _errorReporter.reportErrorForNode(
+            StrongModeCode.IMPLICIT_DYNAMIC_INVOKE,
+            node.function,
+            [declaredType]);
+      }
+    }
+  }
+
+  void _checkForImplicitDynamicReturn(AstNode node, ExecutableElement element) {
+    if (_options.implicitDynamic) {
+      return;
+    }
+    if (element is PropertyAccessorElement && element.isSetter) {
+      return;
+    }
+    if (element != null &&
+        element.hasImplicitReturnType &&
+        element.returnType.isDynamic) {

[Leaf, 2016/06/24 21:40:25]

Can we trust returnType not to be null?

[Jennifer Messerly, 2016/06/27 18:38:17]

I believe so, yes. Most of the methods in this file do not check for null
returnType.

(One place I do know it can happen: via tests. They sometimes build incomplete
Element models, that resolution would never make, then pass them to the APIs. I
have been fixing that in the tests when I encounter it. Invariants == good :) )

+      _errorReporter.reportErrorForNode(
+          StrongModeCode.IMPLICIT_DYNAMIC_RETURN, node, [element.displayName]);
+    }
+  }
+
+  void _checkForImplicitDynamicType(TypeName node) {
+    if (_options.implicitDynamic ||
+        node == null ||
+        node.typeArguments != null) {
+      return;
+    }
+    DartType type = node.type;
+    if (type is ParameterizedType &&
+        type.typeArguments.isNotEmpty &&
+        type.typeArguments.any((t) => t.isDynamic)) {
+      _errorReporter.reportErrorForNode(
+          StrongModeCode.IMPLICIT_DYNAMIC_TYPE, node, [type]);
+    }
+  }
+
+  void _checkForImplicitDynamicTypedLiteral(TypedLiteral node) {
+    if (_options.implicitDynamic || node.typeArguments != null) {
+      return;
+    }
+    DartType type = node.staticType;
+    // It's an error if either the key or value was inferred as dynamic.
+    if (type is InterfaceType && type.typeArguments.any((t) => t.isDynamic)) {
+      ErrorCode errorCode = node is ListLiteral
+          ? StrongModeCode.IMPLICIT_DYNAMIC_LIST_LITERAL
+          : StrongModeCode.IMPLICIT_DYNAMIC_MAP_LITERAL;
+      _errorReporter.reportErrorForNode(errorCode, node);
+    }
+  }
+
   /**
    * Verify that if the given [identifier] is part of a constructor initializer,
    * then it does not implicitly reference 'this' expression.