Issue 39875. Don't report unchecked nullable when resovled to an extension with nullable extended type.

Bug: https://github.com/dart-lang/sdk/issues/39875
Change-Id: I8beff226da4b301b0d2903788b44705985c89823
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/129323
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>

pkg/_fe_analyzer_shared/test/flow_analysis/nullability/data/null_aware_access.dart

@@ -15,7 +15,7 @@ class C {
 
 class D {
   void set setter(value) {}
-  D? operator [](index) => this;
+  C? operator [](index) => C();
   void operator []=(index, value) {}
   D get getterSetter => this;
   void set getterSetter(value) {}

pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart

@@ -148,7 +148,8 @@ class MethodInvocationResolver {
     }
 
     if (receiverType is InterfaceType) {
-      _resolveReceiverInterfaceType(node, receiverType, nameNode, name);
+      _resolveReceiverInterfaceType(
+          node, receiver, receiverType, nameNode, name);
       return;
     }
 
@@ -509,16 +510,20 @@ class MethodInvocationResolver {
     );
   }
 
-  void _resolveReceiverInterfaceType(MethodInvocation node,
+  void _resolveReceiverInterfaceType(MethodInvocation node, Expression receiver,
       InterfaceType receiverType, SimpleIdentifier nameNode, String name) {
     if (_isCoreFunction(receiverType) &&
         name == FunctionElement.CALL_METHOD_NAME) {
+      _resolver.nullableDereferenceVerifier
+          .methodInvocation(receiver, receiverType, name);
       _setDynamicResolution(node);
       return;
     }
 
     var target = _inheritance.getMember(receiverType, _currentName);
     if (target != null) {
+      _resolver.nullableDereferenceVerifier
+          .methodInvocation(receiver, receiverType, name);
       target = _resolver.toLegacyElement(target);
       nameNode.staticElement = target;
       if (target is PropertyAccessorElement) {

pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart

@@ -0,0 +1,71 @@
+// Copyright (c) 2019, 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.
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/error/listener.dart';
+import 'package:analyzer/src/dart/element/type.dart';
+import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/generated/resolver.dart';
+
+/// Helper for checking potentially nullable dereferences.
+class NullableDereferenceVerifier {
+  /// Properties on the object class which are safe to call on nullable types.
+  ///
+  /// Note that this must include tear-offs.
+  ///
+  /// TODO(mfairhurst): Calculate these fields rather than hard-code them.
+  static const _objectPropertyNames = {
+    'hashCode',
+    'runtimeType',
+    'noSuchMethod',
+    'toString',
+  };
+
+  final TypeSystemImpl _typeSystem;
+  final ErrorReporter _errorReporter;
+
+  NullableDereferenceVerifier(this._typeSystem, this._errorReporter);
+
+  void methodInvocation(
+    Expression receiver,
+    DartType receiverType,
+    String methodName,
+  ) {
+    if (!_typeSystem.isNonNullableByDefault) {
+      return;
+    }
+
+    if (methodName == 'toString' || methodName == 'noSuchMethod') {
+      return;
+    }
+
+    _check(receiver, receiverType);
+  }
+
+  void propertyAccess(Expression receiver, DartType receiverType, String name) {
+    if (!_typeSystem.isNonNullableByDefault) {
+      return;
+    }
+
+    if (_objectPropertyNames.contains(name)) {
+      return;
+    }
+
+    _check(receiver, receiverType);
+  }
+
+  /// If the [receiverType] is potentially nullable, report it.
+  void _check(Expression receiver, DartType receiverType) {
+    if (identical(receiverType, DynamicTypeImpl.instance) ||
+        !_typeSystem.isPotentiallyNullable(receiverType)) {
+      return;
+    }
+
+    var errorCode = receiverType == _typeSystem.typeProvider.nullType
+        ? StaticWarningCode.INVALID_USE_OF_NULL_VALUE
+        : StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE;
+    _errorReporter.reportErrorForNode(errorCode, receiver);
+  }
+}

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

@@ -175,8 +175,9 @@ class ElementResolver extends SimpleAstVisitor<void> {
         String methodName = operatorType.lexeme;
         // TODO(brianwilkerson) Change the [methodNameNode] from the left hand
         //  side to the operator.
-        var result = _newPropertyResolver()
-            .resolve(leftHandSide, staticType, methodName, leftHandSide);
+        var result = _newPropertyResolver().resolve(
+            leftHandSide, staticType, methodName, leftHandSide,
+            isNullAware: false);
         node.staticElement = result.getter;
         if (_shouldReportInvalidMember(staticType, result)) {
           _recordUndefinedToken(
@@ -490,8 +491,9 @@ class ElementResolver extends SimpleAstVisitor<void> {
     if (target is ExtensionOverride) {
       result = _extensionResolver.getOverrideMember(target, getterMethodName);
     } else {
-      result = _newPropertyResolver()
-          .resolve(target, targetType, getterMethodName, target);
+      result = _newPropertyResolver().resolve(
+          target, targetType, getterMethodName, target,
+          isNullAware: node.isNullAware);
     }
 
     bool isInGetterContext = node.inGetterContext();
@@ -563,7 +565,7 @@ class ElementResolver extends SimpleAstVisitor<void> {
     String methodName = _getPostfixOperator(node);
     DartType staticType = _getStaticType(operand);
     var result = _newPropertyResolver()
-        .resolve(operand, staticType, methodName, operand);
+        .resolve(operand, staticType, methodName, operand, isNullAware: false);
     node.staticElement = result.getter;
     if (_shouldReportInvalidMember(staticType, result)) {
       if (operand is SuperExpression) {
@@ -686,8 +688,9 @@ class ElementResolver extends SimpleAstVisitor<void> {
         return;
       }
       DartType staticType = _getStaticType(operand, read: true);
-      var result = _newPropertyResolver()
-          .resolve(operand, staticType, methodName, operand);
+      var result = _newPropertyResolver().resolve(
+          operand, staticType, methodName, operand,
+          isNullAware: false);
       node.staticElement = result.getter;
       if (_shouldReportInvalidMember(staticType, result)) {
         if (operand is SuperExpression) {
@@ -888,7 +891,8 @@ class ElementResolver extends SimpleAstVisitor<void> {
         enclosingClass != null) {
       InterfaceType enclosingType = enclosingClass.thisType;
       var propertyResolver = _newPropertyResolver();
-      propertyResolver.resolve(null, enclosingType, node.name, node);
+      propertyResolver.resolve(null, enclosingType, node.name, node,
+          isNullAware: false);
       node.auxiliaryElements = AuxiliaryElements(
         propertyResolver.result.getter,
       );
@@ -1153,7 +1157,8 @@ class ElementResolver extends SimpleAstVisitor<void> {
     } else if (invokeType is InterfaceType) {
       var propertyResolver = _newPropertyResolver();
       propertyResolver.resolve(null, invokeType,
-          FunctionElement.CALL_METHOD_NAME, invocation.function);
+          FunctionElement.CALL_METHOD_NAME, invocation.function,
+          isNullAware: false);
       ExecutableElement callMethod = propertyResolver.result.getter;
       invocation.staticElement = callMethod;
       parameterizableType = _elementTypeProvider.safeExecutableType(callMethod);
@@ -1488,8 +1493,9 @@ class ElementResolver extends SimpleAstVisitor<void> {
         return;
       }
       DartType leftType = _getStaticType(leftOperand);
-      ResolutionResult result = _newPropertyResolver()
-          .resolve(leftOperand, leftType, methodName, node);
+      ResolutionResult result = _newPropertyResolver().resolve(
+          leftOperand, leftType, methodName, node,
+          isNullAware: methodName == '==');
 
       node.staticElement = result.getter;
       node.staticInvokeType =
@@ -1779,8 +1785,9 @@ class ElementResolver extends SimpleAstVisitor<void> {
       return;
     }
 
-    var result = _newPropertyResolver()
-        .resolve(target, staticType, propertyName.name, propertyName);
+    var result = _newPropertyResolver().resolve(
+        target, staticType, propertyName.name, propertyName,
+        isNullAware: isNullAware);
 
     if (propertyName.inGetterContext()) {
       var shouldReportUndefinedGetter = false;
@@ -1855,7 +1862,8 @@ class ElementResolver extends SimpleAstVisitor<void> {
           if (enclosingClass != null) {
             var propertyResolver = _newPropertyResolver();
             propertyResolver.resolve(
-                null, enclosingClass.thisType, identifier.name, identifier);
+                null, enclosingClass.thisType, identifier.name, identifier,
+                isNullAware: false);
             setter = propertyResolver.result.setter;
           }
         }
@@ -1893,7 +1901,8 @@ class ElementResolver extends SimpleAstVisitor<void> {
       if (element == null && enclosingType != null) {
         var propertyResolver = _newPropertyResolver();
         propertyResolver.resolve(
-            null, enclosingType, identifier.name, identifier);
+            null, enclosingType, identifier.name, identifier,
+            isNullAware: false);
         if (identifier.inSetterContext() ||
             identifier.parent is CommentReference) {
           element = propertyResolver.result.setter;
@@ -2071,8 +2080,9 @@ class _PropertyResolver {
     Expression target,
     DartType type,
     String name,
-    Expression errorNode,
-  ) {
+    Expression errorNode, {
+    @required bool isNullAware,
+  }) {
     type = _resolveTypeParameter(type);
 
     ExecutableElement typeGetter;
@@ -2121,6 +2131,10 @@ class _PropertyResolver {
       result = ResolutionResult(getter: typeGetter, setter: typeSetter);
     }
 
+    if (!isNullAware && result.isSingle) {
+      _resolver.nullableDereferenceVerifier.propertyAccess(target, type, name);
+    }
+
     if (result.isNone) {
       result = _extensionResolver.findExtension(type, name, errorNode);
     }

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

@@ -38,20 +38,6 @@ import 'package:meta/meta.dart';
  * warnings not covered by the parser and resolver.
  */
 class ErrorVerifier extends RecursiveAstVisitor<void> {
-  /**
-   * Properties on the object class which are safe to call on nullable types.
-   *
-   * Note that this must include tear-offs.
-   *
-   * TODO(mfairhurst): Calculate these fields rather than hard-code them.
-   */
-  static final _objectPropertyNames = {
-    'hashCode',
-    'runtimeType',
-    'noSuchMethod',
-    'toString',
-  };
-
   /**
    * The error reporter by which errors will be reported.
    */
@@ -415,7 +401,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
           promoteParameterToNullable: true);
     } else if (type != TokenType.QUESTION_QUESTION) {
       _checkForArgumentTypeNotAssignableForArgument(node.rightOperand);
-      _checkForNullableDereference(node.leftOperand);
     } else {
       _checkForArgumentTypeNotAssignableForArgument(node.rightOperand);
     }
@@ -932,8 +917,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         node.realTarget,
         node.period ?? node.leftBracket,
       );
-    } else {
-      _checkForNullableDereference(node.realTarget);
     }
     super.visitIndexExpression(node);
   }
@@ -1040,11 +1023,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     _typeArgumentsVerifier.checkMethodInvocation(node);
     _checkForNullableDereference(methodName);
     _requiredParametersVerifier.visitMethodInvocation(node);
-    if (!node.isNullAware &&
-        methodName.name != 'toString' &&
-        methodName.name != 'noSuchMethod') {
-      _checkForNullableDereference(target);
-    }
     super.visitMethodInvocation(node);
   }
 
@@ -1106,7 +1084,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     } else {
       _checkForAssignmentToFinal(node.operand);
       _checkForIntNotAssignable(node.operand);
-      _checkForNullableDereference(node.operand);
     }
     super.visitPostfixExpression(node);
   }
@@ -1120,11 +1097,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       _checkForStaticAccessToInstanceMember(typeReference, name);
       _checkForInstanceAccessToStaticMember(typeReference, node.prefix, name);
     }
-    String property = node.identifier.name;
-    if (node.staticElement is ExecutableElement &&
-        !_objectPropertyNames.contains(property)) {
-      _checkForNullableDereference(node.prefix);
-    }
     super.visitPrefixedIdentifier(node);
   }
 
@@ -1138,7 +1110,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       if (operatorType.isIncrementOperator) {
         _checkForAssignmentToFinal(operand);
       }
-      _checkForNullableDereference(operand);
       _checkForUseOfVoidResult(operand);
       _checkForIntNotAssignable(operand);
     }
@@ -1153,10 +1124,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     _checkForStaticAccessToInstanceMember(typeReference, propertyName);
     _checkForInstanceAccessToStaticMember(
         typeReference, node.target, propertyName);
-    if (!node.isNullAware &&
-        !_objectPropertyNames.contains(propertyName.name)) {
-      _checkForNullableDereference(target);
-    }
     _checkForUnnecessaryNullAware(target, node.operator);
     super.visitPropertyAccess(node);
   }

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

@@ -30,6 +30,7 @@ import 'package:analyzer/src/dart/resolver/method_invocation_resolver.dart';
 import 'package:analyzer/src/dart/resolver/scope.dart';
 import 'package:analyzer/src/diagnostic/diagnostic_factory.dart';
 import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/error/nullable_dereference_verifier.dart';
 import 'package:analyzer/src/generated/constant.dart';
 import 'package:analyzer/src/generated/element_resolver.dart';
 import 'package:analyzer/src/generated/element_type_provider.dart';
@@ -464,6 +465,9 @@ class ResolverVisitor extends ScopedVisitor {
 
   final ElementTypeProvider _elementTypeProvider;
 
+  /// Helper for checking potentially nullable dereferences.
+  NullableDereferenceVerifier nullableDereferenceVerifier;
+
   /// Helper for extension method resolution.
   ExtensionMemberResolver extensionResolver;
 
@@ -579,6 +583,10 @@ class ResolverVisitor extends ScopedVisitor {
         super(definingLibrary, source, typeProvider, errorListener,
             nameScope: nameScope) {
     this._promoteManager = TypePromotionManager(typeSystem);
+    this.nullableDereferenceVerifier = NullableDereferenceVerifier(
+      typeSystem,
+      errorReporter,
+    );
     this.extensionResolver = ExtensionMemberResolver(this);
     this.elementResolver = ElementResolver(this,
         reportConstEvaluationErrors: reportConstEvaluationErrors,

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

@@ -1448,6 +1448,21 @@ class ExtensionMethodsExternalReferenceWithNnbdTest
   @override
   bool get typeToStringWithNullability => true;
 
+  test_instance_getter_fromInstance_nullable() async {
+    await assertNoErrorsInCode('''
+extension E on int? {
+  int get foo => 0;
+}
+
+f(int? a) {
+  a.foo;
+}
+''');
+    var access = findNode.prefixed('a.foo');
+    assertElement(access, findElement.getter('foo', of: 'E'));
+    assertType(access, 'int');
+  }
+
   test_instance_getter_fromInstance_nullAware() async {
     await assertNoErrorsInCode('''
 extension E on int {
@@ -1463,6 +1478,21 @@ f(int? a) {
     assertType(identifier, 'int');
   }
 
+  test_instance_method_fromInstance_nullable() async {
+    await assertNoErrorsInCode('''
+extension E on int? {
+  void foo() {}
+}
+
+f(int? a) {
+  a.foo();
+}
+''');
+    var invocation = findNode.methodInvocation('a.foo()');
+    assertElement(invocation, findElement.method('foo', of: 'E'));
+    assertInvokeType(invocation, 'void Function()');
+  }
+
   test_instance_method_fromInstance_nullAware() async {
     await assertNoErrorsInCode('''
 extension E on int {
@@ -1478,7 +1508,38 @@ f(int? a) {
     assertInvokeType(invocation, 'void Function()');
   }
 
-  test_instance_operator_index_formInstance_nullAware() async {
+  test_instance_operator_binary_fromInstance_nullable() async {
+    await assertNoErrorsInCode('''
+class A {}
+
+extension E on A? {
+  int operator +(int _) => 0;
+}
+
+f(A? a) {
+  a + 1;
+}
+''');
+    var binary = findNode.binary('a + 1');
+    assertElement(binary, findElement.method('+'));
+    assertType(binary, 'int');
+  }
+
+  test_instance_operator_index_fromInstance_nullable() async {
+    await assertNoErrorsInCode('''
+extension E on int? {
+  int operator [](int index) => 0;
+}
+
+f(int? a) {
+  a[0];
+}
+''');
+    var index = findNode.index('a[0]');
+    assertElement(index, findElement.method('[]'));
+  }
+
+  test_instance_operator_index_fromInstance_nullAware() async {
     await assertNoErrorsInCode('''
 extension E on int {
   int operator [](int index) => 0;
@@ -1489,7 +1550,72 @@ f(int? a) {
 }
 ''');
     var index = findNode.index('a?.[0]');
-    assertElement(index, findElement.method('[]', of: 'E'));
+    assertElement(index, findElement.method('[]'));
+  }
+
+  test_instance_operator_postfixInc_fromInstance_nullable() async {
+    await assertNoErrorsInCode('''
+class A {}
+
+extension E on A? {
+  A? operator +(int _) => this;
+}
+
+f(A? a) {
+  a++;
+}
+''');
+    var expression = findNode.postfix('a++');
+    assertElement(expression, findElement.method('+'));
+    assertType(expression, 'A?');
+  }
+
+  test_instance_operator_prefixInc_fromInstance_nullable() async {
+    await assertNoErrorsInCode('''
+class A {}
+
+extension E on A? {
+  A? operator +(int _) => this;
+}
+
+f(A? a) {
+  ++a;
+}
+''');
+    var expression = findNode.prefix('++a');
+    assertElement(expression, findElement.method('+'));
+    assertType(expression, 'A?');
+  }
+
+  test_instance_operator_unaryMinus_fromInstance_nullable() async {
+    await assertNoErrorsInCode('''
+class A {}
+
+extension E on A? {
+  A? operator -() => this;
+}
+
+f(A? a) {
+  -a;
+}
+''');
+    var expression = findNode.prefix('-a');
+    assertElement(expression, findElement.method('unary-'));
+    assertType(expression, 'A?');
+  }
+
+  test_instance_setter_fromInstance_nullable() async {
+    await assertNoErrorsInCode('''
+extension E on int? {
+  set foo(int _) {}
+}
+
+f(int? a) {
+  a.foo = 1;
+}
+''');
+    var access = findNode.prefixed('a.foo');
+    assertElement(access, findElement.setter('foo'));
   }
 
   test_instance_setter_fromInstance_nullAware() async {

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

@@ -305,6 +305,20 @@ m() {
     ]);
   }
 
+  test_getter_nullable_notNullableExtension() async {
+    await assertErrorsInCode(r'''
+extension E on int {
+  int get foo => 0;
+}
+
+m(int? x) {
+  x.foo;
+}
+''', [
+      error(StaticTypeWarningCode.UNDEFINED_GETTER, 60, 3),
+    ]);
+  }
+
   test_if_nonNullable() async {
     await assertNoErrorsInCode(r'''
 m() {
@@ -536,6 +550,20 @@ m() {
     ]);
   }
 
+  test_method_nullable_notNullableExtension() async {
+    await assertErrorsInCode(r'''
+extension E on int {
+  void foo() {}
+}
+
+m(int? x) {
+  x.foo();
+}
+''', [
+      error(StaticTypeWarningCode.UNDEFINED_METHOD, 56, 3),
+    ]);
+  }
+
   test_method_questionDot_nullable() async {
     await assertNoErrorsInCode(r'''
 m() {
@@ -553,6 +581,24 @@ m(int x) {
 ''');
   }
 
+  test_methodInvocation_call_notNullable() async {
+    await assertNoErrorsInCode(r'''
+m(Function x) {
+  x.call();
+}
+''');
+  }
+
+  test_methodInvocation_call_nullable() async {
+    await assertErrorsInCode(r'''
+m(Function? x) {
+  x.call();
+}
+''', [
+      error(StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE, 19, 1),
+    ]);
+  }
+
   test_minusEq_nonNullable() async {
     await assertNoErrorsInCode(r'''
 m() {
@@ -682,6 +728,22 @@ m(int? x) {
     ]);
   }
 
+  test_operatorPostfixInc_nullable_notNullableExtension() async {
+    await assertErrorsInCode(r'''
+class A {}
+
+extension E on A {
+  A operator +(int _) => this;
+}
+
+m(A? x) {
+  x++;
+}
+''', [
+      error(StaticTypeWarningCode.UNDEFINED_OPERATOR, 78, 2),
+    ]);
+  }
+
   test_operatorPrefixDec_nonNullable() async {
     await assertNoErrorsInCode(r'''
 m() {
@@ -742,6 +804,22 @@ m() {
     ]);
   }
 
+  test_operatorUnaryMinus_nullable_notNullableExtension() async {
+    await assertErrorsInCode(r'''
+class A {}
+
+extension E on A {
+  A operator -() => this;
+}
+
+m(A? x) {
+  -x;
+}
+''', [
+      error(StaticTypeWarningCode.UNDEFINED_OPERATOR, 72, 1),
+    ]);
+  }
+
   test_or_nonNullable() async {
     await assertNoErrorsInCode(r'''
 m() {