respond to comments

Author: sofisl

src/object.js

@@ -44,7 +44,12 @@ function ReflectionObject(name, options) {
     /**
      * Resolved Features.
      */
-    this._features = null;
+    this._features = {};
+
+    /**
+     * Resolved Features.
+     */
+    this._proto_features = null;
 
     /**
      * Parent namespace.
@@ -152,7 +157,8 @@ ReflectionObject.prototype.resolve = function resolve() {
     if (this.resolved)
         return this;
     this._resolveFeatures();
-    this.resolved = true;
+    if (this.root instanceof Root || this.parent)
+        this.resolved = true;
     return this;
 };
 
@@ -166,10 +172,14 @@ ReflectionObject.prototype._resolveFeatures = function _resolveFeatures() {
         // (Breaks the bundler and eslint)
         // If we don't create a shallow copy, we end up also altering the parent's
         // features
-        var parentFeatures = Object.assign({}, this.parent._features);
-        this._features = Object.assign(parentFeatures, this._features || {});
+        var parentFeatures = Object.assign({}, this.parent._proto_features);
+        this._features = Object.assign(parentFeatures, this._proto_features || {});
+        // this._proto_features = this._features;
         this.parent._resolveFeatures();
+    } else {
+        this._features = Object.assign({}, this._proto_features);
     }
+    this._proto_features = this._features;
 };
 
 /**
@@ -207,7 +217,7 @@ ReflectionObject.prototype.setParsedOption = function setParsedOption(name, valu
     if (!this.parsedOptions) {
         this.parsedOptions = [];
     }
-    var isFeature = /features/.test(name);
+    var isFeature = /^features/.test(name);
     var parsedOptions = this.parsedOptions;
     if (propName) {
         // If setting a sub property of an option then try to merge it
@@ -236,8 +246,9 @@ ReflectionObject.prototype.setParsedOption = function setParsedOption(name, valu
 
     if (isFeature) {
         var features = parsedOptions.find(x => {return Object.prototype.hasOwnProperty.call(x, "features");});
-        this._features = features.features || {};
+        this._proto_features = features.features || {};
     }
+    
     return this;
 };
 

src/parse.js

@@ -370,9 +370,9 @@ function parse(source, root, options) {
                     break;
 
                 case "required":
-                case "repeated":
                     if (edition)
                         throw illegal(token);
+                case "repeated":
                     parseField(type, token);
                     break;
 
@@ -632,7 +632,7 @@ function parse(source, root, options) {
         dummy.setParsedOption = function(name, value, propName) {
             // In order to not change existing behavior, only calling
             // this for features
-            if (/features/.test(name)) {
+            if (/^features/.test(name)) {
                 return ReflectionObject.prototype.setParsedOption.call(dummy, name, value, propName);
             }
             return undefined;

src/util.js

@@ -171,10 +171,10 @@ util.decorateEnum = function decorateEnum(object) {
  * @param {Object.<string,*>} dst Destination object
  * @param {string} path dot '.' delimited path of the property to set
  * @param {Object} value the value to set
- * @param {boolean} isFeature whether the option being set is a feature (matches feature regex)
+ * @param {boolean} overWrite whether or not to concatenate the values into an array or overwrite
  * @returns {Object.<string,*>} Destination object
  */
-util.setProperty = function setProperty(dst, path, value, isFeature) {
+util.setProperty = function setProperty(dst, path, value, overWrite) {
     function setProp(dst, path, value) {
         var part = path.shift();
         if (part === "__proto__" || part === "prototype") {
@@ -184,7 +184,7 @@ util.setProperty = function setProperty(dst, path, value, isFeature) {
             dst[part] = setProp(dst[part] || {}, path, value);
         } else {
             var prevValue = dst[part];
-            if (prevValue && !isFeature)
+            if (prevValue && !overWrite)
                 value = [].concat(prevValue).concat(value);
             dst[part] = value;
         }

tests/api_object.js

@@ -10,6 +10,7 @@ tape.test("reflection objects", function(test) {
     var obj = new protobuf.ReflectionObject("Test");
 
     obj.resolve();
+    test.equal(obj.resolved, false, "should not resolve when not part of a root");
 
     obj.resolved = 2;
     obj.resolve();

tests/feature_resolution_editions.js

@@ -31,12 +31,15 @@ message Message {
 `
 tape.test("feautre resolution defaults", function(test) {
     var rootEditions = protobuf.parse(protoEditions2023).root;
+    rootEditions.resolveAll();
     test.same(rootEditions._features, editions2023Defaults);
 
     var rootProto2 = protobuf.parse(proto2).root;
+    rootProto2.resolveAll();
     test.same(rootProto2._features, proto2Defaults);
 
     var rootProto3 = protobuf.parse(proto3).root;
+    rootProto3.resolveAll();
     test.same(rootProto3._features, proto3Defaults);
 
     test.end();
@@ -71,6 +74,7 @@ tape.test("feature resolution editions precedence", function(test) {
     protobuf.load("tests/data/feature-resolution.proto", function(err, root) {
         if (err)
             return test.fail(err.message);
+        root.resolveAll();
         test.same(root._features.amazing_feature, 'A');
         test.same(root.lookup("Message")._features.amazing_feature, 'B')
         test.same(root.lookupEnum("SomeEnum")._features.amazing_feature, 'C')