CLI: Further improved generated static code style

CHANGELOG.md

@@ -1,3 +1,37 @@
+# [6.5.0](https://github.com/dcodeIO/protobuf.js/releases/tag/6.5.0)
+
+## Breaking
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/3946e0fefea415f52a16ea7a74109ff40eee9643) Initial upgrade of converters to real generated functions, see [#620](https://github.com/dcodeIO/protobuf.js/issues/620)<br />
+
+## Fixed
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/c7e14b1d684aaba2080195cc83900288c5019bbc) Use common utility for virtual oneof getters and setters in both reflection and static code, see [#644](https://github.com/dcodeIO/protobuf.js/issues/644)<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/508984b7ff9529906be282375d36fdbada66b8e6) Use Type.toObject/Message.toObject within converters, see [#641](https://github.com/dcodeIO/protobuf.js/issues/641)<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/5bca18f2d32e8687986e23edade7c2aeb6b6bac1) Generate null/undefined assertion in fromObject if actually NOT an enum, see [#620](https://github.com/dcodeIO/protobuf.js/issues/620)<br />
+
+## New
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/cda5c5452fa0797f1e4c375471aef96f844711f1) Removed scoping iifes from generated static code<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/def7b45fb9b5e01028cfa3bf2ecd8272575feb4d) Removed even more clutter from generated static code<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/dbd19fd9d3a57d033aad1d7173f7f66db8f8db3e) Removed various clutter from generated static code<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/340d6aa82ac17c4a761c681fa71d5a0955032c8b) Now also parses comments, sets them on reflected objects and re-uses them when generating static code, see [#640](https://github.com/dcodeIO/protobuf.js/issues/640)<br />
+
+## CLI
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/1cc8a2460c7e161c9bc58fa441ec88e752df409c) Made sure that static target's replacement regexes don't match fields<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/d4272dbf5d0b2577af8efb74a94d246e2e0d728e) Also accept (trailing) triple-slash comments for compatibility with protoc-gen-doc, see [#640](https://github.com/dcodeIO/protobuf.js/issues/640)<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/508984b7ff9529906be282375d36fdbada66b8e6) Replace all occurencies of types[%d].values in static code, see [#641](https://github.com/dcodeIO/protobuf.js/issues/641)<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/0a3862b75fa60ef732e0cd36d623f025acc2fb45) Use semver to validate that CLI dependencies actually satisfy the required version, see [#637](https://github.com/dcodeIO/protobuf.js/issues/637)<br />
+
+## Docs
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/9e360ea6a74d41307483e51f18769df7f5b047b9) Added a hint on documenting .proto files for static code<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/d2a97bb818474645cf7ce1832952b2c3c739b234) Documented internally used codegen partials for what it's worth<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/a08ee2305a8d29290b8203c187a4a453898e8f6f) Updated docs to reflect recent updates<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/079388ca65dfd581d74188a6ae49cfa01b103809) Updated converter documentation<br />
+
+## Other
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/c280a4a18c6d81c3468177b2ea58ae3bc4f25e73) Removed now useless trailing comment checks, see [#640](https://github.com/dcodeIO/protobuf.js/issues/640)<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/3f6ffd01c3cb7d7fc4129c45dc803abb5211a1fd) Restrict comment parsing for static code to explicit /**-blocks because old protos may generate a lot of nonsense otherwise, see [#640](https://github.com/dcodeIO/protobuf.js/issues/640)<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/a4954ed2b63e349c96acc5c471241a5abb79854d) Added semver to dev/cli dependencies<br />
+[:hash:](https://github.com/dcodeIO/protobuf.js/commit/44167db494c49d9e4b561a66ad9ce2d8ed865a21) Ensured that pbjs' beautify does not break regular expressions in generated verify functions<br />
+
 # [6.4.6](https://github.com/dcodeIO/protobuf.js/releases/tag/6.4.6)
 
 ## Fixed

cli/targets/static.js

@@ -1,9 +1,12 @@
 "use strict";
 module.exports = static_target;
 
-var protobuf = require("../.."),
-    cliUtil  = require("../util"),
-    UglifyJS = require("uglify-js");
+var protobuf   = require("../.."),
+    cliUtil    = require("../util"),
+    UglifyJS   = require("uglify-js"),
+    esprima    = require("esprima"),
+    escodegen  = require("escodegen"),
+    estraverse = require("estraverse");
 
 var Type      = protobuf.Type,
     Service   = protobuf.Service,
@@ -139,33 +142,63 @@ function buildNamespace(ref, ns) {
     }
 }
 
-function beautify(code) {
-    return UglifyJS.minify(code
+var reduceableBlockStatements = {
+    IfStatement: true,
+    ForStatement: true,
+    WhileStatement: true
+};
+
+function beautifyCode(code) {
+    // Rename short vars
+    code = code
         .replace(/\b(?!\\)r\b/g, "reader")
         .replace(/\b(?!\\)w\b/g, "writer")
         .replace(/\b(?!\\)m\b/g, "message")
         .replace(/\b(?!\\)t\b/g, "tag")
-        .replace(/\b(?!\\)l\b/g, "len")
+        .replace(/\b(?!\\)l\b/g, "length")
         .replace(/\b(?!\\)c\b/g, "end")
         .replace(/\b(?!\\)c2\b/g, "end2")
         .replace(/\b(?!\\)k\b/g, "key")
         .replace(/\b(?!\\)ks\b/g, "keys")
         .replace(/\b(?!\\)ks2\b/g, "keys2")
-        .replace(/\b(?!\\)e\b/g, "err")
+        .replace(/\b(?!\\)e\b/g, "error")
         .replace(/\b(?!\\)f\b/g, "impl")
         .replace(/\b(?!\\)o\b/g, "options")
         .replace(/\b(?!\\)d\b/g, "object")
-        .replace(/\b(?!\\)n\b/g, "long"),
-        {
-            fromString: true,
-            compress: false,
-            mangle: false,
-            output: {
-                beautify: true,
-                bracketize: true
-            }
+        .replace(/\b(?!\\)n\b/g, "long");
+    // Add semicolons
+    code = UglifyJS.minify(code, {
+        fromString: true,
+        compress: false,
+        mangle: false,
+        output: {
+            beautify: true
+        }
+    }).code;
+    // Properly beautify
+    var ast = esprima.parse(code);
+    estraverse.replace(ast, {
+        enter: function(node, parent) {
+            // remove braces around block statements with a single child
+            if (node.type === "BlockStatement" && reduceableBlockStatements[parent.type] && node.body.length === 1)
+                return node.body[0];
+            return undefined;
+        }
+    });
+    code = escodegen.generate(ast, {
+        format: {
+            newline: "\r\n",
+            quotes: "double"
         }
-    ).code.replace(/ {4}/g, "\t");
+    });
+    // Add id, wireType comments
+    if (config.comments)
+        code = code.replace(/\.uint32\((\d+)\)/g, function($0, $1) {
+            var id = $1 >>> 3,
+                wireType = $1 & 7;
+            return ".uint32(/* id " + id + ", wireType " + wireType + " =*/" + $1 + ")";
+        });
+    return code;
 }
 
 function buildFunction(type, functionName, gen, scope) {
@@ -178,7 +211,9 @@ function buildFunction(type, functionName, gen, scope) {
         .replace(/\b(?!\.)types\[\b/g, "$types[");
 
     if (config.beautify)
-        code = beautify(code);
+        code = beautifyCode(code);
+
+    code = code.replace(/ {4}/g, "\t");
 
     var hasScope = scope && Object.keys(scope).length;
 
@@ -366,8 +401,8 @@ function buildType(ref, type) {
         push("");
         pushComment([
             "Encodes the specified " + type.name + " message.",
-            "@param {" + fullName + "|Object} message " + type.name + " message or plain object to encode",
-            "@param {$protobuf.Writer} [writer] Writer to encode to",
+            "@param {" + fullName + "|Object} " + (config.beautify ? "message" : "m") + " " + type.name + " message or plain object to encode",
+            "@param {$protobuf.Writer} [" + (config.beautify ? "writer" : "w") + "] Writer to encode to",
             "@returns {$protobuf.Writer} Writer"
         ]);
         buildFunction(type, "encode", protobuf.encoder(type));
@@ -394,8 +429,8 @@ function buildType(ref, type) {
         push("");
         pushComment([
             "Decodes " + aOrAn(type.name) + " message from the specified reader or buffer.",
-            "@param {$protobuf.Reader|Uint8Array} readerOrBuffer Reader or buffer to decode from",
-            "@param {number} [length] Message length if known beforehand",
+            "@param {$protobuf.Reader|Uint8Array} " + (config.beautify ? "reader" : "r") + " Reader or buffer to decode from",
+            "@param {number} [" + (config.beautify ? "length" : "l") + "] Message length if known beforehand",
             "@returns {" + fullName + "} " + type.name
         ]);
         buildFunction(type, "decode", protobuf.decoder(type));
@@ -404,13 +439,16 @@ function buildType(ref, type) {
             push("");
             pushComment([
                 "Decodes " + aOrAn(type.name) + " message from the specified reader or buffer, length delimited.",
-                "@param {$protobuf.Reader|Uint8Array} readerOrBuffer Reader or buffer to decode from",
+                "@param {$protobuf.Reader|Uint8Array} reader Reader or buffer to decode from",
                 "@returns {" + fullName + "} " + type.name
             ]);
-            push(name(type.name) + ".decodeDelimited = function decodeDelimited(readerOrBuffer) {");
+            push(name(type.name) + ".decodeDelimited = function decodeDelimited(reader) {");
             ++indent;
-            push("readerOrBuffer = readerOrBuffer instanceof $Reader ? readerOrBuffer : $Reader(readerOrBuffer);");
-            push("return this.decode(readerOrBuffer, readerOrBuffer.uint32());");
+                push("if (!(reader instanceof $Reader))");
+                ++indent;
+                    push("reader = $Reader(reader);");
+                --indent;
+                push("return this.decode(reader, reader.uint32());");
             --indent;
             push("};");
 
@@ -421,7 +459,7 @@ function buildType(ref, type) {
         push("");
         pushComment([
             "Verifies " + aOrAn(type.name) + " message.",
-            "@param {" + fullName + "|Object} message " + type.name + " message or plain object to verify",
+            "@param {" + fullName + "|Object} " + (config.beautify ? "message" : "m") + " " + type.name + " message or plain object to verify",
             "@returns {?string} `null` if valid, otherwise the reason why it is not"
         ]);
         buildFunction(type, "verify", protobuf.verifier(type));
@@ -432,7 +470,7 @@ function buildType(ref, type) {
         push("");
         pushComment([
             "Creates " + aOrAn(type.name) + " message from a plain object. Also converts values to their respective internal types.",
-            "@param {Object.<string,*>} object Plain object",
+            "@param {Object.<string,*>} " + (config.beautify ? "object" : "d") + " Plain object",
             "@returns {" + fullName + "} " + type.name
         ]);
         buildFunction(type, "fromObject", protobuf.converter.fromObject(type));
@@ -449,8 +487,8 @@ function buildType(ref, type) {
         push("");
         pushComment([
             "Creates a plain object from " + aOrAn(type.name) + " message. Also converts values to other types if specified.",
-            "@param {" + fullName + "} message " + type.name,
-            "@param {$protobuf.ConversionOptions} [options] Conversion options",
+            "@param {" + fullName + "} " + (config.beautify ? "message" : "m") + " " + type.name,
+            "@param {$protobuf.ConversionOptions} [" + (config.beautify ? "options" : "o") + "] Conversion options",
             "@returns {Object.<string,*>} Plain object"
         ]);
         buildFunction(type, "toObject", protobuf.converter.toObject(type));
@@ -561,7 +599,7 @@ function buildService(ref, service) {
             "@param {" + cbName + "} callback Node-style callback called with the error, if any, and " + method.resolvedResponseType.name,
             "@returns {undefined}"
         ]);
-        push(name(service.name) + ".prototype[" + JSON.stringify(lcName) + "] = function " + name(lcName) + "(request, callback) {");
+        push(name(service.name) + ".prototype" + util.safeProp(lcName) + " = function " + name(lcName) + "(request, callback) {");
             ++indent;
             push("var requestData;");
             push("try {");