Fix cast and retype implementations. (#88)
* Fix cast and retype implementations.
* Fix and add a few tests.
* Address feedbakc.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cab5521..e5a8b71 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## 1.15.0
+
+* Fixed bugs where `QueueList`, `MapKeySet`, and `MapValueSet` did not adhere to
+ the contract laid out by `List.cast`, `Set.cast` and `Map.cast` respectively.
+ The returned instances of these methods now correctly forward to the existing
+ instance instead of always creating a new copy.
+
## 1.14.8
* Deprecated `Delegating{Name}.typed` static methods in favor of the new Dart 2
diff --git a/lib/src/queue_list.dart b/lib/src/queue_list.dart
index cab687a..dcfee7c 100644
--- a/lib/src/queue_list.dart
+++ b/lib/src/queue_list.dart
@@ -10,6 +10,21 @@
// that are redundant with ListMixin. Remove or simplify it when issue 21330 is
// fixed.
class QueueList<E> extends Object with ListMixin<E> implements Queue<E> {
+ /// Adapts [source] to be a `QueueList<T>`.
+ ///
+ /// Any time the class would produce an element that is not a [T], the element
+ /// access will throw.
+ ///
+ /// Any time a [T] value is attempted stored into the adapted class, the store
+ /// will throw unless the value is also an instance of [S].
+ ///
+ /// If all accessed elements of [source] are actually instances of [T] and if
+ /// all elements stored in the returned are actually instance of [S],
+ /// then the returned instance can be used as a `QueueList<T>`.
+ static QueueList<T> _castFrom<S, T>(QueueList<S> source) {
+ return new _CastQueueList<S, T>(source);
+ }
+
static const int _INITIAL_CAPACITY = 8;
List<E> _table;
int _head;
@@ -31,6 +46,9 @@
_table = new List<E>(initialCapacity);
}
+ // An internal constructor for use by _CastQueueList.
+ QueueList._();
+
/// Create a queue initially containing the elements of [source].
factory QueueList.from(Iterable<E> source) {
if (source is List) {
@@ -81,13 +99,14 @@
}
QueueList<T> cast<T>() {
- if (this is QueueList<T>) {
- return this as QueueList<T>;
+ QueueList<Object> self = this;
+ if (self is QueueList<T>) {
+ return self;
}
- return new QueueList<T>.from(_table.cast<T>());
+ return retype<T>();
}
- QueueList<T> retype<T>() => new QueueList<T>.from(_table.retype<T>());
+ QueueList<T> retype<T>() => QueueList._castFrom<E, T>(this);
String toString() => IterableBase.iterableToFullString(this, "{", "}");
@@ -230,3 +249,17 @@
_head = 0;
}
}
+
+class _CastQueueList<S, T> extends QueueList<T> {
+ final QueueList<S> _delegate;
+
+ _CastQueueList(this._delegate) : super._() {
+ _table = _delegate._table.cast<T>();
+ }
+
+ int get _head => _delegate._head;
+ set _head(int value) => _delegate._head = value;
+
+ int get _tail => _delegate._tail;
+ set _tail(int value) => _delegate._tail = value;
+}
diff --git a/lib/src/wrappers.dart b/lib/src/wrappers.dart
index cfe97c7..cead501 100644
--- a/lib/src/wrappers.dart
+++ b/lib/src/wrappers.dart
@@ -475,7 +475,12 @@
Iterable<E> get _base => _baseMap.keys;
- Set<T> cast<T>() => _baseMap.keys.toSet().cast<T>();
+ Set<T> cast<T>() {
+ if (this is MapKeySet<T>) {
+ return this as MapKeySet<T>;
+ }
+ return Set.castFrom<E, T>(this);
+ }
bool contains(Object element) => _baseMap.containsKey(element);
@@ -513,7 +518,7 @@
E lookup(Object element) =>
throw new UnsupportedError("MapKeySet doesn't support lookup().");
- Set<T> retype<T>() => _baseMap.keys.toSet().retype<T>();
+ Set<T> retype<T>() => Set.castFrom<E, T>(this);
/// Returns a new set which contains all the elements of [this] and [other].
///
@@ -561,7 +566,12 @@
Iterable<V> get _base => _baseMap.values;
- Set<T> cast<T>() => _baseMap.values.toSet().cast<T>();
+ Set<T> cast<T>() {
+ if (this is Set<T>) {
+ return this as Set<T>;
+ }
+ return Set.castFrom<V, T>(this);
+ }
bool contains(Object element) {
if (element != null && element is! V) return false;
@@ -659,7 +669,7 @@
void retainWhere(bool test(V element)) =>
removeWhere((element) => !test(element));
- Set<T> retype<T>() => toSet().retype<T>();
+ Set<T> retype<T>() => Set.castFrom<V, T>(this);
/// Returns a new set which contains all the elements of [this] and [other].
///
diff --git a/pubspec.yaml b/pubspec.yaml
index 300fb9c..5f9ca88 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: collection
-version: 1.14.7
+version: 1.15.0
author: Dart Team <misc@dartlang.org>
description: Collections and utilities functions and classes related to collections.
homepage: https://www.github.com/dart-lang/collection
diff --git a/test/queue_list_test.dart b/test/queue_list_test.dart
index d2e7014..ed26a65 100644
--- a/test/queue_list_test.dart
+++ b/test/queue_list_test.dart
@@ -258,6 +258,42 @@
expect(queue.cast<Pattern>(), same(queue));
}, skip: isDart2 ? false : 'Requires a Dart2 runtime');
+ test("cast does not throw on mutation when the type is valid", () {
+ var patternQueue = new QueueList<Pattern>()..addAll(['a', 'b']);
+ var stringQueue = patternQueue.cast<String>();
+ stringQueue.addAll(['c', 'd']);
+ expect(
+ stringQueue,
+ const isInstanceOf<QueueList<String>>(),
+ reason: 'Expected QueueList<String>, got ${stringQueue.runtimeType}',
+ skip: isDart2 ? false : 'Cast does nothing in Dart1',
+ );
+
+ expect(
+ stringQueue,
+ ['a', 'b', 'c', 'd'],
+ skip: isDart2 ? false : 'Cast does nothing in Dart1',
+ );
+
+ expect(patternQueue, stringQueue, reason: 'Should forward to original');
+ });
+
+ test("cast throws on mutation when the type is not valid", () {
+ QueueList<Object> stringQueue = new QueueList<String>();
+ var numQueue = stringQueue.cast<num>();
+ expect(
+ numQueue,
+ const isInstanceOf<QueueList<num>>(),
+ reason: 'Expected QueueList<num>, got ${numQueue.runtimeType}',
+ skip: isDart2 ? false : 'Cast does nothing in Dart1',
+ );
+ expect(
+ () => numQueue.add(1),
+ throwsCastError,
+ skip: isDart2 ? false : 'In Dart1 a TypeError is not thrown',
+ );
+ });
+
test("retype returns a new QueueList", () {
var queue = new QueueList<String>();
expect(queue.retype<Pattern>(), isNot(same(queue)));
