Skip to content

Commit

Permalink
Other: Properly remove unnecessary (packed) options from JSON descrip…
Browse files Browse the repository at this point in the history
…tors
  • Loading branch information
dcodeIO committed Apr 13, 2017
1 parent 056ecc3 commit 3a20968
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 95 deletions.
2 changes: 1 addition & 1 deletion cli/targets/proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ function buildFieldOptions(field) {
var sb = [];
keys.forEach(function(key) {
var val = field.options[key];
var wireType = types.packed[field.resolvedType instanceof Enum ? "uint32" : field.type];
var wireType = types.packed[field.resolvedType instanceof Enum ? "int32" : field.type];
switch (key) {
case "packed":
val = Boolean(val);
Expand Down
13 changes: 7 additions & 6 deletions ext/descriptor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,16 +234,16 @@ Type.prototype.toDescriptor = function toDescriptor(syntax) {
if (this._fieldsArray[i] instanceof MapField) { // map fields are repeated FieldNameEntry
var keyType = toDescriptorType(this._fieldsArray[i].keyType, this._fieldsArray[i].resolvedKeyType),
valueType = toDescriptorType(this._fieldsArray[i].type, this._fieldsArray[i].resolvedType),
valueTypeName = valueType === 11 || valueType === 14
valueTypeName = valueType === /* type */ 11 || valueType === /* enum */ 14
? this._fieldsArray[i].resolvedType && this._fieldsArray[i].resolvedType.fullName || this._fieldsArray[i].type
: undefined;
descriptor.nestedType.push(exports.DescriptorProto.create({
name: descriptor.field[descriptor.field.length - 1].type_name,
name: descriptor.field[descriptor.field.length - 1].typeName,
field: [
exports.FieldDescriptorProto.create({ name: "key", number: 1, label: 1, type: keyType }), // can't be message
exports.FieldDescriptorProto.create({ name: "key", number: 1, label: 1, type: keyType }), // can't reference a type or enum
exports.FieldDescriptorProto.create({ name: "value", number: 2, label: 1, type: valueType, typeName: valueTypeName })
],
options: exports.MessageOptions({ mapEntry: true })
options: exports.MessageOptions.create({ mapEntry: true })
}));
}
}
Expand Down Expand Up @@ -482,7 +482,7 @@ Field.prototype.toDescriptor = function toDescriptor(syntax) {
case 10: // group
case 11: // type
case 14: // enum
descriptor.typeName = this.resolvedType ? this.resolvedType.fullName : this.type;
descriptor.typeName = this.resolvedType ? this.resolvedType.fullName : this.type; // TODO: shorten
break;
}

Expand Down Expand Up @@ -567,7 +567,8 @@ Enum.fromDescriptor = function fromDescriptor(descriptor) {

return new Enum(
descriptor.name && descriptor.name.length ? descriptor.name : "Enum" + unnamedEnumIndex++,
values
values,
descriptor.options && descriptor.options.allowAlias ? { allowAlias: true } : undefined
);
};

Expand Down
15 changes: 11 additions & 4 deletions ext/descriptor/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,22 @@ var proto = require("../../google/protobuf/descriptor.json")/*{
}
}*/;

var root = protobuf.Root.fromJSON(proto);
var root = protobuf.Root.fromJSON(proto).resolveAll();

console.log("Original proto", JSON.stringify(root, null, 2));
// console.log("Original proto", JSON.stringify(root, null, 2));

var msg = root.toDescriptor();

console.log("\nDescriptor", JSON.stringify(msg.toObject(), null, 2));
// console.log("\nDescriptor", JSON.stringify(msg.toObject(), null, 2));

var buf = descriptor.FileDescriptorSet.encode(msg).finish();
var root2 = protobuf.Root.fromDescriptor(buf, "proto2");

console.log("\nDecoded proto", JSON.stringify(root2, null, 2));
// console.log("\nDecoded proto", JSON.stringify(root2, null, 2));

require("deep-diff").diff(root.toJSON(), root2.toJSON()).forEach(function(diff) {
console.log(diff.kind + " @ " + diff.path.join("."));
console.log("lhs:", diff.lhs);
console.log("rhs:", diff.rhs);
console.log();
});
105 changes: 31 additions & 74 deletions google/protobuf/descriptor.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
"file": {
"rule": "repeated",
"type": "FileDescriptorProto",
"id": 1,
"options": {}
"id": 1
}
}
},
Expand All @@ -27,10 +26,7 @@
"dependency": {
"rule": "repeated",
"type": "string",
"id": 3,
"options": {
"packed": false
}
"id": 3
},
"publicDependency": {
"rule": "repeated",
Expand All @@ -51,26 +47,22 @@
"messageType": {
"rule": "repeated",
"type": "DescriptorProto",
"id": 4,
"options": {}
"id": 4
},
"enumType": {
"rule": "repeated",
"type": "EnumDescriptorProto",
"id": 5,
"options": {}
"id": 5
},
"service": {
"rule": "repeated",
"type": "ServiceDescriptorProto",
"id": 6,
"options": {}
"id": 6
},
"extension": {
"rule": "repeated",
"type": "FieldDescriptorProto",
"id": 7,
"options": {}
"id": 7
},
"options": {
"type": "FileOptions",
Expand All @@ -95,38 +87,32 @@
"field": {
"rule": "repeated",
"type": "FieldDescriptorProto",
"id": 2,
"options": {}
"id": 2
},
"extension": {
"rule": "repeated",
"type": "FieldDescriptorProto",
"id": 6,
"options": {}
"id": 6
},
"nestedType": {
"rule": "repeated",
"type": "DescriptorProto",
"id": 3,
"options": {}
"id": 3
},
"enumType": {
"rule": "repeated",
"type": "EnumDescriptorProto",
"id": 4,
"options": {}
"id": 4
},
"extensionRange": {
"rule": "repeated",
"type": "ExtensionRange",
"id": 5,
"options": {}
"id": 5
},
"oneofDecl": {
"rule": "repeated",
"type": "OneofDescriptorProto",
"id": 8,
"options": {}
"id": 8
},
"options": {
"type": "MessageOptions",
Expand All @@ -135,16 +121,12 @@
"reservedRange": {
"rule": "repeated",
"type": "ReservedRange",
"id": 9,
"options": {}
"id": 9
},
"reservedName": {
"rule": "repeated",
"type": "string",
"id": 10,
"options": {
"packed": false
}
"id": 10
}
},
"nested": {
Expand Down Expand Up @@ -270,8 +252,7 @@
"value": {
"rule": "repeated",
"type": "EnumValueDescriptorProto",
"id": 2,
"options": {}
"id": 2
},
"options": {
"type": "EnumOptions",
Expand Down Expand Up @@ -304,8 +285,7 @@
"method": {
"rule": "repeated",
"type": "MethodDescriptorProto",
"id": 2,
"options": {}
"id": 2
},
"options": {
"type": "ServiceOptions",
Expand Down Expand Up @@ -408,8 +388,7 @@
"uninterpretedOption": {
"rule": "repeated",
"type": "UninterpretedOption",
"id": 999,
"options": {}
"id": 999
}
},
"extensions": [
Expand Down Expand Up @@ -455,8 +434,7 @@
"uninterpretedOption": {
"rule": "repeated",
"type": "UninterpretedOption",
"id": 999,
"options": {}
"id": 999
}
},
"extensions": [
Expand Down Expand Up @@ -507,8 +485,7 @@
"uninterpretedOption": {
"rule": "repeated",
"type": "UninterpretedOption",
"id": 999,
"options": {}
"id": 999
}
},
"extensions": [
Expand Down Expand Up @@ -545,8 +522,7 @@
"uninterpretedOption": {
"rule": "repeated",
"type": "UninterpretedOption",
"id": 999,
"options": {}
"id": 999
}
},
"extensions": [
Expand All @@ -569,8 +545,7 @@
"uninterpretedOption": {
"rule": "repeated",
"type": "UninterpretedOption",
"id": 999,
"options": {}
"id": 999
}
},
"extensions": [
Expand All @@ -589,8 +564,7 @@
"uninterpretedOption": {
"rule": "repeated",
"type": "UninterpretedOption",
"id": 999,
"options": {}
"id": 999
}
},
"extensions": [
Expand All @@ -609,8 +583,7 @@
"uninterpretedOption": {
"rule": "repeated",
"type": "UninterpretedOption",
"id": 999,
"options": {}
"id": 999
}
},
"extensions": [
Expand All @@ -629,8 +602,7 @@
"uninterpretedOption": {
"rule": "repeated",
"type": "UninterpretedOption",
"id": 999,
"options": {}
"id": 999
}
},
"extensions": [
Expand All @@ -645,8 +617,7 @@
"name": {
"rule": "repeated",
"type": "NamePart",
"id": 2,
"options": {}
"id": 2
},
"identifierValue": {
"type": "string",
Expand Down Expand Up @@ -695,8 +666,7 @@
"location": {
"rule": "repeated",
"type": "Location",
"id": 1,
"options": {}
"id": 1
}
},
"nested": {
Expand All @@ -705,18 +675,12 @@
"path": {
"rule": "repeated",
"type": "int32",
"id": 1,
"options": {
"packed": true
}
"id": 1
},
"span": {
"rule": "repeated",
"type": "int32",
"id": 2,
"options": {
"packed": true
}
"id": 2
},
"leadingComments": {
"type": "string",
Expand All @@ -729,10 +693,7 @@
"leadingDetachedComments": {
"rule": "repeated",
"type": "string",
"id": 6,
"options": {
"packed": false
}
"id": 6
}
}
}
Expand All @@ -743,8 +704,7 @@
"annotation": {
"rule": "repeated",
"type": "Annotation",
"id": 1,
"options": {}
"id": 1
}
},
"nested": {
Expand All @@ -753,10 +713,7 @@
"path": {
"rule": "repeated",
"type": "int32",
"id": 1,
"options": {
"packed": true
}
"id": 1
},
"sourceFile": {
"type": "string",
Expand Down
2 changes: 1 addition & 1 deletion src/decoder.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function decoder(mtype) {
var i = 0;
for (; i < /* initializes */ mtype.fieldsArray.length; ++i) {
var field = mtype._fieldsArray[i].resolve(),
type = field.resolvedType instanceof Enum ? "uint32" : field.type,
type = field.resolvedType instanceof Enum ? "int32" : field.type,
ref = "m" + util.safeProp(field.name); gen
("case %d:", field.id);

Expand Down
2 changes: 1 addition & 1 deletion src/encoder.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function encoder(mtype) {
for (var i = 0; i < fields.length; ++i) {
var field = fields[i].resolve(),
index = mtype._fieldsArray.indexOf(field),
type = field.resolvedType instanceof Enum ? "uint32" : field.type,
type = field.resolvedType instanceof Enum ? "int32" : field.type,
wireType = types.basic[type];
ref = "m" + util.safeProp(field.name);

Expand Down
Loading

0 comments on commit 3a20968

Please sign in to comment.