Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

apply null safety syntax to mobile dart:ui #18933

Merged
merged 9 commits into from
Jun 12, 2020
Merged

apply null safety syntax to mobile dart:ui #18933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
62ca6b9
apply null safety syntax to mobile dart:ui
yjbanov Jun 9, 2020
866293f
clean-up /*required*/
yjbanov Jun 9, 2020
7ff0045
pass --enable-experiment=non-nullable to dartanalyzer
yjbanov Jun 9, 2020
7cf0137
remove focalRadius de-nulling
yjbanov Jun 9, 2020
8511602
make api_conform_test.dart non-nullable
yjbanov Jun 9, 2020
2c53f9b
make engine tests pass
yjbanov Jun 9, 2020
b329025
help analyzer with null inference more
yjbanov Jun 10, 2020
b6aba25
better null assert textScaleFactor
yjbanov Jun 10, 2020
e288fbf
address comments
yjbanov Jun 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions analysis_options.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
# private fields, especially on the Window object):

analyzer:
enable-experiment:
- non-nullable
exclude: [
# this test pretends to be part of dart:ui and results in lots of false
# positives.
Expand Down
1 change: 1 addition & 0 deletions ci/analyze.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ dartanalyzer --version

RESULTS=`dartanalyzer \
--options flutter/analysis_options.yaml \
--enable-experiment=non-nullable \
"$1out/host_debug_unopt/gen/sky/bindings/dart_ui/ui.dart" \
2>&1 \
| grep -Ev "No issues found!" \
Expand Down
5 changes: 3 additions & 2 deletions lib/ui/annotations.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

// TODO(dnfield): Remove unused_import ignores when https://github.com/dart-lang/sdk/issues/35164 is resolved.

// @dart = 2.6
// @dart = 2.9

part of dart.ui;

// TODO(dnfield): Update this if/when we default this to on in the tool,
Expand Down Expand Up @@ -39,7 +40,7 @@ part of dart.ui;
/// }
/// }
/// ```
const _KeepToString/*!*/ keepToString = _KeepToString();
const _KeepToString keepToString = _KeepToString();

class _KeepToString {
const _KeepToString();
Expand Down
49 changes: 24 additions & 25 deletions lib/ui/channel_buffers.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart = 2.6
// @dart = 2.9

part of dart.ui;

/// A saved platform message for a channel with its callback.
Expand All @@ -14,12 +15,12 @@ class _StoredMessage {
_StoredMessage(this._data, this._callback);

/// Representation of the message's payload.
final ByteData/*?*/ _data;
ByteData/*?*/ get data => _data;
final ByteData? _data;
ByteData? get data => _data;

/// Callback to be called when the message is received.
final PlatformMessageResponseCallback/*!*/ _callback;
PlatformMessageResponseCallback/*!*/ get callback => _callback;
final PlatformMessageResponseCallback _callback;
PlatformMessageResponseCallback get callback => _callback;
}

/// A fixed-size circular queue.
Expand All @@ -43,7 +44,7 @@ class _RingBuffer<T> {

/// A callback that get's called when items are ejected from the [_RingBuffer]
/// by way of an overflow or a resizing.
Function(T) _dropItemCallback;
Function(T)? _dropItemCallback;
set dropItemCallback(Function(T) callback) {
_dropItemCallback = callback;
}
Expand All @@ -60,7 +61,7 @@ class _RingBuffer<T> {
}

/// Returns null when empty.
T pop() {
T? pop() {
Copy link
Member

@zanderso zanderso Jun 10, 2020

Choose a reason for hiding this comment

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

What if T is already nullable?

Copy link
Contributor

@jonahwilliams jonahwilliams Jun 10, 2020

Choose a reason for hiding this comment

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

IIRC T is potentially non-nullable, so it could be int?. The type system will collapse muliplte ?? to ?, so essentially this forces nullability even a concrete instance of T is non-nullable.

Copy link
Contributor Author

@yjbanov yjbanov Jun 10, 2020

Choose a reason for hiding this comment

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

☝️ What Jonah says. But also, I'm not too worried about what happens inside private classes, as long as there are not breakages or performance regressions. We can revisit specific cases later, when everything uses the same syntax.

return _queue.isEmpty ? null : _queue.removeFirst();
}

Expand All @@ -70,9 +71,7 @@ class _RingBuffer<T> {
int result = 0;
while (_queue.length > lengthLimit) {
final T item = _queue.removeFirst();
if (_dropItemCallback != null) {
_dropItemCallback(item);
}
_dropItemCallback?.call(item);
result += 1;
}
return result;
Expand All @@ -86,7 +85,7 @@ class _RingBuffer<T> {
}

/// Signature for [ChannelBuffers.drain].
typedef DrainChannelCallback = Future<void>/*!*/ Function(ByteData/*?*/, PlatformMessageResponseCallback/*!*/);
typedef DrainChannelCallback = Future<void> Function(ByteData?, PlatformMessageResponseCallback);

/// Storage of channel messages until the channels are completely routed,
/// i.e. when a message handler is attached to the channel on the framework side.
Expand Down Expand Up @@ -116,8 +115,8 @@ class ChannelBuffers {
static const String kControlChannelName = 'dev.flutter/channel-buffers';

/// A mapping between a channel name and its associated [_RingBuffer].
final Map<String, _RingBuffer<_StoredMessage>> _messages =
<String, _RingBuffer<_StoredMessage>>{};
final Map<String, _RingBuffer<_StoredMessage>?> _messages =
Copy link
Member

@zanderso zanderso Jun 10, 2020

Choose a reason for hiding this comment

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

Can this Map have a String explicitly mapped to null? How is that distinguished from the Map not containing null?

Copy link
Contributor Author

@yjbanov yjbanov Jun 10, 2020

Choose a reason for hiding this comment

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

Maps are tricky in that even if you say the values are non-null operator[] will still return nullable value. I can try forcing non-null but I also think internal stuff like this can be cleaned up gradually in follow-up PRs.

Copy link
Contributor

@lrhn lrhn Jun 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

To answer the question: Yes, it can have null as a value. It's distinguished from not having an entry by the key being there in keys or containsKey, and the null value being in values.

Whether you want or need that is up to you.

<String, _RingBuffer<_StoredMessage>?>{};

_RingBuffer<_StoredMessage> _makeRingBuffer(int size) {
final _RingBuffer<_StoredMessage> result = _RingBuffer<_StoredMessage>(size);
Expand All @@ -130,8 +129,8 @@ class ChannelBuffers {
}

/// Returns true on overflow.
bool/*!*/ push(String/*!*/ channel, ByteData/*?*/ data, PlatformMessageResponseCallback/*!*/ callback) {
_RingBuffer<_StoredMessage> queue = _messages[channel];
bool push(String channel, ByteData? data, PlatformMessageResponseCallback callback) {
_RingBuffer<_StoredMessage>? queue = _messages[channel];
if (queue == null) {
queue = _makeRingBuffer(kDefaultBufferSize);
_messages[channel] = queue;
Expand All @@ -150,23 +149,23 @@ class ChannelBuffers {
}

/// Returns null on underflow.
_StoredMessage _pop(String channel) {
final _RingBuffer<_StoredMessage> queue = _messages[channel];
final _StoredMessage result = queue?.pop();
_StoredMessage? _pop(String channel) {
final _RingBuffer<_StoredMessage>? queue = _messages[channel];
final _StoredMessage? result = queue?.pop();
return result;
}

bool _isEmpty(String channel) {
final _RingBuffer<_StoredMessage> queue = _messages[channel];
return (queue == null) ? true : queue.isEmpty;
final _RingBuffer<_StoredMessage>? queue = _messages[channel];
return queue == null || queue.isEmpty;
}

/// Changes the capacity of the queue associated with the given channel.
///
/// This could result in the dropping of messages if newSize is less
/// than the current length of the queue.
void _resize(String channel, int newSize) {
_RingBuffer<_StoredMessage> queue = _messages[channel];
_RingBuffer<_StoredMessage>? queue = _messages[channel];
if (queue == null) {
queue = _makeRingBuffer(newSize);
_messages[channel] = queue;
Expand All @@ -182,9 +181,9 @@ class ChannelBuffers {
///
/// This should be called once a channel is prepared to handle messages
/// (i.e. when a message handler is setup in the framework).
Future<void>/*!*/ drain(String/*!*/ channel, DrainChannelCallback/*!*/ callback) async {
Future<void> drain(String channel, DrainChannelCallback callback) async {
while (!_isEmpty(channel)) {
final _StoredMessage message = _pop(channel);
final _StoredMessage message = _pop(channel)!;
await callback(message.data, message.callback);
}
}
Expand All @@ -204,7 +203,7 @@ class ChannelBuffers {
/// Arity: 2
/// Format: `resize\r<channel name>\r<new size>`
/// Description: Allows you to set the size of a channel's buffer.
void handleMessage(ByteData/*!*/ data) {
void handleMessage(ByteData data) {
final List<String> command = _getString(data).split('\r');
if (command.length == /*arity=*/2 + 1 && command[0] == 'resize') {
_resize(command[1], int.parse(command[2]));
Expand All @@ -220,4 +219,4 @@ class ChannelBuffers {
///
/// See also:
/// * [BinaryMessenger] - The place where ChannelBuffers are typically read.
final ChannelBuffers/*!*/ channelBuffers = ChannelBuffers();
final ChannelBuffers channelBuffers = ChannelBuffers();
Loading