From e25e68ea227c8afc29753e7b94473c756b12fa02 Mon Sep 17 00:00:00 2001 From: David Morgan Date: Mon, 24 Oct 2016 14:10:14 +0200 Subject: [PATCH] Relax type checks based on strong mode guarantees. Remove test cases that strong mode catches for us. Fix the remainder to be strong mode clean. --- .analysis_options | 2 + CHANGELOG.md | 6 +++ README.md | 10 ++++ lib/src/list/list_builder.dart | 10 ++-- .../list_multimap/list_multimap_builder.dart | 12 ++--- lib/src/map/map_builder.dart | 20 ++++---- lib/src/set/set_builder.dart | 10 ++-- .../set_multimap/set_multimap_builder.dart | 8 ++-- pubspec.yaml | 2 +- test/list/list_builder_test.dart | 46 ++++++------------- .../list_multimap_builder_test.dart | 46 ++++--------------- test/map/map_builder_test.dart | 26 ----------- test/set/set_builder_test.dart | 16 +------ .../set_multimap_builder_test.dart | 45 ++++-------------- 14 files changed, 90 insertions(+), 169 deletions(-) create mode 100644 .analysis_options diff --git a/.analysis_options b/.analysis_options new file mode 100644 index 0000000..39114b9 --- /dev/null +++ b/.analysis_options @@ -0,0 +1,2 @@ +analyzer: + strong-mode: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 802a119..4f7d7ca 100644 --- 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 index 8124a40..0145cd4 100644 --- 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 index ca392f2..90c3a1b 100644 --- a/lib/src/list/list_builder.dart +++ b/lib/src/list/list_builder.dart @@ -234,15 +234,17 @@ class ListBuilder { } } - 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 index b2d7e61..2dd826f 100644 --- a/lib/src/list_multimap/list_multimap_builder.dart +++ b/lib/src/list_multimap/list_multimap_builder.dart @@ -219,15 +219,15 @@ class ListMultimapBuilder { } } - 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 index b9fe060..284d111 100644 --- a/lib/src/map/map_builder.dart +++ b/lib/src/map/map_builder.dart @@ -140,27 +140,31 @@ class MapBuilder { } } - 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 index 6e65ceb..0e185f2 100644 --- a/lib/src/set/set_builder.dart +++ b/lib/src/set/set_builder.dart @@ -177,15 +177,17 @@ class SetBuilder { } } - 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 index ae46a73..deecb7c 100644 --- a/lib/src/set_multimap/set_multimap_builder.dart +++ b/lib/src/set_multimap/set_multimap_builder.dart @@ -219,14 +219,14 @@ class SetMultimapBuilder { } } - 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 index 0687c3e..e7e2698 100644 --- 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 index 4f4eddf..13b5c82 100644 --- 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([0])[0] = '0', throws); - }); - - test('throws on wrong type add', () { - expect(() => new ListBuilder().add('0'), throws); - }); - test('throws on wrong type addAll', () { - expect(() => new ListBuilder().addAll([0, 1, '0']), throws); - }); - - test('throws on wrong type insert', () { - expect(() => new ListBuilder().insert(0, '0'), throws); + expect( + () => new ListBuilder().addAll(new List.from([0, 1, '0'])), + throws); }); test('throws on wrong type insertAll', () { - expect(() => new ListBuilder().insertAll(0, [0, 1, '0']), throws); + expect( + () => new ListBuilder() + .insertAll(0, new List.from([0, 1, '0'])), + throws); }); test('throws on wrong type setAll', () { expect( - () => new ListBuilder([0, 1, 2]).setAll(0, [0, 1, '0']), throws); - }); - - test('throws on wrong type setRange', () { - expect(() => new ListBuilder([0, 1, 2]).setRange(0, 2, [0, 1, '0']), + () => new ListBuilder([0, 1, 2]) + .setAll(0, new List.from([0, 1, '0'])), throws); }); - test('throws on wrong type fillRange', () { + test('throws on wrong type setRange', () { expect( - () => new ListBuilder([0, 1, 2]).fillRange(0, 2, '0'), throws); + () => new ListBuilder([0, 1, 2]) + .setRange(0, 2, new List.from([0, 1, '0'])), + throws); }); test('throws on wrong type replaceRange', () { expect( - () => new ListBuilder([0, 1, 2]).replaceRange(0, 2, [0, 1, '0']), - throws); - }); - - test('throws on wrong type map', () { - expect(() => new ListBuilder([0, 1, 2]).map((x) => '0'), throws); - }); - - test('throws on wrong type expand', () { - expect(() => new ListBuilder([0, 1, 2]).expand((x) => [x, '0']), + () => new ListBuilder([0, 1, 2]) + .replaceRange(0, 2, new List.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 index 4104a7d..31f7a26 100644 --- 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().addAll({ - null: ['0'] - }), - throws); - }); - - test('throws on null value addAll', () { - expect( - () => new ListMultimapBuilder().addAll({ - 0: [null] - }), - throws); - }); + final mutableMultimap = new ListMultimap(); + mutableMultimap.add(null, '0'); - test('throws on wrong type key add', () { expect( - () => new ListMultimapBuilder().add('0', '0'), throws); - }); - - test('throws on wrong type value add', () { - expect(() => new ListMultimapBuilder().add(0, [0]), throws); - }); - - test('throws on wrong type key addValues', () { - expect(() => new ListMultimapBuilder().addValues('0', ['0']), + () => new ListMultimapBuilder().addAll(mutableMultimap), throws); }); - test('throws on wrong type value addValues', () { - expect(() => new ListMultimapBuilder().addValues(0, [0]), - throws); - }); + test('throws on null value addAll', () { + final mutableMultimap = new ListMultimap(); + mutableMultimap.add(0, null); - test('throws on wrong type key addAll', () { - final mutableMultimap = new ListMultimap(); - mutableMultimap.add('0', '0'); expect( () => new ListMultimapBuilder().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().addAll(mutableMultimap), + () => new ListMultimapBuilder() + .addValues(0, new List.from([0])), throws); }); @@ -137,7 +111,7 @@ void main() { expect( (new ListMultimapBuilder() ..addIterable([1, 2, 3], - values: (element) => [element, element + 1])) + values: (element) => [element, element + 1])) .build() .toMap(), { diff --git a/test/map/map_builder_test.dart b/test/map/map_builder_test.dart index cecedc0..eb389ad 100644 --- a/test/map/map_builder_test.dart +++ b/test/map/map_builder_test.dart @@ -51,32 +51,6 @@ void main() { expect(() => new MapBuilder().addAll({0: null}), throws); }); - test('throws on wrong type key put', () { - expect(() => new MapBuilder()['0'] = '0', throws); - }); - - test('throws on wrong type value put', () { - expect(() => new MapBuilder()[0] = 0, throws); - }); - - test('throws on wrong type key putIfAbsent', () { - expect(() => new MapBuilder().putIfAbsent('0', () => '0'), - throws); - }); - - test('throws on wrong type value putIfAbsent', () { - expect( - () => new MapBuilder().putIfAbsent(0, () => 0), throws); - }); - - test('throws on wrong type key addAll', () { - expect(() => new MapBuilder().addAll({'0': '0'}), throws); - }); - - test('throws on wrong type value addAll', () { - expect(() => new MapBuilder().addAll({0: 0}), throws); - }); - test('has replace method that replaces all data', () { expect( (new MapBuilder()..replace({1: '1', 2: '2'})) diff --git a/test/set/set_builder_test.dart b/test/set/set_builder_test.dart index efed7e6..3076e70 100644 --- 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().add('0'), throws); - }); - test('throws on wrong type addAll', () { - expect(() => new SetBuilder().addAll([0, 1, '0']), throws); - }); - - test('throws on wrong type map', () { - expect(() => new SetBuilder([0, 1, 2]).map((x) => '0'), throws); - }); - - test('throws on wrong type expand', () { - expect( - () => new SetBuilder([0, 1, 2]).expand((x) => [x, '0']), throws); + expect(() => new SetBuilder().addAll(new List.from([0, 1, '0'])), + throws); }); test('has replace method that replaces all data', () { diff --git a/test/set_multimap/set_multimap_builder_test.dart b/test/set_multimap/set_multimap_builder_test.dart index a112684..74b4c82 100644 --- a/test/set_multimap/set_multimap_builder_test.dart +++ b/test/set_multimap/set_multimap_builder_test.dart @@ -37,52 +37,27 @@ void main() { }); test('throws on null key addAll', () { - expect( - () => new SetMultimapBuilder().addAll({ - null: ['0'] - }), - throws); - }); + final mutableMultimap = new SetMultimap(); + mutableMultimap.add(0, null); - test('throws on null value addAll', () { expect( - () => new SetMultimapBuilder().addAll({ - 0: [null] - }), - throws); - }); - - test('throws on wrong type key add', () { - expect(() => new SetMultimapBuilder().add('0', '0'), throws); - }); - - test('throws on wrong type value add', () { - expect(() => new SetMultimapBuilder().add(0, [0]), throws); - }); - - test('throws on wrong type key addValues', () { - expect(() => new SetMultimapBuilder().addValues('0', ['0']), + () => new SetMultimapBuilder().addAll(mutableMultimap), throws); }); - test('throws on wrong type value addValues', () { - expect(() => new SetMultimapBuilder().addValues(0, [0]), - throws); - }); + test('throws on null value addAll', () { + final mutableMultimap = new SetMultimap(); + mutableMultimap.add(0, null); - test('throws on wrong type key addAll', () { - final mutableMultimap = new SetMultimap(); - mutableMultimap.add('0', '0'); expect( () => new SetMultimapBuilder().addAll(mutableMultimap), throws); }); - test('throws on wrong type value addAll', () { - final mutableMultimap = new SetMultimap(); - mutableMultimap.add(0, 0); + test('throws on wrong type value addValues', () { expect( - () => new SetMultimapBuilder().addAll(mutableMultimap), + () => new SetMultimapBuilder() + .addValues(0, new List.from([0])), throws); }); @@ -134,7 +109,7 @@ void main() { expect( (new SetMultimapBuilder() ..addIterable([1, 2, 3], - values: (element) => [element, element + 1])) + values: (element) => [element, element + 1])) .build() .toMap(), {