Hook up field presence feature

Author: mkruskal-google

cli/targets/static.js

@@ -362,58 +362,16 @@ function toJsType(field, parentIsInterface = false) {
     return type;
 }
 
-function syntaxForType(type) {
-
-    var syntax = null;
-    var namespace = type;
-
-    while (syntax === null && namespace !== null) {
-        if (namespace.options != null && "syntax" in namespace.options) {
-            syntax = namespace.options["syntax"];
-        }
-        else {
-            namespace = namespace.parent;
-        }
-    }
-
-    return syntax !== null ? syntax : "proto2";
-}
-
-function isExplicitPresence(field, syntax) {
-
-    // In proto3, optional fields are explicit
-    if (syntax === "proto3") {
-        return field.options != null && field.options["proto3_optional"] === true;
-    }
-
-    // In proto2, fields are explicitly optional if they are not part of a map, array or oneOf group
-    if (syntax === "proto2") {
-        return field.optional && !(field.partOf || field.repeated || field.map);
-    }
-
-    throw new Error("Unknown proto syntax: [" + syntax + "]");
-}
-
-function isImplicitPresence(field, syntax) {
-
-    // In proto3, everything not marked optional has implicit presence (including maps and repeated fields)
-    if (syntax === "proto3") {
-        return field.options == null || field.options["proto3_optional"] !== true;
-    }
-
-    // In proto2, nothing has implicit presence
-    if (syntax === "proto2") {
+function hasPresence(field) {
+    if (field.repeated || field.map) {
         return false;
     }
-
-    throw new Error("Unknown proto syntax: [" + syntax + "]");
+    return field.partOf || // oneofs
+        field.declaringField || field.extensionField || // extensions
+        field._features.field_presence === "EXPLICIT";
 }
 
-function isOptionalOneOf(oneof, syntax) {
-
-    if (syntax === "proto2") {
-        return false;
-    }
+function isProto3Optional(oneof) {
 
     if (oneof.fieldsArray == null || oneof.fieldsArray.length !== 1) {
         return false;
@@ -426,8 +384,6 @@ function isOptionalOneOf(oneof, syntax) {
 
 function buildType(ref, type) {
 
-    var syntax = syntaxForType(type);
-
     if (config.comments) {
         var typeDef = [
             "Properties of " + aOrAn(type.name) + ".",
@@ -443,13 +399,15 @@ function buildType(ref, type) {
                 // With semantic nulls, only explicit optional fields and one-of members can be set to null
                 // Implicit fields (proto3), maps and lists can be omitted, but if specified must be non-null
                 // Implicit fields will take their default value when the message is constructed
-                if (isExplicitPresence(field, syntax) || field.partOf) {
-                    jsType = jsType + "|null|undefined";
-                    nullable = true;
-                }
-                else if (isImplicitPresence(field, syntax) || field.repeated || field.map) {
-                    jsType = jsType + "|undefined";
-                    nullable = true;
+                if (field.optional) {
+                    if (hasPresence(field)) {
+                        jsType = jsType + "|null|undefined";
+                        nullable = true;
+                    }
+                    else {
+                        jsType = jsType + "|undefined";
+                        nullable = true;
+                    }
                 }
             }
             else {
@@ -490,7 +448,7 @@ function buildType(ref, type) {
                 // With semantic nulls, fields are nullable if they are explicitly optional or part of a one-of
                 // Maps, repeated values and fields with implicit defaults are never null after construction
                 // Members are never undefined, at a minimum they are initialized to null
-                if (isExplicitPresence(field, syntax) || field.partOf) {
+                if (hasPresence(field) && field.optional) {
                     jsType = jsType + "|null";
                 }
             }
@@ -514,7 +472,7 @@ function buildType(ref, type) {
         // With semantic nulls, only explict optional fields and one-of members are null by default
         // Otherwise use field.optional, which doesn't consider proto3, maps, repeated fields etc.
         var nullDefault = config["null-semantics"]
-            ? isExplicitPresence(field, syntax)
+            ? hasPresence(field) && field.optional
             : field.optional && config["null-defaults"];
         if (field.repeated)
             push(escapeName(type.name) + ".prototype" + prop + " = $util.emptyArray;"); // overwritten in constructor
@@ -546,7 +504,7 @@ function buildType(ref, type) {
         }
         oneof.resolve();
         push("");
-        if (isOptionalOneOf(oneof, syntax)) {
+        if (isProto3Optional(oneof)) {
             push("// Virtual OneOf for proto3 optional field");
         }
         else {

src/field.js

@@ -105,18 +105,6 @@ function Field(name, id, type, rule, extend, options, comment) {
      */
     this.extend = extend || undefined; // toJSON
 
-    /**
-     * Whether this field is required.
-     * @type {boolean}
-     */
-    this.required = rule === "required";
-
-    /**
-     * Whether this field is optional.
-     * @type {boolean}
-     */
-    this.optional = !this.required;
-
     /**
      * Whether this field is repeated.
      * @type {boolean}
@@ -190,6 +178,30 @@ function Field(name, id, type, rule, extend, options, comment) {
     this.comment = comment;
 }
 
+/**
+ * Determines whether this field is required.
+ * @name Field#required
+ * @type {boolean}
+ * @readonly
+ */
+Object.defineProperty(Field.prototype, "required", {
+    get: function() {
+        return this._features.field_presence === "LEGACY_REQUIRED";
+    }
+});
+
+/**
+ * Determines whether this field is not required.
+ * @name Field#optional
+ * @type {boolean}
+ * @readonly
+ */
+Object.defineProperty(Field.prototype, "optional", {
+    get: function() {
+        return !this.required;
+    }
+});
+
 /**
  * Determines whether this field is packed. Only relevant when repeated.
  * @name Field#packed
@@ -320,6 +332,9 @@ Field.prototype._inferLegacyProtoFeatures = function _inferLegacyProtoFeatures(e
     if (edition) return {};
 
     var features = {};
+    if (this.rule === "required") {
+        features.field_presence = "LEGACY_REQUIRED";
+    }
     if (this.getOption("packed") === true) {
         features.repeated_field_encoding = "PACKED";
     } else if (this.getOption("packed") === false) {

tests/cli.js

@@ -236,6 +236,45 @@ tape.test("with --null-semantics, optional fields are handled correctly in proto
     });
 });
 
+tape.test("with --null-semantics, optional fields are handled correctly in editions", function(test) {
+    cliTest(test, function() {
+        var root = protobuf.loadSync("tests/data/cli/null-defaults-edition2023.proto");
+        root.resolveAll();
+
+        var staticTarget = require("../cli/targets/static");
+
+        staticTarget(root, {
+            create: true,
+            decode: true,
+            encode: true,
+            convert: true,
+            comments: true,
+            "null-semantics": true,
+        }, function(err, jsCode) {
+
+            test.error(err, 'static code generation worked');
+
+            test.ok(jsCode.includes("@property {OptionalFields.ISubMessage|null|undefined} [a] OptionalFields a"), "Property for a should use an interface")
+            test.ok(jsCode.includes("@member {OptionalFields.SubMessage|null} a"), "Member for a should use a message type")
+            test.ok(jsCode.includes("OptionalFields.prototype.a = null;"), "Initializer for a should be null")
+
+            test.ok(jsCode.includes("@property {string|null|undefined} [e] OptionalFields e"), "Property for e should be nullable")
+            test.ok(jsCode.includes("@member {string|null} e"), "Member for e should be nullable")
+            test.ok(jsCode.includes("OptionalFields.prototype.e = null;"), "Initializer for e should be null")
+
+            test.ok(jsCode.includes("@property {number} r OptionalFields r"), "Property for r should not be nullable")
+            test.ok(jsCode.includes("@member {number} r"), "Member for r should not be nullable")
+            test.ok(jsCode.includes("OptionalFields.prototype.r = 0;"), "Initializer for r should be zero")
+
+            test.ok(jsCode.includes("@property {number|undefined} [i] OptionalFields i"), "Property for i should be optional but not nullable")
+            test.ok(jsCode.includes("@member {number} i"), "Member for i should not be nullable")
+            test.ok(jsCode.includes("OptionalFields.prototype.i = 0;"), "Initializer for i should be zero")
+
+            test.end();
+        });
+    });
+});
+
 
 tape.test("pbjs generates static code with message filter", function (test) {
     cliTest(test, function () {

tests/data/cli/null-defaults-edition2023.proto

@@ -0,0 +1,12 @@
+edition = "2023";
+
+message OptionalFields {
+  message SubMessage {
+    string a = 1 [features.field_presence = IMPLICIT];
+  }
+
+  SubMessage a = 1;
+  string e = 2;
+  uint32 r = 3 [features.field_presence = LEGACY_REQUIRED];
+  uint32 i = 4  [features.field_presence = IMPLICIT];
+}

tests/feature_resolution_editions.js

@@ -445,12 +445,13 @@ tape.test("feature resolution inferred proto2 repeated encoding", function(test)
         repeated int32 unpacked = 3 [packed = false];
     }`).root.resolveAll();
 
-    test.notOk(root.lookup("Message").fields.default.packed)
-    test.equal(root.lookup("Message").fields.default._features.repeated_field_encoding, "EXPANDED")
-    test.ok(root.lookup("Message").fields.packed.packed)
-    test.equal(root.lookup("Message").fields.packed._features.repeated_field_encoding, "PACKED")
-    test.notOk(root.lookup("Message").fields.unpacked.packed)
-    test.equal(root.lookup("Message").fields.unpacked._features.repeated_field_encoding, "EXPANDED")
+    var msg = root.lookup("Message");
+    test.notOk(msg.fields.default.packed)
+    test.equal(msg.fields.default._features.repeated_field_encoding, "EXPANDED")
+    test.ok(msg.fields.packed.packed)
+    test.equal(msg.fields.packed._features.repeated_field_encoding, "PACKED")
+    test.notOk(msg.fields.unpacked.packed)
+    test.equal(msg.fields.unpacked._features.repeated_field_encoding, "EXPANDED")
 
     test.end();
 });
@@ -463,13 +464,32 @@ tape.test("feature resolution inferred proto3 repeated encoding", function(test)
         repeated int32 unpacked = 3 [packed = false];
     }`).root.resolveAll();
 
-    test.ok(root.lookup("Message").fields.default.packed)
-    test.equal(root.lookup("Message").fields.default._features.repeated_field_encoding, "PACKED")
-    test.ok(root.lookup("Message").fields.packed.packed)
-    test.equal(root.lookup("Message").fields.packed._features.repeated_field_encoding, "PACKED")
-    test.notOk(root.lookup("Message").fields.unpacked.packed)
-    test.equal(root.lookup("Message").fields.unpacked._features.repeated_field_encoding, "EXPANDED")
+    var msg = root.lookup("Message");
+    test.ok(msg.fields.default.packed)
+    test.equal(msg.fields.default._features.repeated_field_encoding, "PACKED")
+    test.ok(msg.fields.packed.packed)
+    test.equal(msg.fields.packed._features.repeated_field_encoding, "PACKED")
+    test.notOk(msg.fields.unpacked.packed)
+    test.equal(msg.fields.unpacked._features.repeated_field_encoding, "EXPANDED")
 
     test.end();
 });
 
+
+tape.test("feature resolution inferred proto2 presence", function(test) {
+    var root = protobuf.parse(`syntax = "proto2";
+    message Message {
+        optional int32 default = 1;
+        required int32 required = 2;
+    }`).root.resolveAll();
+
+    var msg = root.lookup("Message");
+    test.ok(msg.fields.default.optional)
+    test.notOk(msg.fields.default.required)
+    test.equal(msg.fields.default._features.field_presence, "EXPLICIT")
+    test.notOk(msg.fields.required.optional)
+    test.ok(msg.fields.required.required)
+    test.equal(msg.fields.required._features.field_presence, "LEGACY_REQUIRED")
+
+    test.end();
+});

tests/other_protocolerror.js

@@ -15,7 +15,7 @@ tape.test("a protocol error", function(test) {
         ).add(
             new protobuf.Field("bar", 2, "string", "required")
         )
-    );
+    ).resolveAll();
 
     var Test = root.lookup("Test");
     var buf  = protobuf.util.newBuffer(2);