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.

Sign up for GitHub

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

Jump to bottom

Support nullable types #790

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support nullable types #790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aa2c83a
Adds the `nullable` option which allows to use nullable types in addi…
chitochi Mar 20, 2024
f6eb0d6
Improved documentation of the two new methods.
chitochi Mar 20, 2024
f9b9106
Replaces isNullable with isOptional.
chitochi Mar 20, 2024
889182d
fixes
kevmoo Jul 9, 2023
d0f4e55
add test for `optional int32`
craiglabenz Jul 21, 2023
aa7210e
Build protos using the nullable option for testing.
chitochi Mar 20, 2024
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
15 changes: 15 additions & 0 deletions protobuf/lib/meta.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const GeneratedMessage_reservedNames = <String>[
'runtimeType',
'setExtension',
'setField',
'setFieldNullable',
'toBuilder',
'toDebugString',
'toProto3Json',
Expand All @@ -59,26 +60,40 @@ const GeneratedMessage_reservedNames = <String>[
r'$_clearField',
r'$_ensure',
r'$_get',
r'$_getNullable',
r'$_getI64',
r'$_getI64Nullable',
r'$_getList',
r'$_getMap',
r'$_getN',
r'$_getB',
r'$_getBF',
r'$_getBNullable',
r'$_getI',
r'$_getIZ',
r'$_getINullable',
r'$_getS',
r'$_getSZ',
r'$_getSNullable',
r'$_has',
r'$_setBool',
r'$_setBoolNullable',
r'$_setBytes',
r'$_setBytesNullable',
r'$_setDouble',
r'$_setDoubleNullable',
r'$_setField',
r'$_setFieldNullable',
r'$_setFloat',
r'$_setFloatNullable',
r'$_setInt64',
r'$_setInt64Nullable',
r'$_setSignedInt32',
r'$_setSignedInt32Nullable',
r'$_setString',
r'$_setStringNullable',
r'$_setUnsignedInt32',
r'$_setUnsignedInt32Nullable',
r'$_whichOneof',
];

Expand Down
60 changes: 60 additions & 0 deletions protobuf/lib/src/protobuf/field_set.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,40 @@ class _FieldSet {
_setNonExtensionFieldUnchecked(meta, fi, value);
}

/// Sets a non-repeated field with error-checking.
/// This method behaves like [_setField], except if `null` is passed as
/// value. In this case, [_clearField] will be called.
///
/// Works for both extended and non-extended fields.
/// Suitable for public API.
void _setFieldNullable(int tagNumber, Object? value) {
final meta = _meta;
final fi = _nonExtensionInfo(meta, tagNumber);
if (fi == null) {
final extensions = _extensions;
if (extensions == null) {
throw ArgumentError('tag $tagNumber not defined in $_messageName');
}
if (value == null) {
_clearField(tagNumber);
return;
}
extensions._setField(tagNumber, value);
return;
}

if (fi.isRepeated) {
throw ArgumentError(_setFieldFailedMessage(
fi, value, 'repeating field (use get + .add())'));
}
if (value == null) {
_clearField(tagNumber);
Copy link
Member

Choose a reason for hiding this comment

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

This will remove required fields from the field set, right? I think that's probably OK (removing required fields is already allowed with clearField), but the documentation says "sets a non-repeated nullable field", and my understanding is only optional fields are nullable.

I think we need two documentation, one for this member with implementation details, one for the GeneratedMessage.setFieldNullable with how the member is supposed to be used. This should say it will just clear the field (same as _clearField). GeneratedMessage.setFieldNullable should clarify what happens when you pass null.

Copy link
Author

Choose a reason for hiding this comment

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

Indeed, I hadn't noticed that!
I'll add some documentation.

Copy link
Author

Choose a reason for hiding this comment

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

I just pushed the changes in the documention, I'm not sure if it's right. 😄

return;
}
_validateField(fi, value);
_setNonExtensionFieldUnchecked(meta, fi, value);
}

/// Sets a non-repeated field without validating it.
///
/// Works for both extended and non-extended fields.
Expand Down Expand Up @@ -421,6 +455,9 @@ class _FieldSet {
/// `false`.
bool _$getBF(int index) => _values[index] ?? false;

/// The implementation of a generated getter for nullable `bool` fields.
bool? _$getBNullable(int index) => _values[index];

/// The implementation of a generated getter for int fields.
int _$getI(int index, int? defaultValue) {
var value = _values[index];
Expand All @@ -435,6 +472,9 @@ class _FieldSet {
/// fixed32, sfixed32) that default to `0`.
int _$getIZ(int index) => _values[index] ?? 0;

/// The implementation of a generated getter for nullable int fields.
int? _$getINullable(int index) => _values[index];

/// The implementation of a generated getter for String fields.
String _$getS(int index, String? defaultValue) {
var value = _values[index];
Expand All @@ -449,13 +489,19 @@ class _FieldSet {
/// the empty string.
String _$getSZ(int index) => _values[index] ?? '';

/// The implementation of a generated getter for nullable String fields.
String? _$getSNullable(int index) => _values[index];

/// The implementation of a generated getter for Int64 fields.
Int64 _$getI64(int index) {
var value = _values[index];
value ??= _getDefault(_nonExtensionInfoByIndex(index));
return value;
}

/// The implementation of a generated getter for nullable Int64 fields.
Int64? _$getI64Nullable(int index) => _values[index];

/// The implementation of a generated 'has' method.
bool _$has(int index) {
final value = _values[index];
Expand Down Expand Up @@ -490,11 +536,25 @@ class _FieldSet {
_values[index] = value;
}

void _$setNullable(int index, Object? value) {
assert(!_nonExtensionInfoByIndex(index).isRepeated);
assert(value == null || _$check(index, value));
if (value == null) {
_clearField(_meta.byIndex[index].tagNumber);
return;
}

_$set(index, value);
}

bool _$check(int index, var newValue) {
_validateField(_nonExtensionInfoByIndex(index), newValue);
return true; // Allows use in an assertion.
}

/// The implementation of a generated nullable getter.
T _$getNullable<T>(int index) => _values[index];

// Bulk operations reading or writing multiple fields

void _clear() {
Expand Down
89 changes: 89 additions & 0 deletions protobuf/lib/src/protobuf/generated_message.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,17 @@ abstract class GeneratedMessage {
_fieldSet._setField(tagNumber, value);
}

/// Sets the value of a field by its [tagNumber].
/// This method should be used for optional fields when the nullable option
/// is set. When `null` is passed as value, the field is cleared.
///
/// Throws an [ArgumentError] if [value] does not match the type associated
/// with [tagNumber].
@pragma('dart2js:noInline')
void setFieldNullable(int tagNumber, Object? value) {
_fieldSet._setFieldNullable(tagNumber, value);
}

/// For generated code only.
/// @nodoc
T $_get<T>(int index, T defaultValue) =>
Expand All @@ -426,6 +437,10 @@ abstract class GeneratedMessage {
/// @nodoc
T $_getN<T>(int index) => _fieldSet._$getND(index);

/// For generated code only.
/// @nodoc
T $_getNullable<T>(int index) => _fieldSet._$getNullable<T>(index);

/// For generated code only.
/// @nodoc
T $_ensure<T>(int index) => _fieldSet._$ensure<T>(index);
Expand All @@ -448,6 +463,10 @@ abstract class GeneratedMessage {
/// @nodoc
bool $_getBF(int index) => _fieldSet._$getBF(index);

/// For generated code only.
/// @nodoc
bool? $_getBNullable(int index) => _fieldSet._$getBNullable(index);

/// For generated code only.
/// @nodoc
int $_getI(int index, int defaultValue) =>
Expand All @@ -457,6 +476,10 @@ abstract class GeneratedMessage {
/// @nodoc
int $_getIZ(int index) => _fieldSet._$getIZ(index);

/// For generated code only.
/// @nodoc
int? $_getINullable(int index) => _fieldSet._$getINullable(index);

/// For generated code only.
/// @nodoc
String $_getS(int index, String defaultValue) =>
Expand All @@ -466,10 +489,18 @@ abstract class GeneratedMessage {
/// @nodoc
String $_getSZ(int index) => _fieldSet._$getSZ(index);

/// For generated code only.
/// @nodoc
String? $_getSNullable(int index) => _fieldSet._$getSNullable(index);

/// For generated code only.
/// @nodoc
Int64 $_getI64(int index) => _fieldSet._$getI64(index);

/// For generated code only.
/// @nodoc
Int64? $_getI64Nullable(int index) => _fieldSet._$getI64Nullable(index);

/// For generated code only.
/// @nodoc
bool $_has(int index) => _fieldSet._$has(index);
Expand All @@ -478,14 +509,29 @@ abstract class GeneratedMessage {
/// @nodoc
void $_setBool(int index, bool value) => _fieldSet._$set(index, value);

/// For generated code only.
/// @nodoc
void $_setBoolNullable(int index, bool? value) =>
_fieldSet._$setNullable(index, value);

/// For generated code only.
/// @nodoc
void $_setBytes(int index, List<int> value) => _fieldSet._$set(index, value);

/// For generated code only.
/// @nodoc
void $_setBytesNullable(int index, List<int>? value) =>
_fieldSet._$setNullable(index, value);

/// For generated code only.
/// @nodoc
void $_setString(int index, String value) => _fieldSet._$set(index, value);

/// For generated code only.
/// @nodoc
void $_setStringNullable(int index, String? value) =>
_fieldSet._$setNullable(index, value);

/// For generated code only.
/// @nodoc
void $_setFloat(int index, double value) {
Expand All @@ -496,10 +542,24 @@ abstract class GeneratedMessage {
_fieldSet._$set(index, value);
}

/// For generated code only.
/// @nodoc
void $_setFloatNullable(int index, double? value) {
if (value != null && !_isFloat32(value)) {
_fieldSet._$check(index, value);
}
_fieldSet._$setNullable(index, value);
}

/// For generated code only.
/// @nodoc
void $_setDouble(int index, double value) => _fieldSet._$set(index, value);

/// For generated code only.
/// @nodoc
void $_setDoubleNullable(int index, double? value) =>
_fieldSet._$setNullable(index, value);

/// For generated code only.
/// @nodoc
void $_setSignedInt32(int index, int value) {
Expand All @@ -510,6 +570,15 @@ abstract class GeneratedMessage {
_fieldSet._$set(index, value);
}

/// For generated code only.
/// @nodoc
void $_setSignedInt32Nullable(int index, int? value) {
if (value != null && !_isSigned32(value)) {
_fieldSet._$check(index, value);
}
_fieldSet._$setNullable(index, value);
}

/// For generated code only.
/// @nodoc
void $_setUnsignedInt32(int index, int value) {
Expand All @@ -520,16 +589,36 @@ abstract class GeneratedMessage {
_fieldSet._$set(index, value);
}

/// For generated code only.
/// @nodoc
void $_setUnsignedInt32Nullable(int index, int? value) {
if (value != null && !_isUnsigned32(value)) {
_fieldSet._$check(index, value);
}
_fieldSet._$setNullable(index, value);
}

/// For generated code only.
/// @nodoc
void $_setInt64(int index, Int64 value) => _fieldSet._$set(index, value);

/// For generated code only.
/// @nodoc
void $_setInt64Nullable(int index, Int64? value) =>
_fieldSet._$setNullable(index, value);

/// For generated code only. Separate from [setField] to distinguish
/// reflective accesses.
/// @nodoc
void $_setField(int tagNumber, Object value) =>
_fieldSet._setField(tagNumber, value);

/// For generated code only. Separate from [setFieldNullable] to distinguish
/// reflective accesses.
/// @nodoc
void $_setFieldNullable(int tagNumber, Object? value) =>
_fieldSet._setFieldNullable(tagNumber, value);

/// For generated code only. Separate from [clearField] to distinguish
/// reflective accesses.
/// @nodoc
Expand Down
13 changes: 13 additions & 0 deletions protoc_plugin/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ TEST_PROTO_LIST = \
google/protobuf/timestamp \
google/protobuf/type \
google/protobuf/unittest_import \
google/protobuf/unittest_import_proto3 \
google/protobuf/unittest_import_public_proto3 \
google/protobuf/unittest_optimize_for \
google/protobuf/unittest_well_known_types \
google/protobuf/unittest \
google/protobuf/unittest_proto3 \
google/protobuf/wrappers \
custom_option \
dart_name \
Expand Down Expand Up @@ -99,6 +102,16 @@ $(TEST_PROTO_LIBS): $(PLUGIN_SRC) $(TEST_PROTO_SRCS)
--plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH))\
$(TEST_PROTO_SRCS)

mkdir -p $(TEST_PROTO_DIR)/nullable

protoc\
--experimental_allow_proto3_optional\
--dart_out="nullable:$(TEST_PROTO_DIR)/nullable"\
-Iprotos\
-I$(TEST_PROTO_SRC_DIR)\
--plugin=protoc-gen-dart=$(realpath $(PLUGIN_PATH))\
$(TEST_PROTO_SRCS)

dart format $(TEST_PROTO_DIR)

update-pregenerated: $(PLUGIN_PATH) $(PREGENERATED_SRCS)
Expand Down
10 changes: 8 additions & 2 deletions protoc_plugin/lib/src/file_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,14 @@ class FileGenerator extends ProtobufContainer {
descriptor.enumType[i], this, usedTopLevelNames, i));
}
for (var i = 0; i < descriptor.messageType.length; i++) {
messageGenerators.add(MessageGenerator.topLevel(descriptor.messageType[i],
this, declaredMixins, defaultMixin, usedTopLevelNames, i));
messageGenerators.add(MessageGenerator.topLevel(
descriptor.messageType[i],
this,
declaredMixins,
defaultMixin,
usedTopLevelNames,
i,
options.useNullable));
}
for (var i = 0; i < descriptor.extension.length; i++) {
extensionGenerators.add(ExtensionGenerator.topLevel(
Expand Down
Loading