Move reporting INVALID_ASSIGNMENT for compound assignments into Stati…

…cTypeAnalyzer.

R=brianwilkerson@google.com

Change-Id: Ia5355023ff5873c7d10eb60b85c5fd8a41dea417
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115800
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

pkg/analyzer/lib/src/generated/error_verifier.dart

@@ -418,7 +418,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         operatorType == TokenType.QUESTION_QUESTION_EQ) {
       _checkForInvalidAssignment(lhs, rhs);
     } else {
-      _checkForInvalidCompoundAssignment(node, lhs, rhs);
       _checkForArgumentTypeNotAssignableForArgument(rhs);
       _checkForNullableDereference(lhs);
     }
@@ -3474,27 +3473,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         rhs, leftType, StaticTypeWarningCode.INVALID_ASSIGNMENT);
   }
 
-  /**
-   * Given an [assignment] using a compound assignment operator, this verifies
-   * that the given assignment is valid. The [lhs] is the left hand side
-   * expression. The [rhs] is the right hand side expression.
-   *
-   * See [StaticTypeWarningCode.INVALID_ASSIGNMENT].
-   */
-  void _checkForInvalidCompoundAssignment(
-      AssignmentExpression assignment, Expression lhs, Expression rhs) {
-    if (lhs == null) {
-      return;
-    }
-    DartType leftType = getStaticType(lhs);
-    DartType rightType = getStaticType(assignment);
-    if (!_typeSystem.isAssignableTo(rightType, leftType,
-        featureSet: _featureSet)) {
-      _errorReporter.reportTypeErrorForNode(
-          StaticTypeWarningCode.INVALID_ASSIGNMENT, rhs, [rightType, leftType]);
-    }
-  }
-
   /**
    * Check the given [initializer] to ensure that the field being initialized is
    * a valid field. The [fieldName] is the field name from the

pkg/analyzer/lib/src/generated/static_type_analyzer.dart

@@ -322,15 +322,26 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<void> {
         operator == TokenType.BAR_BAR_EQ) {
       _recordStaticType(node, _nonNullable(_typeProvider.boolType));
     } else {
-      ExecutableElement staticMethodElement = node.staticElement;
-      DartType staticType = _computeStaticReturnType(staticMethodElement);
-      staticType = _typeSystem.refineBinaryExpressionType(
-          _getStaticType(node.leftHandSide, read: true),
-          operator,
-          node.rightHandSide.staticType,
-          staticType,
-          _featureSet);
-      _recordStaticType(node, staticType);
+      var operatorElement = node.staticElement;
+      var type = operatorElement?.returnType ?? _dynamicType;
+      type = _typeSystem.refineBinaryExpressionType(
+        _getStaticType(node.leftHandSide, read: true),
+        operator,
+        node.rightHandSide.staticType,
+        type,
+        _featureSet,
+      );
+      _recordStaticType(node, type);
+
+      var leftWriteType = _getStaticType(node.leftHandSide);
+      if (!_typeSystem.isAssignableTo(type, leftWriteType,
+          featureSet: _featureSet)) {
+        _resolver.errorReporter.reportTypeErrorForNode(
+          StaticTypeWarningCode.INVALID_ASSIGNMENT,
+          node.rightHandSide,
+          [type, leftWriteType],
+        );
+      }
     }
     _nullShortingTermination(node);
   }

pkg/analyzer/test/generated/static_type_analyzer_test.dart

@@ -428,26 +428,6 @@ class StaticTypeAnalyzerTest extends EngineTestCase with ResourceProviderMixin {
     _listener.assertNoErrors();
   }
 
-  void test_visitAssignmentExpression_compound_II() {
-    validate(TokenType operator) {
-      InterfaceType numType = _typeProvider.numType;
-      InterfaceType intType = _typeProvider.intType;
-      SimpleIdentifier identifier = _resolvedVariable(intType, "i");
-      AssignmentExpression node = AstTestFactory.assignmentExpression(
-          identifier, operator, _resolvedInteger(1));
-      MethodElement plusMethod = getMethod(numType, "+");
-      node.staticElement = plusMethod;
-      expect(_analyze(node), same(intType));
-      _listener.assertNoErrors();
-    }
-
-    validate(TokenType.MINUS_EQ);
-    validate(TokenType.PERCENT_EQ);
-    validate(TokenType.PLUS_EQ);
-    validate(TokenType.STAR_EQ);
-    validate(TokenType.TILDE_SLASH_EQ);
-  }
-
   void test_visitAssignmentExpression_compound_lazy() {
     validate(TokenType operator) {
       InterfaceType boolType = _typeProvider.boolType;
@@ -462,46 +442,6 @@ class StaticTypeAnalyzerTest extends EngineTestCase with ResourceProviderMixin {
     validate(TokenType.BAR_BAR_EQ);
   }
 
-  void test_visitAssignmentExpression_compound_plusID() {
-    validate(TokenType operator) {
-      InterfaceType numType = _typeProvider.numType;
-      InterfaceType intType = _typeProvider.intType;
-      InterfaceType doubleType = _typeProvider.doubleType;
-      SimpleIdentifier identifier = _resolvedVariable(intType, "i");
-      AssignmentExpression node = AstTestFactory.assignmentExpression(
-          identifier, operator, _resolvedDouble(1.0));
-      MethodElement plusMethod = getMethod(numType, "+");
-      node.staticElement = plusMethod;
-      expect(_analyze(node), same(doubleType));
-      _listener.assertNoErrors();
-    }
-
-    validate(TokenType.MINUS_EQ);
-    validate(TokenType.PERCENT_EQ);
-    validate(TokenType.PLUS_EQ);
-    validate(TokenType.STAR_EQ);
-  }
-
-  void test_visitAssignmentExpression_compoundIfNull_differentTypes() {
-    // double d; d ??= 0
-    Expression node = AstTestFactory.assignmentExpression(
-        _resolvedVariable(_typeProvider.doubleType, 'd'),
-        TokenType.QUESTION_QUESTION_EQ,
-        _resolvedInteger(0));
-    expect(_analyze(node), _typeProvider.numType);
-    _listener.assertNoErrors();
-  }
-
-  void test_visitAssignmentExpression_compoundIfNull_sameTypes() {
-    // int i; i ??= 0
-    Expression node = AstTestFactory.assignmentExpression(
-        _resolvedVariable(_typeProvider.intType, 'i'),
-        TokenType.QUESTION_QUESTION_EQ,
-        _resolvedInteger(0));
-    expect(_analyze(node), same(_typeProvider.intType));
-    _listener.assertNoErrors();
-  }
-
   void test_visitAssignmentExpression_simple() {
     // i = 0
     InterfaceType intType = _typeProvider.intType;

pkg/analyzer/test/src/dart/resolution/assignment_test.dart

@@ -4,6 +4,7 @@
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/src/error/codes.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
@@ -131,6 +132,63 @@ class C {
     assertType(right, 'int');
   }
 
+  test_compound_refineType_int_double() async {
+    await assertErrorsInCode(r'''
+main(int i) {
+  i += 1.2;
+  i -= 1.2;
+  i *= 1.2;
+  i %= 1.2;
+}
+''', [
+      error(StaticTypeWarningCode.INVALID_ASSIGNMENT, 21, 3),
+      error(StaticTypeWarningCode.INVALID_ASSIGNMENT, 33, 3),
+      error(StaticTypeWarningCode.INVALID_ASSIGNMENT, 45, 3),
+      error(StaticTypeWarningCode.INVALID_ASSIGNMENT, 57, 3),
+    ]);
+    assertType(findNode.assignment('+='), 'double');
+    assertType(findNode.assignment('-='), 'double');
+    assertType(findNode.assignment('*='), 'double');
+    assertType(findNode.assignment('%='), 'double');
+  }
+
+  test_compound_refineType_int_int() async {
+    await assertNoErrorsInCode(r'''
+main(int i) {
+  i += 1;
+  i -= 1;
+  i *= 1;
+  i ~/= 1;
+  i %= 1;
+}
+''');
+    assertType(findNode.assignment('+='), 'int');
+    assertType(findNode.assignment('-='), 'int');
+    assertType(findNode.assignment('*='), 'int');
+    assertType(findNode.assignment('~/='), 'int');
+    assertType(findNode.assignment('%='), 'int');
+  }
+
+  test_compoundIfNull_differentTypes() async {
+    await assertErrorsInCode(r'''
+main(double a, int b) {
+  a ??= b;
+}
+''', [
+      error(StaticTypeWarningCode.INVALID_ASSIGNMENT, 32, 1),
+    ]);
+    assertType(findNode.assignment('??='), 'num');
+  }
+
+  test_compoundIfNull_sameTypes() async {
+    await assertNoErrorsInCode(r'''
+main(int a) {
+  a ??= 0;
+}
+''');
+    assertType(findNode.assignment('??='), 'int');
+  }
+
   test_in_const_context() async {
     addTestFile('''
 void f(num x, int y) {

pkg/analyzer/test/src/diagnostics/unchecked_use_of_nullable_value_test.dart

@@ -144,13 +144,6 @@ m(B b) {
     assertType(assignment2, 'int');
   }
 
-  @FailingTest(reason: r'''
-This test fails because verifier checks that the type of `b.a?.x += 1`, which
-is the type of `+` invocation, is assignable to `b.a?.x` type. But with NNBD
-it is not. The type of `b.a?.x += 1` is `int?`, because of shortening. But
-because of the same shortening the assignment is performed only when `b.a` is
-not null, and the type to check should be `int`.
-''')
   test_assignment_plusEq_propertyAccess3_short1() async {
     await assertErrorsInCode(r'''
 class A {