Fixed: Post-merge, also tackles #683

cli/targets/static.js

@@ -613,9 +613,15 @@ function buildEnum(ref, enm) {
     push(name(ref) + "." + name(enm.name) + " = (function() {");
     ++indent;
         push((config.es6 ? "const" : "var") + " valuesById = {}, values = Object.create(valuesById);");
+        var aliased = [];
         Object.keys(enm.values).forEach(function(key) {
             var val = enm.values[key];
-            push("values[valuesById[" + val + "] = " + JSON.stringify(key) + "] = " + val + ";");
+            if (aliased.indexOf(val) >= -1)
+                push("values[" + JSON.stringify(key) + "] = " + val + ";");
+            else {
+                push("values[valuesById[" + val + "] = " + JSON.stringify(key) + "] = " + val + ";");
+                aliased.push(val);
+            }
         });
         push("return values;");
     --indent;

index.d.ts

@@ -265,7 +265,7 @@ export class Field extends ReflectionObject {
      * Field rule, if any.
      * @type {string|undefined}
      */
-    rule: (string|undefined);
+    rule?: string;
 
     /**
      * Field type.
@@ -283,7 +283,7 @@ export class Field extends ReflectionObject {
      * Extended type if different from parent.
      * @type {string|undefined}
      */
-    extend: (string|undefined);
+    extend?: string;
 
     /**
      * Whether this field is required.
@@ -711,7 +711,7 @@ export class Method extends ReflectionObject {
      * Whether requests are streamed or not.
      * @type {boolean|undefined}
      */
-    requestStream: (boolean|undefined);
+    requestStream?: boolean;
 
     /**
      * Response type.
@@ -723,7 +723,7 @@ export class Method extends ReflectionObject {
      * Whether responses are streamed or not.
      * @type {boolean|undefined}
      */
-    responseStream: (boolean|undefined);
+    responseStream?: boolean;
 
     /**
      * Resolved request type.
@@ -806,7 +806,7 @@ export abstract class NamespaceBase extends ReflectionObject {
      * Nested objects by name.
      * @type {Object.<string,ReflectionObject>|undefined}
      */
-    nested: ({ [k: string]: ReflectionObject }|undefined);
+    nested?: { [k: string]: ReflectionObject };
 
     /**
      * Nested objects of this namespace as an array for iteration.
@@ -933,7 +933,7 @@ export abstract class ReflectionObject {
      * Options.
      * @type {Object.<string,*>|undefined}
      */
-    options: ({ [k: string]: any }|undefined);
+    options?: { [k: string]: any };
 
     /**
      * Unique name within its namespace.

src/enum.js

@@ -91,12 +91,14 @@ Enum.prototype.add = function(name, id, comment) {
     if (this.values[name] !== undefined)
         throw Error("duplicate name");
 
-    if (this.valuesById[id] !== undefined && !(this.options && this.options.allow_alias))
-        throw Error("duplicate id");
+    if (this.valuesById[id] !== undefined) {
+        if (!(this.options && this.options.allow_alias))
+            throw Error("duplicate id");
+        this.values[name] = id;
+    } else
+        this.valuesById[this.values[name] = id] = name;
 
-    this.valuesById[this.values[name] = id] = name;
     this.comments[name] = comment || null;
-
     return this;
 };
 

src/field.js

@@ -237,6 +237,10 @@ Field.prototype.resolve = function resolve() {
             this.typeDefault = this.resolvedType.values[this.typeDefault];
     }
 
+    // remove unnecessary packed option (parser adds this) if not referencing an enum
+    if (this.options && this.options.packed !== undefined && this.resolvedType && !(this.resolvedType instanceof Enum))
+        delete this.options.packed;
+
     // convert to internal data type if necesssary
     if (this.long) {
         this.typeDefault = util.Long.fromNumber(this.typeDefault, this.type.charAt(0) === "u");

src/parse.js

@@ -335,8 +335,10 @@ function parse(source, root, options) {
         if (!field.comment)
             field.comment = cmnt(trailingLine);
         // JSON defaults to packed=true if not set so we have to set packed=false explicity when
-        // parsing proto2 descriptors without the option, where applicable.
-        if (field.repeated && types.packed[type] !== undefined && !isProto3)
+        // parsing proto2 descriptors without the option, where applicable. This must be done for
+        // any type (not just packable types) because enums also use varint encoding and it is not
+        // yet known whether a type is an enum or not.
+        if (!isProto3 && field.repeated)
             field.setOption("packed", false, /* ifNotSet */ true);
         parent.add(field);
     }

src/root.js

@@ -173,7 +173,7 @@ Root.prototype.load = function load(filename, options, callback) {
                 if (err) {
                     if (!weak)
                         finish(err);
-                    else if (!queued)
+                    else /* istanbul ignore next */ if (!queued) // can't be covered reliably
                         finish(null, self);
                     return;
                 }

src/type.js

@@ -271,7 +271,7 @@ Type.prototype.add = function add(object) {
         // type instead.
 
         // avoids calling the getter if not absolutely necessary because it's called quite frequently
-        if (this._fieldsById ? this._fieldsById[object.id] : this.fieldsById[object.id])
+        if (this._fieldsById ? /* istanbul ignore next */ this._fieldsById[object.id] : this.fieldsById[object.id])
             throw Error("duplicate id " + object.id + " in " + this);
 
         if (object.parent)

tests/data/comments.js

@@ -32,19 +32,19 @@ $root.Test1 = (function() {
 
     /**
      * Field with a comment.
-     * @type {string}
+     * @type {string|undefined}
      */
     Test1.prototype.field1 = "";
 
     /**
      * Test1 field2.
-     * @type {number}
+     * @type {number|undefined}
      */
     Test1.prototype.field2 = 0;
 
     /**
      * Field with a comment and a <a href="http://example.com/foo/">link</a>
-     * @type {boolean}
+     * @type {boolean|undefined}
      */
     Test1.prototype.field3 = false;
 
@@ -132,6 +132,8 @@ $root.Test1 = (function() {
      * @returns {?string} `null` if valid, otherwise the reason why it is not
      */
     Test1.verify = function verify(message) {
+        if (typeof message !== "object" || message === null)
+            return "object expected";
         if (message.field1 !== undefined)
             if (!$util.isString(message.field1))
                 return "field1: string expected";
@@ -297,7 +299,9 @@ $root.Test2 = (function() {
      * @param {Test2|Object} message Test2 message or plain object to verify
      * @returns {?string} `null` if valid, otherwise the reason why it is not
      */
-    Test2.verify = function verify() {
+    Test2.verify = function verify(message) {
+        if (typeof message !== "object" || message === null)
+            return "object expected";
         return null;
     };
 
@@ -361,9 +365,9 @@ $root.Test2 = (function() {
  */
 $root.Test3 = (function() {
     var valuesById = {}, values = Object.create(valuesById);
-    values[valuesById[1] = "ONE"] = 1;
-    values[valuesById[2] = "TWO"] = 2;
-    values[valuesById[3] = "THREE"] = 3;
+    values["ONE"] = 1;
+    values["TWO"] = 2;
+    values["THREE"] = 3;
     return values;
 })();
 

tests/data/convert.js

@@ -28,55 +28,55 @@ $root.Message = (function() {
 
     /**
      * Message stringVal.
-     * @type {string}
+     * @type {string|undefined}
      */
     Message.prototype.stringVal = "";
 
     /**
      * Message stringRepeated.
-     * @type {Array.<string>}
+     * @type {Array.<string>|undefined}
      */
     Message.prototype.stringRepeated = $util.emptyArray;
 
     /**
      * Message uint64Val.
-     * @type {number|$protobuf.Long}
+     * @type {number|$protobuf.Long|undefined}
      */
     Message.prototype.uint64Val = $util.Long ? $util.Long.fromBits(0,0,true) : 0;
 
     /**
      * Message uint64Repeated.
-     * @type {Array.<number|$protobuf.Long>}
+     * @type {Array.<number|$protobuf.Long>|undefined}
      */
     Message.prototype.uint64Repeated = $util.emptyArray;
 
     /**
      * Message bytesVal.
-     * @type {Uint8Array}
+     * @type {Uint8Array|undefined}
      */
     Message.prototype.bytesVal = $util.newBuffer([]);
 
     /**
      * Message bytesRepeated.
-     * @type {Array.<Uint8Array>}
+     * @type {Array.<Uint8Array>|undefined}
      */
     Message.prototype.bytesRepeated = $util.emptyArray;
 
     /**
      * Message enumVal.
-     * @type {number}
+     * @type {number|undefined}
      */
     Message.prototype.enumVal = 1;
 
     /**
      * Message enumRepeated.
-     * @type {Array.<number>}
+     * @type {Array.<number>|undefined}
      */
     Message.prototype.enumRepeated = $util.emptyArray;
 
     /**
      * Message int64Map.
-     * @type {Object.<string,number|$protobuf.Long>}
+     * @type {Object.<string,number|$protobuf.Long>|undefined}
      */
     Message.prototype.int64Map = $util.emptyObject;
 
@@ -234,6 +234,8 @@ $root.Message = (function() {
      * @returns {?string} `null` if valid, otherwise the reason why it is not
      */
     Message.verify = function verify(message) {
+        if (typeof message !== "object" || message === null)
+            return "object expected";
         if (message.stringVal !== undefined)
             if (!$util.isString(message.stringVal))
                 return "stringVal: string expected";
@@ -504,8 +506,8 @@ $root.Message = (function() {
      */
     Message.SomeEnum = (function() {
         var valuesById = {}, values = Object.create(valuesById);
-        values[valuesById[1] = "ONE"] = 1;
-        values[valuesById[2] = "TWO"] = 2;
+        values["ONE"] = 1;
+        values["TWO"] = 2;
         return values;
     })();