Make core collections stronger & consistent

[matanlurey]

Suggestions:

Iterable

• bool contains(Object element) ➡️ bool contains(T element)

You never want to write this:

class Dog {}
class Cat {}

void main() {
  var dogs = [ new Dog(); ]
  var cat = new Cat();
  ...
  print(dogs.contains(cat)); // <-- Will never be true.
}

I ran into this today when I had a Set that used to be Set<A>, got changed to Set<B>, but code was never updated the check it for a B instead of an A, yet static analysis reported everything was fine.

List

• List.from(Iterable elements) ➡️ List.from(Iterable<E> elements)

I'm not sure if this is blocked on #30355 or not.

• List.unmodifiable(Iterable elements) ➡️ List.unmodifiable(Iterable<E> elements)

This is a common way to lose reified type information 😢

• bool remove(Object element) ➡️ bool remove(T element)

Same issue as described above for Iterable#contains.

Map

• Map.from(Map other) ➡️ Map.from(Map<K, V> other)
• Map.fromIterable(Iterable iterable, { K key(element), V value(element) }) ➡️ Map.fromIterable(Iterable<E> iterable, { K key(E element), V value(E element) })
• Map.unmodifiable(Map other) ➡️ Map.unmodifiable(Map<K, V> other)
• bool containsKey(Object key) ➡️ bool containsKey(K key)
• bool containsKey(Object value) ➡️ bool containsValue(V value)

[lrhn]

All of these are there for one reason or another.
In Dart, you can do

Set<Object> x = new Set<int>()..addAll([1, 2, 3]); 
... 
if (x.contains("string")) ...;`.

The assignment is valid due to covariant generics. The lookup is statically safe because of the static type of x. There are two ways to give this meaning at runtime - either throw or return false. We chose to return false because it makes the assignment more useful.

Most other languages, including Java and C#, do not have covariant generics.
The Java collections also accept Object for contains and remove. Maybe it's just because they can - If you know that an Object reference actually has a type compatible with the set, you can remove it without having to cast it first.
C# collections require contains and remove to have the element type, so that's also possible. The C# collections generally has a Cast method that makes it possible to use a List<int> as a List<Object> by adding runtime checks. That could be useful for Dart as well.

For a constructor like Set.from, it not requiring its arguments to have the correct type makes it possible to dynamically cast. Example: new List<num>.from(setOfObjects.where((e) => e is num)). You can't just use the toList method on the iterable because that would give you a List<Object>. These constructors are the escape hatches needed to actually change the type of a collection.
Again, a cast operation might do the same thing (including adding the required dynamic checks), so you could do setOfObjects.where((e) => e is num).cast<num>().toList().

So, there are reasons for these choices. If we change them back, we'll need to add some other feature to alleviate the pain.

[mraleph]

We stepped on all of this in Vipunen too, when trying to type-check core libraries under the assumption that int et al are primitive types (non-nullable, incompatible with Object) and that C<dynamic> is forbidden.

[lrhn]

Assuming that int is-not Object is going to break a lot of things since everything was designed under the assumption that everything is an object.
It's probably possible to have a type system with no greatest element, and generics that work with it, and in those cases, the assumption that Object is a supertype of everything, like in Set.remove, will be very visible. That said, we are not planning to move to such a type system, so that's not an argument for changing anything.

[matanlurey]

@lrhn:

All of these are there for one reason or another.

Sure, but it doesn't mean they should never change.

In Dart, you can do

Set<Object> x = new Set<int>()..addAll([1, 2, 3]); 
... 
if (x.contains("string")) ...;`.

The assignment is valid due to covariant generics. The lookup is statically safe because of the static type of x. There are two ways to give this meaning at runtime - either throw or return false. We chose to return false because it makes the assignment more useful.

It didn't make it more useful for me when I almost submitted a production bug. I imagine if you poll your users, almost nobody finds this a useful feature of Dart. In fact, it reminds me more of JavaScript or Python than C# (HashSet(T).Contains).

The Java collections also accept Object for contains and remove. Maybe it's just because they can - If you know that an Object reference actually has a type compatible with the set, you can remove it without having to cast it first.

Just because Java's aging standard library doesn't isn't a good enough reason IMO.

C# collections require contains and remove to have the element type, so that's also possible. The C# collections generally has a Cast method that makes it possible to use a List<int> as a List<Object> by adding runtime checks. That could be useful for Dart as well.

+1. Opt-in, almost nobody will actually need this, but +1 for allowing opt-in with a cast method.

For a constructor like Set.from, it not requiring its arguments to have the correct type makes it possible to dynamically cast. Example: new List<num>.from(setOfObjects.where((e) => e is num)).

Fair, but I'd like to see a specific set of methods/constructors specifically dealing with casting instead of it being the default. It's really confusing to our users that .from drops type information.

Again, a cast operation might do the same thing (including adding the required dynamic checks), so you could do setOfObjects.where((e) => e is num).cast<num>().toList().

I like this option.

[matanlurey]

I'm guessing these are unlikely to occur or should be broken up into individual requests.

[cedvdb]

The one that always annoys me is Map.fromIterable. Couldn't it use a generic at least ?

Map<String, Booking>.fromIterable(
        bookingList,
        key: (booking) => (booking as Booking).id,
      ),

[lrhn]

@cedvdb It can't, actually. Constructors cannot be generic. (I wish they could, dart-lang/language#647).

We could make fromIterable as static method instead of a constructor, but then the generics would be:

static Map<K, V> fromIterable<S, K, V>(Iterable<S> source, {K Function(S) key, V Function(S) value}) ...

It does mean that fromIterable sucks, and you shouldn't use it. These days, you should use a collection literal instead:

  {for (var value in source) key(source): value(source)}