Refactor non-bool expression checks

MichaelRFairhurst · commit-bot@chromium.org · commit 2eebc1024c94 · 2019-06-14T21:50:30.000Z
Change-Id: I4da7abe3293c4dbf7218f3a276cf9418b3d14f19 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106000 Commit-Queue: Mike Fairhurst <mfairhurst@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -34,6 +34,7 @@ import 'package:analyzer/src/generated/parser.dart' show ParserErrorCode;
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/sdk.dart' show DartSdk, SdkLibrary;
 import 'package:analyzer/src/generated/source.dart';
+import 'package:meta/meta.dart';
 
 /**
  * A visitor used to traverse an AST structure looking for additional errors and
@@ -369,15 +370,15 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
 
   @override
   void visitAssertInitializer(AssertInitializer node) {
-    _checkForNonBoolExpression(node);
-    _checkForNullableDereference(node.condition);
+    _checkForNonBoolExpression(node.condition,
+        errorCode: StaticTypeWarningCode.NON_BOOL_EXPRESSION);
     super.visitAssertInitializer(node);
   }
 
   @override
   void visitAssertStatement(AssertStatement node) {
-    _checkForNonBoolExpression(node);
-    _checkForNullableDereference(node.condition);
+    _checkForNonBoolExpression(node.condition,
+        errorCode: StaticTypeWarningCode.NON_BOOL_EXPRESSION);
     super.visitAssertStatement(node);
   }
 
@@ -418,8 +419,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       _checkForAssignability(node.rightOperand, _boolType,
           StaticTypeWarningCode.NON_BOOL_OPERAND, [lexeme]);
       _checkForUseOfVoidResult(node.rightOperand);
-      _checkForNullableDereference(node.leftOperand);
-      _checkForNullableDereference(node.rightOperand);
     } else if (type == TokenType.EQ_EQ || type == TokenType.BANG_EQ) {
       _checkForArgumentTypeNotAssignableForArgument(node.rightOperand,
           promoteParameterToNullable: true);
@@ -1168,12 +1167,14 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     Expression operand = node.operand;
     if (operatorType == TokenType.BANG) {
       _checkForNonBoolNegationExpression(operand);
-    } else if (operatorType.isIncrementOperator) {
-      _checkForAssignmentToFinal(operand);
+    } else {
+      if (operatorType.isIncrementOperator) {
+        _checkForAssignmentToFinal(operand);
+      }
+      _checkForNullableDereference(operand);
+      _checkForUseOfVoidResult(operand);
+      _checkForIntNotAssignable(operand);
     }
-    _checkForIntNotAssignable(operand);
-    _checkForNullableDereference(operand);
-    _checkForUseOfVoidResult(operand);
     super.visitPrefixExpression(node);
   }
 
@@ -4569,57 +4570,34 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
    * See [StaticTypeWarningCode.NON_BOOL_CONDITION].
    */
   void _checkForNonBoolCondition(Expression condition) {
-    DartType conditionType = getStaticType(condition);
-    if (!_checkForNullableDereference(condition) &&
-        !_checkForUseOfVoidResult(condition) &&
-        conditionType != null &&
-        !_typeSystem.isAssignableTo(conditionType, _boolType,
-            featureSet: _featureSet)) {
-      _errorReporter.reportErrorForNode(
-          StaticTypeWarningCode.NON_BOOL_CONDITION, condition);
-    }
+    _checkForNonBoolExpression(condition,
+        errorCode: StaticTypeWarningCode.NON_BOOL_CONDITION);
   }
 
   /**
-   * Verify that the given [assertion] has either a 'bool' or '() -> bool'
-   * condition.
+   * Verify that the given [expression] is of type 'bool', and report
+   * [errorCode] if not, or a nullability error if its improperly nullable.
    */
-  void _checkForNonBoolExpression(Assertion assertion) {
-    Expression expression = assertion.condition;
+  void _checkForNonBoolExpression(Expression expression,
+      {@required ErrorCode errorCode}) {
     DartType type = getStaticType(expression);
-    if (type is InterfaceType) {
-      if (!_typeSystem.isAssignableTo(type, _boolType,
-          featureSet: _featureSet)) {
-        if (type.element == _boolType.element) {
-          _errorReporter.reportErrorForNode(
-              StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE, expression);
-        } else {
-          _errorReporter.reportErrorForNode(
-              StaticTypeWarningCode.NON_BOOL_EXPRESSION, expression);
-        }
+    if (!_checkForUseOfVoidResult(expression) &&
+        !_typeSystem.isAssignableTo(type, _boolType, featureSet: _featureSet)) {
+      if (type.element == _boolType.element) {
+        _errorReporter.reportErrorForNode(
+            StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE, expression);
+      } else {
+        _errorReporter.reportErrorForNode(errorCode, expression);
       }
-    } else if (type is FunctionType) {
-      _errorReporter.reportErrorForNode(
-          StaticTypeWarningCode.NON_BOOL_EXPRESSION, expression);
     }
   }
 
   /**
    * Checks to ensure that the given [expression] is assignable to bool.
    */
   void _checkForNonBoolNegationExpression(Expression expression) {
-    DartType conditionType = getStaticType(expression);
-    if (conditionType != null &&
-        !_typeSystem.isAssignableTo(conditionType, _boolType,
-            featureSet: _featureSet)) {
-      if (conditionType.element == _boolType.element) {
-        _errorReporter.reportErrorForNode(
-            StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE, expression);
-      } else {
-        _errorReporter.reportErrorForNode(
-            StaticTypeWarningCode.NON_BOOL_NEGATION_EXPRESSION, expression);
-      }
-    }
+    _checkForNonBoolExpression(expression,
+        errorCode: StaticTypeWarningCode.NON_BOOL_NEGATION_EXPRESSION);
   }
 
   /**
diff --git a/pkg/dev_compiler/test/sourcemap/testfiles/next_through_assert.dart b/pkg/dev_compiler/test/sourcemap/testfiles/next_through_assert.dart
@@ -7,6 +7,6 @@ void main() {
   assert(/*bc:1*/ foo());
 }
 
-foo() {
+bool foo() {
   return true;
 }

Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,6 @@ void main() {`
`7`	`7`	`assert(/bc:1/ foo());`
`8`	`8`	`}`
`9`	`9`
`10`		`-foo() {`
	`10`	`+bool foo() {`
`11`	`11`	`return true;`
`12`	`12`	`}`