Remove an old TODO, minor refactoring (#744)

- Remove an old TODO, clarify the code

- Avoid an explicit cast in `GeneratedMessage.mergeFromJson`

- Update reviver argument type in `jsonDecode` calls to avoid any
  potential runtime type checks

- Remove a dead code

- Simplify oneof handling

Author: osa1

protobuf/lib/src/protobuf/builder_info.dart

@@ -27,8 +27,8 @@ class BuilderInfo {
   /// Mapping from [FieldInfo.name]s to [FieldInfo]s.
   final Map<String, FieldInfo> byName = <String, FieldInfo>{};
 
-  /// Mapping from [FieldInfo.tagNumber]s to the corresponding `oneof` indices
-  /// (if any).
+  /// Mapping from `oneof` field [FieldInfo.tagNumber]s to the their indices in
+  /// [_FieldSet._oneofCases].
   final Map<int, int> oneofs = <int, int>{};
 
   /// Whether the message has extension fields.
@@ -301,12 +301,12 @@ class BuilderInfo {
         : qualifiedMessageName.substring(lastDot + 1);
   }
 
-  List<FieldInfo> _computeSortedByTag() {
-    // TODO(skybrian): perhaps the code generator should insert the FieldInfos
-    // in tag number order, to avoid sorting them?
-    return List<FieldInfo>.from(fieldInfo.values, growable: false)
-      ..sort((FieldInfo a, FieldInfo b) => a.tagNumber.compareTo(b.tagNumber));
-  }
+  List<FieldInfo> _computeSortedByTag() =>
+      // Code generator inserts fields in tag order, but it's possible for
+      // user-written code to insert unordered.
+      List<FieldInfo>.from(fieldInfo.values, growable: false)
+        ..sort(
+            (FieldInfo a, FieldInfo b) => a.tagNumber.compareTo(b.tagNumber));
 
   GeneratedMessage _makeEmptyMessage(
       int tagNumber, ExtensionRegistry? extensionRegistry) {

protobuf/lib/src/protobuf/extension_field_set.dart

@@ -12,7 +12,7 @@ class _ExtensionFieldSet {
 
   _ExtensionFieldSet(this._parent);
 
-  Extension? _getInfoOrNull(int? tagNumber) => _info[tagNumber];
+  Extension? _getInfoOrNull(int tagNumber) => _info[tagNumber];
 
   dynamic _getFieldOrDefault(Extension fi) {
     if (fi.isRepeated) return _getList(fi);

protobuf/lib/src/protobuf/field_set.dart

@@ -70,8 +70,8 @@ class _FieldSet {
   int? get _memoizedHashCode =>
       _frozenState is int ? _frozenState as int : null;
 
-  // Maps a oneof decl index to the tag number which is currently set. If the
-  // index is not present, the oneof field is unset.
+  /// Maps a `oneof` field index to the tag number which is currently set. If
+  /// the index is not present, the oneof field is unset.
   final Map<int, int>? _oneofCases;
 
   _FieldSet(this._message, BuilderInfo meta, this._eventPlugin)
@@ -217,24 +217,26 @@ class _FieldSet {
     return _extensions?._hasField(tagNumber) ?? false;
   }
 
-  void _clearField(int? tagNumber) {
+  void _clearField(int tagNumber) {
     _ensureWritable();
     final meta = _meta;
-    var fi = _nonExtensionInfo(meta, tagNumber);
+    final fi = _nonExtensionInfo(meta, tagNumber);
+
     if (fi != null) {
-      // clear a non-extension field
+      assert(tagNumber == fi.tagNumber);
+
+      // Clear a non-extension field
       final eventPlugin = _eventPlugin;
       if (eventPlugin != null && eventPlugin.hasObservers) {
         eventPlugin.beforeClearField(fi);
       }
       _values[fi.index!] = null;
 
-      if (meta.oneofs.containsKey(fi.tagNumber)) {
-        _oneofCases!.remove(meta.oneofs[fi.tagNumber]);
+      final oneofIndex = meta.oneofs[tagNumber];
+      if (oneofIndex != null) {
+        _oneofCases![oneofIndex] = 0;
       }
 
-      var oneofIndex = meta.oneofs[fi.tagNumber];
-      if (oneofIndex != null) _oneofCases![oneofIndex] = 0;
       return;
     }
 
@@ -328,10 +330,13 @@ class _FieldSet {
 
   /// Sets a non-extended field and fires events.
   void _setNonExtensionFieldUnchecked(BuilderInfo meta, FieldInfo fi, value) {
-    var tag = fi.tagNumber;
-    var oneofIndex = meta.oneofs[tag];
+    final tag = fi.tagNumber;
+    final oneofIndex = meta.oneofs[tag];
     if (oneofIndex != null) {
-      _clearField(_oneofCases![oneofIndex]);
+      final currentOneofTag = _oneofCases![oneofIndex];
+      if (currentOneofTag != null) {
+        _clearField(currentOneofTag);
+      }
       _oneofCases![oneofIndex] = tag;
     }
 
@@ -488,7 +493,10 @@ class _FieldSet {
     var oneofIndex = meta.oneofs[tag];
 
     if (oneofIndex != null) {
-      _clearField(_oneofCases![oneofIndex]);
+      final currentOneofTag = _oneofCases![oneofIndex];
+      if (currentOneofTag != null) {
+        _clearField(currentOneofTag);
+      }
       _oneofCases![oneofIndex] = tag;
     }
     _values[index] = value;

protobuf/lib/src/protobuf/generated_message.dart

@@ -279,12 +279,12 @@ abstract class GeneratedMessage {
     /// This is a slight regression on the Dart VM.
     /// TODO(skybrian) we could skip the reviver if we're running
     /// on the Dart VM for a slight speedup.
-    final jsonMap =
-        jsonDecode(data, reviver: _emptyReviver) as Map<String, dynamic>;
+    final Map<String, dynamic> jsonMap =
+        jsonDecode(data, reviver: _emptyReviver);
     _mergeFromJsonMap(_fieldSet, jsonMap, extensionRegistry);
   }
 
-  static dynamic _emptyReviver(k, v) => v;
+  static Object? _emptyReviver(Object? k, Object? v) => v;
 
   /// Merges field values from a JSON object represented as a Dart map.
   ///

protobuf/lib/src/protobuf/proto3_json.dart

@@ -297,8 +297,6 @@ void _mergeFromProto3Json(
               throw context.parseException(
                   'Wrong boolean key, should be one of ("true", "false")', key);
           }
-          // ignore: dead_code
-          throw StateError('(Should have been) unreachable statement');
         case PbFieldType._STRING_BIT:
           return key;
         case PbFieldType._UINT64_BIT: