Make protobuf-ts as conformant as protobuf-es (#567)

timostamm · Jul 31, 2023 · 7cfd1a5 · 7cfd1a5
1 parent 2f0ed59
commit 7cfd1a5
Show file tree

Hide file tree

Showing 30 changed files with 2,158 additions and 163 deletions.
diff --git a/packages/plugin/src/message-type-extensions/well-known-types.ts b/packages/plugin/src/message-type-extensions/well-known-types.ts
@@ -303,6 +303,8 @@ export class WellKnownTypes implements CustomMethodGenerator {
                 if (s > 315576000000 || s < -315576000000)
                     throw new Error("Duration value out of range.");
                 let text = message.seconds.toString();
+                if (s === 0 && message.nanos < 0)
+                    text = "-" + text;
                 if (message.nanos !== 0) {
                     let nanosStr = Math.abs(message.nanos).toString();
                     nanosStr = "0".repeat(9 - nanosStr.length) + nanosStr;
@@ -321,21 +323,19 @@ export class WellKnownTypes implements CustomMethodGenerator {
             function internalJsonRead(json: ${JsonValue}, options: ${JsonReadOptions}, target?: ${Duration}): ${Duration} {
                 if (typeof json !== "string")
                     throw new Error("Unable to parse Duration from JSON " + ${typeofJsonValue}(json) + ". Expected string.");
-                let match = json.match(/^(-?[0-9]+)(?:\\.([0-9]+))?s/);
+                let match = json.match(/^(-?)([0-9]+)(?:\\.([0-9]+))?s/);
                 if (match === null)
                     throw new Error("Unable to parse Duration from JSON string. Invalid format.");
                 if (!target)
                     target = this.create();
-                let longSeconds = ${PbLong}.from(match[1]);
+                let [, sign, secs, nanos] = match;
+                let longSeconds = ${PbLong}.from(sign + secs);
                 if (longSeconds.toNumber() > 315576000000 || longSeconds.toNumber() < -315576000000)
                     throw new Error("Unable to parse Duration from JSON string. Value out of range.");
                 target.seconds = longSeconds.${longConvertMethod}();
-                if (typeof match[2] == "string") {
-                    let nanosStr = match[2] + "0".repeat(9 - match[2].length);
+                if (typeof nanos == "string") {
+                    let nanosStr = sign + nanos + "0".repeat(9 - nanos.length);
                     target.nanos = parseInt(nanosStr);
-                    if (longSeconds.isNegative()) {
-                        target.nanos = -target.nanos;
-                    }
                 }
                 return target;
             }
@@ -455,7 +455,9 @@ export class WellKnownTypes implements CustomMethodGenerator {
                     case "${nullValueField}":
                         return null;
                     case "${numberValueField}":
-                        return message.kind.${numberValueField};
+                        let numberValue = message.kind.${numberValueField};
+                        if (typeof numberValue == "number" && !Number.isFinite(numberValue)) throw new globalThis.Error();
+                        return numberValue;
                     case "${stringValueField}":
                         return message.kind.${stringValueField};
                     case "${listValueField}":

diff --git a/packages/runtime/src/reflection-json-reader.ts b/packages/runtime/src/reflection-json-reader.ts
@@ -78,6 +78,9 @@ export class ReflectionJsonReader {
             // handle oneof ADT
             let target: UnknownMessage | UnknownOneofGroup; // this will be the target for the field value, whether it is member of a oneof or not
             if (field.oneof) {
+                if (jsonValue === null && (field.kind !== 'enum' || field.T()[0] !== 'google.protobuf.NullValue')) {
+                    continue;
+                }
                 // since json objects are unordered by specification, it is not possible to take the last of multiple oneofs
                 if (oneofsHandled.includes(field.oneof)) throw new Error(`Multiple members of the oneof group "${field.oneof}" of ${this.info.typeName} are present in JSON.`);
                 oneofsHandled.push(field.oneof);
@@ -202,11 +205,11 @@ export class ReflectionJsonReader {
     /**
      * Returns `false` for unrecognized string representations.
      *
-     * google.protobuf.NullValue accepts only JSON `null`.
+     * google.protobuf.NullValue accepts only JSON `null` (or the old `"NULL_VALUE"`).
      */
     enum(type: EnumInfo, json: unknown, fieldName: string, ignoreUnknownFields: boolean): UnknownEnum | false {
         if (type[0] == 'google.protobuf.NullValue')
-            assert(json === null, `Unable to parse field ${this.info.typeName}#${fieldName}, enum ${type[0]} only accepts null.`);
+            assert(json === null || json === "NULL_VALUE", `Unable to parse field ${this.info.typeName}#${fieldName}, enum ${type[0]} only accepts null.`);
         if (json === null)
             // we require 0 to be default value for all enums
             return 0;

diff --git a/packages/runtime/src/reflection-json-writer.ts b/packages/runtime/src/reflection-json-writer.ts
@@ -169,11 +169,11 @@ export class ReflectionJsonWriter {
     }
 
     /**
-     * Returns `null` for google.protobuf.NullValue.
+     * Returns `null` as the default for google.protobuf.NullValue.
      */
     enum(type: EnumInfo, value: unknown, fieldName: string, optional: boolean, emitDefaultValues: boolean, enumAsInteger: boolean): JsonValue | undefined {
         if (type[0] == 'google.protobuf.NullValue')
-            return null;
+            return !emitDefaultValues && !optional ? undefined : null;
         if (value === undefined) {
             assert(optional);
             return undefined;

diff --git a/packages/test-conformance/failing_tests_optimize_code_size-long_type_bigint.txt b/packages/test-conformance/failing_tests_optimize_code_size-long_type_bigint.txt
@@ -1,16 +1 @@
 Recommended.Proto2.JsonInput.FieldNameExtension.Validator
-Recommended.Proto3.JsonInput.NullValueInNormalMessage.Validator
-Recommended.Proto3.JsonInput.NullValueInOtherOneofNewFormat.Validator
-Recommended.Proto3.JsonInput.NullValueInOtherOneofOldFormat.Validator
-Recommended.ValueRejectInfNumberValue.JsonOutput
-Recommended.ValueRejectNanNumberValue.JsonOutput
-Required.Proto3.JsonInput.DurationNegativeNanos.JsonOutput
-Required.Proto3.JsonInput.DurationNegativeNanos.ProtobufOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.JsonOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.ProtobufOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasUseAlias.JsonOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasUseAlias.ProtobufOutput
-Required.Proto3.JsonInput.OneofFieldNullFirst.JsonOutput
-Required.Proto3.JsonInput.OneofFieldNullFirst.ProtobufOutput
-Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
-Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput
diff --git a/packages/test-conformance/failing_tests_optimize_code_size-long_type_string.txt b/packages/test-conformance/failing_tests_optimize_code_size-long_type_string.txt
@@ -1,16 +1 @@
 Recommended.Proto2.JsonInput.FieldNameExtension.Validator
-Recommended.Proto3.JsonInput.NullValueInNormalMessage.Validator
-Recommended.Proto3.JsonInput.NullValueInOtherOneofNewFormat.Validator
-Recommended.Proto3.JsonInput.NullValueInOtherOneofOldFormat.Validator
-Recommended.ValueRejectInfNumberValue.JsonOutput
-Recommended.ValueRejectNanNumberValue.JsonOutput
-Required.Proto3.JsonInput.DurationNegativeNanos.JsonOutput
-Required.Proto3.JsonInput.DurationNegativeNanos.ProtobufOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.JsonOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.ProtobufOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasUseAlias.JsonOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasUseAlias.ProtobufOutput
-Required.Proto3.JsonInput.OneofFieldNullFirst.JsonOutput
-Required.Proto3.JsonInput.OneofFieldNullFirst.ProtobufOutput
-Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
-Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput
diff --git a/packages/test-conformance/failing_tests_optimize_speed-long_type_bigint.txt b/packages/test-conformance/failing_tests_optimize_speed-long_type_bigint.txt
@@ -1,16 +1 @@
 Recommended.Proto2.JsonInput.FieldNameExtension.Validator
-Recommended.Proto3.JsonInput.NullValueInNormalMessage.Validator
-Recommended.Proto3.JsonInput.NullValueInOtherOneofNewFormat.Validator
-Recommended.Proto3.JsonInput.NullValueInOtherOneofOldFormat.Validator
-Recommended.ValueRejectInfNumberValue.JsonOutput
-Recommended.ValueRejectNanNumberValue.JsonOutput
-Required.Proto3.JsonInput.DurationNegativeNanos.JsonOutput
-Required.Proto3.JsonInput.DurationNegativeNanos.ProtobufOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.JsonOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.ProtobufOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasUseAlias.JsonOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasUseAlias.ProtobufOutput
-Required.Proto3.JsonInput.OneofFieldNullFirst.JsonOutput
-Required.Proto3.JsonInput.OneofFieldNullFirst.ProtobufOutput
-Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
-Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput
diff --git a/packages/test-conformance/failing_tests_optimize_speed-long_type_string.txt b/packages/test-conformance/failing_tests_optimize_speed-long_type_string.txt
@@ -1,16 +1 @@
 Recommended.Proto2.JsonInput.FieldNameExtension.Validator
-Recommended.Proto3.JsonInput.NullValueInNormalMessage.Validator
-Recommended.Proto3.JsonInput.NullValueInOtherOneofNewFormat.Validator
-Recommended.Proto3.JsonInput.NullValueInOtherOneofOldFormat.Validator
-Recommended.ValueRejectInfNumberValue.JsonOutput
-Recommended.ValueRejectNanNumberValue.JsonOutput
-Required.Proto3.JsonInput.DurationNegativeNanos.JsonOutput
-Required.Proto3.JsonInput.DurationNegativeNanos.ProtobufOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.JsonOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.ProtobufOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasUseAlias.JsonOutput
-Required.Proto3.JsonInput.EnumFieldWithAliasUseAlias.ProtobufOutput
-Required.Proto3.JsonInput.OneofFieldNullFirst.JsonOutput
-Required.Proto3.JsonInput.OneofFieldNullFirst.ProtobufOutput
-Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
-Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput
diff --git a/packages/test-conformance/proto/conformance/conformance.proto b/packages/test-conformance/proto/conformance/conformance.proto
@@ -29,8 +29,11 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 syntax = "proto3";
+
 package conformance;
+
 option java_package = "com.google.protobuf.conformance";
+option objc_class_prefix = "Conformance";
 
 // This defines the conformance testing protocol.  This protocol exists between
 // the conformance test suite itself and the code being tested.  For each test,
@@ -55,7 +58,7 @@ enum WireFormat {
   UNSPECIFIED = 0;
   PROTOBUF = 1;
   JSON = 2;
-  JSPB = 3;  // Google internal only. Opensource testees just skip it.
+  JSPB = 3;  // Only used inside Google. Opensource testees just skip it.
   TEXT_FORMAT = 4;
 }
 
@@ -69,7 +72,8 @@ enum TestCategory {
   // https://developers.google.com/protocol-buffers/docs/proto3#json_options
   // for more detail.
   JSON_IGNORE_UNKNOWN_PARSING_TEST = 3;
-  // Test jspb wire format. Google internal only. Opensource testees just skip it.
+  // Test jspb wire format. Only used inside Google. Opensource testees just
+  // skip it.
   JSPB_TEST = 4;
   // Test text format. For cpp, java and python, testees can already deal with
   // this type. Testees of other languages can simply skip it.
@@ -92,14 +96,10 @@ message ConformanceRequest {
   // The payload (whether protobuf of JSON) is always for a
   // protobuf_test_messages.proto3.TestAllTypes proto (as defined in
   // src/google/protobuf/proto3_test_messages.proto).
-  //
-  // TODO(haberman): if/when we expand the conformance tests to support proto2,
-  // we will want to include a field that lets the payload/response be a
-  // protobuf_test_messages.proto2.TestAllTypes message instead.
   oneof payload {
     bytes protobuf_payload = 1;
     string json_payload = 2;
-    // Google internal only.  Opensource testees just skip it.
+    // Only used inside Google.  Opensource testees just skip it.
     string jspb_payload = 7;
     string text_payload = 8;
   }
@@ -109,12 +109,12 @@ message ConformanceRequest {
 
   // The full name for the test message to use; for the moment, either:
   // protobuf_test_messages.proto3.TestAllTypesProto3 or
-  // protobuf_test_messages.proto2.TestAllTypesProto2.
+  // protobuf_test_messages.google.protobuf.TestAllTypesProto2.
   string message_type = 4;
 
   // Each test is given a specific test category. Some category may need
-  // specific support in testee programs. Refer to the definition of TestCategory
-  // for more information.
+  // specific support in testee programs. Refer to the definition of
+  // TestCategory for more information.
   TestCategory test_category = 5;
 
   // Specify details for how to encode jspb.
@@ -140,6 +140,11 @@ message ConformanceResponse {
     // this field.
     string serialize_error = 6;
 
+    // This should be set if the test program timed out.  The string should
+    // provide more information about what the child process was doing when it
+    // was killed.
+    string timeout_error = 9;
+
     // This should be set if some other error occurred.  This will always
     // indicate that the test failed.  The string can provide more information
     // about the failure.
@@ -158,8 +163,8 @@ message ConformanceResponse {
     string skipped = 5;
 
     // If the input was successfully parsed and the requested output was JSPB,
-    // serialize to JSPB and set it in this field. JSPB is google internal only
-    // format. Opensource testees can just skip it.
+    // serialize to JSPB and set it in this field. JSPB is only used inside
+    // Google. Opensource testees can just skip it.
     string jspb_payload = 7;
 
     // If the input was successfully parsed and the requested output was
@@ -173,4 +178,3 @@ message JspbEncodingConfig {
   // Encode the value field of Any as jspb array if true, otherwise binary.
   bool use_jspb_array_any_format = 1;
 }
-
diff --git a/packages/test-conformance/proto/google/protobuf/test_messages_proto2.proto b/packages/test-conformance/proto/google/protobuf/test_messages_proto2.proto
@@ -40,6 +40,7 @@ syntax = "proto2";
 package protobuf_test_messages.proto2;
 
 option java_package = "com.google.protobuf_test_messages.proto2";
+option objc_class_prefix = "Proto2";
 
 // This is the default, but we specify it here explicitly.
 option optimize_for = SPEED;
@@ -194,6 +195,23 @@ message TestAllTypesProto2 {
     optional uint32 group_uint32 = 203;
   }
 
+  // default values
+  optional int32 default_int32 = 241 [default = -123456789];
+  optional int64 default_int64 = 242 [default = -9123456789123456789];
+  optional uint32 default_uint32 = 243 [default = 2123456789];
+  optional uint64 default_uint64 = 244 [default = 10123456789123456789];
+  optional sint32 default_sint32 = 245 [default = -123456789];
+  optional sint64 default_sint64 = 246 [default = -9123456789123456789];
+  optional fixed32 default_fixed32 = 247 [default = 2123456789];
+  optional fixed64 default_fixed64 = 248 [default = 10123456789123456789];
+  optional sfixed32 default_sfixed32 = 249 [default = -123456789];
+  optional sfixed64 default_sfixed64 = 250 [default = -9123456789123456789];
+  optional float default_float = 251 [default = 9e9];
+  optional double default_double = 252 [default = 7e22];
+  optional bool default_bool = 253 [default = true];
+  optional string default_string = 254 [default = "Rosebud"];
+  optional bytes default_bytes = 255 [default = "joshua"];
+
   // Test field-name-to-JSON-name convention.
   // (protobuf says names can be any valid C/C++ identifier.)
   optional int32 fieldname1 = 401;
@@ -264,3 +282,22 @@ message UnknownToTestAllTypes {
   optional bool optional_bool = 1006;
   repeated int32 repeated_int32 = 1011;
 }
+
+message NullHypothesisProto2 {}
+
+message EnumOnlyProto2 {
+  enum Bool {
+    kFalse = 0;
+    kTrue = 1;
+  }
+}
+
+message OneStringProto2 {
+  optional string data = 1;
+}
+
+message ProtoWithKeywords {
+  optional int32 inline = 1;
+  optional string concept = 2;
+  repeated string requires = 3;
+}
diff --git a/packages/test-conformance/proto/google/protobuf/test_messages_proto3.proto b/packages/test-conformance/proto/google/protobuf/test_messages_proto3.proto
@@ -80,8 +80,8 @@ message TestAllTypesProto3 {
     ALIAS_FOO = 0;
     ALIAS_BAR = 1;
     ALIAS_BAZ = 2;
-    QUX = 2;
-    qux = 2;
+    MOO = 2;
+    moo = 2;
     bAz = 2;
   }
 
@@ -203,6 +203,7 @@ message TestAllTypesProto3 {
     float oneof_float = 117;
     double oneof_double = 118;
     NestedEnum oneof_enum = 119;
+    google.protobuf.NullValue oneof_null_value = 120;
   }
 
   // Well-known types
@@ -232,6 +233,7 @@ message TestAllTypesProto3 {
   google.protobuf.Struct optional_struct = 304;
   google.protobuf.Any optional_any = 305;
   google.protobuf.Value optional_value = 306;
+  google.protobuf.NullValue optional_null_value = 307;
 
   repeated google.protobuf.Duration repeated_duration = 311;
   repeated google.protobuf.Timestamp repeated_timestamp = 312;
@@ -275,3 +277,12 @@ enum ForeignEnum {
   FOREIGN_BAR = 1;
   FOREIGN_BAZ = 2;
 }
+
+message NullHypothesisProto3 {}
+
+message EnumOnlyProto3 {
+  enum Bool {
+    kFalse = 0;
+    kTrue = 1;
+  }
+}