Google::Protobuf DSL generation broken again

[larouxn]

In the latest release of protobuf i.e. v23.3, there's a Map specific bugfix for Ruby which seems to have broken DSL generation again. Again as v23 also broke generation, fixed in #1497.

This appears to be the change, see below.

bool upb_Map_Delete(upb_Map* map, upb_MessageValue key, upb_MessageValue* val) {
  upb_value v;
- const bool ok = _upb_Map_Delete(map, &key, map->key_size, &v);
- if (val) val->uint64_val = v.val;
- return ok;
+ const bool removed = _upb_Map_Delete(map, &key, map->key_size, &v);
+ if (val) _upb_map_fromvalue(v, val, map->val_size);
+ return removed;
}

And this is what is happening typecheck wise on a google-big-table bump which includes v23.3 prorobuf.

bin/typecheck
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/create_instance_request.rbi:10: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    10 |      clusters: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[])),
                                                                       ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/create_instance_request.rbi:10: Wrong number of type parameters for Array. Expected: 1, got: 0 https://srb.help/7010
    10 |      clusters: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[])),
                                                                                   ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/create_instance_request.rbi:30: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    30 |  sig { returns(Google::Protobuf::RepeatedField[]) }
                                                       ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/create_instance_request.rbi:33: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    33 |  sig { params(value: Google::Protobuf::RepeatedField[]).void }
                                                             ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/table.rbi:11: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    11 |      cluster_states: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[])),
                                                                             ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/table.rbi:11: Wrong number of type parameters for Array. Expected: 1, got: 0 https://srb.help/7010
    11 |      cluster_states: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[])),
                                                                                         ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/table.rbi:12: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    12 |      column_families: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[])),
                                                                              ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/table.rbi:12: Wrong number of type parameters for Array. Expected: 1, got: 0 https://srb.help/7010
    12 |      column_families: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[])),
                                                                                          ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/table.rbi:48: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    48 |  sig { returns(Google::Protobuf::RepeatedField[]) }
                                                       ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/table.rbi:51: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    51 |  sig { params(value: Google::Protobuf::RepeatedField[]).void }
                                                             ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/table.rbi:54: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    54 |  sig { returns(Google::Protobuf::RepeatedField[]) }
                                                       ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/table.rbi:57: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    57 |  sig { params(value: Google::Protobuf::RepeatedField[]).void }
                                                             ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/instance.rbi:12: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    12 |      labels: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[])),
                                                                     ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/instance.rbi:12: Wrong number of type parameters for Array. Expected: 1, got: 0 https://srb.help/7010
    12 |      labels: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[])),
                                                                                 ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/instance.rbi:57: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    57 |  sig { returns(Google::Protobuf::RepeatedField[]) }
                                                       ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/instance.rbi:60: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    60 |  sig { params(value: Google::Protobuf::RepeatedField[]).void }
                                                             ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/create_cluster_metadata.rbi:13: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    13 |      tables: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[]))
                                                                     ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/create_cluster_metadata.rbi:13: Wrong number of type parameters for Array. Expected: 1, got: 0 https://srb.help/7010
    13 |      tables: T.nilable(T.any(Google::Protobuf::RepeatedField[], T::Array[]))
                                                                                 ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/create_cluster_metadata.rbi:48: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    48 |  sig { returns(Google::Protobuf::RepeatedField[]) }
                                                       ^^
 
sorbet/rbi/dsl/google/cloud/bigtable/admin/v2/create_cluster_metadata.rbi:51: Wrong number of type parameters for Google::Protobuf::RepeatedField. Expected: 1, got: 0 https://srb.help/7010
    51 |  sig { params(value: Google::Protobuf::RepeatedField[]).void }
                                                             ^^
Errors: 20

Resources

• Fix DSL generation for Google::Protobuf::Struct #1497
• Updated upb dep to the latest 23.x commit. protocolbuffers/protobuf#13043
• https://github.com/protocolbuffers/protobuf/releases/tag/v23.3

[paracycle]

I doubt that the linked change is responsible for this since the change is for Map but the errors are for RepeatedField and T::Array element types.

[dirceu]

The previous fix we sent is incomplete: the only problem we could reliably reproduce then was an issue with the fields descriptor, and updating the "this is a map" condition to accept FieldsEntry solved that specific problem.

The latest google-protobuf patch update changed the value of descriptor.submsg_name for some descriptors like column_families:

• In protobuf 23.2, descriptor.submsg_name is google.bigtable.admin.v2.Table_MapEntry_column_families
• In protobuf 23.3, descriptor.submsg_name is google.bigtable.admin.v2.Table.ColumnFamiliesEntry

This means that the "this is a map" condition will no longer catch them. Updating the condition to match descriptor.submsg_name.to_s.end_with?("#{descriptor.name.split('_').map(&:capitalize).join}Entry") fixes all the errors for the application used to report this bug.

It doesn't look like there are other fields we get that would reliably tell us wether the descriptor is a map, unfortunately.

Interestingly, that map change seems to be the only actual change in the Ruby implementation of protobuf: protocolbuffers/protobuf@v23.2...v23.3. @bitwise-aiden and I tried updating google-cloud-bigtable, google-protobuf, and google-cloud-bigtable-admin-v2 in isolation, and google-protobuf is indeed the culprit here.

[dirceu]

@bitwise-aiden will try and see if there's a better fix, otherwise we'll go with the <ConstantName>Entry match.

[paracycle]

I have a better fix for this, without any name matching

[paracycle]

The actual change that was making this fail was googleapis/google-cloud-ruby@60c6c7f#diff-4fca4ecc3fd8492291b00da2a60b0bd8d6dac67697c6dce3664bf881ebc2ece3

In a nutshell, protoc has started generating Ruby files that consume the binary protocol buffer descriptor, which serializes a map<String, Progress> type as a ProgressTable message with key and value fields of type String and Progress, respectively. This, in turn, means that the internal naming of google-protobuf for map fields isn't even run, which put us in this state.

[larouxn]

Reopening as the fix was reverted.

• Revert: Simplify check for Google::Protobuf::Map type #1539