Migration: properly handle `dynamic` in code declared `external`.
Fixes #47307.
Bug: https://github.com/dart-lang/sdk/issues/47307
Change-Id: Iad6fa8a413e81cbd8902c223fa722c7da6850aa6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214980
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/nnbd_migration/lib/instrumentation.dart b/pkg/nnbd_migration/lib/instrumentation.dart
index c2700c1..1038d97 100644
--- a/pkg/nnbd_migration/lib/instrumentation.dart
+++ b/pkg/nnbd_migration/lib/instrumentation.dart
@@ -249,6 +249,7 @@
dynamicAssignment,
enumValue,
expressionChecks,
+ externalDynamic,
fieldFormalParameter,
fieldNotInitialized,
forEachVariable,
diff --git a/pkg/nnbd_migration/lib/src/edge_origin.dart b/pkg/nnbd_migration/lib/src/edge_origin.dart
index b12f9ec..58ffe89 100644
--- a/pkg/nnbd_migration/lib/src/edge_origin.dart
+++ b/pkg/nnbd_migration/lib/src/edge_origin.dart
@@ -194,6 +194,18 @@
EdgeOriginKind get kind => EdgeOriginKind.enumValue;
}
+/// Edge origin resulting from an explicit or implicit `dynamic` type annotation
+/// appearing in an external declaration.
+class ExternalDynamicOrigin extends EdgeOrigin {
+ ExternalDynamicOrigin(Source? source, AstNode node) : super(source, node);
+
+ @override
+ String get description => 'dynamic type in external declaration';
+
+ @override
+ EdgeOriginKind get kind => EdgeOriginKind.externalDynamic;
+}
+
/// Edge origin resulting from the relationship between a field formal parameter
/// and the corresponding field.
class FieldFormalParameterOrigin extends EdgeOrigin {
diff --git a/pkg/nnbd_migration/lib/src/node_builder.dart b/pkg/nnbd_migration/lib/src/node_builder.dart
index 6673352..653d36b 100644
--- a/pkg/nnbd_migration/lib/src/node_builder.dart
+++ b/pkg/nnbd_migration/lib/src/node_builder.dart
@@ -67,6 +67,10 @@
final TypeProvider _typeProvider;
+ /// Indicates whether the declaration currently being visited is marked
+ /// `external`.
+ bool _visitingExternalDeclaration = false;
+
NodeBuilder(this._variables, this.source, this.listener, this._graph,
this._typeProvider, this._getLineInfo,
{this.instrumentation});
@@ -196,7 +200,8 @@
node.parameters,
node.initializers,
node.body,
- node.redirectedConstructor);
+ node.redirectedConstructor,
+ isExternal: node.externalKeyword != null);
return null;
}
@@ -340,14 +345,16 @@
node.functionExpression.parameters,
null,
node.functionExpression.body,
- null);
+ null,
+ isExternal: node.externalKeyword != null);
return null;
}
@override
DecoratedType? visitFunctionExpression(FunctionExpression node) {
_handleExecutableDeclaration(node, node.declaredElement!, null, null,
- node.typeParameters, node.parameters, null, node.body, null);
+ node.typeParameters, node.parameters, null, node.body, null,
+ isExternal: false);
return null;
}
@@ -456,7 +463,8 @@
node.parameters,
null,
node.body,
- null);
+ null,
+ isExternal: node.externalKeyword != null);
if (declaredElement is PropertyAccessorElement) {
// Store a decorated type for the synthetic field so that in case we try
// to access it later we won't crash (this could happen due to errors in
@@ -537,6 +545,10 @@
var nullabilityNode = NullabilityNode.forTypeAnnotation(target);
var decoratedType = DecoratedType(type, nullabilityNode);
_variables!.recordDecoratedTypeAnnotation(source, node, decoratedType);
+ if (_visitingExternalDeclaration) {
+ _graph.makeNullableUnion(
+ nullabilityNode, ExternalDynamicOrigin(source, node));
+ }
return decoratedType;
}
var typeArguments = const <DecoratedType>[];
@@ -708,49 +720,62 @@
FormalParameterList? parameters,
NodeList<ConstructorInitializer>? initializers,
FunctionBody body,
- ConstructorName? redirectedConstructor) {
+ ConstructorName? redirectedConstructor,
+ {required bool isExternal}) {
metadata?.accept(this);
- var functionType = declaredElement.type;
- DecoratedType? decoratedReturnType;
- var target = NullabilityNodeTarget.element(declaredElement, _getLineInfo);
- if (returnType != null) {
- _pushNullabilityNodeTarget(target.returnType(), () {
- decoratedReturnType = returnType.accept(this);
- });
- } else if (declaredElement is ConstructorElement) {
- // Constructors have no explicit return type annotation, so use the
- // implicit return type.
- decoratedReturnType = _createDecoratedTypeForClass(
- declaredElement.enclosingElement, parameters!.parent);
- instrumentation?.implicitReturnType(source, node, decoratedReturnType);
- } else {
- // Inferred return type.
- decoratedReturnType = DecoratedType.forImplicitType(
- _typeProvider, functionType.returnType, _graph, target);
- instrumentation?.implicitReturnType(source, node, decoratedReturnType);
- }
- var previousPositionalParameters = _positionalParameters;
- var previousNamedParameters = _namedParameters;
- _positionalParameters = [];
- _namedParameters = {};
- DecoratedType decoratedFunctionType;
+ var previouslyVisitingExternalDeclaration = _visitingExternalDeclaration;
try {
- typeParameters?.accept(this);
- _pushNullabilityNodeTarget(target, () => parameters?.accept(this));
- redirectedConstructor?.accept(this);
- initializers?.accept(this);
- decoratedFunctionType = DecoratedType(functionType, _graph.never,
- returnType: decoratedReturnType,
- positionalParameters: _positionalParameters,
- namedParameters: _namedParameters);
- body.accept(this);
+ if (isExternal) {
+ _visitingExternalDeclaration = true;
+ }
+ var functionType = declaredElement.type;
+ DecoratedType? decoratedReturnType;
+ var target = NullabilityNodeTarget.element(declaredElement, _getLineInfo);
+ if (returnType != null) {
+ _pushNullabilityNodeTarget(target.returnType(), () {
+ decoratedReturnType = returnType.accept(this);
+ });
+ } else if (declaredElement is ConstructorElement) {
+ // Constructors have no explicit return type annotation, so use the
+ // implicit return type.
+ decoratedReturnType = _createDecoratedTypeForClass(
+ declaredElement.enclosingElement, parameters!.parent);
+ instrumentation?.implicitReturnType(source, node, decoratedReturnType);
+ } else {
+ // Inferred return type.
+ decoratedReturnType = DecoratedType.forImplicitType(
+ _typeProvider, functionType.returnType, _graph, target);
+ instrumentation?.implicitReturnType(source, node, decoratedReturnType);
+ if (isExternal && functionType.returnType.isDynamic) {
+ _graph.makeNullableUnion(
+ decoratedReturnType.node!, ExternalDynamicOrigin(source, node));
+ }
+ }
+ var previousPositionalParameters = _positionalParameters;
+ var previousNamedParameters = _namedParameters;
+ _positionalParameters = [];
+ _namedParameters = {};
+ DecoratedType decoratedFunctionType;
+ try {
+ typeParameters?.accept(this);
+ _pushNullabilityNodeTarget(target, () => parameters?.accept(this));
+ redirectedConstructor?.accept(this);
+ initializers?.accept(this);
+ decoratedFunctionType = DecoratedType(functionType, _graph.never,
+ returnType: decoratedReturnType,
+ positionalParameters: _positionalParameters,
+ namedParameters: _namedParameters);
+ body.accept(this);
+ } finally {
+ _positionalParameters = previousPositionalParameters;
+ _namedParameters = previousNamedParameters;
+ }
+ _variables!
+ .recordDecoratedElementType(declaredElement, decoratedFunctionType);
+ return decoratedFunctionType;
} finally {
- _positionalParameters = previousPositionalParameters;
- _namedParameters = previousNamedParameters;
+ _visitingExternalDeclaration = previouslyVisitingExternalDeclaration;
}
- _variables!
- .recordDecoratedElementType(declaredElement, decoratedFunctionType);
- return decoratedFunctionType;
}
DecoratedType? _handleFormalParameter(
@@ -768,6 +793,10 @@
} else {
decoratedType = DecoratedType.forImplicitType(
_typeProvider, declaredElement.type, _graph, target);
+ if (_visitingExternalDeclaration) {
+ _graph.makeNullableUnion(
+ decoratedType.node!, ExternalDynamicOrigin(source, node));
+ }
instrumentation?.implicitType(source, node, decoratedType);
}
} else {
@@ -776,6 +805,10 @@
decoratedReturnType = DecoratedType.forImplicitType(_typeProvider,
DynamicTypeImpl.instance, _graph, target.returnType());
instrumentation?.implicitReturnType(source, node, decoratedReturnType);
+ if (_visitingExternalDeclaration) {
+ _graph.makeNullableUnion(
+ decoratedReturnType.node!, ExternalDynamicOrigin(source, node));
+ }
} else {
decoratedReturnType = type.accept(this);
}
diff --git a/pkg/nnbd_migration/test/api_test.dart b/pkg/nnbd_migration/test/api_test.dart
index 7e35d77..a13964d 100644
--- a/pkg/nnbd_migration/test/api_test.dart
+++ b/pkg/nnbd_migration/test/api_test.dart
@@ -3310,6 +3310,114 @@
await _checkSingleFileChanges(content, expected);
}
+ Future<void> test_external_constructor() async {
+ var content = '''
+class C {
+ external C(dynamic Function(dynamic) callback);
+ static Object g(Object Function(Object) callback) => C(callback);
+}
+''';
+ var expected = '''
+class C {
+ external C(dynamic Function(dynamic) callback);
+ static Object g(Object Function(Object?) callback) => C(callback);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_external_function() async {
+ var content = '''
+external dynamic f();
+Object g() => f();
+''';
+ var expected = '''
+external dynamic f();
+Object? g() => f();
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_external_function_implicit_return() async {
+ var content = '''
+external f();
+Object g() => f();
+''';
+ var expected = '''
+external f();
+Object? g() => f();
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_external_function_implicit_variance() async {
+ var content = '''
+external void f(callback(x));
+void g(Object Function(Object) callback) => f(callback);
+''';
+ var expected = '''
+external void f(callback(x));
+void g(Object Function(Object?) callback) => f(callback);
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_external_function_implicit_variance_complex() async {
+ var content = '''
+external void f(callback(x()));
+void g(Object Function(Object Function()) callback) => f(callback);
+''';
+ var expected = '''
+external void f(callback(x()));
+void g(Object Function(Object? Function()) callback) => f(callback);
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_external_function_variance() async {
+ var content = '''
+external void f(dynamic Function(dynamic) callback);
+void g(Object Function(Object) callback) => f(callback);
+''';
+ var expected = '''
+external void f(dynamic Function(dynamic) callback);
+void g(Object Function(Object?) callback) => f(callback);
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_external_method() async {
+ var content = '''
+class C {
+ external dynamic f();
+ Object g() => f();
+}
+''';
+ var expected = '''
+class C {
+ external dynamic f();
+ Object? g() => f();
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_external_method_implicit() async {
+ var content = '''
+class C {
+ external f();
+ Object g() => f();
+}
+''';
+ var expected = '''
+class C {
+ external f();
+ Object? g() => f();
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
Future<void> test_field_final_uninitalized_used() async {
var content = '''
class C {
