fixes #206, add checking for unary ops

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 330 matching lines @@
   bool _failure = false;
   bool get failure => _failure || _overrideChecker._failure;
 
   CodeChecker(
       TypeRules rules, CheckerReporter reporter, CompilerOptions options)
       : _rules = rules,
         _reporter = reporter,
         _overrideChecker = new _OverrideChecker(rules, reporter, options);
 
   @override
-  visitCompilationUnit(CompilationUnit unit) {
+  void visitCompilationUnit(CompilationUnit unit) {
     void report(Expression expr) {
       _reporter.log(new MissingTypeError(expr));
     }
     var callback = _rules.reportMissingType;
     _rules.reportMissingType = report;
     unit.visitChildren(this);
     _rules.reportMissingType = callback;
   }
 
   _visitMaybeConst(AstNode n, visitNode(AstNode n)) {
@@ skipping 10 matching lines @@
       } else if (n is InstanceCreationExpression) {
         _constantContext = o || n.isConst;
       } else if (n is ConstructorDeclaration) {
         _constantContext = o || n.element.isConst;
       }
     }
     visitNode(n);
     _constantContext = o;
   }
 
-  visitComment(Comment node) {
+  @override
+  void visitComment(Comment node) {
     // skip, no need to do typechecking inside comments (they may contain
     // comment references which would require resolution).
   }
 
-  visitClassDeclaration(ClassDeclaration node) {
+  @override
+  void visitClassDeclaration(ClassDeclaration node) {
     _overrideChecker.check(node);
     super.visitClassDeclaration(node);
   }
 
-  visitAssignmentExpression(AssignmentExpression 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);
     }
     node.visitChildren(this);
   }
 
   /// Check constructor declaration to ensure correct super call placement.
   @override
-  visitConstructorDeclaration(ConstructorDeclaration node) {
+  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
-  visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
+  void visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
     var field = node.fieldName;
     DartType staticType = _rules.elementType(field.staticElement);
     node.expression = checkAssignment(node.expression, staticType);
     node.visitChildren(this);
   }
 
   @override
-  visitForEachStatement(ForEachStatement node) {
+  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);
     node.visitChildren(this);
   }
 
-  @override visitListLiteral(ListLiteral node) {
+  @override
+  void visitForStatement(ForStatement node) {
+    if (node.condition != null) {
+      node.condition = checkBoolean(node.condition);
+    }
+    node.visitChildren(this);
+  }
+
+  @override
+  void visitIfStatement(IfStatement node) {
+    node.condition = checkBoolean(node.condition);
+    node.visitChildren(this);
+  }
+
+  @override
+  void visitDoStatement(DoStatement node) {
+    node.condition = checkBoolean(node.condition);
+    node.visitChildren(this);
+  }
+
+  @override
+  void visitWhileStatement(WhileStatement node) {
+    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);
     }
     super.visitListLiteral(node);
   }
 
-  @override visitMapLiteral(MapLiteral 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];
@@ skipping 41 matching lines @@
       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));
     }
   }
 
-  visitMethodInvocation(MethodInvocation node) {
+  @override
+  void visitMethodInvocation(MethodInvocation node) {
     checkFunctionApplication(node, node.methodName, node.argumentList);
     node.visitChildren(this);
   }
 
-  visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
+  @override
+  void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
     checkFunctionApplication(node, node.function, node.argumentList);
     node.visitChildren(this);
   }
 
-  visitRedirectingConstructorInvocation(RedirectingConstructorInvocation node) {
+  @override
+  void visitRedirectingConstructorInvocation(
+      RedirectingConstructorInvocation node) {
     var type = node.staticElement.type;
     bool checked = checkArgumentList(node.argumentList, type);
     assert(checked);
     node.visitChildren(this);
   }
 
-  visitSuperConstructorInvocation(SuperConstructorInvocation node) {
+  @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);
     }
     node.visitChildren(this);
   }
@@ skipping 17 matching lines @@
     }
   }
 
   Expression _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);
   }
 
-  visitExpressionFunctionBody(ExpressionFunctionBody node) {
+  @override
+  void visitExpressionFunctionBody(ExpressionFunctionBody node) {
     node.expression = _checkReturn(node.expression, node);
     node.visitChildren(this);
   }
 
-  visitReturnStatement(ReturnStatement node) {
+  @override
+  void visitReturnStatement(ReturnStatement node) {
     node.expression = _checkReturn(node.expression, node);
     node.visitChildren(this);
   }
 
-  visitPropertyAccess(PropertyAccess node) {
+  @override
+  void visitPropertyAccess(PropertyAccess node) {
     if (node.staticType.isDynamic && _rules.isDynamicTarget(node.realTarget)) {
       _recordDynamicInvoke(node);
     }
     node.visitChildren(this);
   }
 
-  visitPrefixedIdentifier(PrefixedIdentifier node) {
+  @override
+  void visitPrefixedIdentifier(PrefixedIdentifier node) {
     final target = node.prefix;
     if (_rules.isDynamicTarget(target)) {
       _recordDynamicInvoke(node);
     }
     node.visitChildren(this);
   }
 
-  @override visitDefaultFormalParameter(DefaultFormalParameter node) {
+  @override
+  void visitDefaultFormalParameter(DefaultFormalParameter node) {
     _visitMaybeConst(node, (node) {
       // Check that defaults have the proper subtype.
       var parameter = node.parameter;
       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;
       }
 
       node.visitChildren(this);
     });
   }
 
-  visitFieldFormalParameter(FieldFormalParameter node) {
+  @override
+  void visitFieldFormalParameter(FieldFormalParameter node) {
     var element = node.element;
     var typeName = node.type;
     if (typeName != null) {
       var type = _rules.elementType(element);
       var fieldElement =
           node.identifier.staticElement as FieldFormalParameterElement;
       var fieldType = _rules.elementType(fieldElement.field);
       if (!_rules.isSubTypeOf(type, fieldType)) {
         var staticInfo =
             new InvalidParameterDeclaration(_rules, node, fieldType);
         _recordMessage(staticInfo);
       }
     }
     node.visitChildren(this);
   }
 
   @override
-  visitInstanceCreationExpression(InstanceCreationExpression node) {
+  void visitInstanceCreationExpression(InstanceCreationExpression node) {
     _visitMaybeConst(node, (node) {
       var arguments = node.argumentList;
       var element = node.staticElement;
       if (element != null) {
         var type = _rules.elementType(node.staticElement);
         checkArgumentList(arguments, type);
       } else {
         _recordMessage(new MissingTypeError(node));
       }
       node.visitChildren(this);
     });
   }
 
   @override
-  visitVariableDeclarationList(VariableDeclarationList node) {
+  void visitVariableDeclarationList(VariableDeclarationList node) {
     _visitMaybeConst(node, (node) {
       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;
@@ skipping 13 matching lines @@
               _recordMessage(staticInfo);
             }
           }
         }
       }
       node.visitChildren(this);
     });
   }
 
   @override
-  visitVariableDeclaration(VariableDeclaration node) {
+  void visitVariableDeclaration(VariableDeclaration node) {
     _visitMaybeConst(node, super.visitVariableDeclaration);
   }
 
   void _checkRuntimeTypeCheck(AstNode node, TypeName typeName) {
     var type = getType(typeName);
     if (!_rules.isGroundType(type)) {
       _recordMessage(new InvalidRuntimeCheckError(node, type));
     }
   }
 
-  visitAsExpression(AsExpression node) {
+  @override
+  void visitAsExpression(AsExpression node) {
     // We could do the same check as the IsExpression below, but that is
     // potentially too conservative.  Instead, at runtime, we must fail hard
     // if the Dart as and the DDC as would return different values.
     node.visitChildren(this);
   }
 
-  visitIsExpression(IsExpression node) {
+  @override
+  void visitIsExpression(IsExpression node) {
     _checkRuntimeTypeCheck(node, node.type);
     node.visitChildren(this);
   }
 
-  visitBinaryExpression(BinaryExpression node) {
+  @override
+  void visitPrefixExpression(PrefixExpression node) {
+    if (node.operator.type == TokenType.BANG) {
+      node.operand = checkBoolean(node.operand);
+    } else {
+      _checkUnary(node);
+    }
+    node.visitChildren(this);
+  }
+
+  @override
+  void visitPostfixExpression(PostfixExpression node) {
+    _checkUnary(node);
+    node.visitChildren(this);
+  }
+
+  void _checkUnary(/*PrefixExpression|PostfixExpression*/ node) {
+    var op = node.operator;
+    if (op.isUserDefinableOperator ||
+        op.type == TokenType.PLUS_PLUS ||
+        op.type == TokenType.MINUS_MINUS) {
+      if (_rules.isDynamicTarget(node.operand)) {
+        _recordDynamicInvoke(node);
+      }
+      // For ++ and --, even if it is not dynamic, we still need to check
+      // that the user defined method accepts an `int` as the RHS.
+      // We assume Analyzer has done this already.
+    }
+  }
+
+  @override
+  void visitBinaryExpression(BinaryExpression node) {
     var op = node.operator;
     if (op.isUserDefinableOperator) {
       if (_rules.isDynamicTarget(node.leftOperand)) {
         // Dynamic invocation
         // TODO(vsm): Move this logic to the resolver?
         if (op.type != TokenType.EQ_EQ && op.type != TokenType.BANG_EQ) {
           _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]);
           }
         } 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:
-          var boolType = _rules.provider.boolType;
-          node.leftOperand = checkArgument(node.leftOperand, boolType);
-          node.rightOperand = checkArgument(node.rightOperand, boolType);
+          node.leftOperand = checkBoolean(node.leftOperand);
+          node.rightOperand = 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);
+    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]);
         }
       } 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) =>
+      checkAssignment(expr, _rules.provider.boolType);
+
   Expression checkAssignment(Expression expr, DartType type) {
     if (expr is ParenthesizedExpression) {
       expr.expression = checkAssignment(expr.expression, type);
     } else {
       final staticInfo = _rules.checkAssignment(expr, type, _constantContext);
       _recordMessage(staticInfo);
       if (staticInfo is Conversion) expr = staticInfo;
     }
     return expr;
   }
@@ skipping 79 matching lines @@
   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);
   }
 }