Use BoolExpressionVerifier in BinaryExpressionResolver.

Change-Id: I249b475a3a5b35e1de6f59b7f9cd30102709bb29 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/132167 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
NoamDev · Jan 17, 2020 · 746d3fb · 746d3fb
1 parent 7b44fa4
commit 746d3fb
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 37 deletions.
diff --git a/pkg/analyzer/lib/src/dart/resolver/binary_expression_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/binary_expression_resolver.dart
@@ -102,6 +102,12 @@ class BinaryExpressionResolver {
     _inferenceHelper.recordStaticType(node, staticType);
   }
 
+  void _checkNonBoolOperand(Expression operand, String operator) {
+    _resolver.boolExpressionVerifier.checkForNonBoolExpression(operand,
+        errorCode: StaticTypeWarningCode.NON_BOOL_OPERAND,
+        arguments: [operator]);
+  }
+
   /// Gets the definite type of expression, which can be used in cases where
   /// the most precise type is desired, for example computing the least upper
   /// bound.
@@ -263,37 +269,45 @@ class BinaryExpressionResolver {
   }
 
   void _resolveLogicalAnd(BinaryExpressionImpl node) {
-    Expression left = node.leftOperand;
-    Expression right = node.rightOperand;
+    var left = node.leftOperand;
+    var right = node.rightOperand;
     var flow = _flowAnalysis?.flow;
 
     InferenceContext.setType(left, _typeProvider.boolType);
     InferenceContext.setType(right, _typeProvider.boolType);
 
     // TODO(scheglov) Do we need these checks for null?
     left?.accept(_resolver);
+    left = node.leftOperand;
 
     if (_flowAnalysis != null) {
       flow?.logicalBinaryOp_rightBegin(left, isAnd: true);
       _flowAnalysis.checkUnreachableNode(right);
+
       right.accept(_resolver);
+      right = node.rightOperand;
+
       flow?.logicalBinaryOp_end(node, right, isAnd: true);
     } else {
       _promoteManager.visitBinaryExpression_and_rhs(
         left,
         right,
         () {
           right.accept(_resolver);
+          right = node.rightOperand;
         },
       );
     }
 
+    _checkNonBoolOperand(left, '&&');
+    _checkNonBoolOperand(right, '&&');
+
     _resolve1(node);
     _resolve2(node);
   }
 
   void _resolveLogicalOr(BinaryExpressionImpl node) {
-    Expression left = node.leftOperand;
+    var left = node.leftOperand;
     Expression right = node.rightOperand;
     var flow = _flowAnalysis?.flow;
 
@@ -302,12 +316,19 @@ class BinaryExpressionResolver {
 
     // TODO(scheglov) Do we need these checks for null?
     left?.accept(_resolver);
+    left = node.leftOperand;
 
     flow?.logicalBinaryOp_rightBegin(left, isAnd: false);
     _flowAnalysis?.checkUnreachableNode(right);
+
     right.accept(_resolver);
+    right = node.rightOperand;
+
     flow?.logicalBinaryOp_end(node, right, isAnd: false);
 
+    _checkNonBoolOperand(left, '||');
+    _checkNonBoolOperand(right, '||');
+
     _resolve1(node);
     _resolve2(node);
   }

diff --git a/pkg/analyzer/lib/src/error/bool_expression_verifier.dart b/pkg/analyzer/lib/src/error/bool_expression_verifier.dart
@@ -42,7 +42,7 @@ class BoolExpressionVerifier {
   /// Verify that the given [expression] is of type 'bool', and report
   /// [errorCode] if not, or a nullability error if its improperly nullable.
   void checkForNonBoolExpression(Expression expression,
-      {@required ErrorCode errorCode}) {
+      {@required ErrorCode errorCode, List<Object> arguments}) {
     var type = expression.staticType;
     if (!_checkForUseOfVoidResult(expression) &&
         !_typeSystem.isAssignableTo(type, _boolType)) {
@@ -52,7 +52,7 @@ class BoolExpressionVerifier {
           expression,
         );
       } else {
-        _errorReporter.reportErrorForNode(errorCode, expression);
+        _errorReporter.reportErrorForNode(errorCode, expression, arguments);
       }
     }
   }

diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -365,11 +365,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     Token operator = node.operator;
     TokenType type = operator.type;
     if (type == TokenType.AMPERSAND_AMPERSAND || type == TokenType.BAR_BAR) {
-      String lexeme = operator.lexeme;
-      _checkForAssignability(node.leftOperand, _boolType,
-          StaticTypeWarningCode.NON_BOOL_OPERAND, [lexeme]);
-      _checkForAssignability(node.rightOperand, _boolType,
-          StaticTypeWarningCode.NON_BOOL_OPERAND, [lexeme]);
       _checkForUseOfVoidResult(node.rightOperand);
     } else if (type == TokenType.EQ_EQ || type == TokenType.BANG_EQ) {
       _checkForArgumentTypeNotAssignableForArgument(node.rightOperand,
@@ -1690,33 +1685,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
-  /**
-   * Check that the static type of the given expression is assignable to the
-   * given type. If it isn't, report an error with the given error code. The
-   * [type] is the type that the expression must be assignable to. The
-   * [errorCode] is the error code to be reported. The [arguments] are the
-   * arguments to pass in when creating the error.
-   */
-  void _checkForAssignability(Expression expression, InterfaceType type,
-      ErrorCode errorCode, List<Object> arguments) {
-    if (expression == null) {
-      return;
-    }
-    DartType expressionType = expression.staticType;
-    if (expressionType == null) {
-      return;
-    }
-    if (_typeSystem.isAssignableTo(expressionType, type)) {
-      return;
-    }
-    if (expressionType.element == type.element) {
-      _errorReporter.reportErrorForNode(
-          StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE, expression);
-    } else {
-      _errorReporter.reportErrorForNode(errorCode, expression, arguments);
-    }
-  }
-
   bool _checkForAssignableExpression(
       Expression expression, DartType expectedStaticType, ErrorCode errorCode) {
     DartType actualStaticType = getStaticType(expression);