fixes #209, remove tree mutation from CodeChecker

lib/src/checker/checker.dart

 // Copyright (c) 2015, 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 dev_compiler.src.checker.checker;
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/element.dart';
 import 'package:analyzer/src/generated/scanner.dart' show Token, TokenType;
@@ skipping 380 matching lines @@
     super.visitClassDeclaration(node);
   }
 
   @override
   void visitAssignmentExpression(AssignmentExpression node) {
     var token = node.operator;
     if (token.type != TokenType.EQ) {
       _checkCompoundAssignment(node);
     } else {
       DartType staticType = _rules.getStaticType(node.leftHandSide);
-      node.rightHandSide = checkAssignment(node.rightHandSide, staticType);
+      checkAssignment(node.rightHandSide, staticType);
     }
     node.visitChildren(this);
   }
 
   /// Check constructor declaration to ensure correct super call placement.
   @override
   void visitConstructorDeclaration(ConstructorDeclaration node) {
     _visitMaybeConst(node, (node) {
       node.visitChildren(this);
 
       final init = node.initializers;
       for (int i = 0, last = init.length - 1; i < last; i++) {
         final node = init[i];
         if (node is SuperConstructorInvocation) {
           _recordMessage(new InvalidSuperInvocation(node));
         }
       }
     });
   }
 
   @override
   void visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
     var field = node.fieldName;
     DartType staticType = _rules.elementType(field.staticElement);
-    node.expression = checkAssignment(node.expression, staticType);
+    checkAssignment(node.expression, staticType);
     node.visitChildren(this);
   }
 
   @override
   void visitForEachStatement(ForEachStatement node) {
     // Check that the expression is an Iterable.
     var expr = node.iterable;
     var iterableType = _rules.provider.iterableType;
     var loopVariable = node.identifier != null
         ? node.identifier
         : node.loopVariable.identifier;
     var iteratorType = loopVariable.staticType;
     var checkedType = iterableType.substitute4([iteratorType]);
-    node.iterable = checkAssignment(expr, checkedType);
+    checkAssignment(expr, checkedType);
     node.visitChildren(this);
   }
 
   @override
   void visitForStatement(ForStatement node) {
     if (node.condition != null) {
-      node.condition = checkBoolean(node.condition);
+      checkBoolean(node.condition);
     }
     node.visitChildren(this);
   }
 
   @override
   void visitIfStatement(IfStatement node) {
-    node.condition = checkBoolean(node.condition);
+    checkBoolean(node.condition);
     node.visitChildren(this);
   }
 
   @override
   void visitDoStatement(DoStatement node) {
-    node.condition = checkBoolean(node.condition);
+    checkBoolean(node.condition);
     node.visitChildren(this);
   }
 
   @override
   void visitWhileStatement(WhileStatement node) {
-    node.condition = checkBoolean(node.condition);
+    checkBoolean(node.condition);
     node.visitChildren(this);
   }
 
   @override
   void visitSwitchStatement(SwitchStatement node) {
     // SwitchStatement defines a boolean conversion to check the result of the
     // case value == the switch value, but in dev_compiler we require a boolean
     // return type from an overridden == operator (because Object.==), so
     // checking in SwitchStatement shouldn't be necessary.
     node.visitChildren(this);
   }
 
   @override
   void visitListLiteral(ListLiteral node) {
     var type = _rules.provider.dynamicType;
     if (node.typeArguments != null) {
       var targs = node.typeArguments.arguments;
       if (targs.length > 0) type = targs[0].type;
     }
     var elements = node.elements;
     for (int i = 0; i < elements.length; i++) {
-      elements[i] = checkArgument(elements[i], type);
+      checkArgument(elements[i], type);
     }
     super.visitListLiteral(node);
   }
 
   @override
   void visitMapLiteral(MapLiteral node) {
     var ktype = _rules.provider.dynamicType;
     var vtype = _rules.provider.dynamicType;
     if (node.typeArguments != null) {
       var targs = node.typeArguments.arguments;
       if (targs.length > 0) ktype = targs[0].type;
       if (targs.length > 1) vtype = targs[1].type;
     }
     var entries = node.entries;
     for (int i = 0; i < entries.length; i++) {
       var entry = entries[i];
-      entry.key = checkArgument(entry.key, ktype);
-      entry.value = checkArgument(entry.value, vtype);
+      checkArgument(entry.key, ktype);
+      checkArgument(entry.value, vtype);
     }
     super.visitMapLiteral(node);
   }
 
   // Check invocations
-  bool checkArgumentList(ArgumentList node, FunctionType type) {
+  void checkArgumentList(ArgumentList node, FunctionType type) {
     NodeList<Expression> list = node.arguments;
     int len = list.length;
     for (int i = 0; i < len; ++i) {
       Expression arg = list[i];
       ParameterElement element = arg.staticParameterElement;
       if (element == null) {
         if (type.parameters.length < len) {
           // We found an argument mismatch, the analyzer will report this too,
           // so no need to insert an error for this here.
           continue;
         }
         element = type.parameters[i];
         // TODO(vsm): When can this happen?
         assert(element != null);
       }
       DartType expectedType = _rules.elementType(element);
       if (expectedType == null) expectedType = _rules.provider.dynamicType;
-      list[i] = checkArgument(arg, expectedType);
+      checkArgument(arg, expectedType);
     }
-    return true;
   }
 
-  Expression checkArgument(Expression arg, DartType expectedType) {
+  void checkArgument(Expression arg, DartType expectedType) {
     // Preserve named argument structure, so their immediate parent is the
     // method invocation.
     if (arg is NamedExpression) {
-      arg.expression = checkAssignment(arg.expression, expectedType);
-      return arg;
+      arg = (arg as NamedExpression).expression;
     }
-    return checkAssignment(arg, expectedType);
+    checkAssignment(arg, expectedType);
   }
 
   void checkFunctionApplication(
       Expression node, Expression f, ArgumentList list) {
     if (_rules.isDynamicCall(f)) {
       // If f is Function and this is a method invocation, we should have
       // gotten an analyzer error, so no need to issue another error.
       _recordDynamicInvoke(node);
     } else {
       checkArgumentList(list, _rules.getTypeAsCaller(f));
     }
   }
 
   @override
   void visitMethodInvocation(MethodInvocation node) {
     checkFunctionApplication(node, node.methodName, node.argumentList);
     node.visitChildren(this);
   }
 
   @override
   void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
     checkFunctionApplication(node, node.function, node.argumentList);
     node.visitChildren(this);
   }
 
   @override
   void visitRedirectingConstructorInvocation(
       RedirectingConstructorInvocation node) {
     var type = node.staticElement.type;
-    bool checked = checkArgumentList(node.argumentList, type);
-    assert(checked);
+    checkArgumentList(node.argumentList, type);
     node.visitChildren(this);
   }
 
   @override
   void visitSuperConstructorInvocation(SuperConstructorInvocation node) {
     var element = node.staticElement;
     if (element == null) {
       _recordMessage(new MissingTypeError(node));
     } else {
       var type = node.staticElement.type;
-      bool checked = checkArgumentList(node.argumentList, type);
-      assert(checked);
+      checkArgumentList(node.argumentList, type);
     }
     node.visitChildren(this);
   }
 
   AstNode _getOwnerFunction(AstNode node) {
     var parent = node.parent;
     while (parent is! FunctionExpression &&
         parent is! MethodDeclaration &&
         parent is! ConstructorDeclaration) {
       parent = parent.parent;
     }
     return parent;
   }
 
   FunctionType _getFunctionType(AstNode node) {
     if (node is Declaration) {
       return _rules.elementType(node.element);
     } else {
       assert(node is FunctionExpression);
       return _rules.getStaticType(node);
     }
   }
 
-  Expression _checkReturn(Expression expression, AstNode node) {
+  void _checkReturn(Expression expression, AstNode node) {
     var type = _getFunctionType(_getOwnerFunction(node)).returnType;
     // TODO(vsm): Enforce void or dynamic (to void?) when expression is null.
-    if (expression == null) return null;
-    return checkAssignment(expression, type);
+    if (expression != null) checkAssignment(expression, type);
   }
 
   @override
   void visitExpressionFunctionBody(ExpressionFunctionBody node) {
-    node.expression = _checkReturn(node.expression, node);
+    _checkReturn(node.expression, node);
     node.visitChildren(this);
   }
 
   @override
   void visitReturnStatement(ReturnStatement node) {
-    node.expression = _checkReturn(node.expression, node);
+    _checkReturn(node.expression, node);
     node.visitChildren(this);
   }
 
   @override
   void visitPropertyAccess(PropertyAccess node) {
     if (node.staticType.isDynamic && _rules.isDynamicTarget(node.realTarget)) {
       _recordDynamicInvoke(node);
     }
     node.visitChildren(this);
   }
@@ skipping 15 matching lines @@
       var parameterType = _rules.elementType(parameter.element);
       assert(parameterType != null);
       var defaultValue = node.defaultValue;
       if (defaultValue == null) {
         if (_rules.maybeNonNullableType(parameterType)) {
           var staticInfo = new InvalidVariableDeclaration(
               _rules, node.identifier, parameterType);
           _recordMessage(staticInfo);
         }
       } else {
-        var staticInfo = checkAssignment(defaultValue, parameterType);
-        if (staticInfo is! StaticError) node.defaultValue = staticInfo;
+        checkAssignment(defaultValue, parameterType);
       }
 
       node.visitChildren(this);
     });
   }
 
   @override
   void visitFieldFormalParameter(FieldFormalParameter node) {
     var element = node.element;
     var typeName = node.type;
@@ skipping 32 matching lines @@
       TypeName type = node.type;
       if (type == null) {
         // No checks are needed when the type is var. Although internally the
         // typing rules may have inferred a more precise type for the variable
         // based on the initializer.
       } else {
         var dartType = getType(type);
         for (VariableDeclaration variable in node.variables) {
           var initializer = variable.initializer;
           if (initializer != null) {
-            variable.initializer = checkAssignment(initializer, dartType);
+            checkAssignment(initializer, dartType);
           } else if (_rules.maybeNonNullableType(dartType)) {
             var element = variable.element;
             if (element is FieldElement && !element.isStatic) {
               // Initialized - possibly implicitly - during construction.
               // Handle this via a runtime check during code generation.
 
               // TODO(vsm): Detect statically whether this can fail and
               // report a static error (must fail) or warning (can fail).
             } else {
               var staticInfo =
@@ skipping 29 matching lines @@
 
   @override
   void visitIsExpression(IsExpression node) {
     _checkRuntimeTypeCheck(node, node.type);
     node.visitChildren(this);
   }
 
   @override
   void visitPrefixExpression(PrefixExpression node) {
     if (node.operator.type == TokenType.BANG) {
-      node.operand = checkBoolean(node.operand);
+      checkBoolean(node.operand);
     } else {
       _checkUnary(node);
     }
     node.visitChildren(this);
   }
 
   @override
   void visitPostfixExpression(PostfixExpression node) {
     _checkUnary(node);
     node.visitChildren(this);
@@ skipping 24 matching lines @@
           _recordDynamicInvoke(node);
         }
       } else {
         var element = node.staticElement;
         // Method invocation.
         if (element is MethodElement) {
           var type = element.type as FunctionType;
           // Analyzer should enforce number of parameter types, but check in
           // case we have erroneous input.
           if (type.normalParameterTypes.isNotEmpty) {
-            node.rightOperand =
-                checkArgument(node.rightOperand, type.normalParameterTypes[0]);
+            checkArgument(node.rightOperand, type.normalParameterTypes[0]);
           }
         } else {
           // TODO(vsm): Assert that the analyzer found an error here?
         }
       }
     } else {
       // Non-method operator.
       switch (op.type) {
         case TokenType.AMPERSAND_AMPERSAND:
         case TokenType.BAR_BAR:
-          node.leftOperand = checkBoolean(node.leftOperand);
-          node.rightOperand = checkBoolean(node.rightOperand);
+          checkBoolean(node.leftOperand);
+          checkBoolean(node.rightOperand);
           break;
         case TokenType.BANG_EQ:
           break;
         default:
           assert(false);
       }
     }
     node.visitChildren(this);
   }
 
   @override
   void visitConditionalExpression(ConditionalExpression node) {
-    node.condition = checkBoolean(node.condition);
+    checkBoolean(node.condition);
     node.visitChildren(this);
   }
 
   @override
   void visitIndexExpression(IndexExpression node) {
     if (_rules.isDynamicTarget(node.target)) {
       _recordDynamicInvoke(node);
     } else {
       var element = node.staticElement;
       if (element is MethodElement) {
         var type = element.type as FunctionType;
         // Analyzer should enforce number of parameter types, but check in
         // case we have erroneous input.
         if (type.normalParameterTypes.isNotEmpty) {
-          node.index = checkArgument(node.index, type.normalParameterTypes[0]);
+          checkArgument(node.index, type.normalParameterTypes[0]);
         }
       } else {
         // TODO(vsm): Assert that the analyzer found an error here?
       }
     }
     node.visitChildren(this);
   }
 
   DartType getType(TypeName name) {
     return (name == null) ? _rules.provider.dynamicType : name.type;
   }
 
   /// Analyzer checks boolean conversions, but we need to check too, because
   /// it uses the default assignability rules that allow `dynamic` and `Object`
   /// to be assigned to bool with no message.
-  Expression checkBoolean(Expression expr) =>
+  void checkBoolean(Expression expr) =>
       checkAssignment(expr, _rules.provider.boolType);
 
-  Expression checkAssignment(Expression expr, DartType type) {
+  void checkAssignment(Expression expr, DartType type) {
     if (expr is ParenthesizedExpression) {
-      expr.expression = checkAssignment(expr.expression, type);
+      checkAssignment(expr.expression, type);
     } else {
-      final staticInfo = _rules.checkAssignment(expr, type, _constantContext);
-      _recordMessage(staticInfo);
-      if (staticInfo is Conversion) expr = staticInfo;
+      _recordMessage(_rules.checkAssignment(expr, type, _constantContext));
     }
-    return expr;
   }
 
   DartType _specializedBinaryReturnType(
       TokenType op, DartType t1, DartType t2, DartType normalReturnType) {
     // This special cases binary return types as per 16.26 and 16.27 of the
     // Dart language spec.
     switch (op) {
       case TokenType.PLUS:
       case TokenType.MINUS:
       case TokenType.STAR:
@@ skipping 38 matching lines @@
 
       if (!_rules.isSubTypeOf(returnType, lhsType)) {
         final numType = _rules.provider.numType;
         // Try to fix up the numerical case if possible.
         if (_rules.isSubTypeOf(lhsType, numType) &&
             _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, Coercion.cast(rhsType, lhsType));
-          if (staticInfo is DownCast) {
-            expr.rightHandSide = staticInfo;
-          }
           rhsType = lhsType;
         } else {
           // Static type error
           staticInfo = new StaticTypeError(_rules, expr, lhsType);
         }
         _recordMessage(staticInfo);
       }
 
       // Check the rhs type
-      if (staticInfo is! Conversion) {
+      if (staticInfo is! CoercionInfo) {
         var paramType = paramTypes.first;
         staticInfo = _rules.checkAssignment(
             expr.rightHandSide, paramType, _constantContext);
         _recordMessage(staticInfo);
-        if (staticInfo is Conversion) expr.rightHandSide = staticInfo;
       }
     }
   }
 
   void _recordDynamicInvoke(AstNode node) {
     _reporter.log(new DynamicInvoke(_rules, node));
   }
 
   void _recordMessage(StaticInfo info) {
     if (info == null) return;
     if (info.level >= logger.Level.SEVERE) _failure = true;
     _reporter.log(info);
+    if (info is CoercionInfo) {
+      assert(CoercionInfo.get(info.node) == null);
+      CoercionInfo.set(info.node, info);
+    }
   }
 }