Refactor strong mode to use standard Analyzer errors

pkg/analyzer/lib/src/task/strong/checker.dart

Index: pkg/analyzer/lib/src/task/strong/checker.dart
diff --git a/pkg/analyzer/lib/src/task/strong/checker.dart b/pkg/analyzer/lib/src/task/strong/checker.dart
index 01bea7f766a02154655cb08377b114bbc7a91753..1a7c1da0eaddc29bf2e32f15b8fc4a6ec8bba8a6 100644
--- a/pkg/analyzer/lib/src/task/strong/checker.dart
+++ b/pkg/analyzer/lib/src/task/strong/checker.dart
@@ -14,10 +14,11 @@ import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/generated/engine.dart' show AnalysisOptionsImpl;
+import 'package:analyzer/src/generated/error.dart' show StrongModeCode;
 import 'package:analyzer/src/generated/resolver.dart' show TypeProvider;
 import 'package:analyzer/src/generated/type_system.dart';
 
-import 'info.dart';
+import 'ast_properties.dart';
 
 DartType _elementType(Element e) {
   if (e == null) {
@@ -101,8 +102,8 @@ class CodeChecker extends RecursiveAstVisitor {
   final StrongTypeSystemImpl rules;
   final TypeProvider typeProvider;
   final AnalysisErrorListener reporter;
-  final _OverrideChecker _overrideChecker;
   final AnalysisOptionsImpl _options;
+  _OverrideChecker _overrideChecker;
 
   bool _failure = false;
 
@@ -110,10 +111,11 @@ class CodeChecker extends RecursiveAstVisitor {
       AnalysisErrorListener reporter, this._options)
       : typeProvider = typeProvider,
         rules = rules,
-        reporter = reporter,
-        _overrideChecker = new _OverrideChecker(typeProvider, rules, reporter);
+        reporter = reporter {
+    _overrideChecker = new _OverrideChecker(this);
+  }
 
-  bool get failure => _failure || _overrideChecker._failure;
+  bool get failure => _failure;
 
   void checkArgument(Expression arg, DartType expectedType) {
     // Preserve named argument structure, so their immediate parent is the
@@ -170,7 +172,6 @@ class CodeChecker extends RecursiveAstVisitor {
 
   void reset() {
     _failure = false;
-    _overrideChecker._failure = false;
   }
 
   @override
@@ -264,7 +265,7 @@ class CodeChecker extends RecursiveAstVisitor {
     for (int i = 0, last = init.length - 1; i < last; i++) {
       final node = init[i];
       if (node is SuperConstructorInvocation) {
-        _recordMessage(new InvalidSuperInvocation(node));
+        _recordMessage(node, StrongModeCode.INVALID_SUPER_INVOCATION, [node]);
       }
     }
   }
@@ -315,9 +316,8 @@ class CodeChecker extends RecursiveAstVisitor {
           node.identifier.staticElement as FieldFormalParameterElement;
       var fieldType = _elementType(fieldElement.field);
       if (!rules.isSubtypeOf(type, fieldType)) {
-        var staticInfo =
-            new InvalidParameterDeclaration(rules, node, fieldType);
-        _recordMessage(staticInfo);
+        _recordMessage(node, StrongModeCode.INVALID_PARAMETER_DECLARATION,
+            [node, fieldType]);
       }
     }
     node.visitChildren(this);
@@ -345,8 +345,7 @@ class CodeChecker extends RecursiveAstVisitor {
             sequenceInterface.instantiate([DynamicTypeImpl.instance]);
 
         if (rules.isSubtypeOf(sequenceType, iterableType)) {
-          _recordMessage(DownCast.create(
-              rules, node.iterable, iterableType, sequenceType, _options));
+          _recordImplicitCast(node.iterable, iterableType, sequenceType);
           elementType = DynamicTypeImpl.instance;
         }
       }
@@ -500,7 +499,7 @@ class CodeChecker extends RecursiveAstVisitor {
       //
       // The first case is handled here, the second case is handled below when
       // we call [checkFunctionApplication].
-      DynamicInvoke.set(node.methodName, true);
+      setIsDynamicInvoke(node.methodName, true);
     } else {
       checkFunctionApplication(node, node.methodName, node.argumentList);
     }
@@ -619,7 +618,6 @@ class CodeChecker extends RecursiveAstVisitor {
       assert(functionType.optionalParameterTypes.isEmpty);
 
       // Check the LHS type.
-      var staticInfo;
       var rhsType = _getStaticType(expr.rightHandSide);
       var lhsType = _getStaticType(expr.leftHandSide);
       var returnType = rules.refineBinaryExpressionType(
@@ -632,19 +630,17 @@ class CodeChecker extends RecursiveAstVisitor {
             rules.isSubtypeOf(lhsType, rhsType)) {
           // This is also slightly different from spec, but allows us to keep
           // compound operators in the int += num and num += dynamic cases.
-          staticInfo = DownCast.create(
-              rules, expr.rightHandSide, rhsType, lhsType, _options);
-          rhsType = lhsType;
+          _recordImplicitCast(expr.rightHandSide, rhsType, lhsType);
         } else {
-          staticInfo = new StaticTypeError(expr, lhsType);
+          _recordMessage(expr, StrongModeCode.STATIC_TYPE_ERROR,
+              [expr, returnType, lhsType]);
         }
-        _recordMessage(staticInfo);
-      }
-
-      // Check the rhs type
-      if (staticInfo is! CoercionInfo) {
-        var paramType = paramTypes.first;
-        _checkDowncast(expr.rightHandSide, paramType);
+      } else {
+        // Check the RHS type.
+        //
+        // This is only needed if we didn't already need a cast, and avoids
+        // emitting two messages for the same expression.
+        _checkDowncast(expr.rightHandSide, paramTypes.first);
       }
     }
   }
@@ -679,7 +675,7 @@ class CodeChecker extends RecursiveAstVisitor {
 
     // Downcast if toT <: fromT
     if (rules.isSubtypeOf(to, from)) {
-      _recordMessage(DownCast.create(rules, expr, from, to, _options));
+      _recordImplicitCast(expr, from, to);
       return;
     }
 
@@ -694,7 +690,7 @@ class CodeChecker extends RecursiveAstVisitor {
     // Iterable<T> for some concrete T (e.g. Object).  These are unrelated
     // in the restricted system, but List<dynamic> <: Iterable<T> in dart.
     if (from.isAssignableTo(to)) {
-      _recordMessage(DownCast.create(rules, expr, from, to, _options));
+      _recordImplicitCast(expr, from, to);
     }
   }
 
@@ -731,7 +727,7 @@ class CodeChecker extends RecursiveAstVisitor {
   void _checkRuntimeTypeCheck(AstNode node, TypeName typeName) {
     var type = getType(typeName);
     if (!rules.isGroundType(type)) {
-      _recordMessage(new NonGroundTypeCheckInfo(node, type));
+      _recordMessage(node, StrongModeCode.NON_GROUND_TYPE_CHECK_INFO, [type]);
     }
   }
 
@@ -749,6 +745,85 @@ class CodeChecker extends RecursiveAstVisitor {
     }
   }
 
+  /// Records an implicit cast for the [expression] from [fromType] to [toType].
+  ///
+  /// This will emit the appropriate error/warning/hint message as well as mark
+  /// the AST node.
+  void _recordImplicitCast(
+      Expression expression, DartType fromType, DartType toType) {
+    // toT <:_R fromT => to <: fromT
+    // NB: classes with call methods are subtypes of function
+    // types, but the function type is not assignable to the class
+    assert(toType.isSubtypeOf(fromType) || fromType.isAssignableTo(toType));
+
+    // Inference "casts":
+    if (expression is Literal || expression is FunctionExpression) {
+      // fromT should be an exact type - this will almost certainly fail at
+      // runtime.
+      _recordMessage(expression, StrongModeCode.STATIC_TYPE_ERROR,
+          [expression, fromType, toType]);
+      return;
+    }
+
+    if (expression is InstanceCreationExpression) {
+      ConstructorElement e = expression.staticElement;
+      if (e == null || !e.isFactory) {
+        // fromT should be an exact type - this will almost certainly fail at
+        // runtime.
+
+        _recordMessage(expression, StrongModeCode.STATIC_TYPE_ERROR,
+            [expression, fromType, toType]);
+        return;
+      }
+    }
+
+    if (isKnownFunction(expression)) {
+      _recordMessage(expression, StrongModeCode.STATIC_TYPE_ERROR,
+          [expression, fromType, toType]);
+      return;
+    }
+
+    // TODO(vsm): Change this to an assert when we have generic methods and
+    // fix TypeRules._coerceTo to disallow implicit sideways casts.
+    bool downCastComposite = false;
+    if (!rules.isSubtypeOf(toType, fromType)) {
+      assert(toType.isSubtypeOf(fromType) || fromType.isAssignableTo(toType));
+      downCastComposite = true;
+    }
+
+    // Composite cast: these are more likely to fail.
+    if (!rules.isGroundType(toType)) {
+      // This cast is (probably) due to our different treatment of dynamic.
+      // It may be more likely to fail at runtime.
+      if (fromType is InterfaceType) {
+        // For class types, we'd like to allow non-generic down casts, e.g.,
+        // Iterable<T> to List<T>.  The intuition here is that raw (generic)
+        // casts are problematic, and we should complain about those.
+        var typeArgs = fromType.typeArguments;
+        downCastComposite =
+            typeArgs.isEmpty || typeArgs.any((t) => t.isDynamic);
+      } else {
+        downCastComposite = true;
+      }
+    }
+
+    var parent = expression.parent;
+    ErrorCode errorCode;
+    if (downCastComposite) {
+      errorCode = StrongModeCode.DOWN_CAST_COMPOSITE;
+    } else if (fromType.isDynamic) {
+      errorCode = StrongModeCode.DYNAMIC_CAST;
+    } else if (parent is VariableDeclaration &&
+        parent.initializer == expression) {
+      errorCode = StrongModeCode.ASSIGNMENT_CAST;
+    } else {
+      errorCode = StrongModeCode.DOWN_CAST_IMPLICIT;
+    }
+
+    _recordMessage(expression, errorCode, [fromType, toType]);
+    setImplicitCast(expression, toType);
+  }
+
   // Produce a coercion which coerces something of type fromT
   // to something of type toT.
   // Returns the error coercion if the types cannot be coerced
@@ -811,7 +886,7 @@ class CodeChecker extends RecursiveAstVisitor {
     DartType t = expr.staticType ?? DynamicTypeImpl.instance;
 
     // Remove fuzzy arrow if possible.
-    if (t is FunctionType && StaticInfo.isKnownFunction(expr)) {
+    if (t is FunctionType && isKnownFunction(expr)) {
       t = rules.functionTypeToConcreteType(typeProvider, t);
     }
 
@@ -893,46 +968,57 @@ class CodeChecker extends RecursiveAstVisitor {
     return _isObjectGetter(target, id) || _isObjectMethod(target, id);
   }
 
-  void _recordDynamicInvoke(AstNode node, AstNode target) {
-    if (_options.strongModeHints) {
-      reporter.onError(new DynamicInvoke(node).toAnalysisError());
-    }
+  void _recordDynamicInvoke(AstNode node, Expression target) {
+    _recordMessage(node, StrongModeCode.DYNAMIC_INVOKE, [node]);
     // TODO(jmesserly): we may eventually want to record if the whole operation
     // (node) was dynamic, rather than the target, but this is an easier fit
     // with what we used to do.
-    DynamicInvoke.set(target, true);
+    setIsDynamicInvoke(target, true);
   }
 
-  void _recordMessage(StaticInfo info) {
-    if (info == null) return;
-    var error = info.toAnalysisError();
-    var severity = error.errorCode.errorSeverity;
+  void _recordMessage(AstNode node, ErrorCode errorCode, List arguments) {
+    var severity = errorCode.errorSeverity;
     if (severity == ErrorSeverity.ERROR) _failure = true;
     if (severity != ErrorSeverity.INFO || _options.strongModeHints) {
+      int begin = node is AnnotatedNode
+          ? node.firstTokenAfterCommentAndMetadata.offset
+          : node.offset;
+      int length = node.end - begin;
+      var source = (node.root as CompilationUnit).element.source;
+      var error =
+          new AnalysisError(source, begin, length, errorCode, arguments);
       reporter.onError(error);
     }
-
-    if (info is CoercionInfo) {
-      // TODO(jmesserly): if we're run again on the same AST, we'll produce the
-      // same annotations. This should be harmless. This might go away once
-      // CodeChecker is integrated better with analyzer, as it will know that
-      // checking has already been performed.
-      // assert(CoercionInfo.get(info.node) == null);
-      CoercionInfo.set(info.node, info);
-    }
   }
 }
 
+bool isKnownFunction(Expression expression) {
+  Element element = null;
+  if (expression is FunctionExpression) {
+    return true;
+  } else if (expression is PropertyAccess) {
+    element = expression.propertyName.staticElement;
+  } else if (expression is Identifier) {
+    element = expression.staticElement;
+  }
+// First class functions and static methods, where we know the original
+// declaration, will have an exact type, so we know a downcast will fail.
+  return element is FunctionElement ||
+      element is MethodElement && element.isStatic;
+}
+
 /// Checks for overriding declarations of fields and methods. This is used to
 /// check overrides between classes and superclasses, interfaces, and mixin
 /// applications.
 class _OverrideChecker {
-  bool _failure = false;
   final StrongTypeSystemImpl rules;
   final TypeProvider _typeProvider;
-  final AnalysisErrorListener _reporter;
+  final CodeChecker _checker;
 
-  _OverrideChecker(this._typeProvider, this.rules, this._reporter);
+  _OverrideChecker(CodeChecker checker)
+      : _checker = checker,
+        rules = checker.rules,
+        _typeProvider = checker.typeProvider;
 
   void check(ClassDeclaration node) {
     if (node.element.type.isObject) return;
@@ -1196,8 +1282,14 @@ class _OverrideChecker {
       // as a field.  We effectively treat fields as final / non-virtual.
       PropertyInducingElement field = _getMemberField(type, element);
       if (field != null) {
-        _recordMessage(new InvalidFieldOverride(
-            errorLocation, element, type, subType, baseType));
+        _checker._recordMessage(
+            errorLocation, StrongModeCode.INVALID_FIELD_OVERRIDE, [
+          element.enclosingElement.name,
+          element.name,
+          subType,
+          type,
+          baseType
+        ]);
       }
     }
     FunctionType concreteSubType = subType;
@@ -1225,8 +1317,23 @@ class _OverrideChecker {
       //     get foo => e; // no type specified.
       //     toString() { ... } // no return type specified.
       //   }
-      _recordMessage(new InvalidMethodOverride(
-          errorLocation, element, type, subType, baseType));
+
+      ErrorCode errorCode;
+      if (errorLocation is ExtendsClause) {
+        errorCode = StrongModeCode.INVALID_METHOD_OVERRIDE_FROM_BASE;
+      } else if (errorLocation.parent is WithClause) {
+        errorCode = StrongModeCode.INVALID_METHOD_OVERRIDE_FROM_MIXIN;
+      } else {
+        errorCode = StrongModeCode.INVALID_METHOD_OVERRIDE;
+      }
+
+      _checker._recordMessage(errorLocation, errorCode, [
+        element.enclosingElement.name,
+        element.name,
+        subType,
+        type,
+        baseType
+      ]);
     }
     return true;
   }
@@ -1265,11 +1372,4 @@ class _OverrideChecker {
       current = current.superclass;
     } while (!current.isObject && !visited.contains(current));
   }
-
-  void _recordMessage(StaticInfo info) {
-    if (info == null) return;
-    var error = info.toAnalysisError();
-    if (error.errorCode.errorSeverity == ErrorSeverity.ERROR) _failure = true;
-    _reporter.onError(error);
-  }
 }