Strong-mode analysis errors prevent DDC compilation

[ochafik]

The following list of strong-mode errors prevent code that depend on package:quiver from being compiled with DDC.

(ran dartanalyzer --strong find lib -name '*.dart' | sort -u with SDK version 1.16.0-dev.0.0)

[error] Base class introduces an invalid override. The type of DelegatingIterable.expand (((E) → Iterable<dynamic>) → Iterable<dynamic>) is not a subtype of Iterable<E>.expand (<T>((E) → Iterable<T>) → Iterable<T>). (./lib/src/collection/delegates/list.dart, line 29, col 34)
[error] Base class introduces an invalid override. The type of DelegatingIterable.expand (((E) → Iterable<dynamic>) → Iterable<dynamic>) is not a subtype of Iterable<E>.expand (<T>((E) → Iterable<T>) → Iterable<T>). (./lib/src/collection/delegates/queue.dart, line 29, col 35)
[error] Base class introduces an invalid override. The type of DelegatingIterable.expand (((E) → Iterable<dynamic>) → Iterable<dynamic>) is not a subtype of Iterable<E>.expand (<T>((E) → Iterable<T>) → Iterable<T>). (./lib/src/collection/delegates/set.dart, line 29, col 33)
[error] Base class introduces an invalid override. The type of DelegatingIterable.fold ((dynamic, (dynamic, E) → dynamic) → dynamic) is not a subtype of Iterable<E>.fold (<T>(T, (T, E) → T) → T). (./lib/src/collection/delegates/list.dart, line 29, col 34)
[error] Base class introduces an invalid override. The type of DelegatingIterable.fold ((dynamic, (dynamic, E) → dynamic) → dynamic) is not a subtype of Iterable<E>.fold (<T>(T, (T, E) → T) → T). (./lib/src/collection/delegates/queue.dart, line 29, col 35)
[error] Base class introduces an invalid override. The type of DelegatingIterable.fold ((dynamic, (dynamic, E) → dynamic) → dynamic) is not a subtype of Iterable<E>.fold (<T>(T, (T, E) → T) → T). (./lib/src/collection/delegates/set.dart, line 29, col 33)
[error] Base class introduces an invalid override. The type of DelegatingIterable.map (((E) → dynamic) → Iterable<dynamic>) is not a subtype of Iterable<E>.map (<T>((E) → T) → Iterable<T>). (./lib/src/collection/delegates/list.dart, line 29, col 34)
[error] Base class introduces an invalid override. The type of DelegatingIterable.map (((E) → dynamic) → Iterable<dynamic>) is not a subtype of Iterable<E>.map (<T>((E) → T) → Iterable<T>). (./lib/src/collection/delegates/queue.dart, line 29, col 35)
[error] Base class introduces an invalid override. The type of DelegatingIterable.map (((E) → dynamic) → Iterable<dynamic>) is not a subtype of Iterable<E>.map (<T>((E) → T) → Iterable<T>). (./lib/src/collection/delegates/set.dart, line 29, col 33)
[error] Base class introduces an invalid override. The type of _WrappedIterable.expand (((V) → Iterable<dynamic>) → Iterable<dynamic>) is not a subtype of Iterable<V>.expand (<T>((V) → Iterable<T>) → Iterable<T>). (./lib/src/collection/multimap.dart, line 531, col 26)
[error] Base class introduces an invalid override. The type of _WrappedIterable.expand (((V) → Iterable<dynamic>) → Iterable<dynamic>) is not a subtype of Iterable<V>.expand (<T>((V) → Iterable<T>) → Iterable<T>). (./lib/src/collection/multimap.dart, line 684, col 25)
[error] Base class introduces an invalid override. The type of _WrappedIterable.fold ((dynamic, (dynamic, V) → dynamic) → dynamic) is not a subtype of Iterable<V>.fold (<T>(T, (T, V) → T) → T). (./lib/src/collection/multimap.dart, line 531, col 26)
[error] Base class introduces an invalid override. The type of _WrappedIterable.fold ((dynamic, (dynamic, V) → dynamic) → dynamic) is not a subtype of Iterable<V>.fold (<T>(T, (T, V) → T) → T). (./lib/src/collection/multimap.dart, line 684, col 25)
[error] Base class introduces an invalid override. The type of _WrappedIterable.map (((V) → dynamic) → Iterable<dynamic>) is not a subtype of Iterable<V>.map (<T>((V) → T) → Iterable<T>). (./lib/src/collection/multimap.dart, line 531, col 26)
[error] Base class introduces an invalid override. The type of _WrappedIterable.map (((V) → dynamic) → Iterable<dynamic>) is not a subtype of Iterable<V>.map (<T>((V) → T) → Iterable<T>). (./lib/src/collection/multimap.dart, line 684, col 25)
[error] Invalid override. The type of DelegatingIterable.expand (((E) → Iterable<dynamic>) → Iterable<dynamic>) is not a subtype of Iterable<E>.expand (<T>((E) → Iterable<T>) → Iterable<T>). (./lib/src/collection/delegates/iterable.dart, line 41, col 3)
[error] Invalid override. The type of DelegatingIterable.fold ((dynamic, (dynamic, E) → dynamic) → dynamic) is not a subtype of Iterable<E>.fold (<T>(T, (T, E) → T) → T). (./lib/src/collection/delegates/iterable.dart, line 48, col 3)
[error] Invalid override. The type of DelegatingIterable.map (((E) → dynamic) → Iterable<dynamic>) is not a subtype of Iterable<E>.map (<T>((E) → T) → Iterable<T>). (./lib/src/collection/delegates/iterable.dart, line 68, col 3)
[error] Invalid override. The type of InfiniteIterable.fold ((dynamic, (dynamic, T) → dynamic) → bool) is not a subtype of Iterable<T>.fold (<T₀>(T₀, (T₀, T) → T₀) → T₀). (./lib/src/iterables/infinite_iterable.dart, line 34, col 3)
[error] Invalid override. The type of LinkedLruHashMap.[] ((K) → V) is not a subtype of Map<K, V>.[] ((Object) → V). (./lib/src/collection/lru_map.dart, line 196, col 3)
[error] Invalid override. The type of LinkedLruHashMap.containsKey ((K) → bool) is not a subtype of Map<K, V>.containsKey ((Object) → bool). (./lib/src/collection/lru_map.dart, line 105, col 3)
[error] Invalid override. The type of LinkedLruHashMap.containsValue ((V) → bool) is not a subtype of Map<K, V>.containsValue ((Object) → bool). (./lib/src/collection/lru_map.dart, line 108, col 3)
[error] Invalid override. The type of LinkedLruHashMap.remove ((K) → V) is not a subtype of Map<K, V>.remove ((Object) → V). (./lib/src/collection/lru_map.dart, line 225, col 3)
[error] Invalid override. The type of StreamBuffer.addStream ((Stream<T>) → Future<dynamic>) is not a subtype of StreamConsumer<dynamic>.addStream ((Stream<dynamic>) → Future<dynamic>). (./lib/src/async/stream_buffer.dart, line 142, col 3)
[error] Invalid override. The type of _WrappedIterable.expand (((V) → Iterable<dynamic>) → Iterable<dynamic>) is not a subtype of Iterable<V>.expand (<T>((V) → Iterable<T>) → Iterable<T>). (./lib/src/collection/multimap.dart, line 410, col 3)
[error] Invalid override. The type of _WrappedIterable.fold ((dynamic, (dynamic, V) → dynamic) → dynamic) is not a subtype of Iterable<V>.fold (<T>(T, (T, V) → T) → T). (./lib/src/collection/multimap.dart, line 425, col 3)
[error] Invalid override. The type of _WrappedIterable.map (((V) → dynamic) → Iterable<dynamic>) is not a subtype of Iterable<V>.map (<T>((V) → T) → Iterable<T>). (./lib/src/collection/multimap.dart, line 470, col 3)
[error] Type check failed: _id ((dynamic) → dynamic) is not of type (dynamic) → K (./lib/src/collection/multimap.dart, line 161, col 13)
[error] Type check failed: _id ((dynamic) → dynamic) is not of type (dynamic) → V (./lib/src/collection/multimap.dart, line 162, col 15)
[hint] 'FakeTimer' is deprecated (./lib/testing/src/async/async.dart, line 36, col 3)
[hint] 'padLeft' is deprecated (./lib/strings.dart, line 244, col 19)
[hint] 'padRight' is deprecated (./lib/strings.dart, line 244, col 10)
[hint] The value of the local variable 'run' is not used (./lib/testing/src/equality/equality.dart, line 97, col 14)
[hint] The value of the local variable 'string' is not used (./lib/testing/src/runtime/checked_mode.dart, line 36, col 12)
[warning] Missing concrete implementation of 'Iterable.expand', 'Iterable.fold' and 'Iterable.map' (./lib/src/collection/multimap.dart, line 531, col 7)
[warning] Missing concrete implementation of 'Iterable.expand', 'Iterable.fold' and 'Iterable.map' (./lib/src/collection/multimap.dart, line 684, col 7)
[warning] The getter '_left' is not defined for the class '_TreeNode<V>'. (./lib/src/collection/treeset.dart, line 151, col 47)
[warning] The getter 'length' is not defined for the class 'Object'. (./lib/src/async/stream_buffer.dart, line 152, col 41)
[warning] The method 'add' is not defined for the class 'Object'. (./lib/src/async/stream_router.dart, line 72, col 16)
[warning] The method 'compareTo' is not defined for the class 'Object'. (./lib/src/collection/treeset.dart, line 32, col 32)
[warning] The redirected constructor '({maximumSize: int}) → LinkedLruHashMap<dynamic, dynamic>' has incompatible parameters with '({maximumSize: int}) → LruMap<K, V>' (./lib/src/collection/lru_map.dart, line 32, col 39)
[warning] The return type 'bool' is not assignable to 'T' as required by the method it is overriding from 'Iterable' (./lib/src/iterables/infinite_iterable.dart, line 34, col 8)
[warning] Unsound implicit cast from (K, C) → void to (K, Iterable<V>) → void (./lib/src/collection/multimap.dart, line 352, col 64)
[warning] Unsound implicit cast from (dynamic) → void to (Timer) → void (./lib/async.dart, line 53, col 34)
[warning] Unsound implicit cast from Completer<dynamic> to Completer<List<T>> (./lib/src/async/stream_buffer.dart, line 137, col 54)
[warning] Unsound implicit cast from Converter<List<int>, String> to StreamTransformer<List<int>, dynamic> (./lib/io.dart, line 28, col 27)
[warning] Unsound implicit cast from Future<dynamic> to Future<List<T>> (./lib/src/async/stream_buffer.dart, line 138, col 12)
[warning] Unsound implicit cast from Future<dynamic> to Future<Stream<T>> (./lib/src/async/future_stream.dart, line 42, col 15)
[warning] Unsound implicit cast from List<dynamic> to List<T> (./lib/src/async/stream_buffer.dart, line 118, col 12)
[warning] Unsound implicit cast from Map<K, Iterable<V>> to Map<dynamic, List<V>> (./lib/src/collection/multimap.dart, line 280, col 24)
[warning] Unsound implicit cast from Map<dynamic, dynamic> to Map<String, List<dynamic>> (./lib/testing/src/equality/equality.dart, line 98, col 27)
[warning] Unsound implicit cast from Map<dynamic, dynamic> to Map<String, List<dynamic>> (./lib/testing/src/equality/equality.dart, line 99, col 32)
[warning] Unsound implicit cast from Object to V (./lib/src/collection/treeset.dart, line 441, col 29)
[warning] Unsound implicit cast from Object to V (./lib/src/collection/treeset.dart, line 633, col 20)
[warning] Unsound implicit cast from Object to V (./lib/src/collection/treeset.dart, line 693, col 28)
[warning] Unsound implicit cast from Object to V (./lib/src/collection/treeset.dart, line 954, col 38)
[warning] Unsound implicit cast from Object to V (./lib/src/collection/treeset.dart, line 960, col 51)
[warning] Unsound implicit cast from Object to V (./lib/src/collection/treeset.dart, line 968, col 38)
[warning] Unsound implicit cast from Object to V (./lib/src/collection/treeset.dart, line 974, col 51)
[warning] Unsound implicit cast from StreamSubscription<dynamic> to StreamSubscription<DateTime> (./lib/src/async/metronome.dart, line 89, col 7)
[warning] Unsound implicit cast from dynamic to (T) → T (./lib/src/iterables/generating_iterable.dart, line 57, col 66)
[warning] Unsound implicit cast from dynamic to Iterable<Match> (./lib/pattern.dart, line 67, col 38)
[warning] Unsound implicit cast from dynamic to Iterable<T> (./lib/src/iterables/cycle.dart, line 42, col 21)
[warning] Unsound implicit cast from dynamic to Iterable<V> (./lib/src/collection/multimap.dart, line 225, col 12)
[warning] Unsound implicit cast from dynamic to Iterator<T> (./lib/src/iterables/cycle.dart, line 43, col 21)
[warning] Unsound implicit cast from dynamic to T (./lib/src/iterables/generating_iterable.dart, line 57, col 55)
[warning] Unsound implicit cast from dynamic to T (./lib/src/iterables/generating_iterable.dart, line 74, col 16)
[warning] Unsound implicit cast from dynamic to V (./lib/src/cache/map_cache.dart, line 41, col 23)
[warning] Unsound implicit cast from dynamic to V (./lib/src/cache/map_cache.dart, line 42, col 18)
[warning] Unsound implicit cast from dynamic to V (./lib/src/cache/map_cache.dart, line 45, col 21)
[warning] Unsound implicit cast from dynamic to V (./lib/src/collection/treeset.dart, line 802, col 40)

[emesx]

On it, ETA Monday.

[emesx]

At least two pieces of code need rethinking / original authors' comment:

1. TreeSet's constructor.
   This fragment defines a default comparator function, which itself assumes the instances do implement something called compareTo.
   I'm sorry, but this is not possible to express statically. Is this default value necessary?
2. MultiMaps's removeAll. This is a type-unsafe template method with _create() implemented in SetMultimap and ListMultimap. Because they can only agree on returning an Iterable, the piece of code invokes .addAll() and .clear() on those Iterables muting any warnings. Without a major redesign of this fragment it's impossible to express statically. Are you happy to review a change to this code?

[emesx]

@yjbanov @kevmoo - need your opinion on this.

[leafpetersen]

On the first question - somebody (maybe @munificent or maybe @NweiZ ?) recently fixed a similar issue, I'll try to track down what the solution there was. Note that you do still have the option of falling back on dynamic if needed, we just prefer to avoid it if possible.

For the second question - again, I believe from a quick look that the code as written should work fine in strong mode/DDC - you may need to put an explicit cast on the return value, but the cast should succeed. If you're want to avoid the dynamic sends in the body, then you might need to structure this differently (e.g. define an abstract method _createWith which is the composition of _create and addAll).

[munificent]

I believe you can fix the TreeSet constructor like so:

  factory TreeSet({Comparator<V> comparator}) {
    if (comparator == null) {
      comparator = (a, b) => (a as Comparable<V>).compareTo(b);
    }

    return new AvlTreeSet(comparator: comparator);
  }

This is statically unsound, but that's because the class exposes an API that isn't statically sound. It doesn't prevent you from doing new TreeSet<bool>(), but that will fail at runtime.

To do something statically sound, I think you need distinct classes for a TreeSet that takes a Comparable type argument and one that takes a comparator.

[leafpetersen]

Note that this solution is only the right one if this is only supposed to work for things which actually implement Comparable. If you want it to work for anything that has a compareTo, you need to use the dynamic sends.

[emesx]

Yes, Comparable was a my first guess - but there's nothing about it in TreeSet's docs - no requirements, constrains on it. OK - I'll keep it dynamic.

[justinfagnani]

I wish we could have different constraints on different constructors. Then we could do this (pretending TreeSet was not abstract):

class TreeSet<V> implements Set<V> {
  factory TreeSet<V extends Comparable>() => new TreeSet.withComparator(_comparableComparator);

  TreeSet<V>.withComparator(Comparator<V> comparator) { ... }
}

Since we don't, at least we should add to the docs that if no Comparator is provided, then V must implement Comparable, but I actually like Bob's solution of different classes.

edit: My assumption is that anyone needing the strong-mode compliant TreeSet can use a new version. Quiver really needs to go 1.0 anyway (I'm really ignorant of any progress towards that by now).

[leafpetersen]

@justinfagnani I think you could already write a static generic method that does what you describe above (you don't get to use the factory syntax though). I can't immediately think of any problems with supporting something like that for factory constructors as well - maybe an idea to keep in mind for future.

[cbracken]

Fixed by #290