diff --git a/js/debug.js b/js/debug.js index 3701a095..3c1ada02 100644 --- a/js/debug.js +++ b/js/debug.js @@ -94,8 +94,10 @@ jspb.debug.dump_ = function(thing) { var match = /^get([A-Z]\w*)/.exec(name); if (match && name != 'getExtension' && name != 'getJsPbMessageId') { - var val = thing[name](); - if (val != null) { + var has = 'has' + match[1]; + if (!thing[has] || thing[has]()) + { + var val = thing[name](); object[jspb.debug.formatFieldName_(match[1])] = jspb.debug.dump_(val); } } diff --git a/js/message_test.js b/js/message_test.js index 11792423..b7791431 100644 --- a/js/message_test.js +++ b/js/message_test.js @@ -276,9 +276,6 @@ describe('Message test suite', function() { }); it('testClearFields', function() { - // We don't set 'proper' defaults, rather, bools, strings, - // etc, are cleared to undefined or null and take on the Javascript - // meaning for that value. Repeated fields are set to [] when cleared. var data = ['str', true, [11], [[22], [33]], ['s1', 's2']]; var foo = new proto.jspb.test.OptionalFields(data); foo.clearAString(); @@ -286,8 +283,8 @@ describe('Message test suite', function() { foo.clearANestedMessage(); foo.clearARepeatedMessageList(); foo.clearARepeatedStringList(); - assertUndefined(foo.getAString()); - assertUndefined(foo.getABool()); + assertEquals('', foo.getAString()); + assertEquals(false, foo.getABool()); assertUndefined(foo.getANestedMessage()); assertFalse(foo.hasAString()); assertFalse(foo.hasABool()); @@ -310,8 +307,8 @@ describe('Message test suite', function() { foo.setANestedMessage(null); foo.setARepeatedMessageList(null); foo.setARepeatedStringList(null); - assertNull(foo.getAString()); - assertNull(foo.getABool()); + assertEquals('', foo.getAString()); + assertEquals(false, foo.getABool()); assertNull(foo.getANestedMessage()); assertFalse(foo.hasAString()); assertFalse(foo.hasABool()); @@ -328,8 +325,8 @@ describe('Message test suite', function() { foo.setANestedMessage(undefined); foo.setARepeatedMessageList(undefined); foo.setARepeatedStringList(undefined); - assertUndefined(foo.getAString()); - assertUndefined(foo.getABool()); + assertEquals('', foo.getAString()); + assertEquals(false, foo.getABool()); assertUndefined(foo.getANestedMessage()); assertFalse(foo.hasAString()); assertFalse(foo.hasABool()); @@ -346,9 +343,9 @@ describe('Message test suite', function() { {1000: 'unique'}]); var diff = /** @type {proto.jspb.test.HasExtensions} */ (jspb.Message.difference(p1, p2)); - assertUndefined(diff.getStr1()); + assertEquals('', diff.getStr1()); assertEquals('what', diff.getStr2()); - assertUndefined(diff.getStr3()); + assertEquals('', diff.getStr3()); assertEquals('unique', diff.extensionObject_[1000]); }); @@ -806,7 +803,7 @@ describe('Message test suite', function() { var message = new proto.jspb.test.TestMessageWithOneof([,, 'x']); assertEquals('x', message.getPone()); - assertUndefined(message.getPthree()); + assertEquals('', message.getPthree()); assertEquals( proto.jspb.test.TestMessageWithOneof.PartialOneofCase.PONE, message.getPartialOneofCase()); @@ -815,7 +812,7 @@ describe('Message test suite', function() { it('testKeepsLastWireValueSetInUnion_multipleValues', function() { var message = new proto.jspb.test.TestMessageWithOneof([,, 'x',, 'y']); - assertUndefined('x', message.getPone()); + assertEquals('', message.getPone()); assertEquals('y', message.getPthree()); assertEquals( proto.jspb.test.TestMessageWithOneof.PartialOneofCase.PTHREE, @@ -824,19 +821,19 @@ describe('Message test suite', function() { it('testSettingOneofFieldClearsOthers', function() { var message = new proto.jspb.test.TestMessageWithOneof; - assertUndefined(message.getPone()); - assertUndefined(message.getPthree()); + assertEquals('', message.getPone()); + assertEquals('', message.getPthree()); assertFalse(message.hasPone()); assertFalse(message.hasPthree()); message.setPone('hi'); assertEquals('hi', message.getPone()); - assertUndefined(message.getPthree()); + assertEquals('', message.getPthree()); assertTrue(message.hasPone()); assertFalse(message.hasPthree()); message.setPthree('bye'); - assertUndefined(message.getPone()); + assertEquals('', message.getPone()); assertEquals('bye', message.getPthree()); assertFalse(message.hasPone()); assertTrue(message.hasPthree()); @@ -845,8 +842,8 @@ describe('Message test suite', function() { it('testSettingOneofFieldDoesNotClearFieldsFromOtherUnions', function() { var other = new proto.jspb.test.TestMessageWithOneof; var message = new proto.jspb.test.TestMessageWithOneof; - assertUndefined(message.getPone()); - assertUndefined(message.getPthree()); + assertEquals('', message.getPone()); + assertEquals('', message.getPthree()); assertUndefined(message.getRone()); assertFalse(message.hasPone()); assertFalse(message.hasPthree()); @@ -854,13 +851,13 @@ describe('Message test suite', function() { message.setPone('hi'); message.setRone(other); assertEquals('hi', message.getPone()); - assertUndefined(message.getPthree()); + assertEquals('', message.getPthree()); assertEquals(other, message.getRone()); assertTrue(message.hasPone()); assertFalse(message.hasPthree()); message.setPthree('bye'); - assertUndefined(message.getPone()); + assertEquals('', message.getPone()); assertEquals('bye', message.getPthree()); assertEquals(other, message.getRone()); assertFalse(message.hasPone()); @@ -889,7 +886,7 @@ describe('Message test suite', function() { it('testMessageWithDefaultOneofValues', function() { var message = new proto.jspb.test.TestMessageWithOneof; assertEquals(1234, message.getAone()); - assertUndefined(message.getAtwo()); + assertEquals(0, message.getAtwo()); assertEquals( proto.jspb.test.TestMessageWithOneof.DefaultOneofACase .DEFAULT_ONEOF_A_NOT_SET, @@ -897,7 +894,7 @@ describe('Message test suite', function() { message.setAone(567); assertEquals(567, message.getAone()); - assertUndefined(message.getAtwo()); + assertEquals(0, message.getAtwo()); assertEquals( proto.jspb.test.TestMessageWithOneof.DefaultOneofACase.AONE, message.getDefaultOneofACase()); @@ -911,7 +908,7 @@ describe('Message test suite', function() { message.clearAtwo(); assertEquals(1234, message.getAone()); - assertUndefined(message.getAtwo()); + assertEquals(0, message.getAtwo()); assertEquals( proto.jspb.test.TestMessageWithOneof.DefaultOneofACase .DEFAULT_ONEOF_A_NOT_SET, @@ -920,7 +917,7 @@ describe('Message test suite', function() { it('testMessageWithDefaultOneofValues_defaultNotOnFirstField', function() { var message = new proto.jspb.test.TestMessageWithOneof; - assertUndefined(message.getBone()); + assertEquals(0, message.getBone()); assertEquals(1234, message.getBtwo()); assertFalse(message.hasBone()); assertFalse(message.hasBtwo()); @@ -939,7 +936,7 @@ describe('Message test suite', function() { message.getDefaultOneofBCase()); message.setBtwo(3); - assertUndefined(message.getBone()); + assertEquals(0, message.getBone()); assertFalse(message.hasBone()); assertTrue(message.hasBtwo()); assertEquals(3, message.getBtwo()); @@ -948,7 +945,7 @@ describe('Message test suite', function() { message.getDefaultOneofBCase()); message.clearBtwo(); - assertUndefined(message.getBone()); + assertEquals(0, message.getBone()); assertFalse(message.hasBone()); assertFalse(message.hasBtwo()); assertEquals(1234, message.getBtwo()); @@ -962,7 +959,7 @@ describe('Message test suite', function() { var message = new proto.jspb.test.TestMessageWithOneof(new Array(9).concat(567)); assertEquals(567, message.getAone()); - assertUndefined(message.getAtwo()); + assertEquals(0, message.getAtwo()); assertEquals( proto.jspb.test.TestMessageWithOneof.DefaultOneofACase.AONE, message.getDefaultOneofACase()); @@ -998,7 +995,7 @@ describe('Message test suite', function() { message = new proto.jspb.test.TestMessageWithOneof(new Array(12).concat(890)); - assertUndefined(message.getBone()); + assertEquals(0, message.getBone()); assertEquals(890, message.getBtwo()); assertEquals( proto.jspb.test.TestMessageWithOneof.DefaultOneofBCase.BTWO, @@ -1006,7 +1003,7 @@ describe('Message test suite', function() { message = new proto.jspb.test.TestMessageWithOneof( new Array(11).concat(567, 890)); - assertUndefined(message.getBone()); + assertEquals(0, message.getBone()); assertEquals(890, message.getBtwo()); assertEquals( proto.jspb.test.TestMessageWithOneof.DefaultOneofBCase.BTWO, @@ -1023,7 +1020,7 @@ describe('Message test suite', function() { var other = new proto.jspb.test.TestMessageWithOneof; message.setRone(other); assertEquals(other, message.getRone()); - assertUndefined(message.getRtwo()); + assertEquals('', message.getRtwo()); assertEquals( proto.jspb.test.TestMessageWithOneof.RecursiveOneofCase.RONE, message.getRecursiveOneofCase()); @@ -1041,7 +1038,7 @@ describe('Message test suite', function() { var message = new proto.jspb.test.TestMessageWithOneof; message.setPone('x'); assertEquals('x', message.getPone()); - assertUndefined(message.getPthree()); + assertEquals('', message.getPthree()); assertEquals( proto.jspb.test.TestMessageWithOneof.PartialOneofCase.PONE, message.getPartialOneofCase()); diff --git a/js/proto3_test.js b/js/proto3_test.js index 7f76006a..fab0fd44 100644 --- a/js/proto3_test.js +++ b/js/proto3_test.js @@ -221,10 +221,10 @@ describe('proto3Test', function() { it('testOneofs', function() { var msg = new proto.jspb.test.TestProto3(); - assertEquals(msg.getOneofUint32(), undefined); + assertEquals(msg.getOneofUint32(), 0); assertEquals(msg.getOneofForeignMessage(), undefined); - assertEquals(msg.getOneofString(), undefined); - assertEquals(msg.getOneofBytes(), undefined); + assertEquals(msg.getOneofString(), ''); + assertEquals(msg.getOneofBytes(), ''); assertFalse(msg.hasOneofUint32()); assertFalse(msg.hasOneofString()); assertFalse(msg.hasOneofBytes()); @@ -232,8 +232,8 @@ describe('proto3Test', function() { msg.setOneofUint32(42); assertEquals(msg.getOneofUint32(), 42); assertEquals(msg.getOneofForeignMessage(), undefined); - assertEquals(msg.getOneofString(), undefined); - assertEquals(msg.getOneofBytes(), undefined); + assertEquals(msg.getOneofString(), ''); + assertEquals(msg.getOneofBytes(), ''); assertTrue(msg.hasOneofUint32()); assertFalse(msg.hasOneofString()); assertFalse(msg.hasOneofBytes()); @@ -241,27 +241,27 @@ describe('proto3Test', function() { var submsg = new proto.jspb.test.ForeignMessage(); msg.setOneofForeignMessage(submsg); - assertEquals(msg.getOneofUint32(), undefined); + assertEquals(msg.getOneofUint32(), 0); assertEquals(msg.getOneofForeignMessage(), submsg); - assertEquals(msg.getOneofString(), undefined); - assertEquals(msg.getOneofBytes(), undefined); + assertEquals(msg.getOneofString(), ''); + assertEquals(msg.getOneofBytes(), ''); assertFalse(msg.hasOneofUint32()); assertFalse(msg.hasOneofString()); assertFalse(msg.hasOneofBytes()); msg.setOneofString('hello'); - assertEquals(msg.getOneofUint32(), undefined); + assertEquals(msg.getOneofUint32(), 0); assertEquals(msg.getOneofForeignMessage(), undefined); assertEquals(msg.getOneofString(), 'hello'); - assertEquals(msg.getOneofBytes(), undefined); + assertEquals(msg.getOneofBytes(), ''); assertFalse(msg.hasOneofUint32()); assertTrue(msg.hasOneofString()); assertFalse(msg.hasOneofBytes()); msg.setOneofBytes(goog.crypt.base64.encodeString('\u00FF\u00FF')); - assertEquals(msg.getOneofUint32(), undefined); + assertEquals(msg.getOneofUint32(), 0); assertEquals(msg.getOneofForeignMessage(), undefined); - assertEquals(msg.getOneofString(), undefined); + assertEquals(msg.getOneofString(), ''); assertEquals(msg.getOneofBytes_asB64(), goog.crypt.base64.encodeString('\u00FF\u00FF')); assertFalse(msg.hasOneofUint32()); diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index a24686b6..58c77d00 100755 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc @@ -768,7 +768,6 @@ string MaybeNumberString(const FieldDescriptor* field, const string& orig) { } string JSFieldDefault(const FieldDescriptor* field) { - assert(field->has_default_value()); switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_INT32: return MaybeNumberString( @@ -943,7 +942,7 @@ string JSFieldTypeAnnotation(const GeneratorOptions& options, } if (field->is_optional() && is_primitive && - (!field->has_default_value() || force_optional) && !force_present) { + force_optional && !force_present) { jstype += "?"; } else if (field->is_required() && !is_primitive && !force_optional) { jstype = "!" + jstype; @@ -1258,6 +1257,10 @@ string GetPivot(const Descriptor* desc) { // Returns true for fields that represent "null" as distinct from the default // value. See http://go/proto3#heading=h.kozewqqcqhuz for more information. bool HasFieldPresence(const FieldDescriptor* field) { + if (field->is_repeated()) { + return false; + } + return (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) || (field->containing_oneof() != NULL) || @@ -2387,7 +2390,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options, "index", JSFieldIndex(field), "default", Proto3PrimitiveFieldDefault(field)); } else { - if (field->has_default_value()) { + if (!field->is_repeated()) { printer->Print("!this.has$name$() ? $defaultValue$ : ", "name", JSGetterName(options, field), "defaultValue", JSFieldDefault(field)); @@ -2398,10 +2401,6 @@ void Generator::GenerateClassField(const GeneratorOptions& options, printer->Print("jspb.Message.getRepeatedFloatingPointField(" "this, $index$)", "index", JSFieldIndex(field)); - } else if (field->is_optional() && !field->has_default_value()) { - printer->Print("jspb.Message.getOptionalFloatingPointField(" - "this, $index$)", - "index", JSFieldIndex(field)); } else { // Convert "NaN" to NaN. printer->Print("+jspb.Message.getField(this, $index$)", @@ -2477,7 +2476,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options, "returndoc", JSReturnDoc(options, field)); } - if (HasFieldPresence(field)) { + if (HasFieldPresence(field) || field->is_repeated()) { printer->Print( "$class$.prototype.clear$name$ = function() {\n" " jspb.Message.set$oneoftag$Field(this, $index$$oneofgroup$, ", @@ -2494,22 +2493,24 @@ void Generator::GenerateClassField(const GeneratorOptions& options, "\n", "clearedvalue", (field->is_repeated() ? "[]" : "undefined"), "returnvalue", JSReturnClause(field)); - - printer->Print( - "/**\n" - " * Returns whether this field is set.\n" - " * @return{!boolean}\n" - " */\n" - "$class$.prototype.has$name$ = function() {\n" - " return jspb.Message.getField(this, $index$) != null;\n" - "};\n" - "\n" - "\n", - "class", GetPath(options, field->containing_type()), - "name", JSGetterName(options, field), - "index", JSFieldIndex(field)); } } + + if (HasFieldPresence(field)) { + printer->Print( + "/**\n" + " * Returns whether this field is set.\n" + " * @return{!boolean}\n" + " */\n" + "$class$.prototype.has$name$ = function() {\n" + " return jspb.Message.getField(this, $index$) != null;\n" + "};\n" + "\n" + "\n", + "class", GetPath(options, field->containing_type()), + "name", JSGetterName(options, field), + "index", JSFieldIndex(field)); + } } void Generator::GenerateClassExtensionFieldInfo(const GeneratorOptions& options, @@ -2756,11 +2757,18 @@ void Generator::GenerateClassSerializeBinaryField( const GeneratorOptions& options, io::Printer* printer, const FieldDescriptor* field) const { - printer->Print( - " f = this.get$name$($nolazy$);\n", - "name", JSGetterName(options, field, BYTES_U8), - // No lazy creation for maps containers -- fastpath the empty case. - "nolazy", (field->is_map()) ? "true" : ""); + if (HasFieldPresence(field) && + field->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) { + printer->Print( + " f = jspb.Message.getField(this, $index$);\n", + "index", JSFieldIndex(field)); + } else { + printer->Print( + " f = this.get$name$($nolazy$);\n", + "name", JSGetterName(options, field, BYTES_U8), + // No lazy creation for maps containers -- fastpath the empty case. + "nolazy", (field->is_map()) ? "true" : ""); + } // Print an `if (condition)` statement that evaluates to true if the field