Skip to content

Commit c333e14

Browse files
authored
Ensure that JSDoc parsing happens within a ParsingContext (microsoft#52710)
1 parent 840a0bf commit c333e14

File tree

5 files changed

+43
-48
lines changed

5 files changed

+43
-48
lines changed

src/compiler/parser.ts

+25-5
Original file line numberDiff line numberDiff line change
@@ -1491,6 +1491,8 @@ namespace Parser {
14911491
var identifiers: Map<string, string>;
14921492
var identifierCount: number;
14931493

1494+
// TODO(jakebailey): This type is a lie; this value actually contains the result
1495+
// of ORing a bunch of `1 << ParsingContext.XYZ`.
14941496
var parsingContext: ParsingContext;
14951497

14961498
var notParenthesizedArrow: Set<number> | undefined;
@@ -2872,9 +2874,13 @@ namespace Parser {
28722874
return tokenIsIdentifierOrKeyword(token()) || token() === SyntaxKind.OpenBraceToken;
28732875
case ParsingContext.JsxChildren:
28742876
return true;
2877+
case ParsingContext.JSDocComment:
2878+
return true;
2879+
case ParsingContext.Count:
2880+
return Debug.fail("ParsingContext.Count used as a context"); // Not a real context, only a marker.
2881+
default:
2882+
Debug.assertNever(parsingContext, "Non-exhaustive case in 'isListElement'.");
28752883
}
2876-
2877-
return Debug.fail("Non-exhaustive case in 'isListElement'.");
28782884
}
28792885

28802886
function isValidHeritageClauseObjectLiteral() {
@@ -3010,6 +3016,9 @@ namespace Parser {
30103016

30113017
// True if positioned at element or terminator of the current list or any enclosing list
30123018
function isInSomeParsingContext(): boolean {
3019+
// We should be in at least one parsing context, be it SourceElements while parsing
3020+
// a SourceFile, or JSDocComment when lazily parsing JSDoc.
3021+
Debug.assert(parsingContext, "Missing parsing context");
30133022
for (let kind = 0; kind < ParsingContext.Count; kind++) {
30143023
if (parsingContext & (1 << kind)) {
30153024
if (isListElement(kind, /*inErrorRecovery*/ true) || isListTerminator(kind)) {
@@ -3385,6 +3394,7 @@ namespace Parser {
33853394
case ParsingContext.JsxAttributes: return parseErrorAtCurrentToken(Diagnostics.Identifier_expected);
33863395
case ParsingContext.JsxChildren: return parseErrorAtCurrentToken(Diagnostics.Identifier_expected);
33873396
case ParsingContext.AssertEntries: return parseErrorAtCurrentToken(Diagnostics.Identifier_or_string_literal_expected); // AssertionKey.
3397+
case ParsingContext.JSDocComment: return parseErrorAtCurrentToken(Diagnostics.Identifier_expected);
33883398
case ParsingContext.Count: return Debug.fail("ParsingContext.Count used as a context"); // Not a real context, only a marker.
33893399
default: Debug.assertNever(context);
33903400
}
@@ -7289,6 +7299,8 @@ namespace Parser {
72897299

72907300
function tryReuseAmbientDeclaration(pos: number): Statement | undefined {
72917301
return doInsideOfContext(NodeFlags.Ambient, () => {
7302+
// TODO(jakebailey): this is totally wrong; `parsingContext` is the result of ORing a bunch of `1 << ParsingContext.XYZ`.
7303+
// The enum should really be a bunch of flags.
72927304
const node = currentNode(parsingContext, pos);
72937305
if (node) {
72947306
return consumeNode(node) as Statement;
@@ -8492,7 +8504,8 @@ namespace Parser {
84928504
TupleElementTypes, // Element types in tuple element type list
84938505
HeritageClauses, // Heritage clauses for a class or interface declaration.
84948506
ImportOrExportSpecifiers, // Named import clause's import specifier list,
8495-
AssertEntries, // Import entries list.
8507+
AssertEntries, // Import entries list.
8508+
JSDocComment, // Parsing via JSDocParser
84968509
Count // Number of parsing contexts
84978510
}
84988511

@@ -8598,6 +8611,9 @@ namespace Parser {
85988611
}
85998612

86008613
function parseJSDocCommentWorker(start = 0, length: number | undefined): JSDoc | undefined {
8614+
const saveParsingContext = parsingContext;
8615+
parsingContext |= 1 << ParsingContext.JSDocComment;
8616+
86018617
const content = sourceText;
86028618
const end = length === undefined ? content.length : start + length;
86038619
length = end - start;
@@ -8620,7 +8636,11 @@ namespace Parser {
86208636
const parts: JSDocComment[] = [];
86218637

86228638
// + 3 for leading /**, - 5 in total for /** */
8623-
return scanner.scanRange(start + 3, length - 5, () => {
8639+
const result = scanner.scanRange(start + 3, length - 5, doJSDocScan);
8640+
parsingContext = saveParsingContext;
8641+
return result;
8642+
8643+
function doJSDocScan() {
86248644
// Initially we can parse out a tag. We also have seen a starting asterisk.
86258645
// This is so that /** * @type */ doesn't parse.
86268646
let state = JSDocState.SawAsterisk;
@@ -8726,7 +8746,7 @@ namespace Parser {
87268746
if (parts.length && tags) Debug.assertIsDefined(commentsPos, "having parsed tags implies that the end of the comment span should be set");
87278747
const tagsArray = tags && createNodeArray(tags, tagsPos, tagsEnd);
87288748
return finishNode(factory.createJSDocComment(parts.length ? createNodeArray(parts, start, commentsPos) : trimmedComments.length ? trimmedComments : undefined, tagsArray), start, end);
8729-
});
8749+
}
87308750

87318751
function removeLeadingNewlines(comments: string[]) {
87328752
while (comments.length && (comments[0] === "\n" || comments[0] === "\r")) {

tests/baselines/reference/jsDocDontBreakWithNamespaces.baseline

+1-25
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
// zee('');
2626
// ^
2727
// | ----------------------------------------------------------------------
28-
// | zee(**arg0: any**, arg1: any, arg2: any): any
28+
// | zee(**arg0: any**, arg1: any): any
2929
// | ----------------------------------------------------------------------
3030

3131
[
@@ -259,30 +259,6 @@
259259
],
260260
"isOptional": false,
261261
"isRest": false
262-
},
263-
{
264-
"name": "arg2",
265-
"documentation": [],
266-
"displayParts": [
267-
{
268-
"text": "arg2",
269-
"kind": "parameterName"
270-
},
271-
{
272-
"text": ":",
273-
"kind": "punctuation"
274-
},
275-
{
276-
"text": " ",
277-
"kind": "space"
278-
},
279-
{
280-
"text": "any",
281-
"kind": "keyword"
282-
}
283-
],
284-
"isOptional": false,
285-
"isRest": false
286262
}
287263
],
288264
"documentation": [],
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,12 @@
1+
/a.js(1,13): error TS1098: Type parameter list cannot be empty.
12
/a.js(1,14): error TS1139: Type parameter declaration expected.
2-
/a.js(1,17): error TS1003: Identifier expected.
3-
/a.js(1,17): error TS1110: Type expected.
4-
/a.js(1,17): error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
5-
/a.js(1,18): error TS1005: '>' expected.
6-
/a.js(1,18): error TS1005: '}' expected.
73

84

9-
==== /a.js (6 errors) ====
5+
==== /a.js (2 errors) ====
106
/** @param {<} x */
7+
~~
8+
!!! error TS1098: Type parameter list cannot be empty.
119
~
1210
!!! error TS1139: Type parameter declaration expected.
13-
14-
!!! error TS1003: Identifier expected.
15-
16-
!!! error TS1110: Type expected.
17-
18-
!!! error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
19-
20-
!!! error TS1005: '>' expected.
21-
22-
!!! error TS1005: '}' expected.
2311
function f(x) {}
2412

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
=== /a.js ===
22
/** @param {<} x */
33
function f(x) {}
4-
>f : (x: any) => void
5-
>x : any
4+
>f : (x: () => any) => void
5+
>x : () => any
66

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @filename: index.js
4+
//// class I18n {
5+
//// /**
6+
//// * @param {{dot|fulltext}} [stringMode] - which mode our translation keys use
7+
//// */
8+
//// constructor(options = {}) {}
9+
//// }
10+
11+
verify.encodedSyntacticClassificationsLength(69);

0 commit comments

Comments
 (0)