Skip to content

Commit

Permalink
Other: Restrict comment parsing for static code to explicit /**-block…
Browse files Browse the repository at this point in the history
…s because old protos may generate a lot of nonsense otherwise, see #640
  • Loading branch information
dcodeIO committed Jan 12, 2017
1 parent 340d6aa commit 3f6ffd0
Show file tree
Hide file tree
Showing 22 changed files with 186 additions and 540 deletions.
2 changes: 1 addition & 1 deletion cli/targets/static.js
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ function buildEnum(ref, enm) {
);
Object.keys(enm.values).forEach(function(key) {
var val = enm.values[key];
comment.push("@property {number} " + key + "=" + val + " " + (enm.comments[key] ? enm.comments[key] : key + " value"));
comment.push("@property {number} " + key + "=" + val + " " + (enm.comments[key] || key + " value"));
});
pushComment(comment);
push(name(ref) + "." + name(enm.name) + " = (function() {");
Expand Down
4 changes: 3 additions & 1 deletion dist/noparse/protobuf.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/noparse/protobuf.js.map

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dist/noparse/protobuf.min.js

Large diffs are not rendered by default.

Binary file modified dist/noparse/protobuf.min.js.gz
Binary file not shown.
2 changes: 1 addition & 1 deletion dist/noparse/protobuf.min.js.map

Large diffs are not rendered by default.

53 changes: 29 additions & 24 deletions dist/protobuf.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/protobuf.js.map

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions dist/protobuf.min.js

Large diffs are not rendered by default.

Binary file modified dist/protobuf.min.js.gz
Binary file not shown.
2 changes: 1 addition & 1 deletion dist/protobuf.min.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/runtime/protobuf.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/runtime/protobuf.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified dist/runtime/protobuf.min.js.gz
Binary file not shown.
2 changes: 2 additions & 0 deletions src/converter.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ function genValuePartial_toObject(gen, field, fieldIndex, prop) {
converter.toObject = function toObject(mtype) {
/* eslint-disable no-unexpected-multiline, block-scoped-var, no-redeclare */
var fields = mtype.fieldsArray;
if (!fields.length)
return util.codegen()("return {}");
var gen = util.codegen("m", "o")
("if(!o)")
("o={}")
Expand Down
19 changes: 15 additions & 4 deletions src/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,14 @@ function parse(source, root, options) {
throw illegal(name, "name");
name = applyCase(name);
skip("=");
var id = parseId(next());
var field = parseInlineOptions(new Field(name, id, type, rule, extend));
var line = tn.line(),
field = new Field(name, parseId(next()), type, rule, extend);
field.comment = cmnt();
parseInlineOptions(field);
if (!field.comment) {
peek();
field.comment = cmnt(/* if on */ line);
}
// JSON defaults to packed=true if not set so we have to set packed=false explicity when
// parsing proto2 descriptors without the option, where applicable.
if (field.repeated && types.packed[type] !== undefined && !isProto3)
Expand Down Expand Up @@ -379,8 +384,14 @@ function parse(source, root, options) {
name = applyCase(name);
skip("=");
var id = parseId(next());
var field = parseInlineOptions(new MapField(name, id, keyType, valueType));
var field = new MapField(name, id, keyType, valueType);
var line = tn.line();
field.comment = cmnt();
parseInlineOptions(field);
if (!field.comment) {
peek();
field.comment = cmnt(/* if on */ line);
}
parent.add(field);
}

Expand Down Expand Up @@ -447,7 +458,7 @@ function parse(source, root, options) {
parent.add(name, value, comment);
parseInlineOptions({}); // skips enum value options
if (!comment) {
peek(); // trailing comment?
peek();
parent.comments[name] = cmnt(/* if on */ line);
}
}
Expand Down
30 changes: 11 additions & 19 deletions src/tokenize.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ function tokenize(source) {
length = source.length,
line = 1,
comment = null,
commentLine = 0,
commentType = 0;
commentLine = 0;


var stack = [];
Expand Down Expand Up @@ -96,23 +95,19 @@ function tokenize(source) {
* @returns {undefined}
* @inner
*/
function setComment(start, end, type) {
var count = 0;
function setComment(start, end) {
var text = source
.substring(start, end)
.substring(start, end);
if (text.charAt(0) !== "*") // use /**-blocks only
return;
text = text
.split(/\n/g)
.map(function(line) {
++count;
return line.replace(/ *[*/]+ */, "").trim();
})
.join("\n")
.trim();
if (comment && commentLine === line - count && type === commentType)
comment += "\n" + text;
else {
comment = text;
commentType = type;
}
comment = text;
commentLine = line;
}

Expand Down Expand Up @@ -149,7 +144,6 @@ function tokenize(source) {
if (offset === length)
return null;
++offset;
setComment(start, offset - 1, 0);
++line;
repeat = true;
} else if ((curr = charAt(offset)) === "*") { /* Block */
Expand Down Expand Up @@ -234,13 +228,11 @@ function tokenize(source) {
* @returns {?string} Comment, if any
* @inner
*/
function cmnt(ifOnLine) {
var ret = (ifOnLine !== undefined
? commentLine === ifOnLine
: commentLine === line - 1) && comment || null;
if (comment) {
function cmnt() {
var ret = commentLine === line - 1 && comment || null;
if (ret) {
comment = null;
commentLine = -1;
commentLine = 0;
}
return ret;
}
Expand Down
11 changes: 5 additions & 6 deletions tests/comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,15 @@ tape.test("comments", function(test) {
if (err)
throw test.fail(err.message);

test.equal(root.lookup("Test1").comment, "Message\nwith\na\ncomment.", "should parse * blocks");
test.equal(root.lookup("Test2").comment, "Message\nwith\na\ncomment.", "should parse // blocks");
test.equal(root.lookup("Test3").comment, "a\ncomment.", "should parse the last coherent type of mixed blocks");
test.equal(root.lookup("Test1").comment, "Message\nwith\na\ncomment.", "should parse /**-blocks");
test.equal(root.lookup("Test2").comment, null, "should not parse //-blocks");
test.equal(root.lookup("Test3").comment, null, "should not parse /*-blocks");

test.equal(root.lookup("Test1.field1").comment, "Field with a comment.", "should parse blocks for message fields");
test.equal(root.lookup("Test1.field2").comment, "Field with a comment.", "should parse lines for message fields");
test.equal(root.lookup("Test1.field2").comment, null, "should not parse lines for message fields");

test.equal(root.lookup("Test3").comments.ONE, "Value with a comment.", "should parse blocks for enum values");
test.equal(root.lookup("Test3").comments.TWO, "Value with a comment.", "should parse lines for enum values");
test.equal(root.lookup("Test3").comments.THREE, "Value with a comment.", "should parse trailing lines for enum values");
test.equal(root.lookup("Test3").comments.TWO, null, "should not parse lines for enum values");

test.end();
});
Expand Down
24 changes: 5 additions & 19 deletions tests/data/comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ $root.Test1 = (function() {
$prototype.field1 = "";

/**
* Field with a comment.
* Test1 field2.
* @type {number}
*/
$prototype.field2 = 0;
Expand Down Expand Up @@ -230,10 +230,6 @@ $root.Test2 = (function() {

/**
* Constructs a new Test2.
* @classdesc Message
* with
* a
* comment.
* @exports Test2
* @constructor
* @param {Object} [properties] Properties to set
Expand Down Expand Up @@ -350,15 +346,8 @@ $root.Test2 = (function() {
* @param {$protobuf.ConversionOptions} [options] Conversion options
* @returns {Object.<string,*>} Plain object
*/
Test2.toObject = (function() { return function toObject(message, options) {
if (!options) {
options = {};
}
var object = {};
for (var keys = Object.keys(message), i = 0; i < keys.length; ++i) {
switch (keys[i]) {}
}
return object;
Test2.toObject = (function() { return function toObject() {
return {};
};})();

/**
Expand Down Expand Up @@ -386,20 +375,17 @@ $root.Test2 = (function() {
})();

/**
* a
* comment.
* Test3 enum.
* @exports Test3
* @enum {number}
* @property {number} ONE=1 Value with a comment.
* @property {number} TWO=2 Value with a comment.
* @property {number} THREE=3 Value with a comment.
* @property {number} TWO=2 TWO value
*/
$root.Test3 = (function() {
var valuesById = {},
values = Object.create(valuesById);
values[valuesById[1] = "ONE"] = 1;
values[valuesById[2] = "TWO"] = 2;
values[valuesById[3] = "THREE"] = 3;
return values;
})();

Expand Down
Loading

0 comments on commit 3f6ffd0

Please sign in to comment.