Merge pull request #75 from davidmorgan/fewer-checks

Relax type checks based on strong mode guarantees.
google · Oct 24, 2016 · c5a505a · c5a505a
2 parents 4047ace + e25e68e
commit c5a505a
Show file tree

Hide file tree

Showing 14 changed files with 90 additions and 169 deletions.
diff --git a/.analysis_options b/.analysis_options
@@ -0,0 +1,2 @@
+analyzer:
+ strong-mode: true
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## 1.1.0
+
+- Remove runtime checks that are unnecessary if the project using
+  built_collection is "strong mode clean", that is, if it has no errors with
+  strong mode. See note in `README.md` about strong mode.
+
 ## 1.0.6
 
 - Allow quiver 0.23.

diff --git a/README.md b/README.md
@@ -25,6 +25,16 @@ Built Collections:
 See below for details on each of these points.
 
 
+## A note about strong mode
+
+Please note that from version `1.1.0` built_collection must be used in
+conjunction with
+[strong mode](https://github.com/dart-lang/dev_compiler/blob/master/STRONG_MODE.md)
+to get all the type guarantees. That is, your project must have no warnings or
+errors when analyzed with the strong mode analyzer. This allows some runtime
+checks to be skipped because the equivalent check can be done statically.
+
+
 ## Recommended Style
 
 A project can benefit greatly from using Built Collections throughout.

diff --git a/lib/src/list/list_builder.dart b/lib/src/list/list_builder.dart
@@ -234,15 +234,17 @@ class ListBuilder<E> {
     }
   }
 
-  void _checkElement(Object element) {
-    if (element is! E) {
-      throw new ArgumentError('invalid element: ${element}');
+  void _checkElement(E element) {
+    if (identical(element, null)) {
+      throw new ArgumentError('null element');
     }
   }
 
   void _checkElements(Iterable elements) {
     for (final element in elements) {
-      _checkElement(element);
+      if (element is! E) {
+        throw new ArgumentError('invalid element: ${element}');
+      }
     }
   }
 }
diff --git a/lib/src/list_multimap/list_multimap_builder.dart b/lib/src/list_multimap/list_multimap_builder.dart
@@ -219,15 +219,15 @@ class ListMultimapBuilder<K, V> {
     }
   }
 
-  void _checkKey(Object key) {
-    if (key is! K) {
-      throw new ArgumentError('invalid key: ${key}');
+  void _checkKey(K key) {
+    if (identical(key, null)) {
+      throw new ArgumentError('null key');
     }
   }
 
-  void _checkValue(Object value) {
-    if (value is! V) {
-      throw new ArgumentError('invalid value: ${value}');
+  void _checkValue(V value) {
+    if (identical(value, null)) {
+      throw new ArgumentError('null value');
     }
   }
 }
diff --git a/lib/src/map/map_builder.dart b/lib/src/map/map_builder.dart
@@ -140,27 +140,31 @@ class MapBuilder<K, V> {
     }
   }
 
-  void _checkKey(Object key) {
-    if (key is! K) {
-      throw new ArgumentError('invalid key: ${key}');
+  void _checkKey(K key) {
+    if (identical(key, null)) {
+      throw new ArgumentError('null key');
     }
   }
 
   void _checkKeys(Iterable keys) {
     for (final key in keys) {
-      _checkKey(key);
+      if (key is! K) {
+        throw new ArgumentError('invalid key: ${key}');
+      }
     }
   }
 
-  void _checkValue(Object value) {
-    if (value is! V) {
-      throw new ArgumentError('invalid value: ${value}');
+  void _checkValue(V value) {
+    if (identical(value, null)) {
+      throw new ArgumentError('null value');
     }
   }
 
   void _checkValues(Iterable values) {
     for (final value in values) {
-      _checkValue(value);
+      if (value is! V) {
+        throw new ArgumentError('invalid value: ${value}');
+      }
     }
   }
 }
diff --git a/lib/src/set/set_builder.dart b/lib/src/set/set_builder.dart
@@ -177,15 +177,17 @@ class SetBuilder<E> {
     }
   }
 
-  void _checkElement(Object element) {
-    if (element is! E) {
-      throw new ArgumentError('invalid element: ${element}');
+  void _checkElement(E element) {
+    if (identical(element, null)) {
+      throw new ArgumentError('null element');
     }
   }
 
   void _checkElements(Iterable elements) {
     for (final element in elements) {
-      _checkElement(element);
+      if (element is! E) {
+        throw new ArgumentError('invalid element: ${element}');
+      }
     }
   }
 }
diff --git a/lib/src/set_multimap/set_multimap_builder.dart b/lib/src/set_multimap/set_multimap_builder.dart
@@ -219,14 +219,14 @@ class SetMultimapBuilder<K, V> {
     }
   }
 
-  void _checkKey(Object key) {
-    if (key is! K) {
+  void _checkKey(K key) {
+    if (identical(key, null)) {
       throw new ArgumentError('invalid key: ${key}');
     }
   }
 
-  void _checkValue(Object value) {
-    if (value is! V) {
+  void _checkValue(V value) {
+    if (identical(value, null)) {
       throw new ArgumentError('invalid value: ${value}');
     }
   }

diff --git a/pubspec.yaml b/pubspec.yaml
@@ -1,5 +1,5 @@
 name: built_collection
-version: 1.0.6
+version: 1.1.0
 description: >
   Builder pattern wrappers for core Dart collections.
 authors:

diff --git a/test/list/list_builder_test.dart b/test/list/list_builder_test.dart
@@ -70,53 +70,37 @@ void main() {
           throws);
     });
 
-    test('throws on wrong type assign', () {
-      expect(() => new ListBuilder<int>([0])[0] = '0', throws);
-    });
-
-    test('throws on wrong type add', () {
-      expect(() => new ListBuilder<int>().add('0'), throws);
-    });
-
     test('throws on wrong type addAll', () {
-      expect(() => new ListBuilder<int>().addAll([0, 1, '0']), throws);
-    });
-
-    test('throws on wrong type insert', () {
-      expect(() => new ListBuilder<int>().insert(0, '0'), throws);
+      expect(
+          () => new ListBuilder<int>().addAll(new List<int>.from([0, 1, '0'])),
+          throws);
     });
 
     test('throws on wrong type insertAll', () {
-      expect(() => new ListBuilder<int>().insertAll(0, [0, 1, '0']), throws);
+      expect(
+          () => new ListBuilder<int>()
+              .insertAll(0, new List<int>.from([0, 1, '0'])),
+          throws);
     });
 
     test('throws on wrong type setAll', () {
       expect(
-          () => new ListBuilder<int>([0, 1, 2]).setAll(0, [0, 1, '0']), throws);
-    });
-
-    test('throws on wrong type setRange', () {
-      expect(() => new ListBuilder<int>([0, 1, 2]).setRange(0, 2, [0, 1, '0']),
+          () => new ListBuilder<int>([0, 1, 2])
+              .setAll(0, new List<int>.from([0, 1, '0'])),
           throws);
     });
 
-    test('throws on wrong type fillRange', () {
+    test('throws on wrong type setRange', () {
       expect(
-          () => new ListBuilder<int>([0, 1, 2]).fillRange(0, 2, '0'), throws);
+          () => new ListBuilder<int>([0, 1, 2])
+              .setRange(0, 2, new List<int>.from([0, 1, '0'])),
+          throws);
     });
 
     test('throws on wrong type replaceRange', () {
       expect(
-          () => new ListBuilder<int>([0, 1, 2]).replaceRange(0, 2, [0, 1, '0']),
-          throws);
-    });
-
-    test('throws on wrong type map', () {
-      expect(() => new ListBuilder<int>([0, 1, 2]).map((x) => '0'), throws);
-    });
-
-    test('throws on wrong type expand', () {
-      expect(() => new ListBuilder<int>([0, 1, 2]).expand((x) => [x, '0']),
+          () => new ListBuilder<int>([0, 1, 2])
+              .replaceRange(0, 2, new List<int>.from([0, 1, '0'])),
           throws);
     });
 

diff --git a/test/list_multimap/list_multimap_builder_test.dart b/test/list_multimap/list_multimap_builder_test.dart
@@ -39,53 +39,27 @@ void main() {
     });
 
     test('throws on null key addAll', () {
-      expect(
-          () => new ListMultimapBuilder<int, String>().addAll({
-                null: ['0']
-              }),
-          throws);
-    });
-
-    test('throws on null value addAll', () {
-      expect(
-          () => new ListMultimapBuilder<int, String>().addAll({
-                0: [null]
-              }),
-          throws);
-    });
+      final mutableMultimap = new ListMultimap<int, String>();
+      mutableMultimap.add(null, '0');
 
-    test('throws on wrong type key add', () {
       expect(
-          () => new ListMultimapBuilder<int, String>().add('0', '0'), throws);
-    });
-
-    test('throws on wrong type value add', () {
-      expect(() => new ListMultimapBuilder<int, String>().add(0, [0]), throws);
-    });
-
-    test('throws on wrong type key addValues', () {
-      expect(() => new ListMultimapBuilder<int, String>().addValues('0', ['0']),
+          () => new ListMultimapBuilder<int, String>().addAll(mutableMultimap),
           throws);
     });
 
-    test('throws on wrong type value addValues', () {
-      expect(() => new ListMultimapBuilder<int, String>().addValues(0, [0]),
-          throws);
-    });
+    test('throws on null value addAll', () {
+      final mutableMultimap = new ListMultimap<int, String>();
+      mutableMultimap.add(0, null);
 
-    test('throws on wrong type key addAll', () {
-      final mutableMultimap = new ListMultimap();
-      mutableMultimap.add('0', '0');
       expect(
           () => new ListMultimapBuilder<int, String>().addAll(mutableMultimap),
           throws);
     });
 
-    test('throws on wrong type value addAll', () {
-      final mutableMultimap = new ListMultimap();
-      mutableMultimap.add(0, 0);
+    test('throws on wrong type value addValues', () {
       expect(
-          () => new ListMultimapBuilder<int, String>().addAll(mutableMultimap),
+          () => new ListMultimapBuilder<int, String>()
+              .addValues(0, new List<String>.from([0])),
           throws);
     });
 
@@ -137,7 +111,7 @@ void main() {
       expect(
           (new ListMultimapBuilder<int, int>()
                 ..addIterable([1, 2, 3],
-                    values: (element) => [element, element + 1]))
+                    values: (element) => <int>[element, element + 1]))
               .build()
               .toMap(),
           {

diff --git a/test/map/map_builder_test.dart b/test/map/map_builder_test.dart
@@ -51,32 +51,6 @@ void main() {
       expect(() => new MapBuilder<int, String>().addAll({0: null}), throws);
     });
 
-    test('throws on wrong type key put', () {
-      expect(() => new MapBuilder<int, String>()['0'] = '0', throws);
-    });
-
-    test('throws on wrong type value put', () {
-      expect(() => new MapBuilder<int, String>()[0] = 0, throws);
-    });
-
-    test('throws on wrong type key putIfAbsent', () {
-      expect(() => new MapBuilder<int, String>().putIfAbsent('0', () => '0'),
-          throws);
-    });
-
-    test('throws on wrong type value putIfAbsent', () {
-      expect(
-          () => new MapBuilder<int, String>().putIfAbsent(0, () => 0), throws);
-    });
-
-    test('throws on wrong type key addAll', () {
-      expect(() => new MapBuilder<int, String>().addAll({'0': '0'}), throws);
-    });
-
-    test('throws on wrong type value addAll', () {
-      expect(() => new MapBuilder<int, String>().addAll({0: 0}), throws);
-    });
-
     test('has replace method that replaces all data', () {
       expect(
           (new MapBuilder<int, String>()..replace({1: '1', 2: '2'}))

diff --git a/test/set/set_builder_test.dart b/test/set/set_builder_test.dart
@@ -34,21 +34,9 @@ void main() {
           throws);
     });
 
-    test('throws on wrong type add', () {
-      expect(() => new SetBuilder<int>().add('0'), throws);
-    });
-
     test('throws on wrong type addAll', () {
-      expect(() => new SetBuilder<int>().addAll([0, 1, '0']), throws);
-    });
-
-    test('throws on wrong type map', () {
-      expect(() => new SetBuilder<int>([0, 1, 2]).map((x) => '0'), throws);
-    });
-
-    test('throws on wrong type expand', () {
-      expect(
-          () => new SetBuilder<int>([0, 1, 2]).expand((x) => [x, '0']), throws);
+      expect(() => new SetBuilder<int>().addAll(new List.from([0, 1, '0'])),
+          throws);
     });
 
     test('has replace method that replaces all data', () {