chore: respond to comments

Author: sofisl

src/enum.js

@@ -89,20 +89,14 @@ function Enum(name, values, options, comment, comments, valuesOptions) {
  * @returns {Enum} `this`
  */
 Enum.prototype.resolve = function resolve() {
-
-    if (this.resolved)
-        return this;
+    ReflectionObject.prototype.resolve.call(this);
 
     for (var key of Object.keys(this._valuesProtoFeatures)) {
-
-        if (this.parent) {
-            var parentFeaturesCopy = Object.assign({}, this.parent._features);
-            this._valuesFeatures[key] = Object.assign(parentFeaturesCopy, this._valuesProtoFeatures[key] || {});
-        } else {
-            this._valuesFeatures[key] = Object.assign({}, this._valuesProtoFeatures[key]);
-        }
+        var parentFeaturesCopy = Object.assign({}, this._features);
+        this._valuesFeatures[key] = Object.assign(parentFeaturesCopy, this._valuesProtoFeatures[key] || {});
     }
-    return ReflectionObject.prototype.resolve.call(this);
+
+    return this;
 };
 
 

src/field.js

@@ -319,6 +319,13 @@ Field.prototype.resolve = function resolve() {
     if (this.parent instanceof Type)
         this.parent.ctor.prototype[this.name] = this.defaultValue;
 
+
+    if (this.parent) {
+        var parentFeaturesCopy = Object.assign({}, this.parent._features);
+        this._features = Object.assign(parentFeaturesCopy, this._protoFeatures || {});
+    } else {
+        this._features = Object.assign({}, this._protoFeatures);
+    }
     return ReflectionObject.prototype.resolve.call(this);
 };
 

src/object.js

@@ -3,6 +3,7 @@ module.exports = ReflectionObject;
 
 ReflectionObject.className = "ReflectionObject";
 
+const OneOf = require("./oneof");
 var util = require("./util");
 
 var Root; // cyclic
@@ -162,10 +163,10 @@ ReflectionObject.prototype.onRemove = function onRemove(parent) {
 ReflectionObject.prototype.resolve = function resolve() {
     if (this.resolved)
         return this;
-    if (this instanceof Root || this.parent && this.parent.resolved)
+    if (this instanceof Root || this.parent && this.parent.resolved) {
         this._resolveFeatures();
-    if (this.root instanceof Root)
         this.resolved = true;
+    }
     return this;
 };
 
@@ -188,6 +189,11 @@ ReflectionObject.prototype._resolveFeatures = function _resolveFeatures() {
 
     if (this instanceof Root) {
         this._features = Object.assign(defaults, this._protoFeatures || {});
+    // fields in Oneofs aren't actually children of them, so we have to
+    // special-case it
+    } else if (this.partOf instanceof OneOf) {
+        var lexicalParentFeaturesCopy = Object.assign({}, this.partOf._features);
+        this._features = Object.assign(lexicalParentFeaturesCopy, this._protoFeatures || {});
     } else if (this.parent) {
         var parentFeaturesCopy = Object.assign({}, this.parent._features);
         this._features = Object.assign(parentFeaturesCopy, this._protoFeatures || {});

src/root.js

@@ -88,20 +88,26 @@ Root.prototype.load = function load(filename, options, callback) {
         options = undefined;
     }
     var self = this;
-    if (!callback)
+    if (!callback) {
         return util.asPromise(load, self, filename, options);
+    }
 
     var sync = callback === SYNC; // undocumented
 
     // Finishes loading by calling the callback (exactly once)
     function finish(err, root) {
         /* istanbul ignore if */
-        if (!callback)
+        if (!callback) {
             return;
-        if (sync)
+        }
+        if (sync) {
             throw err;
+        }
         var cb = callback;
         callback = null;
+        if (root) {
+            root.resolveAll();
+        }
         cb(err, root);
     }
 
@@ -139,24 +145,26 @@ Root.prototype.load = function load(filename, options, callback) {
         } catch (err) {
             finish(err);
         }
-        if (!sync && !queued)
+        if (!sync && !queued) {
             finish(null, self); // only once anyway
+        }
     }
 
     // Fetches a single file
     function fetch(filename, weak) {
         filename = getBundledFileName(filename) || filename;
 
         // Skip if already loaded / attempted
-        if (self.files.indexOf(filename) > -1)
+        if (self.files.indexOf(filename) > -1) {
             return;
+        }
         self.files.push(filename);
 
         // Shortcut bundled definitions
         if (filename in common) {
-            if (sync)
+            if (sync) {
                 process(filename, common[filename]);
-            else {
+            } else {
                 ++queued;
                 setTimeout(function() {
                     --queued;
@@ -182,8 +190,9 @@ Root.prototype.load = function load(filename, options, callback) {
             self.fetch(filename, function(err, source) {
                 --queued;
                 /* istanbul ignore if */
-                if (!callback)
+                if (!callback) {
                     return; // terminated meanwhile
+                }
                 if (err) {
                     /* istanbul ignore else */
                     if (!weak)
@@ -200,17 +209,21 @@ Root.prototype.load = function load(filename, options, callback) {
 
     // Assembling the root namespace doesn't require working type
     // references anymore, so we can load everything in parallel
-    if (util.isString(filename))
+    if (util.isString(filename)) {
         filename = [ filename ];
+    }
     for (var i = 0, resolved; i < filename.length; ++i)
         if (resolved = self.resolvePath("", filename[i]))
             fetch(resolved);
     self.resolveAll();
-    if (sync)
+    if (sync) {
         return self;
-    if (!queued)
+    }
+    if (!queued) {
         finish(null, self);
-    return undefined;
+    }
+
+    return self;
 };
 // function load(filename:string, options:IParseOptions, callback:LoadCallback):undefined
 

src/type.js

@@ -300,12 +300,12 @@ Type.prototype.toJSON = function toJSON(toJSONOptions) {
  */
 Type.prototype.resolveAll = function resolveAll() {
     Namespace.prototype.resolveAll.call(this);
-    var fields = this.fieldsArray, i = 0;
-    while (i < fields.length)
-        fields[i++].resolve();
     var oneofs = this.oneofsArray; i = 0;
     while (i < oneofs.length)
         oneofs[i++].resolve();
+    var fields = this.fieldsArray, i = 0;
+    while (i < fields.length)
+        fields[i++].resolve();
     return this;
 };
 

tests/data/feature-resolution.proto

@@ -0,0 +1,62 @@
+edition = "2023";
+
+option features.amazing_feature = A;
+option (mo_single_msg).nested.value = "x";
+service MyService {
+    option features.amazing_feature = E;
+    message MyRequest {};
+    message MyResponse {};
+    rpc MyMethod (MyRequest) returns (MyResponse) {
+        option features.amazing_feature = L;
+    };
+}
+
+message Message {
+    option features.amazing_feature = B;
+
+    string string_val = 1;
+    string string_repeated = 2 [features.amazing_feature = F];
+
+    uint64 uint64_val = 3;
+    uint64 uint64_repeated = 4;
+
+    bytes bytes_val = 5;
+    bytes bytes_repeated = 6;
+
+    SomeEnum enum_val = 7;
+    SomeEnum enum_repeated = 8;
+
+    extensions 10 to 100;
+    extend Message {
+        int32 bar = 10 [features.amazing_feature = I];
+    }
+
+    message Nested {
+        option features.amazing_feature = H;
+        int64 count = 9;
+    }
+
+    enum SomeEnumInMessage {
+        option features.amazing_feature = G;
+        ONE = 11;
+        TWO = 12;
+    }
+    
+    oneof SomeOneOf {
+        option features.amazing_feature = J;
+        int32 a = 13;
+        string b = 14;
+    }
+
+    map<string,int64> int64_map = 15;
+}
+
+extend Message {
+    int32 bar = 16 [features.amazing_feature = D];
+}
+
+enum SomeEnum {
+    option features.amazing_feature = C;
+    ONE = 1 [features.amazing_feature = K];
+    TWO = 2;
+}