From bd528bfbd69deecd3e8ad21e634da495bf0c09bb Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Thu, 30 Jul 2020 19:12:38 +0000 Subject: [PATCH] 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 Commit-Queue: Konstantin Shcheglov --- .../element/replace_top_bottom_visitor.dart | 39 +++-- .../lib/src/dart/element/type_schema.dart | 18 --- .../src/error/type_arguments_verifier.dart | 3 - .../dart/element/replace_top_bottom_test.dart | 135 ++++++++++++++++++ .../test/src/dart/element/test_all.dart | 2 + 5 files changed, 156 insertions(+), 41 deletions(-) create mode 100644 pkg/analyzer/test/src/dart/element/replace_top_bottom_test.dart diff --git a/pkg/analyzer/lib/src/dart/element/replace_top_bottom_visitor.dart b/pkg/analyzer/lib/src/dart/element/replace_top_bottom_visitor.dart index 7525af9745c..7e5e4573590 100644 --- a/pkg/analyzer/lib/src/dart/element/replace_top_bottom_visitor.dart +++ b/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 diff --git a/pkg/analyzer/lib/src/dart/element/type_schema.dart b/pkg/analyzer/lib/src/dart/element/type_schema.dart index a26acd1685e..3be3d578411 100644 --- a/pkg/analyzer/lib/src/dart/element/type_schema.dart +++ b/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 argumentTypes, List parameterTypes) { diff --git a/pkg/analyzer/lib/src/error/type_arguments_verifier.dart b/pkg/analyzer/lib/src/error/type_arguments_verifier.dart index 02b7fad9460..ecf7baa58c2 100644 --- a/pkg/analyzer/lib/src/error/type_arguments_verifier.dart +++ b/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) { diff --git a/pkg/analyzer/test/src/dart/element/replace_top_bottom_test.dart b/pkg/analyzer/test/src/dart/element/replace_top_bottom_test.dart new file mode 100644 index 00000000000..e3a64cf6a35 --- /dev/null +++ b/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*'); + } + + 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'); + _check(listNone(intQuestion), 'List'); + _check(listQuestion(intNone), 'List?'); + _check(listQuestion(intQuestion), 'List?'); + } + + 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); + } +} diff --git a/pkg/analyzer/test/src/dart/element/test_all.dart b/pkg/analyzer/test/src/dart/element/test_all.dart index 0d31f546628..f59885f7084 100644 --- a/pkg/analyzer/test/src/dart/element/test_all.dart +++ b/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();