Updates and tests for ReplaceTopBottomVisitor, fix hints.

Bug: https://github.com/dart-lang/sdk/issues/42196
Change-Id: If9350ccc4ad2908a2c7cde7281fe9e2f01c52f97
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/156561
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

pkg/analyzer/lib/src/dart/element/replace_top_bottom_visitor.dart

@@ -2,18 +2,13 @@
 // 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/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/replacement_visitor.dart';
 import 'package:analyzer/src/dart/element/type_system.dart';
 import 'package:meta/meta.dart';
 
-/// Visitor that computes replaces covariant uses of Top with Bottom, and
-/// contravariant uses of Bottom with Top.
-///
-/// Each visitor method returns `null` if there are no `_`s contained in the
-/// type, otherwise it returns the result of substituting `_` with [_bottomType]
-/// or [_topType], as appropriate.
+/// Replace every "top" type in a covariant position with [_bottomType].
+/// Replace every "bottom" type in a contravariant position with [_topType].
 class ReplaceTopBottomVisitor extends ReplacementVisitor {
   final DartType _topType;
   final DartType _bottomType;
@@ -34,30 +29,34 @@ class ReplaceTopBottomVisitor extends ReplacementVisitor {
   }
 
   @override
-  DartType visitDynamicType(DynamicType type) =>
-      _isCovariant ? _bottomType : type;
+  DartType visitDynamicType(DynamicType type) {
+    return _isCovariant ? _bottomType : null;
+  }
 
   @override
   DartType visitInterfaceType(InterfaceType type) {
-    if (_typeSystem.isTop(type)) {
-      return _isCovariant ? _bottomType : type;
-    }
-    if (_typeSystem.isBottom(type) ||
-        (!_typeSystem.isNonNullableByDefault && type.isDartCoreNull)) {
-      return _isCovariant ? type : _topType;
+    if (_isCovariant) {
+      if (_typeSystem.isTop(type)) {
+        return _bottomType;
+      }
+    } else {
+      if (!_typeSystem.isNonNullableByDefault && type.isDartCoreNull) {
+        return _topType;
+      }
     }
 
     return super.visitInterfaceType(type);
   }
 
   @override
-  DartType visitNeverType(NeverType type) =>
-      _isCovariant && type.nullabilitySuffix != NullabilitySuffix.question
-          ? type
-          : _topType;
+  DartType visitNeverType(NeverType type) {
+    return _isCovariant ? null : _topType;
+  }
 
   @override
-  DartType visitVoidType(VoidType type) => _isCovariant ? _bottomType : type;
+  DartType visitVoidType(VoidType type) {
+    return _isCovariant ? _bottomType : null;
+  }
 
   /// Runs an instance of the visitor on the given [type] and returns the
   /// resulting type.  If the type contains no instances of Top or Bottom, the

pkg/analyzer/lib/src/dart/element/type_schema.dart

@@ -5,7 +5,6 @@
 import 'package:analyzer/dart/ast/token.dart' show Keyword;
 import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
-import 'package:analyzer/dart/element/type_provider.dart';
 import 'package:analyzer/dart/element/type_visitor.dart';
 import 'package:analyzer/src/dart/element/display_string_builder.dart';
 import 'package:analyzer/src/dart/element/type.dart';
@@ -53,23 +52,6 @@ class UnknownInferredType extends TypeImpl {
     builder.writeUnknownInferredType();
   }
 
-  @override
-  DartType replaceTopAndBottom(TypeProvider typeProvider,
-      {bool isCovariant = true}) {
-    // In theory this should never happen, since we only need to do this
-    // replacement when checking super-boundedness of explicitly-specified
-    // types, or types produced by mixin inference or instantiate-to-bounds, and
-    // the unknown type can't occur in any of those cases.
-    assert(
-        false, 'Attempted to check super-boundedness of a type including "_"');
-    // But just in case it does, behave similar to `dynamic`.
-    if (isCovariant) {
-      return typeProvider.nullType;
-    } else {
-      return this;
-    }
-  }
-
   @override
   DartType substitute2(
       List<DartType> argumentTypes, List<DartType> parameterTypes) {

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

@@ -7,7 +7,6 @@ import "dart:math" as math;
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
-import 'package:analyzer/dart/element/type_provider.dart';
 import 'package:analyzer/error/error.dart';
 import 'package:analyzer/error/listener.dart';
 import 'package:analyzer/src/dart/element/type_algebra.dart';
@@ -28,8 +27,6 @@ class TypeArgumentsVerifier {
     this._errorReporter,
   );
 
-  TypeProvider get _typeProvider => _libraryElement.typeProvider;
-
   TypeSystemImpl get _typeSystem => _libraryElement.typeSystem;
 
   void checkFunctionExpressionInvocation(FunctionExpressionInvocation node) {

pkg/analyzer/test/src/dart/element/replace_top_bottom_test.dart

@@ -0,0 +1,135 @@
+// Copyright (c) 2020, 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/element/null_safety_understanding_flag.dart';
+import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/src/dart/element/type.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../../../generated/type_system_test.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(ReplaceTopBottomLegacyTest);
+    defineReflectiveTests(ReplaceTopBottomNullSafetyTest);
+  });
+}
+
+@reflectiveTest
+class ReplaceTopBottomLegacyTest extends AbstractTypeSystemTest {
+  test_contravariant_bottom() {
+    // Not contravariant.
+    _check(nullStar, 'Null*');
+
+    _check(
+      functionTypeStar(returnType: intStar, parameters: [
+        requiredParameter(type: nullStar),
+      ]),
+      'int* Function(dynamic)*',
+    );
+  }
+
+  test_covariant_top() {
+    _check(objectStar, 'Null*');
+    _check(dynamicNone, 'Null*');
+    _check(voidNone, 'Null*');
+
+    _check(futureOrStar(objectStar), 'Null*');
+    _check(futureOrStar(dynamicNone), 'Null*');
+    _check(futureOrStar(voidNone), 'Null*');
+    _check(futureOrStar(futureOrStar(voidNone)), 'Null*');
+
+    _check(
+      functionTypeStar(returnType: intStar, parameters: [
+        requiredParameter(
+          type: functionTypeStar(returnType: intStar, parameters: [
+            requiredParameter(type: objectStar),
+          ]),
+        ),
+      ]),
+      'int* Function(int* Function(Null*)*)*',
+      typeStr: 'int* Function(int* Function(Object*)*)*',
+    );
+
+    _check(listStar(intStar), 'List<int*>*');
+  }
+
+  void _check(DartType type, String expectedStr, {String typeStr}) {
+    NullSafetyUnderstandingFlag.enableNullSafetyTypes(() {
+      if (typeStr != null) {
+        expect(_typeString(type), typeStr);
+      }
+
+      var result = typeSystem.replaceTopAndBottom(type);
+      var resultStr = _typeString(result);
+      expect(resultStr, expectedStr);
+    });
+  }
+
+  String _typeString(TypeImpl type) {
+    return type.getDisplayString(withNullability: true);
+  }
+}
+
+@reflectiveTest
+class ReplaceTopBottomNullSafetyTest extends AbstractTypeSystemNullSafetyTest {
+  test_contravariant_bottom() {
+    // Not contravariant.
+    _check(neverNone, 'Never');
+
+    _check(
+      functionTypeNone(returnType: intNone, parameters: [
+        requiredParameter(type: neverNone),
+      ]),
+      'int Function(Object?)',
+    );
+  }
+
+  test_covariant_top() {
+    _check(objectQuestion, 'Never');
+    _check(objectStar, 'Never');
+    _check(dynamicNone, 'Never');
+    _check(voidNone, 'Never');
+
+    _check(futureOrNone(objectQuestion), 'Never');
+    _check(futureOrNone(objectStar), 'Never');
+    _check(futureOrNone(dynamicNone), 'Never');
+    _check(futureOrNone(voidNone), 'Never');
+    _check(futureOrNone(futureOrNone(voidNone)), 'Never');
+
+    _check(
+      functionTypeNone(returnType: intNone, parameters: [
+        requiredParameter(
+          type: functionTypeNone(returnType: intNone, parameters: [
+            requiredParameter(type: objectQuestion),
+          ]),
+        ),
+      ]),
+      'int Function(int Function(Never))',
+      typeStr: 'int Function(int Function(Object?))',
+    );
+
+    _check(listNone(intNone), 'List<int>');
+    _check(listNone(intQuestion), 'List<int?>');
+    _check(listQuestion(intNone), 'List<int>?');
+    _check(listQuestion(intQuestion), 'List<int?>?');
+  }
+
+  void _check(DartType type, String expectedStr, {String typeStr}) {
+    NullSafetyUnderstandingFlag.enableNullSafetyTypes(() {
+      if (typeStr != null) {
+        expect(_typeString(type), typeStr);
+      }
+
+      var result = typeSystem.replaceTopAndBottom(type);
+      var resultStr = _typeString(result);
+      expect(resultStr, expectedStr);
+    });
+  }
+
+  String _typeString(TypeImpl type) {
+    return type.getDisplayString(withNullability: true);
+  }
+}

pkg/analyzer/test/src/dart/element/test_all.dart

@@ -17,6 +17,7 @@ import 'least_upper_bound_helper_test.dart' as least_upper_bound_helper;
 import 'normalize_type_test.dart' as normalize_type;
 import 'nullability_eliminator_test.dart' as nullability_eliminator;
 import 'nullable_test.dart' as nullable;
+import 'replace_top_bottom_test.dart' as replace_top_bottom;
 import 'runtime_type_equality_test.dart' as runtime_type_equality;
 import 'subtype_test.dart' as subtype;
 import 'top_merge_test.dart' as top_merge;
@@ -42,6 +43,7 @@ main() {
     normalize_type.main();
     nullability_eliminator.main();
     nullable.main();
+    replace_top_bottom.main();
     runtime_type_equality.main();
     subtype.main();
     top_merge.main();