Improve enum decoding -- alternative

protobuf/lib/src/protobuf/protobuf_enum.dart

@@ -2,6 +2,8 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+// ignore_for_file: non_constant_identifier_names
+
 part of '../../protobuf.dart';
 
 /// A base class for all proto enum types.
@@ -37,12 +39,28 @@ class ProtobufEnum {
   /// Creates a new constant [ProtobufEnum] using [value] and [name].
   const ProtobufEnum(this.value, this.name);
 
-  /// Creates a Map for all of the [ProtobufEnum]s in [byIndex], mapping each
+  /// This function is for generated code.
+  ///
+  /// Creates a Map for all of the [ProtobufEnum]s in [enumValues], mapping each
   /// [ProtobufEnum]'s [value] to the [ProtobufEnum].
-  static Map<int, T> initByValue<T extends ProtobufEnum>(List<T> byIndex) {
+  ///
+  /// @nodoc
+  static Map<int, T> initByValue<T extends ProtobufEnum>(List<T> enumValues) {
     final byValue = <int, T>{};
-    for (final v in byIndex) {
-      byValue[v.value] = v;
+    for (final enumValue in enumValues) {
+      byValue[enumValue.value] = enumValue;
+    }
+    return byValue;
+  }
+
+  /// This function is for generated code.
+  ///
+  /// @nodoc
+  static List<T?> $_initByValueList<T extends ProtobufEnum>(
+      List<T> enumValues, int maxEnumValue) {
+    final byValue = List<T?>.filled(maxEnumValue + 1, null);
+    for (final enumValue in enumValues) {
+      byValue[enumValue.value] = enumValue;
     }
     return byValue;
   }

protoc_plugin/Makefile

@@ -30,6 +30,7 @@ TEST_PROTO_LIST = \
 	entity \
 	enum_extension \
 	enum_name \
+	enums \
 	extend_unittest \
 	ExtensionEnumNameConflict \
 	ExtensionNameConflict \

protoc_plugin/lib/src/enum_generator.dart

@@ -175,11 +175,37 @@ class EnumGenerator extends ProtobufContainer {
       out.println('];');
       out.println();
 
-      out.println(
-          'static final $coreImportPrefix.Map<$coreImportPrefix.int, $classname> _byValue ='
-          ' $protobufImportPrefix.ProtobufEnum.initByValue(values);');
-      out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
-          ' _byValue[value];');
+      var maxEnumValue = -1;
+      for (final valueDescriptor in _canonicalValues) {
+        if (valueDescriptor.number.isNegative) {
+          maxEnumValue = -1; // don't use list
+          break;
+        }
+        if (valueDescriptor.number > maxEnumValue) {
+          maxEnumValue = valueDescriptor.number;
+        }
+      }
+
+      final useList = _canonicalValues.isEmpty ||
+          (maxEnumValue >= 0 &&
+              _canonicalValues.length / (maxEnumValue + 1) >= 0.7);
+
+      if (useList) {
+        out.println(
+            'static final $coreImportPrefix.List<$classname?> _byValue ='
+            ' $protobufImportPrefix.ProtobufEnum.\$_initByValueList(values, $maxEnumValue);');
+
+        out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
+            '  value < 0 || value >= _byValue.length ? null : _byValue[value];');
+      } else {
+        out.println(
+            'static final $coreImportPrefix.Map<$coreImportPrefix.int, $classname> _byValue ='
+            ' $protobufImportPrefix.ProtobufEnum.initByValue(values);');
+
+        out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
+            ' _byValue[value];');
+      }
+
       out.println();
 
       out.println('const $classname._(super.v, super.n);');

protoc_plugin/test/generated_message_test.dart

@@ -10,6 +10,7 @@ import '../out/protos/constructor_args/google/protobuf/unittest.pb.dart'
 import '../out/protos/constructor_args/google/protobuf/unittest_import.pb.dart'
     as constructor_args_unittest_import;
 import '../out/protos/duplicate_names_import.pb.dart';
+import '../out/protos/enums.pb.dart';
 import '../out/protos/google/protobuf/unittest.pb.dart';
 import '../out/protos/google/protobuf/unittest_import.pb.dart';
 import '../out/protos/google/protobuf/unittest_optimize_for.pb.dart';
@@ -890,4 +891,27 @@ void main() {
     // constructor arguments, to be able to reuse `assertAllFieldsSet`.
     assertAllFieldsSet(TestAllTypes.fromBuffer(value.writeToBuffer()));
   });
+
+  test('Handling enums defined out of order', () {
+    final message = MessageWithEnums();
+    for (final enum_ in DenseEnum.values) {
+      message.denseEnums.add(enum_);
+    }
+    for (final enum_ in DenseEnumOutOfOrder.values) {
+      message.denseOutOfOrderEnums.add(enum_);
+    }
+    for (final enum_ in SparseEnum.values) {
+      message.sparseEnums.add(enum_);
+    }
+    for (final enum_ in SparseEnumOutOfOrder.values) {
+      message.sparseOutOfOrderEnums.add(enum_);
+    }
+
+    final encoded = message.writeToBuffer();
+    final decoded = MessageWithEnums.fromBuffer(encoded);
+    expect(decoded.denseEnums, DenseEnum.values);
+    expect(decoded.denseOutOfOrderEnums, DenseEnumOutOfOrder.values);
+    expect(decoded.sparseEnums, SparseEnum.values);
+    expect(decoded.sparseOutOfOrderEnums, SparseEnumOutOfOrder.values);
+  });
 }

protoc_plugin/test/goldens/deprecations.pbenum

@@ -24,9 +24,10 @@ class A extends $pb.ProtobufEnum {
     A2,
   ];
 
-  static final $core.Map<$core.int, A> _byValue =
-      $pb.ProtobufEnum.initByValue(values);
-  static A? valueOf($core.int value) => _byValue[value];
+  static final $core.List<A?> _byValue =
+      $pb.ProtobufEnum.$_initByValueList(values, 1);
+  static A? valueOf($core.int value) =>
+      value < 0 || value >= _byValue.length ? null : _byValue[value];
 
   const A._(super.v, super.n);
 }

protoc_plugin/test/goldens/doc_comments.pbenum

@@ -26,9 +26,10 @@ class A extends $pb.ProtobufEnum {
     A2,
   ];
 
-  static final $core.Map<$core.int, A> _byValue =
-      $pb.ProtobufEnum.initByValue(values);
-  static A? valueOf($core.int value) => _byValue[value];
+  static final $core.List<A?> _byValue =
+      $pb.ProtobufEnum.$_initByValueList(values, 1);
+  static A? valueOf($core.int value) =>
+      value < 0 || value >= _byValue.length ? null : _byValue[value];
 
   const A._(super.v, super.n);
 }

protoc_plugin/test/goldens/enum

@@ -11,8 +11,8 @@ class PhoneType extends $pb.ProtobufEnum {
     WORK,
   ];
 
-  static final $core.Map<$core.int, PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
-  static PhoneType? valueOf($core.int value) => _byValue[value];
+  static final $core.List<PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
+  static PhoneType? valueOf($core.int value) =>  value < 0 || value >= _byValue.length ? null : _byValue[value];
 
   const PhoneType._(super.v, super.n);
 }

protoc_plugin/test/goldens/messageGeneratorEnums

@@ -11,8 +11,8 @@ class PhoneNumber_PhoneType extends $pb.ProtobufEnum {
     WORK,
   ];
 
-  static final $core.Map<$core.int, PhoneNumber_PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
-  static PhoneNumber_PhoneType? valueOf($core.int value) => _byValue[value];
+  static final $core.List<PhoneNumber_PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
+  static PhoneNumber_PhoneType? valueOf($core.int value) =>  value < 0 || value >= _byValue.length ? null : _byValue[value];
 
   const PhoneNumber_PhoneType._(super.v, super.n);
 }

protoc_plugin/test/goldens/topLevelEnum.pbenum

@@ -26,8 +26,8 @@ class PhoneType extends $pb.ProtobufEnum {
     WORK,
   ];
 
-  static final $core.Map<$core.int, PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
-  static PhoneType? valueOf($core.int value) => _byValue[value];
+  static final $core.List<PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
+  static PhoneType? valueOf($core.int value) =>  value < 0 || value >= _byValue.length ? null : _byValue[value];
 
   const PhoneType._(super.v, super.n);
 }

protoc_plugin/test/protos/enums.proto

@@ -0,0 +1,54 @@
+enum DenseEnum {
+    DENSE_ENUM_0 = 0;
+    DENSE_ENUM_1 = 1;
+    DENSE_ENUM_2 = 2;
+    DENSE_ENUM_3 = 3;
+    DENSE_ENUM_4 = 4;
+}
+
+enum DenseEnumOutOfOrder {
+    DENSE_ENUM_OOO_0 = 0;
+    DENSE_ENUM_OOO_2 = 2;
+    DENSE_ENUM_OOO_4 = 4;
+    DENSE_ENUM_OOO_3 = 3;
+    DENSE_ENUM_OOO_1 = 1;
+}
+
+enum SparseEnum {
+    SPARSE_ENUM_ZERO = 0;
+    SPARSE_ENUM_MIN_INT = -2147483648;
+    SPARSE_ENUM_1 = -1000000000;
+    SPARSE_ENUM_2 = -100000000;
+    SPARSE_ENUM_3 = -10000000;
+    SPARSE_ENUM_4 = -1000000;
+    SPARSE_ENUM_5 = -100000;
+    SPARSE_ENUM_6 = 100000;
+    SPARSE_ENUM_7 = 1000000;
+    SPARSE_ENUM_8 = 10000000;
+    SPARSE_ENUM_9 = 100000000;
+    SPARSE_ENUM_10 = 1000000000;
+    SPARSE_ENUM_MAX_INT = 2147483647;
+}
+
+enum SparseEnumOutOfOrder {
+    SPARSE_ENUM_OOO_ZERO = 0;
+    SPARSE_ENUM_OOO_1 = -1000000000;
+    SPARSE_ENUM_OOO_MAX_INT = 2147483647;
+    SPARSE_ENUM_OOO_4 = -1000000;
+    SPARSE_ENUM_OOO_6 = 100000;
+    SPARSE_ENUM_OOO_MIN_INT = -2147483648;
+    SPARSE_ENUM_OOO_7 = 1000000;
+    SPARSE_ENUM_OOO_5 = -100000;
+    SPARSE_ENUM_OOO_8 = 10000000;
+    SPARSE_ENUM_OOO_3 = -10000000;
+    SPARSE_ENUM_OOO_10 = 1000000000;
+    SPARSE_ENUM_OOO_2 = -100000000;
+    SPARSE_ENUM_OOO_9 = 100000000;
+}
+
+message MessageWithEnums {
+    repeated DenseEnum denseEnums = 1;
+    repeated DenseEnumOutOfOrder denseOutOfOrderEnums = 2;
+    repeated SparseEnum sparseEnums = 3;
+    repeated SparseEnumOutOfOrder sparseOutOfOrderEnums = 4;
+}