Skip to content

--no-implicit-dynamic should take @optionalTypeArgs into account #26901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zoechi opened this issue Jul 16, 2016 · 8 comments
Closed

--no-implicit-dynamic should take @optionalTypeArgs into account #26901

zoechi opened this issue Jul 16, 2016 · 8 comments
Labels
legacy-area-analyzer Use area-devexp instead. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@zoechi
Copy link
Contributor

zoechi commented Jul 16, 2016

image

@optionalTypeArgs
class MapDataItem<K, V> extends DelegatingMap<K, V> implements DataItem<K, V> { ... }
@kevmoo kevmoo added the legacy-area-analyzer Use area-devexp instead. label Jul 18, 2016
@kevmoo
Copy link
Member

kevmoo commented Jul 18, 2016

FYI @jmesserly

@jmesserly
Copy link

what is @optionalTypeArgs ...?

@zoechi
Copy link
Contributor Author

zoechi commented Jul 19, 2016

@jmesserly
Copy link

jmesserly commented Jul 19, 2016

Hmmm. But what's it used for? I don't understand why it's a good idea. It seems to defeat the purpose of --no-implicit-dynamic, which is opt in, but is intended to give you confidence there is no "accidental dynamic" in your code anywhere.

There is a way to get similar behavior with strong mode+no-implicit-dynamic:

class MapDataItem<K extends Object, V extends Object> extends ...

By adding extends Object it changes the default behavior, now it will implicitly instantiate with Object, which unlike dynamic, is a somewhat safe default. At least, much safer and seems more in the spirit of "no-implicit-dynamic".

I suppose we could combine them and have @optionalTypeArgs mean implicit Object. Or we just make implicit Object always. The problem is, it doesn't help if you plan on downcasting:

f(MapDataItem<String, int> m) { ... }

var myItem = new MapDataItem(...);
f(myItem); // boom, cast failure!

That's why we were trying to encourage type arguments. With a type like MapDataItem<K, V> we should pretty much always infer from the constructor key/value type anyway (ctors have upwards+downwards inference now).

CC @leafpetersen as well

@zoechi
Copy link
Contributor Author

zoechi commented Jul 19, 2016

I didn't realize that generic type parameter constraints are used this way. I tried it and it seems to solve my problem.

I like this better than @optionalTypeArgs which I used for exceptions to always_specify_types

@zoechi
Copy link
Contributor Author

zoechi commented Jul 19, 2016

BTW 🍻 🍻 🍻 for --no-implicit-dynamic. I was missing this since I started using Dart.

@jmesserly jmesserly added P3 A lower priority bug or feature request analyzer-strong-mode type-enhancement A request for a change that isn't a bug labels Jul 19, 2016
@jmesserly
Copy link

jmesserly commented Jul 19, 2016

awesome! Let us know if that feeling changes based on experience :) I will leave this open because it's at least an inconsistency we should think about. And I do worry about taking away the nice "raw type" syntax. If it seems to come up a lot, we can bump the priority to figure out something better.

@jmesserly
Copy link

Closing this issue as stale; we do have work planned for new --strict flags in Analyzer that should be easier to use compared to --no-implicit-dynamic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants