Rework feature resolution to not rely on resolved types

Author: mkruskal-google

bench/index.js

@@ -102,12 +102,12 @@ newSuite("fromJSON")
     protobuf.Root.fromJSON(json).resolveAll();
 })
 .add("shared (unique)", function() {
-    var jsonCopy = { 
+    var jsonCopy = {
         options: json.options,
         nested: {}
     };
     Object.keys(json).forEach(key => {
-        jsonCopy.nested[key + fromJsonIndex] = json[key]
+        jsonCopy.nested[key + fromJsonIndex] = json[key];
     });
     fromJsonIndex++;
 
@@ -125,5 +125,5 @@ newSuite("load")
 .add("sync", function() {
     protobuf.loadSync("bench/data/bench.proto");
 })
-.run()
+.run();
 

package.json

@@ -33,7 +33,9 @@
     "build:bundle": "gulp --gulpfile scripts/gulpfile.js",
     "build:types": "node cli/bin/pbts --main --global protobuf --out index.d.ts src/ lib/aspromise/index.js lib/base64/index.js lib/codegen/index.js lib/eventemitter/index.js lib/float/index.js lib/fetch/index.js lib/inquire/index.js lib/path/index.js lib/pool/index.js lib/utf8/index.js",
     "changelog": "node scripts/changelog -w",
-    "coverage": "nyc tape -r ./lib/tape-adapter tests/*.js tests/node/*.js",
+    "coverage": "npm run coverage:test && npm run coverage:report",
+    "coverage:test": "nyc --silent tape -r ./lib/tape-adapter tests/*.js tests/node/*.js",
+    "coverage:report": "nyc report --reporter=lcov --reporter=text",
     "docs": "jsdoc -c config/jsdoc.json -R README.md --verbose --pedantic",
     "lint": "npm run lint:sources && npm run lint:types",
     "lint:sources": "eslint \"**/*.js\" -c config/eslint.json",

src/field.js

@@ -370,12 +370,18 @@ Field.prototype._inferLegacyProtoFeatures = function _inferLegacyProtoFeatures(e
     }
 
     var features = {};
-    this.resolve();
+
     if (this.rule === "required") {
         features.field_presence = "LEGACY_REQUIRED";
     }
-    if (this.resolvedType instanceof Type && this.resolvedType.group) {
-        features.message_encoding = "DELIMITED";
+    if (this.parent && types.defaults[this.type] === undefined) {
+        // We can't use resolvedType because types may not have been resolved yet.  However,
+        // legacy groups are always in the same scope as the field so we don't have to do a
+        // full scan of the tree.
+        var type = this.parent.get(this.type.split(".").pop());
+        if (type && type instanceof Type && type.group) {
+            features.message_encoding = "DELIMITED";
+        }
     }
     if (this.getOption("packed") === true) {
         features.repeated_field_encoding = "PACKED";

src/root.js

@@ -51,7 +51,7 @@ Root.fromJSON = function fromJSON(json, root) {
         root = new Root();
     if (json.options)
         root.setOptions(json.options);
-    return root.addJSON(json.nested).resolveAll();
+    return root.addJSON(json.nested)._resolveFeaturesRecursive();
 };
 
 /**
@@ -99,6 +99,9 @@ Root.prototype.load = function load(filename, options, callback) {
 
     // Finishes loading by calling the callback (exactly once)
     function finish(err, root) {
+        if (root) {
+            root._resolveFeaturesRecursive();
+        }
         /* istanbul ignore if */
         if (!callback) {
             return;
@@ -108,9 +111,6 @@ Root.prototype.load = function load(filename, options, callback) {
         }
         var cb = callback;
         callback = null;
-        if (root) {
-            root.resolveAll();
-        }
         cb(err, root);
     }
 
@@ -218,8 +218,8 @@ Root.prototype.load = function load(filename, options, callback) {
     for (var i = 0, resolved; i < filename.length; ++i)
         if (resolved = self.resolvePath("", filename[i]))
             fetch(resolved);
-    self.resolveAll();
     if (sync) {
+        self._resolveFeaturesRecursive();
         return self;
     }
     if (!queued) {