NOTE: This issue is not time-boxed (i.e. it could be a Dart 2.1.0 w/ a flag, a 3.0.0 change, etc)
When I first started using Dart 2.0 semantics/strong-mode, I didn’t have a single strong argument for or against allowing implicit downcasts, but after working almost exclusively in Dart 2.0 semantics for the last 6 months (DDC+Analyzer) – and seeing both Googlers and external folks file issues and request help I now strongly (pun intended) believe implicit downcasts by default is a mistake on the order of ever introducing “null” into a language.
Overview
Most of the arguments I’ve used (and seen) for implicit downcasts are:
- Originally strong-mode was “opt-in”, so a goal was minimizing changing existing code.
- It makes the leap from Dart 1.0 -> 2.0 easier.
- It keeps the language “terse” and “concise” for common downcasts.
Most of the arguments against:
- “Why would we not make implicit downcasts an error in general… they’re not sound”
- “Why are implicit downcasts allowed by default in strong mode?”
In general, the arguments against are stronger than the arguments for:
Soundness
One of the goals of strong mode is to add a sound type system to the Dart language. Heap soundness has a lot of nice properties – both in developer productivity (“oops, I didn’t mean to do that, thanks for catching that IDE!”), and AOT compiler optimizations (“I know for sure, that var x at this point of time only can ever be a String, therefore I don’t need a trampoline/de-opt”).
Implicit downcasts are still sound (the checking occurs at runtime), but now we’ve taken something that could have been fixed at code review/development time and shifted to something we need to hope exhaustive test coverage catches before we ship code to production.
Least surprise/most predictable
Dart does not currently have type coercion (as of 2017-11-19), and has a huge limit on the amount of semantic “magic” that occurs at either compile or runtime. Implicit downcasts is one of the areas that is surprising to me and a lot of our users both internally and externally:
- Dart 2.0 runtimes are required to check the type (at runtime), but it’s not apparent in the code.
- We require the keyword
covariant
in signatures, but not in assignments or return values.
It’s scary to change framework or library-level APIs
THIS IS MY NUMBER ONE ISSUE SO FAR. HAVE HIT REAL PRODUCTION ISSUES.
Assume I have an API for listing all of the countries a user account has visited:
abstract class Account {
Set<String> get visitedCountries;
}
I can change it to the following…
abstract class Account {
// Now computed lazily and not hashed, since that was a rare use-case.
Iterable<String> get visitedCountries;
}
…and not trigger a single compiler warning or error where users were doing the following:
Iterable<String> warnForCountries(Account account, Set<String> bannedCountries) {
return account.visitedCountries.intersection(bannedCountries);
}
In fact, in a lot of tests, folks will mock out Account
:
class MockAccount extends Mock implements Account {}
void main() {
final account = new MockAccount();
when(account.visitedCountries).thenReturn(['New Zealand'].toSet());
test('should trigger a warning', () {
expect(
warnForCountries(account, ['New Zealand'].toSet()),
['New Zealand'],
);
});
}
… and will never hit my subtle re-typing until they get a production issue
This is an area where implicit-downcasts: false
can’t help – because I need other libraries that haven’t opted into the flag to fail, and they of course, won’t, so I’ll have to assume they have good test coverage and move on.
Accepted language and library features are removing downcasts
int.clamp
instead ofnum.clamp
.- Adding types
T
instead of relying ondynamic
orObject
.
See related issues at the bottom for more context and details.
Examples
The following examples are tested in 2.0.0-dev.6.0
.
Null aware operator
void noError(int x) {
// Implicit downcast: x = <int|String> = <Object>.
x = x ?? 'Hello';
}
Ternary operator
void noError(int x) {
// Implicit downcast: x = <int|String> = <Object>.
x = x == 5 ? 6 : 'Hello';
}
Returning a List
class UserModel {
final List<User> _users;
UserModel(this._users);
// Implicit downcast: Iterable --> List. Will fail at runtime only.
List<String> toNames() => _users.map((u) => u.name);
}
Cascade operator
void hasError() {
// Constructor returns type 'Base' that isn't of expected type 'Derived'.
Derived e = new Base();
}
void noError() {
// Implicit downcast: Base --> Derived.
Derived d = new Base()..value = 'd';
}
class Base {
String value;
}
class Derived extends Base {}
Related Issues
- Disallow downcasts of ternary operators: #25368
- Cascade operators break strong mode typing: #29718
- Strong-mode analyzer inconsistent about when types must match: #30385
- Stricter strong-mode: #30397
- Issues blocking Flutter’s use of “implicit-casts: false”: #30402
Summary
Don’t read this issue as a prescriptive “remove implicit downcasts!”, but rather, given the type of errors we hide today (above), and the rationale for-and-against implicit downcasts, consider introducing changes in Dart 2.1+ that allow better static type safety. If assignment terseness is still a user requirement (I’d love to see the UX feedback showing this), then perhaps something like an implicit cast operator:
void futureDart(Object o) {
int x =~ o; // Same as int x = o as int;
}
I imagine we will get non-nullable types sometime in the future. If we do, you’ll never want this:
void futureDart(int? maybeNull) {
int definitelyNotNull = maybeNull;
}
… so the argument for making this OK is much weaker:
void futureDart(Object maybeInt) {
int definitelyInt = maybeInt;
}
If there were a ‘gold medal’ reaction I would give it to you, for the thorough writeup, inclusion of both sides, convincing argument, sources cited, examples given, affected users, and, for just how important I think the point you’re making is.
100%% agree, I’ve hit the List/Iterable problem myself a number of times
Me again! 😉
I hit this again… I totally forgot about implicit downcasts and then made this runtime error in the Flutter stocks app which should’ve been caught by analysis -> flutter/flutter#13815
The decision to allow implicit downcasts by default has always confused me. The benefits seem tiny (saving a few characters on a cast) for crazy issues (pushing a statically detectable errors to runtime – the opposite of what Strong Dart aims for, and removing any clues that the developer believed the cast was safe).
I’ve mentioned this many times but it’s always been met with a wall of silence. Can we at least flip a coin for it? I’ll provide a coin.
We’ve hit this at Workiva a number of times .. and just found this issue after hitting it today. We’re all for it.
I don’t know if that’s still being used as an argument, but it doesn’t seem like a good one to me. There are many places the language could be made less terse if we want to throw away dev-time errors and find them at run time. If people wanted shorter code that behaves unpredictably, they’ll pick JS 😉
Why can’t you change the setting in your analysis_options? This is what always bugged me in discussions about this. Is there some rule that was you have to use the defaults? The default should be set for what’s best for the future/most people and if you’d rather the opposite, change your setting (unless I’m misunderstanding something – I must admit I don’t know how pub packages you pull in are affected by the switch).
I guess when I said
explicitly
I meant in the same scope. I think that example is bad – you shouldn’t be able to create a method takingdynamic
that will crash if the value isn’t aList<String>
unless you’ve written some code that shows you actually believe your value is aList<String>
. I think you should have to either fix the type on the signature (preferred for that example) it at least addas List<string>
in the call to show that you believe it’s a list of string.Imagine in future you needed to change the type of
List<String>
there (maybe you’d originally made itIterable
and were now changing it toList
), you can’t just change it and use analysis errors to make sure you found all the places that needed updating. Find References is no good either because it’ll show those you’ve already fixed (or didn’t need fixing) as well as those that are bad.I think the short-term gains (slightly less typing) are outweighed by the additional risks in maintaining/refactoring 🙂
I know I have a preference for much stronger typing than most.. but this one seems really loose to me, and at-odds with the idea of Dart going strong. I think (though have no data to back it up ;-)) that most people would expect that last code block to give them a static warning.
This has been accepted as part of the NNBD release planned for late this year. Closing this issue since it is now tracked as part of that scope of work. Discussion issue here: dart-lang/language#192 .