Fixed: Use common utility for virtual oneof getters and setters in bo…

…th reflection and static code, see #644

README.md

@@ -307,7 +307,7 @@ protobuf.load("bundle.json", function(err, root) {
 });
 ```
 
-**ProTip!** Documenting your .proto files with `/** ... */`-blocks translates to generated static code.
+**ProTip!** Documenting your .proto files with `/** ... */`-blocks or (trailing) `/// ...` lines translates to generated static code.
 
 ### Generating TypeScript definitions from static modules
 

cli/targets/static.js

@@ -25,8 +25,12 @@ function static_target(root, options, callback) {
             push("// Lazily resolved type references");
         push("var $lazyTypes = [];");
         push("");
-        if (config.comments)
-            push("// Exported root namespace");
+        if (config.comments) {
+            if (root.comment)
+                pushComment("@fileoverview " + root.comment);
+            else
+                push("// Exported root namespace");
+        }
         push("var $root = {};");
         buildNamespace(null, root);
         push("");
@@ -76,7 +80,7 @@ function pushComment(lines) {
         return;
     var split = [];
     for (var i = 0; i < lines.length; ++i)
-        if (lines[i] !== null)
+        if (lines[i] !== null && lines[i].substring(0, 8) !== "@exclude")
             Array.prototype.push.apply(split, lines[i].split(/\r?\n/g));
     push("/**");
     split.forEach(function(line) {
@@ -126,7 +130,7 @@ function buildNamespace(ref, ns) {
     else if (ns.name !== "") {
         push("");
         pushComment([
-            "Namespace " + ns.name + ".",
+            ns.comment || "Namespace " + ns.name + ".",
             "@exports " + ns.fullName.substring(1),
             "@namespace"
         ]);
@@ -301,7 +305,15 @@ function buildType(ref, type) {
     });
 
     // virtual oneof fields
+    var firstOneOf = true;;
     type.oneofsArray.forEach(function(oneof) {
+        if (firstOneOf) {
+            firstOneOf = false;
+            push("");
+            if (config.comments)
+                push("// OneOf field names bound to virtual getters and setters");
+            push("var $oneOfFields;");
+        }
         oneof.resolve();
         push("");
         pushComment([
@@ -311,27 +323,8 @@ function buildType(ref, type) {
         ]);
         push("Object.defineProperty($prototype, " + JSON.stringify(oneof.name) +", {");
         ++indent;
-            push("get: function() {");
-            ++indent;
-            oneof.oneof.forEach(function(name) {
-                push("if (this[" + JSON.stringify(name) + "] !== undefined)");
-                ++indent;
-                    push("return " + JSON.stringify(name) + ";");
-                --indent;
-            });
-            push("return undefined;");
-            --indent;
-            push("},");
-            push("set: function(value) {");
-            ++indent;
-            oneof.oneof.forEach(function(name) {
-                push("if (value !== " + JSON.stringify(name) + ")");
-                ++indent;
-                    push("delete this[" + JSON.stringify(name) + "];");
-                --indent;
-            });
-            --indent;
-            push("}");
+            push("get: $protobuf.util.oneOfGetter($oneOfFields = [" + oneof.oneof.map(JSON.stringify).join(", ") + "]),");
+            push("set: $protobuf.util.oneOfSetter($oneOfFields)");
         --indent;
         push("});");
     });

index.d.ts

@@ -1378,7 +1378,9 @@ export class Root extends NamespaceBase {
     load(filename: (string|string[]), options?: ParseOptions): Promise<Root>;
 
     /**
-     * Synchronously loads one or multiple .proto or preprocessed .json files into this root namespace.
+     * Synchronously loads one or multiple .proto or preprocessed .json files into this root namespace (node only).
+     * @name Root#loadSync
+     * @function
      * @param {string|string[]} filename Names of one or multiple files to load
      * @param {ParseOptions} [options] Parse options. Defaults to {@link parse.defaults} when omitted.
      * @returns {Root} Root namespace
@@ -2358,6 +2360,20 @@ export namespace util {
      */
     function merge(dst: { [k: string]: any }, src: { [k: string]: any }, ifNotSet?: boolean): { [k: string]: any };
 
+    /**
+     * Builds a getter for a oneof's present field name.
+     * @param {string[]} fieldNames Field names
+     * @returns {function():string|undefined} Unbound getter
+     */
+    function oneOfGetter(fieldNames: string[]): () => any;
+
+    /**
+     * Builds a setter for a oneof's present field name.
+     * @param {string[]} fieldNames Field names
+     * @returns {function(?string):undefined} Unbound setter
+     */
+    function oneOfSetter(fieldNames: string[]): () => any;
+
     /**
      * A minimal UTF8 implementation for number arrays.
      * @memberof util

src/class.js

@@ -70,18 +70,8 @@ function create(type, ctor) {
     // Messages have non-enumerable getters and setters for each virtual oneof field
     type.oneofsArray.forEach(function(oneof) {
         Object.defineProperty(prototype, oneof.resolve().name, {
-            get: function() {
-                // > If the parser encounters multiple members of the same oneof on the wire, only the last member seen is used in the parsed message.
-                for (var keys = Object.keys(this), i = keys.length - 1; i > -1; --i)
-                    if (oneof.oneof.indexOf(keys[i]) > -1)
-                        return keys[i];
-                return undefined;
-            },
-            set: function(value) {
-                for (var keys = oneof.oneof, i = 0; i < keys.length; ++i)
-                    if (keys[i] !== value)
-                        delete this[keys[i]];
-            }
+            get: util.oneOfGetter(oneof.oneof),
+            set: util.oneOfSetter(oneof.oneof)
         });
     });