Skip to content

Map operator[] allows any type #48030

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
Hixie opened this issue Dec 31, 2021 · 3 comments
Closed

Map operator[] allows any type #48030

Hixie opened this issue Dec 31, 2021 · 3 comments

Comments

@Hixie
Copy link
Contributor

Hixie commented Dec 31, 2021

A Map<K, V> will never contain a key with a type other than K, so it's almost certainly a programming error if the argument to Map.operator[] is not of type K. However, Map.operator[] takes an Object?!

We should tighten the argument to Map, with a dart fix that inserts an as dynamic to migrate people (we already have a lint that catches use of dynamic so people will be warned without being broken).

@Hixie
Copy link
Contributor Author

Hixie commented Dec 31, 2021

Yeah, extending this change to cover the methods as well would make sense too. I think that's similarly likely to always be a programming error, though I can see situations where you might not know the incoming type (e.g. you got something from JSON) and where it might therefore make sense?

On the whole I would be biased towards stronger typing here.

@lrhn
Copy link
Member

lrhn commented Dec 31, 2021

Definitely a deliberate design choice.

The reason that Map.operator[], Map.remove, Map.containsKey, Map.containsValue, Iterable.contains, List.remove, Set.lookup, Set.containsAll, Set.difference, Set.removeAll, and Set.retainAll all allow Object? as input(s) is that it supports covariant generics and up-casting.

A List<int> is-a List<num>, and you can freely cast a list of integers to List<num>.
Because of that, element query methods like contains should work at that type. They do, because they accept Object? as argument. The implementation can work for any object, there is no problem asking whether a list of integers contains 0.5 (the answer is false, just as asking if it contains "string").

The remove methods might not need that extra flexibility often, but it's consistent and it always works (you can always do .removeWhere((x) => x == anyObject) anyway).

I believe the original source of that choice was #9893.

The only functions that need to prevent you from passing in unrelated values are ones that add to the collection, because the things you add must have the actual key/value/element type.

If anything, the odd methods are the query methods indexOf/lastIndexOf which do not accept Object?.

Changing this now is a breaking change.
The one approach that I can see might fly is to change the interfaces to expect E/V/K, but keep the implementation classes accepting Object?. Then a List<int> cast to List<num> would seemingly accept num arguments to remove, but would actually accept any value, and not be restricted to the reified int type argument.

@Hixie
Copy link
Contributor Author

Hixie commented Jan 1, 2022

Fair enough. I'll suggest a lint instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants