From ed99b3c7a07b82f852cd42b309c7d386fa003a67 Mon Sep 17 00:00:00 2001 From: gideong Date: Thu, 18 Jan 2018 18:55:32 -0800 Subject: [PATCH 1/6] Preserve comments when serializing/deserializing with toJSON and fromJSON. --- src/enum.js | 18 +++++++++++++---- src/field.js | 14 ++++++++++--- src/method.js | 14 ++++++++++--- src/service.js | 4 +++- src/type.js | 5 ++++- tests/api_enum.js | 3 +++ tests/api_namespace.js | 4 ++-- tests/api_service.js | 5 +++-- tests/api_type.js | 5 +++-- tests/comment_serialization.js | 28 ++++++++++++++++++++++++++ tests/data/comment_serialization.proto | 19 +++++++++++++++++ 11 files changed, 101 insertions(+), 18 deletions(-) create mode 100644 tests/comment_serialization.js create mode 100644 tests/data/comment_serialization.proto diff --git a/src/enum.js b/src/enum.js index 59a6ef7f0..2a54380b0 100644 --- a/src/enum.js +++ b/src/enum.js @@ -16,8 +16,10 @@ var Namespace = require("./namespace"), * @param {string} name Unique name within its namespace * @param {Object.} [values] Enum values as an object, by name * @param {Object.} [options] Declared options + * @param {string} comment The comment for this enum + * @param {Object.} comments The value comments for this enum */ -function Enum(name, values, options) { +function Enum(name, values, options, comment, comments) { ReflectionObject.call(this, name, options); if (values && typeof values !== "object") @@ -35,11 +37,17 @@ function Enum(name, values, options) { */ this.values = Object.create(this.valuesById); // toJSON, marker + /** + * Enum comment text. + * @type {string|undefined} + */ + this.comment = comment; + /** * Value comment texts, if any. * @type {Object.} */ - this.comments = {}; + this.comments = comments || {}; /** * Reserved ranges, if any. @@ -72,7 +80,7 @@ function Enum(name, values, options) { * @throws {TypeError} If arguments are invalid */ Enum.fromJSON = function fromJSON(name, json) { - var enm = new Enum(name, json.values, json.options); + var enm = new Enum(name, json.values, json.options, json.comment, json.comments); enm.reserved = json.reserved; return enm; }; @@ -85,7 +93,9 @@ Enum.prototype.toJSON = function toJSON() { return util.toObject([ "options" , this.options, "values" , this.values, - "reserved" , this.reserved && this.reserved.length ? this.reserved : undefined + "reserved" , this.reserved && this.reserved.length ? this.reserved : undefined, + "comment" , this.comment, + "comments" , this.comments ]); }; diff --git a/src/field.js b/src/field.js index c33a34456..6716dad7b 100644 --- a/src/field.js +++ b/src/field.js @@ -35,7 +35,7 @@ var ruleRe = /^required|optional|repeated$/; * @throws {TypeError} If arguments are invalid */ Field.fromJSON = function fromJSON(name, json) { - return new Field(name, json.id, json.type, json.rule, json.extend, json.options); + return new Field(name, json.id, json.type, json.rule, json.extend, json.options, json.comment); }; /** @@ -50,8 +50,9 @@ Field.fromJSON = function fromJSON(name, json) { * @param {string|Object.} [rule="optional"] Field rule * @param {string|Object.} [extend] Extended type if different from parent * @param {Object.} [options] Declared options + * @param {string} comment Comment associated with this field */ -function Field(name, id, type, rule, extend, options) { +function Field(name, id, type, rule, extend, options, comment) { if (util.isObject(rule)) { options = rule; @@ -183,6 +184,12 @@ function Field(name, id, type, rule, extend, options) { * @private */ this._packed = null; + + /** + * Comment for this field. + * @type {string|undefined} + */ + this.comment = comment; } /** @@ -235,7 +242,8 @@ Field.prototype.toJSON = function toJSON() { "type" , this.type, "id" , this.id, "extend" , this.extend, - "options" , this.options + "options" , this.options, + "comment" , this.comment ]); }; diff --git a/src/method.js b/src/method.js index dac99b260..f28630e54 100644 --- a/src/method.js +++ b/src/method.js @@ -19,8 +19,9 @@ var util = require("./util"); * @param {boolean|Object.} [requestStream] Whether the request is streamed * @param {boolean|Object.} [responseStream] Whether the response is streamed * @param {Object.} [options] Declared options + * @param {string} comment The comment for this method */ -function Method(name, type, requestType, responseType, requestStream, responseStream, options) { +function Method(name, type, requestType, responseType, requestStream, responseStream, options, comment) { /* istanbul ignore next */ if (util.isObject(requestStream)) { @@ -86,6 +87,12 @@ function Method(name, type, requestType, responseType, requestStream, responseSt * @type {Type|null} */ this.resolvedResponseType = null; + + /** + * Comment for this method + * @type {string|undefined} + */ + this.comment = comment; } /** @@ -107,7 +114,7 @@ function Method(name, type, requestType, responseType, requestStream, responseSt * @throws {TypeError} If arguments are invalid */ Method.fromJSON = function fromJSON(name, json) { - return new Method(name, json.type, json.requestType, json.responseType, json.requestStream, json.responseStream, json.options); + return new Method(name, json.type, json.requestType, json.responseType, json.requestStream, json.responseStream, json.options, json.comment); }; /** @@ -121,7 +128,8 @@ Method.prototype.toJSON = function toJSON() { "requestStream" , this.requestStream, "responseType" , this.responseType, "responseStream" , this.responseStream, - "options" , this.options + "options" , this.options, + "comment" , this.comment ]); }; diff --git a/src/service.js b/src/service.js index cc419f6a7..3cd52f6d0 100644 --- a/src/service.js +++ b/src/service.js @@ -57,6 +57,7 @@ Service.fromJSON = function fromJSON(name, json) { service.add(Method.fromJSON(names[i], json.methods[names[i]])); if (json.nested) service.addJSON(json.nested); + service.comment = json.comment; return service; }; @@ -69,7 +70,8 @@ Service.prototype.toJSON = function toJSON() { return util.toObject([ "options" , inherited && inherited.options || undefined, "methods" , Namespace.arrayToJSON(this.methodsArray) || /* istanbul ignore next */ {}, - "nested" , inherited && inherited.nested || undefined + "nested" , inherited && inherited.nested || undefined, + "comment" , this.comment ]); }; diff --git a/src/type.js b/src/type.js index e22b65113..449f81ec7 100644 --- a/src/type.js +++ b/src/type.js @@ -270,6 +270,8 @@ Type.fromJSON = function fromJSON(name, json) { type.reserved = json.reserved; if (json.group) type.group = true; + if (json.comment) + type.comment = json.comment; return type; }; @@ -286,7 +288,8 @@ Type.prototype.toJSON = function toJSON() { "extensions" , this.extensions && this.extensions.length ? this.extensions : undefined, "reserved" , this.reserved && this.reserved.length ? this.reserved : undefined, "group" , this.group || undefined, - "nested" , inherited && inherited.nested || undefined + "nested" , inherited && inherited.nested || undefined, + "comment" , this.comment || undefined ]); }; diff --git a/tests/api_enum.js b/tests/api_enum.js index 705491331..2b0c0a6b0 100644 --- a/tests/api_enum.js +++ b/tests/api_enum.js @@ -71,6 +71,9 @@ tape.test("reflected enums", function(test) { values: { a: 1, c: 3 + }, + comments: { + c: null } }, "should export values to JSON"); diff --git a/tests/api_namespace.js b/tests/api_namespace.js index b7203da68..db3a26fd7 100644 --- a/tests/api_namespace.js +++ b/tests/api_namespace.js @@ -113,7 +113,7 @@ tape.test("reflected namespaces", function(test) { ns = protobuf.Namespace.fromJSON("My", { nested: { Message: { fields: {} }, - Enum: { values: {} }, + Enum: { values: {}, comments: {} }, Service: { methods: {} }, extensionField: { type: "string", id: 1000, extend: "Message" }, Other: { nested: {} } @@ -122,7 +122,7 @@ tape.test("reflected namespaces", function(test) { test.same(ns.toJSON(), { nested: { Message: { fields: {} }, - Enum: { values: {} }, + Enum: { values: {}, comments: {} }, Service: { methods: {} }, extensionField: { extend: "Message", id: 1000, type: "string" }, Other: { } diff --git a/tests/api_service.js b/tests/api_service.js index be857daa1..87b8f1c61 100644 --- a/tests/api_service.js +++ b/tests/api_service.js @@ -6,7 +6,8 @@ var def = { methods: {}, nested: { SomeEnum: { - values: {} + values: {}, + comments: {} } } }; @@ -44,4 +45,4 @@ tape.test("reflected services", function(test) { MyMethod = MyService.get("MyMethod"); test.end(); -}); \ No newline at end of file +}); diff --git a/tests/api_type.js b/tests/api_type.js index 593066190..e88c74c62 100644 --- a/tests/api_type.js +++ b/tests/api_type.js @@ -22,7 +22,8 @@ var def2 = { reserved: [[900, 999], "b"], nested: { Type: { - values: { ONE: 1, TWO: 2 } + values: { ONE: 1, TWO: 2 }, + comments: {} }, Service: { methods: {} @@ -74,7 +75,7 @@ tape.test("reflected types", function(test) { reserved: [[900, 999], "b"], nested: { Type: { fields: {} }, - Enum: { values: {} }, + Enum: { values: {}, comments: {} }, Service: { methods: {} }, extensionField: { extend: "Message", id: 1000, type: "string" }, Other: { } diff --git a/tests/comment_serialization.js b/tests/comment_serialization.js new file mode 100644 index 000000000..54471e93b --- /dev/null +++ b/tests/comment_serialization.js @@ -0,0 +1,28 @@ +var tape = require("tape"); + +var protobuf = require(".."); + +tape.test("preserve comments through de/serialization", function(test) { + test.plan(6); + protobuf.load("tests/data/comment_serialization.proto", function(err, root) { + if (err) { + throw test.fail(err.message); + } + + var copy = protobuf.Root.fromJSON(root.toJSON()); + test.equal(root.lookup("TestMessage").comment, copy.lookup("TestMessage").comment); + test.equal(root.lookup("TestMessage.testField").comment, copy.lookup("TestMessage.testField").comment); + + var rootService = root.lookupService("TestService"); + var copyService = copy.lookupService("TestService"); + test.equal(rootService.comment, copyService.comment); + test.equal(rootService.methods["testMethod"].comment, copyService.methods["testMethod"].comment); + + var rootEnum = root.lookup("TestEnum"); + var copyEnum = copy.lookup("TestEnum"); + test.equal(rootEnum.comment, copyEnum.comment); + test.equal(rootEnum.comments.VALUE, copyEnum.comments.VALUE); + + test.end(); + }); +}); diff --git a/tests/data/comment_serialization.proto b/tests/data/comment_serialization.proto new file mode 100644 index 000000000..05f2d358f --- /dev/null +++ b/tests/data/comment_serialization.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +/// message comment +message TestMessage { + /// field comment + string testField = 1; +} + +/// service comment +service TestService { + /// method comment + rpc testMethod(TestMessage) returns (TestMessage) {} +} + +/// enum comment +enum TestEnum { + /// enum value comment + VALUE = 1; +} From c4fe23eeeb3e74469964cc367fff32955986019a Mon Sep 17 00:00:00 2001 From: gideong Date: Tue, 6 Feb 2018 14:19:37 -0800 Subject: [PATCH 2/6] Adding an option keepComments for all toJSON implementations. --- src/enum.js | 8 +++-- src/field.js | 6 ++-- src/mapfield.js | 14 ++++++--- src/method.js | 6 ++-- src/namespace.js | 10 +++--- src/oneof.js | 18 ++++++++--- src/parse.js | 7 +++++ src/service.js | 10 +++--- src/type.js | 12 +++++--- tests/api_enum.js | 3 -- tests/api_namespace.js | 4 +-- tests/api_service.js | 3 +- tests/api_type.js | 5 ++- tests/comment_serialization.js | 42 ++++++++++++++++++++++++-- tests/data/comment_serialization.proto | 11 +++++++ 15 files changed, 118 insertions(+), 41 deletions(-) diff --git a/src/enum.js b/src/enum.js index 2a54380b0..12f55f616 100644 --- a/src/enum.js +++ b/src/enum.js @@ -87,15 +87,17 @@ Enum.fromJSON = function fromJSON(name, json) { /** * Converts this enum to an enum descriptor. + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {IEnum} Enum descriptor */ -Enum.prototype.toJSON = function toJSON() { +Enum.prototype.toJSON = function toJSON(toJSONOptions) { + var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; return util.toObject([ "options" , this.options, "values" , this.values, "reserved" , this.reserved && this.reserved.length ? this.reserved : undefined, - "comment" , this.comment, - "comments" , this.comments + "comment" , keepComments ? this.comment : undefined, + "comments" , keepComments ? this.comments : undefined ]); }; diff --git a/src/field.js b/src/field.js index 6716dad7b..f8199d0aa 100644 --- a/src/field.js +++ b/src/field.js @@ -234,16 +234,18 @@ Field.prototype.setOption = function setOption(name, value, ifNotSet) { /** * Converts this field to a field descriptor. + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {IField} Field descriptor */ -Field.prototype.toJSON = function toJSON() { +Field.prototype.toJSON = function toJSON(toJSONOptions) { + var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; return util.toObject([ "rule" , this.rule !== "optional" && this.rule || undefined, "type" , this.type, "id" , this.id, "extend" , this.extend, "options" , this.options, - "comment" , this.comment + "comment" , keepComments ? this.comment : undefined ]); }; diff --git a/src/mapfield.js b/src/mapfield.js index 7e1714fa3..86d145b9b 100644 --- a/src/mapfield.js +++ b/src/mapfield.js @@ -18,9 +18,10 @@ var types = require("./types"), * @param {string} keyType Key type * @param {string} type Value type * @param {Object.} [options] Declared options + * @param {string} comment Comment associated with this field */ -function MapField(name, id, keyType, type, options) { - Field.call(this, name, id, type, options); +function MapField(name, id, keyType, type, options, comment) { + Field.call(this, name, id, type, undefined, undefined, options, comment); /* istanbul ignore if */ if (!util.isString(keyType)) @@ -64,20 +65,23 @@ function MapField(name, id, keyType, type, options) { * @throws {TypeError} If arguments are invalid */ MapField.fromJSON = function fromJSON(name, json) { - return new MapField(name, json.id, json.keyType, json.type, json.options); + return new MapField(name, json.id, json.keyType, json.type, json.options, json.comment); }; /** * Converts this map field to a map field descriptor. + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {IMapField} Map field descriptor */ -MapField.prototype.toJSON = function toJSON() { +MapField.prototype.toJSON = function toJSON(toJSONOptions) { + var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; return util.toObject([ "keyType" , this.keyType, "type" , this.type, "id" , this.id, "extend" , this.extend, - "options" , this.options + "options" , this.options, + "comment" , keepComments ? this.comment : undefined ]); }; diff --git a/src/method.js b/src/method.js index f28630e54..25796bfac 100644 --- a/src/method.js +++ b/src/method.js @@ -119,9 +119,11 @@ Method.fromJSON = function fromJSON(name, json) { /** * Converts this method to a method descriptor. + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {IMethod} Method descriptor */ -Method.prototype.toJSON = function toJSON() { +Method.prototype.toJSON = function toJSON(toJSONOptions) { + var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; return util.toObject([ "type" , this.type !== "rpc" && /* istanbul ignore next */ this.type || undefined, "requestType" , this.requestType, @@ -129,7 +131,7 @@ Method.prototype.toJSON = function toJSON() { "responseType" , this.responseType, "responseStream" , this.responseStream, "options" , this.options, - "comment" , this.comment + "comment" , keepComments ? this.comment : undefined ]); }; diff --git a/src/namespace.js b/src/namespace.js index f1fc5f087..a5c3ed730 100644 --- a/src/namespace.js +++ b/src/namespace.js @@ -39,14 +39,15 @@ Namespace.fromJSON = function fromJSON(name, json) { * Converts an array of reflection objects to JSON. * @memberof Namespace * @param {ReflectionObject[]} array Object array + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {Object.|undefined} JSON object or `undefined` when array is empty */ -function arrayToJSON(array) { +function arrayToJSON(array, toJSONOptions) { if (!(array && array.length)) return undefined; var obj = {}; for (var i = 0; i < array.length; ++i) - obj[array[i].name] = array[i].toJSON(); + obj[array[i].name] = array[i].toJSON(toJSONOptions); return obj; } @@ -147,12 +148,13 @@ Object.defineProperty(Namespace.prototype, "nestedArray", { /** * Converts this namespace to a namespace descriptor. + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {INamespace} Namespace descriptor */ -Namespace.prototype.toJSON = function toJSON() { +Namespace.prototype.toJSON = function toJSON(toJSONOptions) { return util.toObject([ "options" , this.options, - "nested" , arrayToJSON(this.nestedArray) + "nested" , arrayToJSON(this.nestedArray, toJSONOptions) ]); }; diff --git a/src/oneof.js b/src/oneof.js index 70a0ebb01..562a1c9da 100644 --- a/src/oneof.js +++ b/src/oneof.js @@ -16,8 +16,9 @@ var Field = require("./field"), * @param {string} name Oneof name * @param {string[]|Object.} [fieldNames] Field names * @param {Object.} [options] Declared options + * @param {string} comment Comment associated with this field */ -function OneOf(name, fieldNames, options) { +function OneOf(name, fieldNames, options, comment) { if (!Array.isArray(fieldNames)) { options = fieldNames; fieldNames = undefined; @@ -40,6 +41,12 @@ function OneOf(name, fieldNames, options) { * @readonly */ this.fieldsArray = []; // declared readonly for conformance, possibly not yet added to parent + + /** + * Comment for this field. + * @type {string|undefined} + */ + this.comment = comment; } /** @@ -57,17 +64,20 @@ function OneOf(name, fieldNames, options) { * @throws {TypeError} If arguments are invalid */ OneOf.fromJSON = function fromJSON(name, json) { - return new OneOf(name, json.oneof, json.options); + return new OneOf(name, json.oneof, json.options, json.comment); }; /** * Converts this oneof to a oneof descriptor. + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {IOneOf} Oneof descriptor */ -OneOf.prototype.toJSON = function toJSON() { +OneOf.prototype.toJSON = function toJSON(toJSONOptions) { + var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; return util.toObject([ "options" , this.options, - "oneof" , this.oneof + "oneof" , this.oneof, + "comment" , keepComments ? this.comment : undefined ]); }; diff --git a/src/parse.js b/src/parse.js index e1e572de2..c57c8f5e7 100644 --- a/src/parse.js +++ b/src/parse.js @@ -41,6 +41,13 @@ var base10Re = /^[1-9][0-9]*$/, * Options modifying the behavior of {@link parse}. * @interface IParseOptions * @property {boolean} [keepCase=false] Keeps field casing instead of converting to camel case + * @property {boolean} [alternateCommentMode=false] Recognize double-slash comments in addition to doc-block comments. + */ + +/** + * Options modifying the behavior of JSON serialization. + * @interface IToJSONOptions + * @property {boolean} [keepComments=false] Serializes comments. */ /** diff --git a/src/service.js b/src/service.js index 3cd52f6d0..e180192c9 100644 --- a/src/service.js +++ b/src/service.js @@ -63,15 +63,17 @@ Service.fromJSON = function fromJSON(name, json) { /** * Converts this service to a service descriptor. + * @param {IToJSONOptions} [options] JSON conversion options * @returns {IService} Service descriptor */ -Service.prototype.toJSON = function toJSON() { - var inherited = Namespace.prototype.toJSON.call(this); +Service.prototype.toJSON = function toJSON(toJSONOptions) { + var inherited = Namespace.prototype.toJSON.call(this, toJSONOptions); + var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; return util.toObject([ "options" , inherited && inherited.options || undefined, - "methods" , Namespace.arrayToJSON(this.methodsArray) || /* istanbul ignore next */ {}, + "methods" , Namespace.arrayToJSON(this.methodsArray, toJSONOptions) || /* istanbul ignore next */ {}, "nested" , inherited && inherited.nested || undefined, - "comment" , this.comment + "comment" , keepComments ? this.comment : undefined ]); }; diff --git a/src/type.js b/src/type.js index 449f81ec7..76d3b5f8a 100644 --- a/src/type.js +++ b/src/type.js @@ -277,19 +277,21 @@ Type.fromJSON = function fromJSON(name, json) { /** * Converts this message type to a message type descriptor. + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {IType} Message type descriptor */ -Type.prototype.toJSON = function toJSON() { - var inherited = Namespace.prototype.toJSON.call(this); +Type.prototype.toJSON = function toJSON(toJSONOptions) { + var inherited = Namespace.prototype.toJSON.call(this, toJSONOptions); + var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; return util.toObject([ "options" , inherited && inherited.options || undefined, - "oneofs" , Namespace.arrayToJSON(this.oneofsArray), - "fields" , Namespace.arrayToJSON(this.fieldsArray.filter(function(obj) { return !obj.declaringField; })) || {}, + "oneofs" , Namespace.arrayToJSON(this.oneofsArray, toJSONOptions), + "fields" , Namespace.arrayToJSON(this.fieldsArray.filter(function(obj) { return !obj.declaringField; }), toJSONOptions) || {}, "extensions" , this.extensions && this.extensions.length ? this.extensions : undefined, "reserved" , this.reserved && this.reserved.length ? this.reserved : undefined, "group" , this.group || undefined, "nested" , inherited && inherited.nested || undefined, - "comment" , this.comment || undefined + "comment" , keepComments ? this.comment : undefined ]); }; diff --git a/tests/api_enum.js b/tests/api_enum.js index 2b0c0a6b0..705491331 100644 --- a/tests/api_enum.js +++ b/tests/api_enum.js @@ -71,9 +71,6 @@ tape.test("reflected enums", function(test) { values: { a: 1, c: 3 - }, - comments: { - c: null } }, "should export values to JSON"); diff --git a/tests/api_namespace.js b/tests/api_namespace.js index db3a26fd7..b7203da68 100644 --- a/tests/api_namespace.js +++ b/tests/api_namespace.js @@ -113,7 +113,7 @@ tape.test("reflected namespaces", function(test) { ns = protobuf.Namespace.fromJSON("My", { nested: { Message: { fields: {} }, - Enum: { values: {}, comments: {} }, + Enum: { values: {} }, Service: { methods: {} }, extensionField: { type: "string", id: 1000, extend: "Message" }, Other: { nested: {} } @@ -122,7 +122,7 @@ tape.test("reflected namespaces", function(test) { test.same(ns.toJSON(), { nested: { Message: { fields: {} }, - Enum: { values: {}, comments: {} }, + Enum: { values: {} }, Service: { methods: {} }, extensionField: { extend: "Message", id: 1000, type: "string" }, Other: { } diff --git a/tests/api_service.js b/tests/api_service.js index 87b8f1c61..805caf7d5 100644 --- a/tests/api_service.js +++ b/tests/api_service.js @@ -6,8 +6,7 @@ var def = { methods: {}, nested: { SomeEnum: { - values: {}, - comments: {} + values: {} } } }; diff --git a/tests/api_type.js b/tests/api_type.js index e88c74c62..593066190 100644 --- a/tests/api_type.js +++ b/tests/api_type.js @@ -22,8 +22,7 @@ var def2 = { reserved: [[900, 999], "b"], nested: { Type: { - values: { ONE: 1, TWO: 2 }, - comments: {} + values: { ONE: 1, TWO: 2 } }, Service: { methods: {} @@ -75,7 +74,7 @@ tape.test("reflected types", function(test) { reserved: [[900, 999], "b"], nested: { Type: { fields: {} }, - Enum: { values: {}, comments: {} }, + Enum: { values: {} }, Service: { methods: {} }, extensionField: { extend: "Message", id: 1000, type: "string" }, Other: { } diff --git a/tests/comment_serialization.js b/tests/comment_serialization.js index 54471e93b..f696d24c7 100644 --- a/tests/comment_serialization.js +++ b/tests/comment_serialization.js @@ -2,16 +2,54 @@ var tape = require("tape"); var protobuf = require(".."); -tape.test("preserve comments through de/serialization", function(test) { - test.plan(6); +tape.test("by default, drop comments through de/serialization", function(test) { + test.plan(16); protobuf.load("tests/data/comment_serialization.proto", function(err, root) { if (err) { throw test.fail(err.message); } var copy = protobuf.Root.fromJSON(root.toJSON()); + test.ok(root.lookup("TestMessage").comment); + test.notOk(copy.lookup("TestMessage").comment); + test.ok(root.lookup("TestMessage.testField").comment); + test.notOk(copy.lookup("TestMessage.testField").comment); + test.ok(root.lookup("TestMessage.testMap").comment); + test.notOk(copy.lookup("TestMessage.testMap").comment); + test.ok(root.lookup("TestMessage.testOneof").comment); + test.notOk(copy.lookup("TestMessage.testOneof").comment); + + var rootService = root.lookupService("TestService"); + var copyService = copy.lookupService("TestService"); + test.ok(rootService.comment); + test.notOk(copyService.comment); + test.ok(rootService.methods["testMethod"].comment); + test.notOk(copyService.methods["testMethod"].comment); + + var rootEnum = root.lookup("TestEnum"); + var copyEnum = copy.lookup("TestEnum"); + test.ok(rootEnum.comment); + test.notOk(copyEnum.comment); + test.ok(rootEnum.comments.VALUE); + test.notOk(copyEnum.comments.VALUE); + + test.end(); + }); +}); + +tape.test("preserve comments through de/serialization if option set", function(test) { + test.plan(8); + protobuf.load("tests/data/comment_serialization.proto", function(err, root) { + if (err) { + throw test.fail(err.message); + } + + var toJSONOptions = {keepComments: true}; + var copy = protobuf.Root.fromJSON(root.toJSON(toJSONOptions)); test.equal(root.lookup("TestMessage").comment, copy.lookup("TestMessage").comment); test.equal(root.lookup("TestMessage.testField").comment, copy.lookup("TestMessage.testField").comment); + test.equal(root.lookup("TestMessage.testMap").comment, copy.lookup("TestMessage.testMap").comment); + test.equal(root.lookup("TestMessage.testOneof").comment, copy.lookup("TestMessage.testOneof").comment); var rootService = root.lookupService("TestService"); var copyService = copy.lookupService("TestService"); diff --git a/tests/data/comment_serialization.proto b/tests/data/comment_serialization.proto index 05f2d358f..8ddce09e5 100644 --- a/tests/data/comment_serialization.proto +++ b/tests/data/comment_serialization.proto @@ -4,6 +4,17 @@ syntax = "proto3"; message TestMessage { /// field comment string testField = 1; + + /// map field comment + map testMap = 2; + + /// oneof field comment + oneof testOneof { + /// oneof string comment + string testOneofString = 3; + /// oneof bool comment + bool testOneofBool = 4; + } } /// service comment From 7ba9bf7d42ad4f4b5d9bf70dd077f01a4c8ce0ae Mon Sep 17 00:00:00 2001 From: gideong Date: Tue, 6 Feb 2018 14:34:13 -0800 Subject: [PATCH 3/6] Fix code climate issues. --- src/enum.js | 2 +- src/field.js | 2 +- src/mapfield.js | 2 +- src/method.js | 2 +- src/oneof.js | 2 +- src/service.js | 4 ++-- src/type.js | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/enum.js b/src/enum.js index 12f55f616..6967c46bd 100644 --- a/src/enum.js +++ b/src/enum.js @@ -91,7 +91,7 @@ Enum.fromJSON = function fromJSON(name, json) { * @returns {IEnum} Enum descriptor */ Enum.prototype.toJSON = function toJSON(toJSONOptions) { - var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; + var keepComments = toJSONOptions ? Boolean(toJSONOptions.keepComments) : false; return util.toObject([ "options" , this.options, "values" , this.values, diff --git a/src/field.js b/src/field.js index f8199d0aa..8790d56bf 100644 --- a/src/field.js +++ b/src/field.js @@ -238,7 +238,7 @@ Field.prototype.setOption = function setOption(name, value, ifNotSet) { * @returns {IField} Field descriptor */ Field.prototype.toJSON = function toJSON(toJSONOptions) { - var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; + var keepComments = toJSONOptions ? Boolean(toJSONOptions.keepComments) : false; return util.toObject([ "rule" , this.rule !== "optional" && this.rule || undefined, "type" , this.type, diff --git a/src/mapfield.js b/src/mapfield.js index 86d145b9b..0097c27ef 100644 --- a/src/mapfield.js +++ b/src/mapfield.js @@ -74,7 +74,7 @@ MapField.fromJSON = function fromJSON(name, json) { * @returns {IMapField} Map field descriptor */ MapField.prototype.toJSON = function toJSON(toJSONOptions) { - var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; + var keepComments = toJSONOptions ? Boolean(toJSONOptions.keepComments) : false; return util.toObject([ "keyType" , this.keyType, "type" , this.type, diff --git a/src/method.js b/src/method.js index 25796bfac..9b2abf386 100644 --- a/src/method.js +++ b/src/method.js @@ -123,7 +123,7 @@ Method.fromJSON = function fromJSON(name, json) { * @returns {IMethod} Method descriptor */ Method.prototype.toJSON = function toJSON(toJSONOptions) { - var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; + var keepComments = toJSONOptions ? Boolean(toJSONOptions.keepComments) : false; return util.toObject([ "type" , this.type !== "rpc" && /* istanbul ignore next */ this.type || undefined, "requestType" , this.requestType, diff --git a/src/oneof.js b/src/oneof.js index 562a1c9da..a443bc4ec 100644 --- a/src/oneof.js +++ b/src/oneof.js @@ -73,7 +73,7 @@ OneOf.fromJSON = function fromJSON(name, json) { * @returns {IOneOf} Oneof descriptor */ OneOf.prototype.toJSON = function toJSON(toJSONOptions) { - var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; + var keepComments = toJSONOptions ? Boolean(toJSONOptions.keepComments) : false; return util.toObject([ "options" , this.options, "oneof" , this.oneof, diff --git a/src/service.js b/src/service.js index e180192c9..bc2c3080c 100644 --- a/src/service.js +++ b/src/service.js @@ -63,12 +63,12 @@ Service.fromJSON = function fromJSON(name, json) { /** * Converts this service to a service descriptor. - * @param {IToJSONOptions} [options] JSON conversion options + * @param {IToJSONOptions} [toJSONOptions] JSON conversion options * @returns {IService} Service descriptor */ Service.prototype.toJSON = function toJSON(toJSONOptions) { var inherited = Namespace.prototype.toJSON.call(this, toJSONOptions); - var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; + var keepComments = toJSONOptions ? Boolean(toJSONOptions.keepComments) : false; return util.toObject([ "options" , inherited && inherited.options || undefined, "methods" , Namespace.arrayToJSON(this.methodsArray, toJSONOptions) || /* istanbul ignore next */ {}, diff --git a/src/type.js b/src/type.js index 76d3b5f8a..2e7bda49b 100644 --- a/src/type.js +++ b/src/type.js @@ -282,7 +282,7 @@ Type.fromJSON = function fromJSON(name, json) { */ Type.prototype.toJSON = function toJSON(toJSONOptions) { var inherited = Namespace.prototype.toJSON.call(this, toJSONOptions); - var keepComments = toJSONOptions ? (!!toJSONOptions.keepComments) : false; + var keepComments = toJSONOptions ? Boolean(toJSONOptions.keepComments) : false; return util.toObject([ "options" , inherited && inherited.options || undefined, "oneofs" , Namespace.arrayToJSON(this.oneofsArray, toJSONOptions), From ac36320309a83c7f4a794b208e8dda0f2d7fd7e8 Mon Sep 17 00:00:00 2001 From: gideong Date: Tue, 6 Feb 2018 15:28:32 -0800 Subject: [PATCH 4/6] Fixing Field constructor type shuffling for comment. --- src/field.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/field.js b/src/field.js index 8790d56bf..3396e9773 100644 --- a/src/field.js +++ b/src/field.js @@ -55,9 +55,11 @@ Field.fromJSON = function fromJSON(name, json) { function Field(name, id, type, rule, extend, options, comment) { if (util.isObject(rule)) { + comment = extend; options = rule; rule = extend = undefined; } else if (util.isObject(extend)) { + comment = options options = extend; extend = undefined; } From a5e4048e7824a58255363872cb598f90f031ed8e Mon Sep 17 00:00:00 2001 From: gideong Date: Wed, 7 Feb 2018 10:37:08 -0800 Subject: [PATCH 5/6] CodeClimate fix: missing semicolon. --- src/field.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/field.js b/src/field.js index 3396e9773..0e8abcf32 100644 --- a/src/field.js +++ b/src/field.js @@ -59,7 +59,7 @@ function Field(name, id, type, rule, extend, options, comment) { options = rule; rule = extend = undefined; } else if (util.isObject(extend)) { - comment = options + comment = options; options = extend; extend = undefined; } From 34d532705896bde60c0ca8217da0e18b543af3fa Mon Sep 17 00:00:00 2001 From: gideong Date: Wed, 7 Feb 2018 12:29:23 -0800 Subject: [PATCH 6/6] Updating types, fixing some type comments. --- index.d.ts | 65 ++++++++++++++++++++++++++++++++++++++----------- src/enum.js | 6 ++--- src/field.js | 4 +-- src/mapfield.js | 2 +- src/method.js | 4 +-- src/oneof.js | 4 +-- 6 files changed, 61 insertions(+), 24 deletions(-) diff --git a/index.d.ts b/index.d.ts index 5b5406703..66d0e06b2 100644 --- a/index.d.ts +++ b/index.d.ts @@ -156,8 +156,10 @@ export class Enum extends ReflectionObject { * @param name Unique name within its namespace * @param [values] Enum values as an object, by name * @param [options] Declared options + * @param [comment] The comment for this enum + * @param [comments] The value comments for this enum */ - constructor(name: string, values?: { [k: string]: number }, options?: { [k: string]: any }); + constructor(name: string, values?: { [k: string]: number }, options?: { [k: string]: any }, comment?: string, comments?: { [k: string]: string }); /** Enum values by id. */ public valuesById: { [k: number]: string }; @@ -165,6 +167,9 @@ export class Enum extends ReflectionObject { /** Enum values by name. */ public values: { [k: string]: number }; + /** Enum comment text. */ + public comment: (string|null); + /** Value comment texts, if any. */ public comments: { [k: string]: string }; @@ -182,9 +187,10 @@ export class Enum extends ReflectionObject { /** * Converts this enum to an enum descriptor. + * @param [toJSONOptions] JSON conversion options * @returns Enum descriptor */ - public toJSON(): IEnum; + public toJSON(toJSONOptions?: IToJSONOptions): IEnum; /** * Adds a value to this enum. @@ -288,8 +294,9 @@ export class FieldBase extends ReflectionObject { * @param [rule="optional"] Field rule * @param [extend] Extended type if different from parent * @param [options] Declared options + * @param [comment] Comment associated with this field */ - constructor(name: string, id: number, type: string, rule?: (string|{ [k: string]: any }), extend?: (string|{ [k: string]: any }), options?: { [k: string]: any }); + constructor(name: string, id: number, type: string, rule?: (string|{ [k: string]: any }), extend?: (string|{ [k: string]: any }), options?: { [k: string]: any }, comment?: string); /** Field rule, if any. */ public rule?: string; @@ -342,11 +349,15 @@ export class FieldBase extends ReflectionObject { /** Sister-field within the declaring namespace if an extended field. */ public declaringField: (Field|null); + /** Comment for this field. */ + public comment: (string|null); + /** * Converts this field to a field descriptor. + * @param [toJSONOptions] JSON conversion options * @returns Field descriptor */ - public toJSON(): IField; + public toJSON(toJSONOptions?: IToJSONOptions): IField; /** * Resolves this field's type references. @@ -445,8 +456,9 @@ export class MapField extends FieldBase { * @param keyType Key type * @param type Value type * @param [options] Declared options + * @param [comment] Comment associated with this field */ - constructor(name: string, id: number, keyType: string, type: string, options?: { [k: string]: any }); + constructor(name: string, id: number, keyType: string, type: string, options?: { [k: string]: any }, comment?: string); /** Key type. */ public keyType: string; @@ -465,9 +477,10 @@ export class MapField extends FieldBase { /** * Converts this map field to a map field descriptor. + * @param [toJSONOptions] JSON conversion options * @returns Map field descriptor */ - public toJSON(): IMapField; + public toJSON(toJSONOptions?: IToJSONOptions): IMapField; /** * Map field decorator (TypeScript). @@ -586,8 +599,9 @@ export class Method extends ReflectionObject { * @param [requestStream] Whether the request is streamed * @param [responseStream] Whether the response is streamed * @param [options] Declared options + * @param [comment] The comment for this method */ - constructor(name: string, type: (string|undefined), requestType: string, responseType: string, requestStream?: (boolean|{ [k: string]: any }), responseStream?: (boolean|{ [k: string]: any }), options?: { [k: string]: any }); + constructor(name: string, type: (string|undefined), requestType: string, responseType: string, requestStream?: (boolean|{ [k: string]: any }), responseStream?: (boolean|{ [k: string]: any }), options?: { [k: string]: any }, comment?: string); /** Method type. */ public type: string; @@ -610,6 +624,9 @@ export class Method extends ReflectionObject { /** Resolved response type. */ public resolvedResponseType: (Type|null); + /** Comment for this method */ + public comment: (string|null); + /** * Constructs a method from a method descriptor. * @param name Method name @@ -621,9 +638,10 @@ export class Method extends ReflectionObject { /** * Converts this method to a method descriptor. + * @param [toJSONOptions] JSON conversion options * @returns Method descriptor */ - public toJSON(): IMethod; + public toJSON(toJSONOptions?: IToJSONOptions): IMethod; } /** Method descriptor. */ @@ -670,9 +688,10 @@ export class Namespace extends NamespaceBase { /** * Converts an array of reflection objects to JSON. * @param array Object array + * @param [toJSONOptions] JSON conversion options * @returns JSON object or `undefined` when array is empty */ - public static arrayToJSON(array: ReflectionObject[]): ({ [k: string]: any }|undefined); + public static arrayToJSON(array: ReflectionObject[], toJSONOptions?: IToJSONOptions): ({ [k: string]: any }|undefined); /** * Tests if the specified id is reserved. @@ -702,9 +721,10 @@ export abstract class NamespaceBase extends ReflectionObject { /** * Converts this namespace to a namespace descriptor. + * @param [toJSONOptions] JSON conversion options * @returns Namespace descriptor */ - public toJSON(): INamespace; + public toJSON(toJSONOptions?: IToJSONOptions): INamespace; /** * Adds nested objects to this namespace from nested object descriptors. @@ -921,8 +941,9 @@ export class OneOf extends ReflectionObject { * @param name Oneof name * @param [fieldNames] Field names * @param [options] Declared options + * @param [comment] Comment associated with this field */ - constructor(name: string, fieldNames?: (string[]|{ [k: string]: any }), options?: { [k: string]: any }); + constructor(name: string, fieldNames?: (string[]|{ [k: string]: any }), options?: { [k: string]: any }, comment?: string); /** Field names that belong to this oneof. */ public oneof: string[]; @@ -930,6 +951,9 @@ export class OneOf extends ReflectionObject { /** Fields that belong to this oneof as an array for iteration. */ public readonly fieldsArray: Field[]; + /** Comment for this field. */ + public comment: (string|null); + /** * Constructs a oneof from a oneof descriptor. * @param name Oneof name @@ -941,9 +965,10 @@ export class OneOf extends ReflectionObject { /** * Converts this oneof to a oneof descriptor. + * @param [toJSONOptions] JSON conversion options * @returns Oneof descriptor */ - public toJSON(): IOneOf; + public toJSON(toJSONOptions?: IToJSONOptions): IOneOf; /** * Adds a field to this oneof and removes it from its current parent, if any. @@ -1016,6 +1041,16 @@ export interface IParseOptions { /** Keeps field casing instead of converting to camel case */ keepCase?: boolean; + + /** Recognize double-slash comments in addition to doc-block comments. */ + alternateCommentMode?: boolean; +} + +/** Options modifying the behavior of JSON serialization. */ +export interface IToJSONOptions { + + /** Serializes comments. */ + keepComments?: boolean; } /** @@ -1345,9 +1380,10 @@ export class Service extends NamespaceBase { /** * Converts this service to a service descriptor. + * @param [toJSONOptions] JSON conversion options * @returns Service descriptor */ - public toJSON(): IService; + public toJSON(toJSONOptions?: IToJSONOptions): IService; /** Methods of this service as an array for iteration. */ public readonly methodsArray: Method[]; @@ -1497,9 +1533,10 @@ export class Type extends NamespaceBase { /** * Converts this message type to a message type descriptor. + * @param [toJSONOptions] JSON conversion options * @returns Message type descriptor */ - public toJSON(): IType; + public toJSON(toJSONOptions?: IToJSONOptions): IType; /** * Adds a nested object to this type. diff --git a/src/enum.js b/src/enum.js index 6967c46bd..b11014be3 100644 --- a/src/enum.js +++ b/src/enum.js @@ -16,8 +16,8 @@ var Namespace = require("./namespace"), * @param {string} name Unique name within its namespace * @param {Object.} [values] Enum values as an object, by name * @param {Object.} [options] Declared options - * @param {string} comment The comment for this enum - * @param {Object.} comments The value comments for this enum + * @param {string} [comment] The comment for this enum + * @param {Object.} [comments] The value comments for this enum */ function Enum(name, values, options, comment, comments) { ReflectionObject.call(this, name, options); @@ -39,7 +39,7 @@ function Enum(name, values, options, comment, comments) { /** * Enum comment text. - * @type {string|undefined} + * @type {string|null} */ this.comment = comment; diff --git a/src/field.js b/src/field.js index 0e8abcf32..9e6f5a6b8 100644 --- a/src/field.js +++ b/src/field.js @@ -50,7 +50,7 @@ Field.fromJSON = function fromJSON(name, json) { * @param {string|Object.} [rule="optional"] Field rule * @param {string|Object.} [extend] Extended type if different from parent * @param {Object.} [options] Declared options - * @param {string} comment Comment associated with this field + * @param {string} [comment] Comment associated with this field */ function Field(name, id, type, rule, extend, options, comment) { @@ -189,7 +189,7 @@ function Field(name, id, type, rule, extend, options, comment) { /** * Comment for this field. - * @type {string|undefined} + * @type {string|null} */ this.comment = comment; } diff --git a/src/mapfield.js b/src/mapfield.js index 0097c27ef..67c70978f 100644 --- a/src/mapfield.js +++ b/src/mapfield.js @@ -18,7 +18,7 @@ var types = require("./types"), * @param {string} keyType Key type * @param {string} type Value type * @param {Object.} [options] Declared options - * @param {string} comment Comment associated with this field + * @param {string} [comment] Comment associated with this field */ function MapField(name, id, keyType, type, options, comment) { Field.call(this, name, id, type, undefined, undefined, options, comment); diff --git a/src/method.js b/src/method.js index 9b2abf386..f5159225f 100644 --- a/src/method.js +++ b/src/method.js @@ -19,7 +19,7 @@ var util = require("./util"); * @param {boolean|Object.} [requestStream] Whether the request is streamed * @param {boolean|Object.} [responseStream] Whether the response is streamed * @param {Object.} [options] Declared options - * @param {string} comment The comment for this method + * @param {string} [comment] The comment for this method */ function Method(name, type, requestType, responseType, requestStream, responseStream, options, comment) { @@ -90,7 +90,7 @@ function Method(name, type, requestType, responseType, requestStream, responseSt /** * Comment for this method - * @type {string|undefined} + * @type {string|null} */ this.comment = comment; } diff --git a/src/oneof.js b/src/oneof.js index a443bc4ec..ba0e90279 100644 --- a/src/oneof.js +++ b/src/oneof.js @@ -16,7 +16,7 @@ var Field = require("./field"), * @param {string} name Oneof name * @param {string[]|Object.} [fieldNames] Field names * @param {Object.} [options] Declared options - * @param {string} comment Comment associated with this field + * @param {string} [comment] Comment associated with this field */ function OneOf(name, fieldNames, options, comment) { if (!Array.isArray(fieldNames)) { @@ -44,7 +44,7 @@ function OneOf(name, fieldNames, options, comment) { /** * Comment for this field. - * @type {string|undefined} + * @type {string|null} */ this.comment = comment; }