From ffb5ad49bfa8665758220082d6b7a6bee7a0eceb Mon Sep 17 00:00:00 2001 From: eyalp Date: Sun, 23 Jun 2019 20:49:38 +0300 Subject: [PATCH 1/9] Add option parsing tests to ensure no regressions are caused --- tests/comp_options-parse.js | 72 +++++++++++++++++++++++ tests/data/options_test.proto | 108 ++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 tests/comp_options-parse.js create mode 100644 tests/data/options_test.proto diff --git a/tests/comp_options-parse.js b/tests/comp_options-parse.js new file mode 100644 index 000000000..db65dabcd --- /dev/null +++ b/tests/comp_options-parse.js @@ -0,0 +1,72 @@ +var tape = require("tape"); +var protobuf = require(".."); + +tape.test("Options", function (test) { + var root = protobuf.loadSync("tests/data/options_test.proto"); + + test.test(test.name + " - field options (Int)", function (test) { + var TestFieldOptionsInt = root.lookup("TestFieldOptionsInt"); + test.equal(TestFieldOptionsInt.fields.field1.options["(fo_rep_int)"], 2, "should take second repeated int option"); + test.equal(TestFieldOptionsInt.fields.field2.options["(fo_single_int)"], 3, "should correctly parse single int option"); + test.end(); + }); + + test.test(test.name + " - message options (Int)", function (test) { + var TestMessageOptionsInt = root.lookup("TestMessageOptionsInt"); + test.equal(TestMessageOptionsInt.options["(mo_rep_int)"], 2, "should take second repeated int message option"); + test.equal(TestMessageOptionsInt.options["(mo_single_int)"], 3, "should correctly parse single int message option"); + test.end(); + }); + + test.test(test.name + " - field options (Message)", function (test) { + var TestFieldOptionsMsg = root.lookup("TestFieldOptionsMsg"); + test.equal(TestFieldOptionsMsg.fields.field1.options["(fo_rep_msg).value"], 4, "should take second repeated message option"); + test.equal(TestFieldOptionsMsg.fields.field1.options["(fo_rep_msg).rep_value"], 6, "should take second repeated int in second repeated option"); + test.equal(TestFieldOptionsMsg.fields.field2.options["(fo_single_msg).value"], 7, "should correctly parse single msg option"); + test.equal(TestFieldOptionsMsg.fields.field2.options["(fo_single_msg).rep_value"], 9, "should take second repeated int in single msg option"); + test.end(); + }); + + test.test(test.name + " - message options (Message)", function (test) { + var TestMessageOptionsMsg = root.lookup("TestMessageOptionsMsg"); + test.equal(TestMessageOptionsMsg.options["(mo_rep_msg).value"], 4, "should take second repeated message option"); + test.equal(TestMessageOptionsMsg.options["(mo_rep_msg).rep_value"], 6, "should take second repeated int in second repeated option"); + test.equal(TestMessageOptionsMsg.options["(mo_single_msg).value"], 7, "should correctly parse single msg option"); + test.equal(TestMessageOptionsMsg.options["(mo_single_msg).rep_value"], 9, "should take second repeated int in single msg option"); + test.end(); + }); + + test.test(test.name + " - field options (Nested)", function (test) { + var TestFieldOptionsNested = root.lookup("TestFieldOptionsNested"); + test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).value"], 1, "should merge repeated options messages"); + test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).rep_value"], 3, "should parse in any order"); + test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).nested.nested.value"], "x", "should correctly parse nested field options"); + test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).rep_nested.value"], "z", "should take second repeated nested options"); + test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).nested.value"], "w", "should merge nested options"); + + test.equal(TestFieldOptionsNested.fields.field2.options["(fo_single_msg).nested.value"], "x", "should correctly parse nested property name"); + test.equal(TestFieldOptionsNested.fields.field2.options["(fo_single_msg).rep_nested.value"], "y", "should take second repeated nested options"); + + test.equal(TestFieldOptionsNested.fields.field3.options["(fo_single_msg).nested.value"], "x", "should correctly parse nested field options"); + test.equal(TestFieldOptionsNested.fields.field3.options["(fo_single_msg).nested.nested.nested.value"], "y", "should correctly parse several nesting levels"); + + test.end(); + }); + + test.test(test.name + " - message options (Nested)", function (test) { + var TestMessageOptionsNested = root.lookup("TestMessageOptionsNested"); + test.equal(TestMessageOptionsNested.options["(mo_rep_msg).value"], 1, "should merge repeated options messages"); + test.equal(TestMessageOptionsNested.options["(mo_rep_msg).rep_value"], 3, "should parse in any order"); + test.equal(TestMessageOptionsNested.options["(mo_rep_msg).nested.nested.value"], "x", "should correctly parse nested field options"); + test.equal(TestMessageOptionsNested.options["(mo_rep_msg).rep_nested.value"], "z", "should take second repeated nested options"); + test.equal(TestMessageOptionsNested.options["(mo_rep_msg).nested.value"], "w", "should merge nested options"); + + test.equal(TestMessageOptionsNested.options["(mo_single_msg).nested.value"], "x", "should correctly parse nested property name"); + test.equal(TestMessageOptionsNested.options["(mo_single_msg).rep_nested.value"], "y", "should take second repeated nested options"); + test.equal(TestMessageOptionsNested.options["(mo_single_msg).rep_nested.nested.nested.value"], "y", "should correctly parse several nesting levels"); + + test.end(); + }); + + test.end(); +}); diff --git a/tests/data/options_test.proto b/tests/data/options_test.proto new file mode 100644 index 000000000..1bdc0e470 --- /dev/null +++ b/tests/data/options_test.proto @@ -0,0 +1,108 @@ +syntax = "proto3"; + +import "google/protobuf/descriptor.proto"; + +package test; + +//Simple int32 based options (both single and repeated) +//for fields and messages + +extend google.protobuf.FieldOptions { + repeated int32 fo_rep_int = 50000; + int32 fo_single_int = 50001; +} + +extend google.protobuf.MessageOptions { + repeated int32 mo_rep_int = 50000; + int32 mo_single_int = 50001; +} + +message TestFieldOptionsInt { + string field1 = 2 [(fo_rep_int) = 1, (fo_rep_int) = 2]; + string field2 = 1 [(fo_single_int) = 3]; +} + +message TestMessageOptionsInt { + option (mo_rep_int) = 1; + option (mo_rep_int) = 2; + option (mo_single_int) = 3; +} + +//Message based options including nested sub messages (both single and repeated) +//for fields and messages + +message Msg { + int32 value = 1; + repeated int32 rep_value = 2; + SubMsg nested = 3; + repeated SubMsg rep_nested = 4; +} + +message SubMsg { + string value = 1; + SubMsg nested = 2; +} + +extend google.protobuf.FieldOptions { + repeated Msg fo_rep_msg = 50002; + Msg fo_single_msg = 50003; +} + +extend google.protobuf.MessageOptions { + repeated Msg mo_rep_msg = 50002; + Msg mo_single_msg = 50003; +} + +message TestFieldOptionsMsg { + string field1 = 1 [(fo_rep_msg) = {value: 1 rep_value: 2 rep_value: 3}, (fo_rep_msg) = {value: 4 rep_value: 5 rep_value: 6}]; + string field2 = 2 [(fo_single_msg).value = 7, (fo_single_msg).rep_value = 8, (fo_single_msg).rep_value = 9]; +} + +message TestMessageOptionsMsg { + option (mo_rep_msg) = { + value: 1 + rep_value: 2 + rep_value: 3 + }; + option (mo_rep_msg) = { + value: 4 + rep_value: 5 + rep_value: 6 + }; + option (mo_single_msg).value = 7; + option (mo_single_msg).rep_value = 8; + option (mo_single_msg).rep_value = 9; +} + +message TestFieldOptionsNested { + string field1 = 1 [(fo_rep_msg) = {value: 1 nested { nested {value: "x"} } rep_nested { value: "y"} rep_nested { value: "z" } rep_value: 3}, (fo_rep_msg) = { nested { value: "w" }}]; + string field2 = 2 [(fo_single_msg).nested.value = "x", (fo_single_msg).rep_nested = {value : "x"}, (fo_single_msg).rep_nested = {value : "y"}]; + string field3 = 3 [(fo_single_msg).nested = {value: "x" nested {nested{value: "y"}}}]; +} + + +message TestMessageOptionsNested { + option (mo_rep_msg) = { + value: 1 + nested { + nested { + value: "x" + } + } + rep_nested { + value: "y" + } + rep_nested { + value: "z" + } + rep_value: 3 + }; + option (mo_rep_msg) = { + nested { + value: "w" + } + }; + option (mo_single_msg).nested.value = "x"; + option (mo_single_msg).rep_nested = {value : "x" nested {nested{value: "y"}}}; + option (mo_single_msg).rep_nested = {value : "y"}; +} From eeeeeb1423dba974570627e4cb16eceafbcd1860 Mon Sep 17 00:00:00 2001 From: eyalp Date: Mon, 24 Jun 2019 18:15:40 +0300 Subject: [PATCH 2/9] Properly parse option values into objects in addition to the regular option parsing. Repeated options are preserved as well as repeated fields within nested options. Parsed options are kept in a parsedOptions field on every level (message, field etc.) --- index.d.ts | 22 +++++++++++++++++ src/object.js | 40 ++++++++++++++++++++++++++++++ src/parse.js | 38 ++++++++++++++++++++++------ src/util.js | 31 +++++++++++++++++++++++ tests/api_object.js | 12 +++++++++ tests/api_util.js | 29 ++++++++++++++++++++++ tests/comp_options-parse.js | 49 +++++++++++++++++++++++++++++++++++++ 7 files changed, 214 insertions(+), 7 deletions(-) diff --git a/index.d.ts b/index.d.ts index e65d5fff3..94dc47590 100644 --- a/index.d.ts +++ b/index.d.ts @@ -859,6 +859,9 @@ export abstract class ReflectionObject { /** Options. */ public options?: { [k: string]: any }; + /** Options. */ + public parsedOptions?: { [k: string]: any }[]; + /** Unique name within its namespace. */ public name: string; @@ -920,6 +923,15 @@ export abstract class ReflectionObject { */ public setOption(name: string, value: any, ifNotSet?: boolean): ReflectionObject; + /** + * Sets a parsed option. + * @param name parsed Option name + * @param value Option value + * @param [propName] dot '.' delimited full path of property within the option to set. if undefined\empty, will add a new option with that value + * @returns `this` + */ + public setParsedOption(name: string, value: any, propName?: string): ReflectionObject; + /** * Sets multiple options. * @param options Options to set @@ -2151,6 +2163,16 @@ export namespace util { */ function decorateEnum(object: object): Enum; + + /** + * Sets the value of a property by property path. If a value already exists, it is turned to an array + * @param dst Destination object + * @param path dot '.' delimited path of the property to set + * @param value the value to set + * @returns Destination object + */ + function setProperty(dst: { [k: string]: any }, path: string, value: object); + /** Decorator root (TypeScript). */ let decorateRoot: Root; diff --git a/src/object.js b/src/object.js index b6a5e5635..fbcebf03b 100644 --- a/src/object.js +++ b/src/object.js @@ -29,6 +29,12 @@ function ReflectionObject(name, options) { */ this.options = options; // toJSON + /** + * Parsed Options. + * @type {Object.[]|undefined} + */ + this.parsedOptions = null; + /** * Unique name within its namespace. * @type {string} @@ -169,6 +175,40 @@ ReflectionObject.prototype.setOption = function setOption(name, value, ifNotSet) return this; }; +/** + * Sets a parsed option. + * @param {string} name parsed Option name + * @param {*} value Option value + * @param {string} propName dot '.' delimited full path of property within the option to set. if undefined\empty, will add a new option with that value + * @returns {ReflectionObject} `this` + */ +ReflectionObject.prototype.setParsedOption = function setParsedOption(name, value, propName) { + var parsedOptions = (this.parsedOptions || (this.parsedOptions = [])); + if (propName) { + //If setting a sub property of an option then try to merge it + //with an existing option + var opt = parsedOptions.find(function (opt) { + return opt.hasOwnProperty(name) + }); + if (opt) { + //If we found an existing option - just merge the property value + var newValue = opt[name]; + util.setProperty(newValue, propName, value); + } else { + //otherwise, create a new option, set it's property and add it to the list + opt = {}; + opt[name] = util.setProperty({}, propName, value); + parsedOptions.push(opt); + } + } else { + //Always create a new option when setting the value of the option itself + var newOpt = {}; + newOpt[name] = value; + parsedOptions.push(newOpt); + } + return this; +}; + /** * Sets multiple options. * @param {Object.} options Options to set diff --git a/src/parse.js b/src/parse.js index 47f94e494..6b32b8363 100644 --- a/src/parse.js +++ b/src/parse.js @@ -540,39 +540,58 @@ function parse(source, root, options) { throw illegal(token, "name"); var name = token; + var option = name; + var propName; + if (isCustom) { skip(")"); name = "(" + name + ")"; + option = name; token = peek(); if (fqTypeRefRe.test(token)) { + propName = token.substr(1); //remove '.' before property name name += token; next(); } } skip("="); - parseOptionValue(parent, name); + var optionValue = parseOptionValue(parent, name); + setParsedOption(parent, option, optionValue, propName); } function parseOptionValue(parent, name) { if (skip("{", true)) { // { a: "foo" b { c: "bar" } } + var result = {}; do { /* istanbul ignore if */ if (!nameRe.test(token = next())) throw illegal(token, "name"); + var value; + var propName = token; if (peek() === "{") - parseOptionValue(parent, name + "." + token); + value = parseOptionValue(parent, name + "." + token); else { skip(":"); if (peek() === "{") - parseOptionValue(parent, name + "." + token); - else - setOption(parent, name + "." + token, readValue(true)); + value = parseOptionValue(parent, name + "." + token); + else { + value = readValue(true); + setOption(parent, name + "." + token, value); + } } + var prevValue = result[propName]; + if (prevValue) + value = [].concat(prevValue).concat(value); + result[propName] = value; skip(",", true); } while (!skip("}", true)); - } else - setOption(parent, name, readValue(true)); + return result; + } else { + var simpleValue = readValue(true); + setOption(parent, name, simpleValue); + return simpleValue; + } // Does not enforce a delimiter to be universal } @@ -581,6 +600,11 @@ function parse(source, root, options) { parent.setOption(name, value); } + function setParsedOption(parent, name, value, propName) { + if (parent.setParsedOption) + parent.setParsedOption(name, value, propName); + } + function parseInlineOptions(parent) { if (skip("[", true)) { do { diff --git a/src/util.js b/src/util.js index a5a9a8354..85e764d33 100644 --- a/src/util.js +++ b/src/util.js @@ -165,6 +165,37 @@ util.decorateEnum = function decorateEnum(object) { return enm; }; + +/** + * Sets the value of a property by property path. If a value already exists, it is turned to an array + * @param {Object.} dst Destination object + * @param {string} path dot '.' delimited path of the property to set + * @param {Object} value the value to set + * @returns {Object.} Destination object + */ +util.setProperty = function setProperty(dst, path, value) { + function setProp(dst, path, value) { + var part = path.shift(); + if (path.length > 0) { + dst[part] = setProp(dst[part] || {}, path, value) + } else { + var prevValue = dst[part]; + if (prevValue) + value = [].concat(prevValue).concat(value); + dst[part] = value; + } + return dst; + } + + if (typeof dst !== "object") + throw TypeError("dst must be an object"); + if (!path) + throw TypeError("path must be specified"); + + path = path.split("."); + return setProp(dst, path, value) +}; + /** * Decorator root (TypeScript). * @name util.decorateRoot diff --git a/tests/api_object.js b/tests/api_object.js index b6b5f6982..9dda73351 100644 --- a/tests/api_object.js +++ b/tests/api_object.js @@ -24,6 +24,18 @@ tape.test("reflection objects", function(test) { obj.setOption("c", 3); test.same(obj.options, { a: 1, b: 2, c: 3 }, "should set single options"); + obj.setParsedOption("opt1", {a: 1, b: 2}); + test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}], "should set single parsed option"); + obj.setParsedOption("opt1", {a: 3, b: 4}); + test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}, {"opt1": {a: 3, b: 4}}], "should allow same option twice"); + obj.setParsedOption("opt2", 1, "x"); + test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}, {"opt1": {a: 3, b: 4}}, {"opt2": {x: 1}}], "should create new option using property path"); + obj.setParsedOption("opt2", 5, "a.b"); + test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}, {"opt1": {a: 3, b: 4}}, {"opt2": {x: 1, a: {b :5}}}], "should merge new property path in existing option"); + obj.setParsedOption("opt2", 6, "x"); + test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}, {"opt1": {a: 3, b: 4}}, {"opt2": {x: [1,6], a: {b :5}}}], "should convert property to array when set more than once"); + + test.equal(obj.toString(), "ReflectionObject Test", "should convert to a string (even if not part of a root)"); obj.name = ""; test.equal(obj.toString(), "ReflectionObject", "should convert to a string even with no full name"); diff --git a/tests/api_util.js b/tests/api_util.js index ae2a3e15a..7b6f50ffe 100644 --- a/tests/api_util.js +++ b/tests/api_util.js @@ -70,5 +70,34 @@ tape.test("util", function(test) { test.end(); }); + test.test(test.name + " - setProperty", function(test) { + var o = {}; + + test.throws(function() { + util.setProperty(5, 'prop1', 5); + }, TypeError, "dst must be an object"); + + test.throws(function () { + util.setProperty(o, '', 5); + }, TypeError, "path must be specified"); + + util.setProperty(o, 'prop1', 5); + test.same(o, {prop1: 5}, "should set single property value"); + + util.setProperty(o, 'prop1', 6); + test.same(o, {prop1: [5, 6]}, "should convert to array if same property is set"); + + util.setProperty(o, 'prop.subprop', { subsub: 5}); + test.same(o, {prop1: [5, 6], prop: {subprop: {subsub: 5}}}, "should handle nested properties properly"); + + util.setProperty(o, 'prop.subprop.subsub', 6); + test.same(o, {prop1: [5, 6], prop: {subprop: {subsub: [5, 6]}}}, "should convert to array nested property"); + + util.setProperty(o, 'prop.subprop', { subsub2: 7}); + test.same(o, {prop1: [5, 6], prop: {subprop: [{subsub: [5,6]}, {subsub2: 7}]}}, "should convert nested properties to array"); + + test.end(); + }); + test.end(); }); \ No newline at end of file diff --git a/tests/comp_options-parse.js b/tests/comp_options-parse.js index db65dabcd..90c4299c0 100644 --- a/tests/comp_options-parse.js +++ b/tests/comp_options-parse.js @@ -7,7 +7,10 @@ tape.test("Options", function (test) { test.test(test.name + " - field options (Int)", function (test) { var TestFieldOptionsInt = root.lookup("TestFieldOptionsInt"); test.equal(TestFieldOptionsInt.fields.field1.options["(fo_rep_int)"], 2, "should take second repeated int option"); + test.same(TestFieldOptionsInt.fields.field1.parsedOptions, [{"(fo_rep_int)": 1}, {"(fo_rep_int)": 2}], "should take all repeated int option"); + test.equal(TestFieldOptionsInt.fields.field2.options["(fo_single_int)"], 3, "should correctly parse single int option"); + test.same(TestFieldOptionsInt.fields.field2.parsedOptions, [{"(fo_single_int)": 3}], "should correctly parse single int option"); test.end(); }); @@ -15,6 +18,7 @@ tape.test("Options", function (test) { var TestMessageOptionsInt = root.lookup("TestMessageOptionsInt"); test.equal(TestMessageOptionsInt.options["(mo_rep_int)"], 2, "should take second repeated int message option"); test.equal(TestMessageOptionsInt.options["(mo_single_int)"], 3, "should correctly parse single int message option"); + test.same(TestMessageOptionsInt.parsedOptions, [{"(mo_rep_int)": 1}, {"(mo_rep_int)": 2}, {"(mo_single_int)": 3}], "should take all int message option"); test.end(); }); @@ -22,8 +26,12 @@ tape.test("Options", function (test) { var TestFieldOptionsMsg = root.lookup("TestFieldOptionsMsg"); test.equal(TestFieldOptionsMsg.fields.field1.options["(fo_rep_msg).value"], 4, "should take second repeated message option"); test.equal(TestFieldOptionsMsg.fields.field1.options["(fo_rep_msg).rep_value"], 6, "should take second repeated int in second repeated option"); + test.same(TestFieldOptionsMsg.fields.field1.parsedOptions, [ + {"(fo_rep_msg)": {value: 1, rep_value: [2, 3]}}, + {"(fo_rep_msg)": {value: 4, rep_value: [5, 6]}}], "should take all repeated message option"); test.equal(TestFieldOptionsMsg.fields.field2.options["(fo_single_msg).value"], 7, "should correctly parse single msg option"); test.equal(TestFieldOptionsMsg.fields.field2.options["(fo_single_msg).rep_value"], 9, "should take second repeated int in single msg option"); + test.same(TestFieldOptionsMsg.fields.field2.parsedOptions, [{"(fo_single_msg)": {value: 7, rep_value: [8,9]}}], "should take all repeated message option"); test.end(); }); @@ -33,6 +41,11 @@ tape.test("Options", function (test) { test.equal(TestMessageOptionsMsg.options["(mo_rep_msg).rep_value"], 6, "should take second repeated int in second repeated option"); test.equal(TestMessageOptionsMsg.options["(mo_single_msg).value"], 7, "should correctly parse single msg option"); test.equal(TestMessageOptionsMsg.options["(mo_single_msg).rep_value"], 9, "should take second repeated int in single msg option"); + test.same(TestMessageOptionsMsg.parsedOptions, [ + {"(mo_rep_msg)": {value: 1, rep_value: [2, 3]}}, + {"(mo_rep_msg)": {value: 4, rep_value: [5, 6]}}, + {"(mo_single_msg)": {value: 7, rep_value: [8, 9]}}, + ], "should take all message options"); test.end(); }); @@ -43,12 +56,31 @@ tape.test("Options", function (test) { test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).nested.nested.value"], "x", "should correctly parse nested field options"); test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).rep_nested.value"], "z", "should take second repeated nested options"); test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).nested.value"], "w", "should merge nested options"); + test.same(TestFieldOptionsNested.fields.field1.parsedOptions,[ + {"(fo_rep_msg)": {value: 1, nested: { nested: { value: "x"}}, rep_nested: [{value: "y"},{value: "z"}], rep_value: 3}}, + {"(fo_rep_msg)": { nested: { value: "w"}}}, + ],"should parse all options including nested"); test.equal(TestFieldOptionsNested.fields.field2.options["(fo_single_msg).nested.value"], "x", "should correctly parse nested property name"); test.equal(TestFieldOptionsNested.fields.field2.options["(fo_single_msg).rep_nested.value"], "y", "should take second repeated nested options"); + test.same(TestFieldOptionsNested.fields.field2.parsedOptions, [{ + "(fo_single_msg)": { + nested: {value: "x"}, + rep_nested: [{value: "x"}, {value: "y"}] + } + } + ], "should parse single nested option correctly"); test.equal(TestFieldOptionsNested.fields.field3.options["(fo_single_msg).nested.value"], "x", "should correctly parse nested field options"); test.equal(TestFieldOptionsNested.fields.field3.options["(fo_single_msg).nested.nested.nested.value"], "y", "should correctly parse several nesting levels"); + test.same(TestFieldOptionsNested.fields.field3.parsedOptions, [{ + "(fo_single_msg)": { + nested: { + value: "x", + nested: {nested: {value: "y"}} + } + } + }], "should correctly parse several nesting levels"); test.end(); }); @@ -65,6 +97,23 @@ tape.test("Options", function (test) { test.equal(TestMessageOptionsNested.options["(mo_single_msg).rep_nested.value"], "y", "should take second repeated nested options"); test.equal(TestMessageOptionsNested.options["(mo_single_msg).rep_nested.nested.nested.value"], "y", "should correctly parse several nesting levels"); + test.same(TestMessageOptionsNested.parsedOptions, [ + { + "(mo_rep_msg)": { + value: 1, + nested: {nested: {value: "x"}}, + rep_nested: [{value: "y"}, {value: "z"}], + rep_value: 3 + } + }, + {"(mo_rep_msg)": {nested: {value: "w"}}}, + { + "(mo_single_msg)": { + nested: {value: "x"}, + rep_nested: [{value: "x", nested: {nested: {value: "y"}}}, {value: "y"}] + } + } + ], "should correctly parse all nested message options"); test.end(); }); From 79cb7e637d4ef26839a9c31e0f7bc99ba2fd0595 Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 1 Jul 2020 08:37:26 -0700 Subject: [PATCH 3/9] fix: bad merge --- src/parse.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/parse.js b/src/parse.js index 7fc0094a2..d313e8e4e 100644 --- a/src/parse.js +++ b/src/parse.js @@ -564,6 +564,7 @@ function parse(source, root, options) { function parseOptionValue(parent, name) { if (skip("{", true)) { // { a: "foo" b { c: "bar" } } var result = {}; + while (!skip("}", true)) { /* istanbul ignore if */ if (!nameRe.test(token = next())) throw illegal(token, "name"); From 03ffc138318743adda215331b1473f9d4f4fa14c Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 1 Jul 2020 08:39:50 -0700 Subject: [PATCH 4/9] fix: lint --- src/parse.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/parse.js b/src/parse.js index d313e8e4e..a5f8cef88 100644 --- a/src/parse.js +++ b/src/parse.js @@ -589,11 +589,11 @@ function parse(source, root, options) { skip(",", true); } return result; - } else { - var simpleValue = readValue(true); - setOption(parent, name, simpleValue); - return simpleValue; } + + var simpleValue = readValue(true); + setOption(parent, name, simpleValue); + return simpleValue; // Does not enforce a delimiter to be universal } From 32a40e8f80991992d2c86b0a33f603877c9d570e Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 1 Jul 2020 08:43:48 -0700 Subject: [PATCH 5/9] fix: lint --- src/object.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/object.js b/src/object.js index fbcebf03b..ec4cf3cbb 100644 --- a/src/object.js +++ b/src/object.js @@ -183,25 +183,28 @@ ReflectionObject.prototype.setOption = function setOption(name, value, ifNotSet) * @returns {ReflectionObject} `this` */ ReflectionObject.prototype.setParsedOption = function setParsedOption(name, value, propName) { - var parsedOptions = (this.parsedOptions || (this.parsedOptions = [])); + if (!this.parsedOptions) { + this.parsedOptions = []; + } + var parsedOptions = this.parsedOptions; if (propName) { - //If setting a sub property of an option then try to merge it - //with an existing option + // If setting a sub property of an option then try to merge it + // with an existing option var opt = parsedOptions.find(function (opt) { - return opt.hasOwnProperty(name) + return Object.prototype.hasOwnProperty.call(opt, name); }); if (opt) { - //If we found an existing option - just merge the property value + // If we found an existing option - just merge the property value var newValue = opt[name]; util.setProperty(newValue, propName, value); } else { - //otherwise, create a new option, set it's property and add it to the list + // otherwise, create a new option, set it's property and add it to the list opt = {}; opt[name] = util.setProperty({}, propName, value); parsedOptions.push(opt); } } else { - //Always create a new option when setting the value of the option itself + // Always create a new option when setting the value of the option itself var newOpt = {}; newOpt[name] = value; parsedOptions.push(newOpt); From b53ebfd6b20d101290eabdc771d47101aea94ad1 Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 1 Jul 2020 08:44:18 -0700 Subject: [PATCH 6/9] fix: lint --- src/util.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util.js b/src/util.js index 85e764d33..5ae88cc7f 100644 --- a/src/util.js +++ b/src/util.js @@ -177,7 +177,7 @@ util.setProperty = function setProperty(dst, path, value) { function setProp(dst, path, value) { var part = path.shift(); if (path.length > 0) { - dst[part] = setProp(dst[part] || {}, path, value) + dst[part] = setProp(dst[part] || {}, path, value); } else { var prevValue = dst[part]; if (prevValue) @@ -193,7 +193,7 @@ util.setProperty = function setProperty(dst, path, value) { throw TypeError("path must be specified"); path = path.split("."); - return setProp(dst, path, value) + return setProp(dst, path, value); }; /** From b59e2e0619248c63098279a1f1271ff007e63970 Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 1 Jul 2020 08:45:57 -0700 Subject: [PATCH 7/9] fix: lint --- src/parse.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parse.js b/src/parse.js index a5f8cef88..918d7c2eb 100644 --- a/src/parse.js +++ b/src/parse.js @@ -590,7 +590,7 @@ function parse(source, root, options) { } return result; } - + var simpleValue = readValue(true); setOption(parent, name, simpleValue); return simpleValue; From 45daedd64b7b6818466ad475ee5374b4d177c974 Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 1 Jul 2020 08:48:19 -0700 Subject: [PATCH 8/9] fix: lint --- index.d.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/index.d.ts b/index.d.ts index 7f5e823fd..867cfdcaf 100644 --- a/index.d.ts +++ b/index.d.ts @@ -2174,7 +2174,6 @@ export namespace util { */ function decorateEnum(object: object): Enum; - /** * Sets the value of a property by property path. If a value already exists, it is turned to an array * @param dst Destination object From 160178616b099195adeebd976fd2594c6f9152e6 Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 1 Jul 2020 09:25:36 -0700 Subject: [PATCH 9/9] fix: build types --- src/object.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object.js b/src/object.js index ec4cf3cbb..bd04ceca8 100644 --- a/src/object.js +++ b/src/object.js @@ -31,7 +31,7 @@ function ReflectionObject(name, options) { /** * Parsed Options. - * @type {Object.[]|undefined} + * @type {Array.>|undefined} */ this.parsedOptions = null;