Skip to content

Commit 608009b

Browse files
sofislsofisl
committed
respond to comments
1 parent 087a33e commit 608009b

File tree

4 files changed

+210
-26
lines changed

4 files changed

+210
-26
lines changed

‎src/object.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,10 @@ ReflectionObject.prototype.onRemove = function onRemove(parent) {
156156
ReflectionObject.prototype.resolve = function resolve() {
157157
if (this.resolved)
158158
return this;
159-
this._resolveFeatures();
160-
if (this.root instanceof Root || this.parent)
159+
if (this.root instanceof Root || this.parent) {
160+
this._resolveFeatures();
161161
this.resolved = true;
162+
}
162163
return this;
163164
};
164165

@@ -216,7 +217,7 @@ ReflectionObject.prototype.setParsedOption = function setParsedOption(name, valu
216217
if (!this.parsedOptions) {
217218
this.parsedOptions = [];
218219
}
219-
var isFeature = /^features/.test(name);
220+
var isFeature = /^features\./.test(name);
220221
var parsedOptions = this.parsedOptions;
221222
if (propName) {
222223
// If setting a sub property of an option then try to merge it

‎src/parse.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ function parse(source, root, options) {
131131
try {
132132
return parseNumber(token, /* insideTryCatch */ true);
133133
} catch (e) {
134-
135134
/* istanbul ignore else */
136135
if (acceptTypeRef && typeRefRe.test(token))
137136
return token;
@@ -146,8 +145,15 @@ function parse(source, root, options) {
146145
do {
147146
if (acceptStrings && ((token = peek()) === "\"" || token === "'"))
148147
target.push(readString());
149-
else
150-
target.push([ start = parseId(next()), skip("to", true) ? parseId(next()) : start ]);
148+
else {
149+
try {
150+
target.push([ start = parseId(next()), skip("to", true) ? parseId(next()) : start ]);
151+
} catch (err) {
152+
if (typeRefRe.test(token) && edition) {
153+
target.push(token);
154+
}
155+
}
156+
}
151157
} while (skip(",", true));
152158
var dummy = {options: undefined};
153159
dummy.setOption = function(name, value) {
@@ -634,7 +640,7 @@ function parse(source, root, options) {
634640
dummy.setParsedOption = function(name, value, propName) {
635641
// In order to not change existing behavior, only calling
636642
// this for features
637-
if (/^features/.test(name)) {
643+
if (/^features\./.test(name)) {
638644
return ReflectionObject.prototype.setParsedOption.call(dummy, name, value, propName);
639645
}
640646
return undefined;

‎tests/feature_grammar.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ var protoEditionsGroup = `edition = "2023";
1616
group uint32 a = 1;\
1717
}`;
1818

19+
var noQuote = `edition = "2023";
20+
message Foo {
21+
reserved bar, baz;
22+
}`;
23+
1924
tape.test("editions grammar", function(test) {
2025
test.throws(function() {
2126
protobuf.parse(protoEditionsRequired);
@@ -29,5 +34,7 @@ tape.test("editions grammar", function(test) {
2934
protobuf.parse(protoEditionsGroup);
3035
}, Error, "Error: illegal token 'group'");
3136

37+
test.ok(protobuf.parse(noQuote));
38+
3239
test.end();
3340
});

‎tests/feature_resolution_editions.js

Lines changed: 189 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,67 @@
1+
/*
2+
edition = "2023";
3+
4+
option features.amazing_feature = A;
5+
option (mo_single_msg).nested.value = "x";
6+
service MyService {
7+
option features.amazing_feature = E;
8+
message MyRequest {};
9+
message MyResponse {};
10+
rpc MyMethod (MyRequest) returns (MyResponse) {
11+
option features.amazing_feature = L;
12+
};
13+
}
14+
15+
message Message {
16+
option features.amazing_feature = B;
17+
18+
string string_val = 1;
19+
string string_repeated = 2 [features.amazing_feature = F];
20+
21+
uint64 uint64_val = 3;
22+
uint64 uint64_repeated = 4;
23+
24+
bytes bytes_val = 5;
25+
bytes bytes_repeated = 6;
26+
27+
SomeEnum enum_val = 7;
28+
SomeEnum enum_repeated = 8;
29+
30+
extensions 10 to 100;
31+
extend Message {
32+
int32 bar = 10 [features.amazing_feature = I];
33+
}
34+
35+
message Nested {
36+
option features.amazing_feature = H;
37+
int64 count = 9;
38+
}
39+
40+
enum SomeEnumInMessage {
41+
option features.amazing_feature = G;
42+
ONE = 11;
43+
TWO = 12;
44+
}
45+
46+
oneof SomeOneOf {
47+
option features.amazing_feature = J;
48+
int32 a = 13;
49+
string b = 14;
50+
}
51+
52+
map<string,int64> int64_map = 15;
53+
}
54+
55+
extend Message {
56+
int32 bar = 16 [features.amazing_feature = D];
57+
}
58+
59+
enum SomeEnum {
60+
option features.amazing_feature = C;
61+
ONE = 1 [features.amazing_feature = K];
62+
TWO = 2;
63+
}
64+
*/
165
var tape = require("tape");
266

367
var protobuf = require("..");
@@ -29,6 +93,106 @@ message Message {
2993
}
3094
}
3195
`
96+
97+
var test1 =`edition = "2023";
98+
99+
option features.amazing_feature = A;`
100+
var test2 = `edition = "2023";
101+
option features.amazing_feature = A;
102+
103+
message Message {
104+
option features.amazing_feature = B;
105+
}`
106+
var test3 = `edition = "2023";
107+
option features.amazing_feature = A;
108+
enum SomeEnum {
109+
option features.amazing_feature = C;
110+
ONE = 1;
111+
TWO = 2;
112+
}`
113+
114+
var test4 = `edition = "2023";
115+
option features.amazing_feature = A;
116+
117+
message Message {
118+
option features.amazing_feature = B;
119+
}
120+
121+
extend Message {
122+
int32 bar = 16 [features.amazing_feature = D];
123+
}
124+
`
125+
var test5 = `edition = "2023";
126+
option features.amazing_feature = A;
127+
service MyService {
128+
option features.amazing_feature = E;
129+
message MyRequest {};
130+
message MyResponse {};
131+
}
132+
`
133+
var test6 = `edition = "2023";
134+
option features.amazing_feature = A;
135+
message Message {
136+
string string_val = 1;
137+
string string_repeated = 2 [features.amazing_feature = F];
138+
}`
139+
140+
var test7 = `edition = "2023";
141+
option features.amazing_feature = A;
142+
message Message {
143+
enum SomeEnumInMessage {
144+
option features.amazing_feature = G;
145+
ONE = 11;
146+
TWO = 12;
147+
}
148+
}`
149+
150+
var test8 = `edition = "2023";
151+
option features.amazing_feature = A;
152+
message Message {
153+
message Nested {
154+
option features.amazing_feature = H;
155+
int64 count = 9;
156+
}
157+
}`
158+
159+
var test9 = `edition = "2023";
160+
option features.amazing_feature = A;
161+
message Message {
162+
extend Message {
163+
int32 bar = 10 [features.amazing_feature = I];
164+
}
165+
}`
166+
167+
var test10 = `edition = "2023";
168+
option features.amazing_feature = A;
169+
message Message {
170+
oneof SomeOneOf {
171+
option features.amazing_feature = J;
172+
int32 a = 13;
173+
string b = 14;
174+
}
175+
}`
176+
177+
var test11 = `edition = "2023";
178+
option features.amazing_feature = A;
179+
enum SomeEnum {
180+
option features.amazing_feature = C;
181+
ONE = 1 [features.amazing_feature = K];
182+
TWO = 2;
183+
}`
184+
185+
var test12 = `edition = "2023";
186+
option features.amazing_feature = A;
187+
service MyService {
188+
option features.amazing_feature = E;
189+
message MyRequest {};
190+
message MyResponse {};
191+
rpc MyMethod (MyRequest) returns (MyResponse) {
192+
option features.amazing_feature = L;
193+
};
194+
}
195+
`
32196
tape.test("feautre resolution defaults", function(test) {
33197
var rootEditions = protobuf.parse(protoEditions2023).root;
34198
rootEditions.resolveAll();
@@ -70,24 +234,30 @@ tape.test("feature resolution inheritance", function(test) {
70234
})
71235
// Tests precedence for different levels of feature resolution
72236
tape.test("feature resolution editions precedence", function(test) {
237+
var root1 = protobuf.parse(test1).root.resolveAll()
238+
var root2 = protobuf.parse(test2).root.resolveAll();
239+
var root3 = protobuf.parse(test3).root.resolveAll();
240+
var root4 = protobuf.parse(test4).root.resolveAll();
241+
var root5 = protobuf.parse(test5).root.resolveAll();
242+
var root6 = protobuf.parse(test6).root.resolveAll();
243+
var root7 = protobuf.parse(test7).root.resolveAll();
244+
var root8 = protobuf.parse(test8).root.resolveAll();
245+
var root9 = protobuf.parse(test9).root.resolveAll();
246+
var root10 = protobuf.parse(test10).root.resolveAll();
247+
var root11 = protobuf.parse(test11).root.resolveAll();
248+
var root12 = protobuf.parse(test12).root.resolveAll();
249+
test.same(root1._features.amazing_feature, 'A');
250+
test.same(root2.lookup("Message")._features.amazing_feature, 'B')
251+
test.same(root3.lookupEnum("SomeEnum")._features.amazing_feature, 'C')
252+
test.same(root4.lookup("Message").fields[".bar"].declaringField._features.amazing_feature, 'D')
253+
test.same(root5.lookupService("MyService")._features.amazing_feature, 'E');
254+
test.same(root6.lookup("Message").fields.stringRepeated._features.amazing_feature, 'F')
255+
test.same(root7.lookup("Message").lookupEnum("SomeEnumInMessage")._features.amazing_feature, 'G')
256+
test.same(root8.lookup("Message").lookup("Nested")._features.amazing_feature, 'H')
257+
test.same(root9.lookup("Message").lookup(".Message.bar")._features.amazing_feature, 'I')
258+
test.same(root10.lookup("Message").lookup("SomeOneOf")._features.amazing_feature, 'J')
259+
test.same(root11.lookupEnum("SomeEnum")._valuesFeatures["ONE"].amazing_feature, 'K')
260+
test.same(root12.lookupService("MyService").lookup("MyMethod")._features.amazing_feature, 'L')
73261

74-
protobuf.load("tests/data/feature-resolution.proto", function(err, root) {
75-
if (err)
76-
return test.fail(err.message);
77-
root.resolveAll();
78-
test.same(root._features.amazing_feature, 'A');
79-
test.same(root.lookup("Message")._features.amazing_feature, 'B')
80-
test.same(root.lookupEnum("SomeEnum")._features.amazing_feature, 'C')
81-
test.same(root.lookup("Message").fields[".bar"].declaringField._features.amazing_feature, 'D')
82-
test.same(root.lookupService("MyService")._features.amazing_feature, 'E');
83-
test.same(root.lookup("Message").fields.stringRepeated._features.amazing_feature, 'F')
84-
test.same(root.lookup("Message").lookupEnum("SomeEnumInMessage")._features.amazing_feature, 'G')
85-
test.same(root.lookup("Message").lookup("Nested")._features.amazing_feature, 'H')
86-
test.same(root.lookup("Message").lookup(".Message.bar")._features.amazing_feature, 'I')
87-
test.same(root.lookup("Message").lookup("SomeOneOf")._features.amazing_feature, 'J')
88-
test.same(root.lookupEnum("SomeEnum")._valuesFeatures["ONE"].amazing_feature, 'K')
89-
test.same(root.lookupService("MyService").lookup("MyMethod")._features.amazing_feature, 'L')
90-
91-
test.end();
92-
})
262+
test.end();
93263
})

0 commit comments

Comments
 (0)