Enable experimental presence detection in JS. (#7592)

Co-authored-by: David L. Jones <dlj@google.com>

js/gulpfile.js

@@ -72,7 +72,7 @@ gulp.task('genproto_group1_closure', function (cb) {
 });
 
 gulp.task('genproto_group2_closure', function (cb) {
-  exec(protoc + ' --js_out=library=testproto_libs2,binary:.  -I ../src -I . -I commonjs ' + group2Protos.join(' '),
+  exec(protoc + ' --experimental_allow_proto3_optional --js_out=library=testproto_libs2,binary:.  -I ../src -I . -I commonjs ' + group2Protos.join(' '),
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -99,7 +99,7 @@ gulp.task('genproto_group1_commonjs', function (cb) {
 });
 
 gulp.task('genproto_group2_commonjs', function (cb) {
-  exec('mkdir -p commonjs_out && ' + protoc + ' --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I commonjs -I . ' + group2Protos.join(' '),
+  exec('mkdir -p commonjs_out && ' + protoc + ' --experimental_allow_proto3_optional --js_out=import_style=commonjs,binary:commonjs_out -I ../src -I commonjs -I . ' + group2Protos.join(' '),
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);

js/proto3_test.js

@@ -77,7 +77,7 @@ describe('proto3Test', function() {
   it('testEqualsProto3', function() {
     var msg1 = new proto.jspb.test.TestProto3();
     var msg2 = new proto.jspb.test.TestProto3();
-    msg2.setOptionalString('');
+    msg2.setSingularString('');
 
     assertTrue(jspb.Message.equals(msg1, msg2));
   });
@@ -90,12 +90,12 @@ describe('proto3Test', function() {
     var msg = new proto.jspb.test.TestProto3();
 
     // Setting should work normally.
-    msg.setOptionalString('optionalString');
-    assertEquals(msg.getOptionalString(), 'optionalString');
+    msg.setSingularString('optionalString');
+    assertEquals(msg.getSingularString(), 'optionalString');
 
     // Clearing should work too ...
-    msg.setOptionalString('');
-    assertEquals(msg.getOptionalString(), '');
+    msg.setSingularString('');
+    assertEquals(msg.getSingularString(), '');
 
     // ... and shouldn't affect the equality with a brand new message.
     assertTrue(jspb.Message.equals(msg, new proto.jspb.test.TestProto3()));
@@ -107,6 +107,58 @@ describe('proto3Test', function() {
   it('testProto3FieldDefaults', function() {
     var msg = new proto.jspb.test.TestProto3();
 
+    assertEquals(msg.getSingularInt32(), 0);
+    assertEquals(msg.getSingularInt64(), 0);
+    assertEquals(msg.getSingularUint32(), 0);
+    assertEquals(msg.getSingularUint64(), 0);
+    assertEquals(msg.getSingularSint32(), 0);
+    assertEquals(msg.getSingularSint64(), 0);
+    assertEquals(msg.getSingularFixed32(), 0);
+    assertEquals(msg.getSingularFixed64(), 0);
+    assertEquals(msg.getSingularSfixed32(), 0);
+    assertEquals(msg.getSingularSfixed64(), 0);
+    assertEquals(msg.getSingularFloat(), 0);
+    assertEquals(msg.getSingularDouble(), 0);
+    assertEquals(msg.getSingularString(), '');
+
+    // TODO(b/26173701): when we change bytes fields default getter to return
+    // Uint8Array, we'll want to switch this assertion to match the u8 case.
+    assertEquals(typeof msg.getSingularBytes(), 'string');
+    assertEquals(msg.getSingularBytes_asU8() instanceof Uint8Array, true);
+    assertEquals(typeof msg.getSingularBytes_asB64(), 'string');
+    assertEquals(msg.getSingularBytes().length, 0);
+    assertEquals(msg.getSingularBytes_asU8().length, 0);
+    assertEquals(msg.getSingularBytes_asB64(), '');
+
+    assertEquals(msg.getSingularForeignEnum(),
+                 proto.jspb.test.Proto3Enum.PROTO3_FOO);
+    assertEquals(msg.getSingularForeignMessage(), undefined);
+    assertEquals(msg.getSingularForeignMessage(), undefined);
+
+    assertEquals(msg.getRepeatedInt32List().length, 0);
+    assertEquals(msg.getRepeatedInt64List().length, 0);
+    assertEquals(msg.getRepeatedUint32List().length, 0);
+    assertEquals(msg.getRepeatedUint64List().length, 0);
+    assertEquals(msg.getRepeatedSint32List().length, 0);
+    assertEquals(msg.getRepeatedSint64List().length, 0);
+    assertEquals(msg.getRepeatedFixed32List().length, 0);
+    assertEquals(msg.getRepeatedFixed64List().length, 0);
+    assertEquals(msg.getRepeatedSfixed32List().length, 0);
+    assertEquals(msg.getRepeatedSfixed64List().length, 0);
+    assertEquals(msg.getRepeatedFloatList().length, 0);
+    assertEquals(msg.getRepeatedDoubleList().length, 0);
+    assertEquals(msg.getRepeatedStringList().length, 0);
+    assertEquals(msg.getRepeatedBytesList().length, 0);
+    assertEquals(msg.getRepeatedForeignEnumList().length, 0);
+    assertEquals(msg.getRepeatedForeignMessageList().length, 0);
+  });
+
+  /**
+   * Test presence for proto3 optional fields.
+   */
+  it('testProto3Optional', function() {
+    var msg = new proto.jspb.test.TestProto3();
+
     assertEquals(msg.getOptionalInt32(), 0);
     assertEquals(msg.getOptionalInt64(), 0);
     assertEquals(msg.getOptionalUint32(), 0);
@@ -135,51 +187,65 @@ describe('proto3Test', function() {
     assertEquals(msg.getOptionalForeignMessage(), undefined);
     assertEquals(msg.getOptionalForeignMessage(), undefined);
 
-    assertEquals(msg.getRepeatedInt32List().length, 0);
-    assertEquals(msg.getRepeatedInt64List().length, 0);
-    assertEquals(msg.getRepeatedUint32List().length, 0);
-    assertEquals(msg.getRepeatedUint64List().length, 0);
-    assertEquals(msg.getRepeatedSint32List().length, 0);
-    assertEquals(msg.getRepeatedSint64List().length, 0);
-    assertEquals(msg.getRepeatedFixed32List().length, 0);
-    assertEquals(msg.getRepeatedFixed64List().length, 0);
-    assertEquals(msg.getRepeatedSfixed32List().length, 0);
-    assertEquals(msg.getRepeatedSfixed64List().length, 0);
-    assertEquals(msg.getRepeatedFloatList().length, 0);
-    assertEquals(msg.getRepeatedDoubleList().length, 0);
-    assertEquals(msg.getRepeatedStringList().length, 0);
-    assertEquals(msg.getRepeatedBytesList().length, 0);
-    assertEquals(msg.getRepeatedForeignEnumList().length, 0);
-    assertEquals(msg.getRepeatedForeignMessageList().length, 0);
+    // Serializing an empty proto yields the empty string.
+    assertEquals(msg.serializeBinary().length, 0);
 
-  });
+    // Values start as unset, but can be explicitly set even to default values
+    // like 0.
+    assertFalse(msg.hasOptionalInt32());
+    msg.setOptionalInt32(0);
+    assertTrue(msg.hasOptionalInt32());
+
+    assertFalse(msg.hasOptionalInt64());
+    msg.setOptionalInt64(0);
+    assertTrue(msg.hasOptionalInt64());
+
+    assertFalse(msg.hasOptionalString());
+    msg.setOptionalString("");
+    assertTrue(msg.hasOptionalString());
+
+    // Now the proto will have a non-zero size, even though its values are 0.
+    var serialized = msg.serializeBinary();
+    assertNotEquals(serialized.length, 0);
 
+    var msg2 = proto.jspb.test.TestProto3.deserializeBinary(serialized);
+    assertTrue(msg2.hasOptionalInt32());
+    assertTrue(msg2.hasOptionalInt64());
+    assertTrue(msg2.hasOptionalString());
+
+    // We can clear fields to go back to empty.
+    msg2.clearOptionalInt32();
+    assertFalse(msg2.hasOptionalInt32());
+
+    msg2.clearOptionalString();
+    assertFalse(msg2.hasOptionalString());
+  });
 
   /**
-   * Test that all fields can be set and read via a serialization roundtrip.
+   * Test that all fields can be set ,and read via a serialization roundtrip.
    */
-  it('testProto3FieldSetGet', function() {
+  it('testProto3FieldSetGet', function () {
     var msg = new proto.jspb.test.TestProto3();
 
-    msg.setOptionalInt32(-42);
-    msg.setOptionalInt64(-0x7fffffff00000000);
-    msg.setOptionalUint32(0x80000000);
-    msg.setOptionalUint64(0xf000000000000000);
-    msg.setOptionalSint32(-100);
-    msg.setOptionalSint64(-0x8000000000000000);
-    msg.setOptionalFixed32(1234);
-    msg.setOptionalFixed64(0x1234567800000000);
-    msg.setOptionalSfixed32(-1234);
-    msg.setOptionalSfixed64(-0x1234567800000000);
-    msg.setOptionalFloat(1.5);
-    msg.setOptionalDouble(-1.5);
-    msg.setOptionalBool(true);
-    msg.setOptionalString('hello world');
-    msg.setOptionalBytes(BYTES);
+    msg.setSingularInt32(-42);
+    msg.setSingularInt64(-0x7fffffff00000000);
+    msg.setSingularUint32(0x80000000);
+    msg.setSingularUint64(0xf000000000000000);
+    msg.setSingularSint32(-100);
+    msg.setSingularSint64(-0x8000000000000000);
+    msg.setSingularFixed32(1234);
+    msg.setSingularFixed64(0x1234567800000000);
+    msg.setSingularSfixed32(-1234);
+    msg.setSingularSfixed64(-0x1234567800000000);
+    msg.setSingularFloat(1.5);
+    msg.setSingularDouble(-1.5);
+    msg.setSingularBool(true);
+    msg.setSingularString('hello world');
+    msg.setSingularBytes(BYTES);
     var submsg = new proto.jspb.test.ForeignMessage();
     submsg.setC(16);
-    msg.setOptionalForeignMessage(submsg);
-    msg.setOptionalForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_BAR);
+    msg.setSingularForeignMessage(submsg);
+    msg.setSingularForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_BAR);
 
     msg.setRepeatedInt32List([-42]);
     msg.setRepeatedInt64List([-0x7fffffff00000000]);
@@ -206,23 +272,23 @@ describe('proto3Test', function() {
     var serialized = msg.serializeBinary();
     msg = proto.jspb.test.TestProto3.deserializeBinary(serialized);
 
-    assertEquals(msg.getOptionalInt32(), -42);
-    assertEquals(msg.getOptionalInt64(), -0x7fffffff00000000);
-    assertEquals(msg.getOptionalUint32(), 0x80000000);
-    assertEquals(msg.getOptionalUint64(), 0xf000000000000000);
-    assertEquals(msg.getOptionalSint32(), -100);
-    assertEquals(msg.getOptionalSint64(), -0x8000000000000000);
-    assertEquals(msg.getOptionalFixed32(), 1234);
-    assertEquals(msg.getOptionalFixed64(), 0x1234567800000000);
-    assertEquals(msg.getOptionalSfixed32(), -1234);
-    assertEquals(msg.getOptionalSfixed64(), -0x1234567800000000);
-    assertEquals(msg.getOptionalFloat(), 1.5);
-    assertEquals(msg.getOptionalDouble(), -1.5);
-    assertEquals(msg.getOptionalBool(), true);
-    assertEquals(msg.getOptionalString(), 'hello world');
-    assertEquals(true, bytesCompare(msg.getOptionalBytes(), BYTES));
-    assertEquals(msg.getOptionalForeignMessage().getC(), 16);
-    assertEquals(msg.getOptionalForeignEnum(),
+    assertEquals(msg.getSingularInt32(), -42);
+    assertEquals(msg.getSingularInt64(), -0x7fffffff00000000);
+    assertEquals(msg.getSingularUint32(), 0x80000000);
+    assertEquals(msg.getSingularUint64(), 0xf000000000000000);
+    assertEquals(msg.getSingularSint32(), -100);
+    assertEquals(msg.getSingularSint64(), -0x8000000000000000);
+    assertEquals(msg.getSingularFixed32(), 1234);
+    assertEquals(msg.getSingularFixed64(), 0x1234567800000000);
+    assertEquals(msg.getSingularSfixed32(), -1234);
+    assertEquals(msg.getSingularSfixed64(), -0x1234567800000000);
+    assertEquals(msg.getSingularFloat(), 1.5);
+    assertEquals(msg.getSingularDouble(), -1.5);
+    assertEquals(msg.getSingularBool(), true);
+    assertEquals(msg.getSingularString(), 'hello world');
+    assertEquals(true, bytesCompare(msg.getSingularBytes(), BYTES));
+    assertEquals(msg.getSingularForeignMessage().getC(), 16);
+    assertEquals(msg.getSingularForeignEnum(),
         proto.jspb.test.Proto3Enum.PROTO3_BAR);
 
     assertElementsEquals(msg.getRepeatedInt32List(), [-42]);
@@ -327,20 +393,20 @@ describe('proto3Test', function() {
     // Set each primitive to a non-default value, then back to its default, to
     // ensure that the serialization is actually checking the value and not just
     // whether it has ever been set.
-    msg.setOptionalInt32(42);
-    msg.setOptionalInt32(0);
-    msg.setOptionalDouble(3.14);
-    msg.setOptionalDouble(0.0);
-    msg.setOptionalBool(true);
-    msg.setOptionalBool(false);
-    msg.setOptionalString('hello world');
-    msg.setOptionalString('');
-    msg.setOptionalBytes(goog.crypt.base64.encodeString('\u00FF\u00FF'));
-    msg.setOptionalBytes('');
-    msg.setOptionalForeignMessage(new proto.jspb.test.ForeignMessage());
-    msg.setOptionalForeignMessage(null);
-    msg.setOptionalForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_BAR);
-    msg.setOptionalForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_FOO);
+    msg.setSingularInt32(42);
+    msg.setSingularInt32(0);
+    msg.setSingularDouble(3.14);
+    msg.setSingularDouble(0.0);
+    msg.setSingularBool(true);
+    msg.setSingularBool(false);
+    msg.setSingularString('hello world');
+    msg.setSingularString('');
+    msg.setSingularBytes(goog.crypt.base64.encodeString('\u00FF\u00FF'));
+    msg.setSingularBytes('');
+    msg.setSingularForeignMessage(new proto.jspb.test.ForeignMessage());
+    msg.setSingularForeignMessage(null);
+    msg.setSingularForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_BAR);
+    msg.setSingularForeignEnum(proto.jspb.test.Proto3Enum.PROTO3_FOO);
     msg.setOneofUint32(32);
     msg.clearOneofUint32();
 
@@ -355,27 +421,25 @@ describe('proto3Test', function() {
   it('testBytesFieldsInterop', function() {
     var msg = new proto.jspb.test.TestProto3();
     // Set as a base64 string and check all the getters work.
-    msg.setOptionalBytes(BYTES_B64);
-    assertTrue(bytesCompare(msg.getOptionalBytes_asU8(), BYTES));
-    assertTrue(bytesCompare(msg.getOptionalBytes_asB64(), BYTES));
-    assertTrue(bytesCompare(msg.getOptionalBytes(), BYTES));
+    msg.setSingularBytes(BYTES_B64);
+    assertTrue(bytesCompare(msg.getSingularBytes_asU8(), BYTES));
+    assertTrue(bytesCompare(msg.getSingularBytes_asB64(), BYTES));
+    assertTrue(bytesCompare(msg.getSingularBytes(), BYTES));
 
     // Test binary serialize round trip doesn't break it.
     msg = proto.jspb.test.TestProto3.deserializeBinary(msg.serializeBinary());
-    assertTrue(bytesCompare(msg.getOptionalBytes_asU8(), BYTES));
-    assertTrue(bytesCompare(msg.getOptionalBytes_asB64(), BYTES));
-    assertTrue(bytesCompare(msg.getOptionalBytes(), BYTES));
+    assertTrue(bytesCompare(msg.getSingularBytes_asU8(), BYTES));
+    assertTrue(bytesCompare(msg.getSingularBytes_asB64(), BYTES));
+    assertTrue(bytesCompare(msg.getSingularBytes(), BYTES));
 
     msg = new proto.jspb.test.TestProto3();
     // Set as a Uint8Array and check all the getters work.
-    msg.setOptionalBytes(BYTES);
-    assertTrue(bytesCompare(msg.getOptionalBytes_asU8(), BYTES));
-    assertTrue(bytesCompare(msg.getOptionalBytes_asB64(), BYTES));
-    assertTrue(bytesCompare(msg.getOptionalBytes(), BYTES));
-
+    msg.setSingularBytes(BYTES);
+    assertTrue(bytesCompare(msg.getSingularBytes_asU8(), BYTES));
+    assertTrue(bytesCompare(msg.getSingularBytes_asB64(), BYTES));
+    assertTrue(bytesCompare(msg.getSingularBytes(), BYTES));
   });
 
-
   it('testTimestampWellKnownType', function() {
     var msg = new proto.google.protobuf.Timestamp();
     msg.fromDate(new Date(123456789));

js/proto3_test.proto

@@ -35,24 +35,43 @@ import "testbinary.proto";
 package jspb.test;
 
 message TestProto3 {
-  int32 optional_int32 = 1;
-  int64 optional_int64 = 2;
-  uint32 optional_uint32 = 3;
-  uint64 optional_uint64 = 4;
-  sint32 optional_sint32 = 5;
-  sint64 optional_sint64 = 6;
-  fixed32 optional_fixed32 = 7;
-  fixed64 optional_fixed64 = 8;
-  sfixed32 optional_sfixed32 = 9;
-  sfixed64 optional_sfixed64 = 10;
-  float optional_float = 11;
-  double optional_double = 12;
-  bool optional_bool = 13;
-  string optional_string = 14;
-  bytes optional_bytes = 15;
+  int32 singular_int32 = 1;
+  int64 singular_int64 = 2;
+  uint32 singular_uint32 = 3;
+  uint64 singular_uint64 = 4;
+  sint32 singular_sint32 = 5;
+  sint64 singular_sint64 = 6;
+  fixed32 singular_fixed32 = 7;
+  fixed64 singular_fixed64 = 8;
+  sfixed32 singular_sfixed32 = 9;
+  sfixed64 singular_sfixed64 = 10;
+  float singular_float = 11;
+  double singular_double = 12;
+  bool singular_bool = 13;
+  string singular_string = 14;
+  bytes singular_bytes = 15;
 
-  ForeignMessage optional_foreign_message = 19;
-  Proto3Enum optional_foreign_enum = 22;
+  ForeignMessage singular_foreign_message = 19;
+  Proto3Enum singular_foreign_enum = 22;
+
+  optional int32 optional_int32 = 121;
+  optional int64 optional_int64 = 122;
+  optional uint32 optional_uint32 = 123;
+  optional uint64 optional_uint64 = 124;
+  optional sint32 optional_sint32 = 125;
+  optional sint64 optional_sint64 = 126;
+  optional fixed32 optional_fixed32 = 127;
+  optional fixed64 optional_fixed64 = 128;
+  optional sfixed32 optional_sfixed32 = 129;
+  optional sfixed64 optional_sfixed64 = 130;
+  optional float optional_float = 131;
+  optional double optional_double = 132;
+  optional bool optional_bool = 133;
+  optional string optional_string = 134;
+  optional bytes optional_bytes = 135;
+
+  optional ForeignMessage optional_foreign_message = 136;
+  optional Proto3Enum optional_foreign_enum =137;
 
   repeated int32 repeated_int32 = 31;
   repeated int64 repeated_int64 = 32;

src/google/protobuf/compiler/js/js_generator.cc

@@ -467,6 +467,7 @@ bool IgnoreMessage(const Descriptor* d) { return d->options().map_entry(); }
 
 // Does JSPB ignore this entire oneof? True only if all fields are ignored.
 bool IgnoreOneof(const OneofDescriptor* oneof) {
+  if (oneof->is_synthetic()) return true;
   for (int i = 0; i < oneof->field_count(); i++) {
     if (!IgnoreField(oneof->field(i))) {
       return false;
@@ -576,6 +577,7 @@ std::string JSOneofIndex(const OneofDescriptor* oneof) {
   int index = -1;
   for (int i = 0; i < oneof->containing_type()->oneof_decl_count(); i++) {
     const OneofDescriptor* o = oneof->containing_type()->oneof_decl(i);
+    if (o->is_synthetic()) continue;
     // If at least one field in this oneof is not JSPB-ignored, count the oneof.
     for (int j = 0; j < o->field_count(); j++) {
       const FieldDescriptor* f = o->field(j);
@@ -794,6 +796,11 @@ std::string DoubleToString(double value) {
   return PostProcessFloat(result);
 }
 
+bool InRealOneof(const FieldDescriptor* field) {
+  return field->containing_oneof() &&
+         !field->containing_oneof()->is_synthetic();
+}
+
 // Return true if this is an integral field that should be represented as string
 // in JS.
 bool IsIntegralFieldWithStringJSType(const FieldDescriptor* field) {
@@ -1173,7 +1180,7 @@ std::string RepeatedFieldsArrayName(const GeneratorOptions& options,
 
 bool HasOneofFields(const Descriptor* desc) {
   for (int i = 0; i < desc->field_count(); i++) {
-    if (desc->field(i)->containing_oneof()) {
+    if (InRealOneof(desc->field(i))) {
       return true;
     }
   }
@@ -1411,15 +1418,9 @@ std::string GetPivot(const Descriptor* desc) {
 // generate extra methods (clearFoo() and hasFoo()) for this field.
 bool HasFieldPresence(const GeneratorOptions& options,
                       const FieldDescriptor* field) {
-  if (field->is_repeated() || field->is_map()) {
-    // We say repeated fields and maps don't have presence, but we still do
-    // generate clearFoo() methods for them through a special case elsewhere.
-    return false;
-  }
-
-  return field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ||
-         field->containing_oneof() != NULL ||
-         field->file()->syntax() == FileDescriptor::SYNTAX_PROTO2;
+  // This returns false for repeated fields and maps, but we still do
+  // generate clearFoo() methods for these through a special case elsewhere.
+  return field->has_presence();
 }
 
 // We use this to implement the semantics that same file can be generated
@@ -2676,7 +2677,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
                               /* singular_if_not_packed = */ false),
         "class", GetMessagePath(options, field->containing_type()),
         "settername", "set" + JSGetterName(options, field), "oneoftag",
-        (field->containing_oneof() ? "Oneof" : ""), "repeatedtag",
+        (InRealOneof(field) ? "Oneof" : ""), "repeatedtag",
         (field->is_repeated() ? "Repeated" : ""));
     printer->Annotate("settername", field);
 
@@ -2686,7 +2687,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
         "\n"
         "\n",
         "index", JSFieldIndex(field), "oneofgroup",
-        (field->containing_oneof() ? (", " + JSOneofArray(options, field))
+        (InRealOneof(field) ? (", " + JSOneofArray(options, field))
                                    : ""));
 
     if (field->is_repeated()) {
@@ -2811,8 +2812,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
           "  return jspb.Message.set$oneoftag$Field(this, $index$",
           "class", GetMessagePath(options, field->containing_type()),
           "settername", "set" + JSGetterName(options, field), "oneoftag",
-          (field->containing_oneof() ? "Oneof" : ""), "index",
-          JSFieldIndex(field));
+          (InRealOneof(field) ? "Oneof" : ""), "index", JSFieldIndex(field));
       printer->Annotate("settername", field);
       printer->Print(
           "$oneofgroup$, $type$value$rptvalueinit$$typeclose$);\n"
@@ -2822,8 +2822,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
           "type",
           untyped ? "/** @type{string|number|boolean|Array|undefined} */(" : "",
           "typeclose", untyped ? ")" : "", "oneofgroup",
-          (field->containing_oneof() ? (", " + JSOneofArray(options, field))
-                                     : ""),
+          (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : ""),
           "rptvalueinit", (field->is_repeated() ? " || []" : ""));
     }
 
@@ -2899,8 +2898,8 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
             "$index$$maybeoneofgroup$, ",
         "class", GetMessagePath(options, field->containing_type()),
         "clearername", "clear" + JSGetterName(options, field),
-        "maybeoneof", (field->containing_oneof() ? "Oneof" : ""),
-        "maybeoneofgroup", (field->containing_oneof()
+        "maybeoneof", (InRealOneof(field) ? "Oneof" : ""),
+        "maybeoneofgroup", (InRealOneof(field)
                             ? (", " + JSOneofArray(options, field))
                             : ""),
         "index", JSFieldIndex(field));
@@ -2965,7 +2964,7 @@ void Generator::GenerateRepeatedPrimitiveHelperMethods(
       "\n",
       "type", untyped ? "/** @type{string|number|boolean|!Uint8Array} */(" : "",
       "typeclose", untyped ? ")" : "", "oneofgroup",
-      (field->containing_oneof() ? (", " + JSOneofArray(options, field)) : ""),
+      (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : ""),
       "rptvalueinit", "");
   // clang-format on
 }
@@ -2994,7 +2993,7 @@ void Generator::GenerateRepeatedMessageHelperMethods(
       "\n"
       "\n",
       "index", JSFieldIndex(field), "oneofgroup",
-      (field->containing_oneof() ? (", " + JSOneofArray(options, field)) : ""),
+      (InRealOneof(field) ? (", " + JSOneofArray(options, field)) : ""),
       "ctor", GetMessagePath(options, field->message_type()));
 }
 

src/google/protobuf/compiler/js/js_generator.h

@@ -142,18 +142,21 @@ class PROTOC_EXPORT Generator : public CodeGenerator {
   Generator() {}
   virtual ~Generator() {}
 
-  virtual bool Generate(const FileDescriptor* file,
-                        const std::string& parameter, GeneratorContext* context,
-                        std::string* error) const {
+  bool Generate(const FileDescriptor* file, const std::string& parameter,
+                GeneratorContext* context, std::string* error) const override {
     *error = "Unimplemented Generate() method. Call GenerateAll() instead.";
     return false;
   }
 
-  virtual bool HasGenerateAll() const { return true; }
+  bool HasGenerateAll() const override { return true; }
 
-  virtual bool GenerateAll(const std::vector<const FileDescriptor*>& files,
-                           const std::string& parameter,
-                           GeneratorContext* context, std::string* error) const;
+  bool GenerateAll(const std::vector<const FileDescriptor*>& files,
+                   const std::string& parameter, GeneratorContext* context,
+                   std::string* error) const override;
+
+  uint64 GetSupportedFeatures() const override {
+    return FEATURE_PROTO3_OPTIONAL;
+  }
 
  private:
   void GenerateHeader(const GeneratorOptions& options,