Skip to content

Commit 5d5b4b3

Browse files
hegemonichegemonic
committed
fix!: allow JSDoc-style type expressions only if support is enabled
A Peggy grammar can specify an _action_ to run when a rule matches the input string. Catharsis's grammar uses these actions to build an AST for each type expression. In some cases, we need a rule to match some construct, then void the match if JSDoc-style types are not enabled. Apparently I thought that you can undo the match by returning `null` from the action. In reality, `null` doesn't work that way. As a result, we've been allowing at least some JSDoc-specific constructs even when JSDoc-style types aren't enabled. Instead of returning `null`, we now use Peggy's `error()` function to complain about these constructs when JSDoc-style types aren't enabled. We also now have tests to confirm that JSDoc-style types can be parsed only when they're enabled. I'm calling this a breaking change because, if JSDoc-style types aren't enabled, then some type expressions that we used to allow are now prohibited. To fix this breakage, enable JSDoc-style types, or update the prohibited type expressions.
1 parent 25339f1 commit 5d5b4b3

6 files changed

Lines changed: 293 additions & 260 deletions

File tree

‎lib/parser.js‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@
2727
return obj;
2828
}
2929

30+
// Examples of property chains that contain JSDoc-style separators. We want to provide an example
31+
// that uses the same separator as the type expression. We also want the examples to be realistic.
32+
const PROPERTY_CHAIN_EXAMPLES = {
33+
'#': 'Item#get',
34+
'~': 'Item~get',
35+
':': 'module:core',
36+
'/': 'module:core/setup'
37+
};
38+
3039
class peg$SyntaxError extends SyntaxError {
3140
constructor(message, expected, found, location) {
3241
super(message);
@@ -428,9 +437,11 @@ function peg$parse(input, options) {
428437
function peg$f2(expr, optionalPre, postfix, optionalPost) {
429438
let result = expr;
430439

431-
// we only allow one optional operator
432440
if (optionalPre && optionalPost) {
433-
return null;
441+
error(
442+
'You can use the optional (`=`) operator only once, either before or after the ' +
443+
`\`${postfix}\` operator.`
444+
);
434445
}
435446

436447
// "non-nullable, yet optional" makes no sense, but we allow it
@@ -511,7 +522,7 @@ function peg$parse(input, options) {
511522

512523
// we only allow this if JSDoc parsing is enabled
513524
if (!options.jsdoc) {
514-
return null;
525+
jsdocError('a name expression followed by one or more pairs of square brackets (`[]`)');
515526
}
516527

517528
result = nest(name);
@@ -576,9 +587,8 @@ function peg$parse(input, options) {
576587
function peg$f20() {
577588
let result;
578589

579-
// we only allow this if JSDoc parsing is enabled
580590
if (!options.jsdoc) {
581-
return null;
591+
jsdocError('the expression `function[]`');
582592
}
583593

584594
result = {
@@ -598,9 +608,8 @@ function peg$parse(input, options) {
598608
return result;
599609
}
600610
function peg$f21(sig, opt) {
601-
// signature is required unless JSDoc parsing is enabled
602611
if (!sig && !options.jsdoc) {
603-
return null;
612+
jsdocError('the expression `function` without a function signature');
604613
} else if (sig === null) {
605614
sig = {
606615
params: []
@@ -801,14 +810,16 @@ function peg$parse(input, options) {
801810
}
802811
function peg$f48(t) {
803812
if (!options.jsdoc) {
804-
return null;
813+
jsdocError('a type other than a name expression')
805814
}
806815

807816
return t;
808817
}
809818
function peg$f49(lit) {
810819
if (!options.jsdoc) {
811-
return null;
820+
jsdocError(
821+
'a name expression that contains only a double-quoted string, like `"myExpression"`'
822+
);
812823
}
813824

814825
return {
@@ -817,7 +828,9 @@ function peg$parse(input, options) {
817828
}
818829
function peg$f50(lit) {
819830
if (!options.jsdoc) {
820-
return null;
831+
jsdocError(
832+
"a name expression that contains only a single-quoted string, like `'myExpression'`"
833+
);
821834
}
822835

823836
return {
@@ -845,15 +858,18 @@ function peg$parse(input, options) {
845858
function peg$f55(sep, prop) {
846859
// we only allow '.' unless JSDoc parsing is enabled
847860
if (sep !== '.' && !options.jsdoc) {
848-
return null;
861+
jsdocError(
862+
`the separator \`${sep}\` in a chain of identifiers, like ` +
863+
`\`${PROPERTY_CHAIN_EXAMPLES[sep]}\``
864+
);
849865
}
850866

851867
return sep + prop;
852868
}
853869
function peg$f56(name) { return name; }
854870
function peg$f57(parts) {
855871
if (!options.jsdoc) {
856-
return null;
872+
jsdocError('an identifier that ends with a value enclosed in parentheses, like `my(item)`');
857873
}
858874

859875
parts = parts || [];
@@ -862,7 +878,9 @@ function peg$parse(input, options) {
862878
}
863879
function peg$f58(params) {
864880
if (!options.jsdoc) {
865-
return null;
881+
jsdocError(
882+
'an identifier that ends with a list of function parameters, like `my(name, ...values)`'
883+
);
866884
}
867885

868886
params = params || [];
@@ -5029,6 +5047,11 @@ function peg$parse(input, options) {
50295047
return s0;
50305048
}
50315049

5050+
5051+
function jsdocError(msg) {
5052+
error(`To allow ${msg}, you must enable JSDoc-style type expressions.`)
5053+
}
5054+
50325055
peg$result = peg$startRuleFunction();
50335056

50345057
const peg$success = (peg$result !== peg$FAILED && peg$currPos === input.length);

‎lib/parser.pegjs‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* @license MIT License <http://opensource.org/licenses/mit-license.php/>
1010
*/
1111

12+
// Global initializer
1213
{{
1314
const Types = require('./types');
1415

@@ -31,8 +32,24 @@
3132

3233
return obj;
3334
}
35+
36+
// Examples of property chains that contain JSDoc-style separators. We want to provide an example
37+
// that uses the same separator as the type expression. We also want the examples to be realistic.
38+
const PROPERTY_CHAIN_EXAMPLES = {
39+
'#': 'Item#get',
40+
'~': 'Item~get',
41+
':': 'module:core',
42+
'/': 'module:core/setup'
43+
};
3444
}}
3545

46+
// Per-parse initializer
47+
{
48+
function jsdocError(msg) {
49+
error(`To allow ${msg}, you must enable JSDoc-style type expressions.`)
50+
}
51+
}
52+
3653
TypeExpression
3754
= TypeUnionLegacySyntax
3855
/ r:Repeatable? unk:UnknownLiteral !BasicTypeExpression {
@@ -56,9 +73,11 @@ TypeExpression
5673
/ expr:BasicTypeExpression optionalPre:Optional? postfix:('?' / '!') optionalPost:Optional? {
5774
let result = expr;
5875

59-
// we only allow one optional operator
6076
if (optionalPre && optionalPost) {
61-
return null;
77+
error(
78+
'You can use the optional (`=`) operator only once, either before or after the ' +
79+
`\`${postfix}\` operator.`
80+
);
6281
}
6382

6483
// "non-nullable, yet optional" makes no sense, but we allow it
@@ -188,7 +207,7 @@ NameExpressionTypeNonRepeatable
188207

189208
// we only allow this if JSDoc parsing is enabled
190209
if (!options.jsdoc) {
191-
return null;
210+
jsdocError('a name expression followed by one or more pairs of square brackets (`[]`)');
192211
}
193212

194213
result = nest(name);
@@ -277,9 +296,8 @@ FunctionTypeNonRepeatable
277296
= FunctionLiteral '[]' {
278297
let result;
279298

280-
// we only allow this if JSDoc parsing is enabled
281299
if (!options.jsdoc) {
282-
return null;
300+
jsdocError('the expression `function[]`');
283301
}
284302

285303
result = {
@@ -299,9 +317,8 @@ FunctionTypeNonRepeatable
299317
return result;
300318
}
301319
/ FunctionLiteral !NameExpression _ sig:FunctionSignatureType? opt:Optional? {
302-
// signature is required unless JSDoc parsing is enabled
303320
if (!sig && !options.jsdoc) {
304-
return null;
321+
jsdocError('the expression `function` without a function signature');
305322
} else if (sig === null) {
306323
sig = {
307324
params: []
@@ -560,7 +577,7 @@ FieldName
560577
/ ReservedWordNameExpressionTypeNonRepeatable
561578
/ t:TypeExpression {
562579
if (!options.jsdoc) {
563-
return null;
580+
jsdocError('a type other than a name expression')
564581
}
565582

566583
return t;
@@ -569,7 +586,9 @@ FieldName
569586
NameExpression
570587
= lit:DoubleStringLiteral {
571588
if (!options.jsdoc) {
572-
return null;
589+
jsdocError(
590+
'a name expression that contains only a double-quoted string, like `"myExpression"`'
591+
);
573592
}
574593

575594
return {
@@ -578,7 +597,9 @@ NameExpression
578597
}
579598
/ lit:SingleStringLiteral {
580599
if (!options.jsdoc) {
581-
return null;
600+
jsdocError(
601+
"a name expression that contains only a single-quoted string, like `'myExpression'`"
602+
);
582603
}
583604

584605
return {
@@ -620,7 +641,10 @@ PropertyChainItem
620641
= sep:('.' / '#' / '~' / ':' / '/') !'<' prop:PropertyIdentifier {
621642
// we only allow '.' unless JSDoc parsing is enabled
622643
if (sep !== '.' && !options.jsdoc) {
623-
return null;
644+
jsdocError(
645+
`the separator \`${sep}\` in a chain of identifiers, like ` +
646+
`\`${PROPERTY_CHAIN_EXAMPLES[sep]}\``
647+
);
624648
}
625649

626650
return sep + prop;
@@ -657,7 +681,7 @@ IdentifierPart
657681
IdentifierEnd
658682
= '(' _ parts:IdentifierPart* _ ')' {
659683
if (!options.jsdoc) {
660-
return null;
684+
jsdocError('an identifier that ends with a value enclosed in parentheses, like `my(item)`');
661685
}
662686

663687
parts = parts || [];
@@ -666,7 +690,9 @@ IdentifierEnd
666690
}
667691
/ '(' _ params:ParametersType? _ ')' {
668692
if (!options.jsdoc) {
669-
return null;
693+
jsdocError(
694+
'an identifier that ends with a list of function parameters, like `my(name, ...values)`'
695+
);
670696
}
671697

672698
params = params || [];

‎test/parser.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,24 @@ function checkTypes(filepath, options) {
5757
expect(validationErrors).toBeEmptyArray();
5858
}
5959

60+
function checkBadTypes(filepath, options) {
61+
const errors = [];
62+
const types = require(filepath);
63+
64+
types.forEach((type) => {
65+
try {
66+
parseIt(type, options);
67+
errors.push(`\`${type.expression}\` was parsed successfully but should have failed`);
68+
} catch (e) {
69+
// This is supposed to throw, so do nothing.
70+
}
71+
});
72+
73+
errors.forEach((e) => {
74+
expect(e).toBeNull();
75+
});
76+
}
77+
6078
describe('parser', () => {
6179
describe('parse()', () => {
6280
const specs = './test/specs';
@@ -74,7 +92,14 @@ describe('parser', () => {
7492
});
7593
}
7694

95+
function nonJsdocTester(specPath, basename) {
96+
it(`can't parse types in the "${basename}" spec when JSDoc type parsing is not enabled`, () => {
97+
checkBadTypes(path.join(specPath, basename), {});
98+
});
99+
}
100+
77101
runTestSpecs(specs, tester);
78102
runTestSpecs(jsdocSpecs, jsdocTester);
103+
runTestSpecs(jsdocSpecs, nonJsdocTester);
79104
});
80105
});

0 commit comments

Comments
 (0)