Skip to content

Commit ba60781

Browse files
committed
Fix warnings on implicit casts
[email protected] Review URL: https://codereview.chromium.org/1418653003 .
1 parent bf0ba96 commit ba60781

File tree

3 files changed

+34
-13
lines changed

3 files changed

+34
-13
lines changed

pkg/analyzer/lib/src/task/strong/info.dart

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,28 @@ abstract class DownCast extends CoercionInfo {
154154
return new StaticTypeError(rules, expression, toT, reason: reason);
155155
}
156156

157+
// TODO(vsm): Change this to an assert when we have generic methods and
158+
// fix TypeRules._coerceTo to disallow implicit sideways casts.
159+
if (!rules.isSubTypeOf(toT, fromT)) {
160+
assert(toT.isSubtypeOf(fromT) || fromT.isAssignableTo(toT));
161+
return new DownCastComposite(rules, expression, cast);
162+
}
163+
157164
// Composite cast: these are more likely to fail.
158165
if (!rules.isGroundType(toT)) {
159166
// This cast is (probably) due to our different treatment of dynamic.
160167
// It may be more likely to fail at runtime.
161-
return new DownCastComposite(rules, expression, cast);
168+
if (fromT is InterfaceType) {
169+
// For class types, we'd like to allow non-generic down casts, e.g.,
170+
// Iterable<T> to List<T>. The intuition here is that raw (generic)
171+
// casts are problematic, and we should complain about those.
172+
var typeArgs = fromT.typeArguments;
173+
if (typeArgs.isEmpty || typeArgs.any((t) => t.isDynamic)) {
174+
return new DownCastComposite(rules, expression, cast);
175+
}
176+
} else {
177+
return new DownCastComposite(rules, expression, cast);
178+
}
162179
}
163180

164181
// Dynamic cast

pkg/analyzer/lib/src/task/strong/rules.dart

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,6 @@ class TypeRules {
376376

377377
// Produce a coercion which coerces something of type fromT
378378
// to something of type toT.
379-
// If wrap is true and both are function types, a closure
380-
// wrapper coercion is produced using _wrapTo (see above)
381379
// Returns the error coercion if the types cannot be coerced
382380
// according to our current criteria.
383381
Coercion _coerceTo(DartType fromT, DartType toT) {
@@ -387,11 +385,12 @@ class TypeRules {
387385
// fromT <: toT, no coercion needed
388386
if (isSubTypeOf(fromT, toT)) return Coercion.identity(toT);
389387

390-
// For now, reject conversions between function types and
391-
// call method objects. We could choose to allow casts here.
392-
// Wrapping a function type to assign it to a call method
393-
// object will never succeed. Wrapping the other way could
394-
// be allowed.
388+
// TODO(vsm): We can get rid of the second clause if we disallow
389+
// all sideways casts - see TODO below.
390+
// -------
391+
// Note: a function type is never assignable to a class per the Dart
392+
// spec - even if it has a compatible call method. We disallow as
393+
// well for consistency.
395394
if ((fromT is FunctionType && getCallMethodType(toT) != null) ||
396395
(toT is FunctionType && getCallMethodType(fromT) != null)) {
397396
return Coercion.error();
@@ -400,6 +399,10 @@ class TypeRules {
400399
// Downcast if toT <: fromT
401400
if (isSubTypeOf(toT, fromT)) return Coercion.cast(fromT, toT);
402401

402+
// TODO(vsm): Once we have generic methods, we should delete this
403+
// workaround. These sideways casts are always ones we warn about
404+
// - i.e., we think they are likely to fail at runtime.
405+
// -------
403406
// Downcast if toT <===> fromT
404407
// The intention here is to allow casts that are sideways in the restricted
405408
// type system, but allowed in the regular dart type system, since these
@@ -409,6 +412,7 @@ class TypeRules {
409412
if (fromT.isAssignableTo(toT)) {
410413
return Coercion.cast(fromT, toT);
411414
}
415+
412416
return Coercion.error();
413417
}
414418

pkg/analyzer/test/src/task/strong/checker_test.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,7 @@ void main() {
984984
lOfAs = /*severe:StaticTypeError*/mOfOs;
985985
lOfAs = mOfAs;
986986
lOfAs = /*warning:DownCastComposite*/lOfDs;
987-
lOfAs = /*warning:DownCastComposite*/lOfOs;
987+
lOfAs = /*info:DownCastImplicit*/lOfOs;
988988
lOfAs = lOfAs;
989989
}
990990
{
@@ -993,7 +993,7 @@ void main() {
993993
mOfDs = mOfAs;
994994
mOfDs = /*info:DownCastImplicit*/lOfDs;
995995
mOfDs = /*info:DownCastImplicit*/lOfOs;
996-
mOfDs = /*info:DownCastImplicit*/lOfAs;
996+
mOfDs = /*warning:DownCastComposite*/lOfAs;
997997
}
998998
{
999999
mOfOs = mOfDs;
@@ -1005,11 +1005,11 @@ void main() {
10051005
}
10061006
{
10071007
mOfAs = /*warning:DownCastComposite*/mOfDs;
1008-
mOfAs = /*warning:DownCastComposite*/mOfOs;
1008+
mOfAs = /*info:DownCastImplicit*/mOfOs;
10091009
mOfAs = mOfAs;
10101010
mOfAs = /*warning:DownCastComposite*/lOfDs;
1011-
mOfAs = /*warning:DownCastComposite*/lOfOs;
1012-
mOfAs = /*warning:DownCastComposite*/lOfAs;
1011+
mOfAs = /*info:DownCastImplicit*/lOfOs;
1012+
mOfAs = /*info:DownCastImplicit*/lOfAs;
10131013
}
10141014
10151015
}

0 commit comments

Comments
 (0)