Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly handle nullable type arguments in a number of APIs #711

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
@@ -1,5 +1,6 @@
#### 3.2.0
#### 3.2.0-dev

* Explicitly disallow a `nullable` type argument for `Optional`.
* Expose `TreeIterator` for iterating `TreeSet` instead of using
`BidirectionalIterator`.
* Require Dart 2.17
Expand Down
5 changes: 0 additions & 5 deletions analysis_options.yaml
@@ -1,11 +1,6 @@
# https://dart.dev/guides/language/analysis-options
include: package:lints/recommended.yaml

analyzer:
errors:
# TODO(kevmoo): see https://github.com/dart-lang/linter/issues/3844
null_check_on_nullable_type_parameter: ignore

linter:
rules:
- always_declare_return_types
Expand Down
2 changes: 1 addition & 1 deletion lib/src/collection/bimap.dart
Expand Up @@ -151,7 +151,7 @@ class HashBiMap<K, V> implements BiMap<K, V> {
@override
void updateAll(V Function(K key, V value) update) {
for (final key in keys) {
_add(key, update(key, _map[key]!), true);
_add(key, update(key, _map[key] as V), true);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handles the nullable V case correctly!

This could also iterate over entries to avoid the as check...

}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/src/collection/lru_map.dart
Expand Up @@ -290,7 +290,7 @@ class LinkedLruHashMap<K, V> implements LruMap<K, V> {
V update(K key, V Function(V value) update, {V Function()? ifAbsent}) {
V newValue;
if (containsKey(key)) {
newValue = update(this[key]!);
newValue = update(this[key] as V);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No way around this, if I understand things correctly

} else {
if (ifAbsent == null) {
throw ArgumentError.value(key, 'key', 'Key not in map');
Expand Down
14 changes: 7 additions & 7 deletions lib/src/collection/treeset.dart
Expand Up @@ -899,7 +899,9 @@ class TreeIterator<V>
{this.reversed = false, this.inclusive = true, V? anchorObject})
: _anchorObject = anchorObject,
_modCountGuard = tree._modCount {
if (_anchorObject == null || tree.isEmpty) {
final anchor = _anchorObject;

if (anchor == null || tree.isEmpty) {
// If the anchor is far left or right, we're just a normal iterator.
_state = reversed ? _right : _left;
_moveNext = reversed ? _movePreviousNormal : _moveNextNormal;
Expand All @@ -911,28 +913,26 @@ class TreeIterator<V>
// Else we've got an anchor we have to worry about initializing from.
// This isn't known till the caller actually performs a previous/next.
_moveNext = () {
_current = tree._searchNearest(_anchorObject,
_current = tree._searchNearest(anchor,
option: reversed ? TreeSearch.LESS_THAN : TreeSearch.GREATER_THAN);
_moveNext = reversed ? _movePreviousNormal : _moveNextNormal;
_movePrevious = reversed ? _moveNextNormal : _movePreviousNormal;
if (_current == null) {
_state = reversed ? _left : _right;
} else if (tree.comparator(_current!.object, _anchorObject!) == 0 &&
!inclusive) {
} else if (tree.comparator(_current!.object, anchor) == 0 && !inclusive) {
_moveNext();
}
return _state == _walk;
};

_movePrevious = () {
_current = tree._searchNearest(_anchorObject,
_current = tree._searchNearest(anchor,
option: reversed ? TreeSearch.GREATER_THAN : TreeSearch.LESS_THAN);
_moveNext = reversed ? _movePreviousNormal : _moveNextNormal;
_movePrevious = reversed ? _moveNextNormal : _movePreviousNormal;
if (_current == null) {
_state = reversed ? _right : _left;
} else if (tree.comparator(_current!.object, _anchorObject!) == 0 &&
!inclusive) {
} else if (tree.comparator(_current!.object, anchor) == 0 && !inclusive) {
_movePrevious();
}
return _state == _walk;
Expand Down
2 changes: 1 addition & 1 deletion lib/src/core/optional.dart
Expand Up @@ -23,7 +23,7 @@ import 'dart:collection';
/// With the introduction of non-null by default in Dart SDK 2.12, developers
/// should avoid adding more uses of this type. Existing users should migrate
/// away from the `Optional` type to types marked nullable: `T?`.
class Optional<T> extends IterableBase<T> {
class Optional<T extends Object> extends IterableBase<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably breaking, but if you can pull this through, hats off to you!

/// Constructs an empty Optional.
const Optional.absent() : _value = null;

Expand Down
5 changes: 3 additions & 2 deletions lib/src/iterables/generating_iterable.dart
Expand Up @@ -67,9 +67,10 @@ class _GeneratingIterator<T> implements Iterator<T> {

@override
bool moveNext() {
if (object == null) return false;
final obj = object;
if (obj == null) return false;
if (started) {
object = next(object!);
object = next(obj);
} else {
started = true;
}
Expand Down