Skip to content

Commit 89dde05

Browse files
stereotype441commit-bot@chromium.org
stereotype441
authored and
[email protected]
committed
Migration: fix handling of assignments involving FutureOr.
Fixes ~96 exceptions whose stack trace contains the line: DecoratedClassHierarchy.getDecoratedSupertype (package:nnbd_migration/src/decorated_class_hierarchy.dart:59:10) Change-Id: I0a4d31f4822107453b2881618817c8a7c20e0e84 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115740 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent ec48af6 commit 89dde05

File tree

5 files changed

+183
-4
lines changed

5 files changed

+183
-4
lines changed

‎pkg/nnbd_migration/lib/src/already_migrated_code_decorator.dart‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'package:analyzer/dart/element/element.dart';
56
import 'package:analyzer/dart/element/type.dart';
67
import 'package:analyzer/src/dart/element/type.dart';
78
import 'package:analyzer/src/generated/resolver.dart';
@@ -76,4 +77,24 @@ class AlreadyMigratedCodeDecorator {
7677
'Unable to decorate already-migrated type $type');
7778
}
7879
}
80+
81+
/// Get all the decorated immediate supertypes of the non-migrated class
82+
/// [class_].
83+
Iterable<DecoratedType> getImmediateSupertypes(ClassElement class_) {
84+
var allSupertypes = <DartType>[];
85+
var supertype = class_.supertype;
86+
if (supertype != null) {
87+
allSupertypes.add(supertype);
88+
}
89+
allSupertypes.addAll(class_.superclassConstraints);
90+
allSupertypes.addAll(class_.interfaces);
91+
allSupertypes.addAll(class_.mixins);
92+
var type = class_.type;
93+
if (type.isDartAsyncFuture) {
94+
// Add FutureOr<T> as a supertype of Future<T>.
95+
allSupertypes
96+
.add(_typeProvider.futureOrType.instantiate(type.typeArguments));
97+
}
98+
return allSupertypes.map(decorate);
99+
}
79100
}

‎pkg/nnbd_migration/lib/src/edge_builder.dart‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1923,6 +1923,15 @@ mixin _AssignmentChecker {
19231923
@required DecoratedType destination,
19241924
@required bool hard}) {
19251925
_connect(source.node, destination.node, origin, hard: hard);
1926+
_checkAssignment_recursion(origin,
1927+
source: source, destination: destination);
1928+
}
1929+
1930+
/// Does the recursive part of [_checkAssignment], visiting all of the types
1931+
/// constituting [source] and [destination], and creating the appropriate
1932+
/// edges between them.
1933+
void _checkAssignment_recursion(EdgeOrigin origin,
1934+
{@required DecoratedType source, @required DecoratedType destination}) {
19261935
var sourceType = source.type;
19271936
var destinationType = destination.type;
19281937
if (sourceType.isBottom || sourceType.isDartCoreNull) {
@@ -1957,6 +1966,15 @@ mixin _AssignmentChecker {
19571966
destinationType is InterfaceType) {
19581967
if (_typeSystem.isSubtypeOf(sourceType, destinationType)) {
19591968
// Ordinary (upcast) assignment. No cast necessary.
1969+
if (destinationType.isDartAsyncFutureOr) {
1970+
if (_typeSystem.isSubtypeOf(
1971+
sourceType, destinationType.typeArguments[0])) {
1972+
// We are looking at T <: FutureOr<U>. So treat this as T <: U.
1973+
_checkAssignment_recursion(origin,
1974+
source: source, destination: destination.typeArguments[0]);
1975+
return;
1976+
}
1977+
}
19601978
var rewrittenSource = _decoratedClassHierarchy.asInstanceOf(
19611979
source, destinationType.element);
19621980
assert(rewrittenSource.typeArguments.length ==

‎pkg/nnbd_migration/lib/src/variables.dart‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,8 @@ class Variables implements VariableRecorder, VariableRepository {
248248
Map<ClassElement, DecoratedType> _decorateDirectSupertypes(
249249
ClassElement class_) {
250250
var result = <ClassElement, DecoratedType>{};
251-
for (var supertype in class_.allSupertypes) {
252-
var decoratedSupertype =
253-
_alreadyMigratedCodeDecorator.decorate(supertype);
251+
for (var decoratedSupertype
252+
in _alreadyMigratedCodeDecorator.getImmediateSupertypes(class_)) {
254253
assert(identical(decoratedSupertype.node, _graph.never));
255254
var class_ = (decoratedSupertype.type as InterfaceType).element;
256255
if (class_ is ClassElementHandle) {

‎pkg/nnbd_migration/test/already_migrated_code_decorator_test.dart‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'package:analyzer/dart/element/element.dart';
56
import 'package:analyzer/dart/element/type.dart';
67
import 'package:analyzer/src/dart/element/element.dart';
78
import 'package:analyzer/src/dart/element/type.dart';
89
import 'package:analyzer/src/generated/resolver.dart';
10+
import 'package:analyzer/src/generated/testing/element_factory.dart';
911
import 'package:analyzer/src/generated/testing/test_type_provider.dart';
1012
import 'package:analyzer/src/generated/utilities_dart.dart';
1113
import 'package:nnbd_migration/src/already_migrated_code_decorator.dart';
@@ -45,6 +47,15 @@ class _AlreadyMigratedCodeDecoratorTest {
4547
expect(decoratedType.node, same(always));
4648
}
4749

50+
void checkFutureOr(
51+
DecoratedType decoratedType,
52+
NullabilityNode expectedNullability,
53+
void Function(DecoratedType) checkArgument) {
54+
expect(decoratedType.type.element, typeProvider.futureOrType.element);
55+
expect(decoratedType.node, expectedNullability);
56+
checkArgument(decoratedType.typeArguments[0]);
57+
}
58+
4859
void checkInt(
4960
DecoratedType decoratedType, NullabilityNode expectedNullability) {
5061
expect(decoratedType.type.element, typeProvider.intType.element);
@@ -76,7 +87,7 @@ class _AlreadyMigratedCodeDecoratorTest {
7687
void checkTypeParameter(
7788
DecoratedType decoratedType,
7889
NullabilityNode expectedNullability,
79-
TypeParameterElementImpl expectedElement) {
90+
TypeParameterElement expectedElement) {
8091
var type = decoratedType.type as TypeParameterTypeImpl;
8192
expect(type.element, same(expectedElement));
8293
expect(decoratedType.node, expectedNullability);
@@ -202,4 +213,63 @@ class _AlreadyMigratedCodeDecoratorTest {
202213
test_decorate_void() {
203214
checkVoid(decorate(typeProvider.voidType));
204215
}
216+
217+
test_getImmediateSupertypes_future() {
218+
var element = typeProvider.futureType.element;
219+
var decoratedSupertypes =
220+
decorator.getImmediateSupertypes(element).toList();
221+
var typeParam = element.typeParameters[0];
222+
expect(decoratedSupertypes, hasLength(2));
223+
checkObject(decoratedSupertypes[0], never);
224+
// Since Future<T> is a subtype of FutureOr<T>, we consider FutureOr<T> to
225+
// be an immediate supertype, even though the class declaration for Future
226+
// doesn't mention FutureOr.
227+
checkFutureOr(decoratedSupertypes[1], never,
228+
(t) => checkTypeParameter(t, never, typeParam));
229+
}
230+
231+
test_getImmediateSupertypes_generic() {
232+
var t = ElementFactory.typeParameterElement('T');
233+
var class_ = ElementFactory.classElement3(
234+
name: 'C',
235+
typeParameters: [t],
236+
supertype: typeProvider.iterableType.instantiate([t.type]));
237+
var decoratedSupertypes = decorator.getImmediateSupertypes(class_).toList();
238+
expect(decoratedSupertypes, hasLength(1));
239+
checkIterable(decoratedSupertypes[0], never,
240+
(type) => checkTypeParameter(type, never, t));
241+
}
242+
243+
test_getImmediateSupertypes_interface() {
244+
var class_ = ElementFactory.classElement('C', typeProvider.objectType);
245+
class_.interfaces = [typeProvider.numType];
246+
var decoratedSupertypes = decorator.getImmediateSupertypes(class_).toList();
247+
expect(decoratedSupertypes, hasLength(2));
248+
checkObject(decoratedSupertypes[0], never);
249+
checkNum(decoratedSupertypes[1], never);
250+
}
251+
252+
test_getImmediateSupertypes_mixin() {
253+
var class_ = ElementFactory.classElement('C', typeProvider.objectType);
254+
class_.mixins = [typeProvider.numType];
255+
var decoratedSupertypes = decorator.getImmediateSupertypes(class_).toList();
256+
expect(decoratedSupertypes, hasLength(2));
257+
checkObject(decoratedSupertypes[0], never);
258+
checkNum(decoratedSupertypes[1], never);
259+
}
260+
261+
test_getImmediateSupertypes_superclassConstraint() {
262+
var class_ = ElementFactory.mixinElement(
263+
name: 'C', constraints: [typeProvider.numType]);
264+
var decoratedSupertypes = decorator.getImmediateSupertypes(class_).toList();
265+
expect(decoratedSupertypes, hasLength(1));
266+
checkNum(decoratedSupertypes[0], never);
267+
}
268+
269+
test_getImmediateSupertypes_supertype() {
270+
var class_ = ElementFactory.classElement('C', typeProvider.objectType);
271+
var decoratedSupertypes = decorator.getImmediateSupertypes(class_).toList();
272+
expect(decoratedSupertypes, hasLength(1));
273+
checkObject(decoratedSupertypes[0], never);
274+
}
205275
}

‎pkg/nnbd_migration/test/edge_builder_test.dart‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,77 @@ int f(dynamic d) => d;
392392
hard: true);
393393
}
394394

395+
test_assign_future_to_futureOr_complex() async {
396+
await analyze('''
397+
import 'dart:async';
398+
FutureOr<List<int>> f(Future<List<int>> x) => x;
399+
''');
400+
// If `x` is `Future<List<int?>>`, then the only way to migrate is to make
401+
// the return type `FutureOr<List<int?>>`.
402+
assertEdge(decoratedTypeAnnotation('int>> x').node,
403+
decoratedTypeAnnotation('int>> f').node,
404+
hard: false);
405+
assertNoEdge(decoratedTypeAnnotation('int>> x').node,
406+
decoratedTypeAnnotation('List<int>> f').node);
407+
assertNoEdge(decoratedTypeAnnotation('int>> x').node,
408+
decoratedTypeAnnotation('FutureOr<List<int>> f').node);
409+
}
410+
411+
test_assign_future_to_futureOr_simple() async {
412+
await analyze('''
413+
import 'dart:async';
414+
FutureOr<int> f(Future<int> x) => x;
415+
''');
416+
// If `x` is nullable, then there are two migrations possible: we could make
417+
// the return type `FutureOr<int?>` or we could make it `FutureOr<int>?`.
418+
// We choose `FutureOr<int>?` because it's strictly more conservative (it's
419+
// a subtype of `FutureOr<int?>`).
420+
assertEdge(decoratedTypeAnnotation('Future<int> x').node,
421+
decoratedTypeAnnotation('FutureOr<int>').node,
422+
hard: true);
423+
assertNoEdge(decoratedTypeAnnotation('Future<int> x').node,
424+
decoratedTypeAnnotation('int> f').node);
425+
// If `x` is `Future<int?>`, then the only way to migrate is to make the
426+
// return type `FutureOr<int?>`.
427+
assertEdge(substitutionNode(decoratedTypeAnnotation('int> x').node, never),
428+
decoratedTypeAnnotation('int> f').node,
429+
hard: false);
430+
assertNoEdge(decoratedTypeAnnotation('int> x').node,
431+
decoratedTypeAnnotation('FutureOr<int>').node);
432+
}
433+
434+
test_assign_non_future_to_futureOr_complex() async {
435+
await analyze('''
436+
import 'dart:async';
437+
FutureOr<List<int>> f(List<int> x) => x;
438+
''');
439+
// If `x` is `List<int?>`, then the only way to migrate is to make the
440+
// return type `FutureOr<List<int?>>`.
441+
assertEdge(decoratedTypeAnnotation('int> x').node,
442+
decoratedTypeAnnotation('int>> f').node,
443+
hard: false);
444+
assertNoEdge(decoratedTypeAnnotation('int> x').node,
445+
decoratedTypeAnnotation('List<int>> f').node);
446+
assertNoEdge(decoratedTypeAnnotation('int> x').node,
447+
decoratedTypeAnnotation('FutureOr<List<int>> f').node);
448+
}
449+
450+
test_assign_non_future_to_futureOr_simple() async {
451+
await analyze('''
452+
import 'dart:async';
453+
FutureOr<int> f(int x) => x;
454+
''');
455+
// If `x` is nullable, then there are two migrations possible: we could make
456+
// the return type `FutureOr<int?>` or we could make it `FutureOr<int>?`.
457+
// We choose `FutureOr<int>?` because it's strictly more conservative (it's
458+
// a subtype of `FutureOr<int?>`).
459+
assertEdge(decoratedTypeAnnotation('int x').node,
460+
decoratedTypeAnnotation('FutureOr<int>').node,
461+
hard: true);
462+
assertNoEdge(decoratedTypeAnnotation('int x').node,
463+
decoratedTypeAnnotation('int>').node);
464+
}
465+
395466
test_assign_null_to_generic_type() async {
396467
await analyze('''
397468
main() {

0 commit comments

Comments
 (0)