Skip to content

Commit

Permalink
Handle JSDoc backticks in the parser, not scanner (#30939)
Browse files Browse the repository at this point in the history 
* Scan backticks in jsdoc as a single token less often

Previously, matching backticks in jsdoc would always be scanned as one
token to aid parsing incorrect jsdoc that uses backticks for parameter
names:

``js
/** @param {string} `nope`
 * @param {number} `not needed`
 */
```

However, this is wrong for code fences, which use triple backticks. This
fix parses a single backtick as a single token if it's immediately
followed by another backtick or by a newline. It retains the
questionable tokenisation of backticks-as-pairs in other cases, however.
A better fix might be to have the parser ignore backticks in jsdoc
instead.

* Add test case

* Handle jsdoc backticks in the parser, not scanner

Previously, the jsdoc scanner had ad-hoc handling of backticks that was
similar in structure to the normal scanner's handling, but much simpler.
This was a smaller code change, but is handled backwards: the special
case of backtick-quoted parameter names was handled in the scanner
instead of in the jsdoc identifier parsing code. That made it overapply
and block correct handling of asterisks across newlines, which was most
obvious in code fences inside jsdoc, as in #23517.

Fixes #23517

* More cleanup
  • Loading branch information
@sandersn
sandersn committed Apr 18, 2019
1 parent 0574c1f commit c3a9429
Show file tree
Hide file tree
Showing 9 changed files with 772 additions and 606 deletions.
86 changes: 59 additions & 27 deletions src/compiler/parser.ts
View file
Expand Up @@ -1081,6 +1081,10 @@ namespace ts {
return currentToken = scanner.scan();
}

function nextTokenJSDoc(): JSDocSyntaxKind {
return currentToken = scanner.scanJsDocToken();
}

function reScanGreaterToken(): SyntaxKind {
return currentToken = scanner.reScanGreaterToken();
}
Expand Down Expand Up @@ -1198,6 +1202,15 @@ namespace ts {
return false;
}

function parseExpectedJSDoc(kind: JSDocSyntaxKind) {
if (token() === kind) {
nextTokenJSDoc();
return true;
}
parseErrorAtCurrentToken(Diagnostics._0_expected, tokenToString(kind));
return false;
}

function parseOptional(t: SyntaxKind): boolean {
if (token() === t) {
nextToken();
Expand All @@ -1214,18 +1227,38 @@ namespace ts {
return undefined;
}

function parseOptionalTokenJSDoc<TKind extends JSDocSyntaxKind>(t: TKind): Token<TKind>;
function parseOptionalTokenJSDoc(t: JSDocSyntaxKind): Node | undefined {
if (token() === t) {
return parseTokenNodeJSDoc();
}
return undefined;
}

function parseExpectedToken<TKind extends SyntaxKind>(t: TKind, diagnosticMessage?: DiagnosticMessage, arg0?: any): Token<TKind>;
function parseExpectedToken(t: SyntaxKind, diagnosticMessage?: DiagnosticMessage, arg0?: any): Node {
return parseOptionalToken(t) ||
createMissingNode(t, /*reportAtCurrentPosition*/ false, diagnosticMessage || Diagnostics._0_expected, arg0 || tokenToString(t));
}

function parseExpectedTokenJSDoc<TKind extends JSDocSyntaxKind>(t: TKind): Token<TKind>;
function parseExpectedTokenJSDoc(t: JSDocSyntaxKind): Node {
return parseOptionalTokenJSDoc(t) ||
createMissingNode(t, /*reportAtCurrentPosition*/ false, Diagnostics._0_expected, tokenToString(t));
}

function parseTokenNode<T extends Node>(): T {
const node = <T>createNode(token());
nextToken();
return finishNode(node);
}

function parseTokenNodeJSDoc<T extends Node>(): T {
const node = <T>createNode(token());
nextTokenJSDoc();
return finishNode(node);
}

function canParseSemicolon() {
// If there's a real semicolon, then we can always parse it out.
if (token() === SyntaxKind.SemicolonToken) {
Expand Down Expand Up @@ -6345,7 +6378,7 @@ namespace ts {
const hasBrace = (mayOmitBraces ? parseOptional : parseExpected)(SyntaxKind.OpenBraceToken);
result.type = doInsideOfContext(NodeFlags.JSDoc, parseJSDocType);
if (!mayOmitBraces || hasBrace) {
parseExpected(SyntaxKind.CloseBraceToken);
parseExpectedJSDoc(SyntaxKind.CloseBraceToken);
}

fixupParentReferences(result);
Expand Down Expand Up @@ -6432,7 +6465,7 @@ namespace ts {
indent += text.length;
}

nextJSDocToken();
nextTokenJSDoc();
while (parseOptionalJsdoc(SyntaxKind.WhitespaceTrivia));
if (parseOptionalJsdoc(SyntaxKind.NewLineTrivia)) {
state = JSDocState.BeginningOfLine;
Expand Down Expand Up @@ -6493,7 +6526,7 @@ namespace ts {
pushComment(scanner.getTokenText());
break;
}
nextJSDocToken();
nextTokenJSDoc();
}
removeLeadingNewlines(comments);
removeTrailingWhitespace(comments);
Expand Down Expand Up @@ -6522,7 +6555,7 @@ namespace ts {
function isNextNonwhitespaceTokenEndOfFile(): boolean {
// We must use infinite lookahead, as there could be any number of newlines :(
while (true) {
nextJSDocToken();
nextTokenJSDoc();
if (token() === SyntaxKind.EndOfFileToken) {
return true;
}
Expand All @@ -6539,7 +6572,7 @@ namespace ts {
}
}
while (token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) {
nextJSDocToken();
nextTokenJSDoc();
}
}

Expand All @@ -6563,15 +6596,15 @@ namespace ts {
else if (token() === SyntaxKind.AsteriskToken) {
precedingLineBreak = false;
}
nextJSDocToken();
nextTokenJSDoc();
}
return seenLineBreak ? indentText : "";
}

function parseTag(margin: number) {
Debug.assert(token() === SyntaxKind.AtToken);
const start = scanner.getTokenPos();
nextJSDocToken();
nextTokenJSDoc();

const tagName = parseJSDocIdentifierName(/*message*/ undefined);
const indentText = skipWhitespaceOrAsterisk();
Expand Down Expand Up @@ -6643,7 +6676,7 @@ namespace ts {
pushComment(initialMargin);
state = JSDocState.SavingComments;
}
let tok = token() as JsDocSyntaxKind;
let tok = token() as JSDocSyntaxKind;
loop: while (true) {
switch (tok) {
case SyntaxKind.NewLineTrivia:
Expand Down Expand Up @@ -6674,11 +6707,11 @@ namespace ts {
break;
case SyntaxKind.OpenBraceToken:
state = JSDocState.SavingComments;
if (lookAhead(() => nextJSDocToken() === SyntaxKind.AtToken && tokenIsIdentifierOrKeyword(nextJSDocToken()) && scanner.getTokenText() === "link")) {
if (lookAhead(() => nextTokenJSDoc() === SyntaxKind.AtToken && tokenIsIdentifierOrKeyword(nextTokenJSDoc()) && scanner.getTokenText() === "link")) {
pushComment(scanner.getTokenText());
nextJSDocToken();
nextTokenJSDoc();
pushComment(scanner.getTokenText());
nextJSDocToken();
nextTokenJSDoc();
}
pushComment(scanner.getTokenText());
break;
Expand All @@ -6696,7 +6729,7 @@ namespace ts {
pushComment(scanner.getTokenText());
break;
}
tok = nextJSDocToken();
tok = nextTokenJSDoc();
}

removeLeadingNewlines(comments);
Expand Down Expand Up @@ -6730,16 +6763,19 @@ namespace ts {
}

function parseBracketNameInPropertyAndParamTag(): { name: EntityName, isBracketed: boolean } {
if (token() === SyntaxKind.NoSubstitutionTemplateLiteral) {
// a markdown-quoted name: `arg` is not legal jsdoc, but occurs in the wild
return { name: createIdentifier(/*isIdentifier*/ true), isBracketed: false };
}
// Looking for something like '[foo]', 'foo', '[foo.bar]' or 'foo.bar'
const isBracketed = parseOptional(SyntaxKind.OpenBracketToken);
const isBracketed = parseOptionalJsdoc(SyntaxKind.OpenBracketToken);
if (isBracketed) {
skipWhitespace();
}
// a markdown-quoted name: `arg` is not legal jsdoc, but occurs in the wild
const isBackquoted = parseOptionalJsdoc(SyntaxKind.BacktickToken);
const name = parseJSDocEntityName();
if (isBackquoted) {
parseExpectedTokenJSDoc(SyntaxKind.BacktickToken);
}
if (isBracketed) {
skipWhitespace();

// May have an optional default, e.g. '[foo = 42]'
if (parseOptionalToken(SyntaxKind.EqualsToken)) {
parseExpression();
Expand Down Expand Up @@ -7022,7 +7058,7 @@ namespace ts {
let canParseTag = true;
let seenAsterisk = false;
while (true) {
switch (nextJSDocToken()) {
switch (nextTokenJSDoc()) {
case SyntaxKind.AtToken:
if (canParseTag) {
const child = tryParseChildTag(target, indent);
Expand Down Expand Up @@ -7057,7 +7093,7 @@ namespace ts {
function tryParseChildTag(target: PropertyLikeParse, indent: number): JSDocTypeTag | JSDocPropertyTag | JSDocParameterTag | false {
Debug.assert(token() === SyntaxKind.AtToken);
const start = scanner.getStartPos();
nextJSDocToken();
nextTokenJSDoc();

const tagName = parseJSDocIdentifierName();
skipWhitespace();
Expand Down Expand Up @@ -7109,13 +7145,9 @@ namespace ts {
return result;
}

function nextJSDocToken(): JsDocSyntaxKind {
return currentToken = scanner.scanJSDocToken();
}

function parseOptionalJsdoc(t: JsDocSyntaxKind): boolean {
function parseOptionalJsdoc(t: JSDocSyntaxKind): boolean {
if (token() === t) {
nextJSDocToken();
nextTokenJSDoc();
return true;
}
return false;
Expand Down Expand Up @@ -7150,7 +7182,7 @@ namespace ts {
result.escapedText = escapeLeadingUnderscores(scanner.getTokenText());
finishNode(result, end);

nextJSDocToken();
nextTokenJSDoc();
return result;
}
}
Expand Down
13 changes: 4 additions & 9 deletions src/compiler/scanner.ts
View file
Expand Up @@ -33,7 +33,7 @@ namespace ts {
reScanJsxToken(): JsxTokenSyntaxKind;
reScanLessThanToken(): SyntaxKind;
scanJsxToken(): JsxTokenSyntaxKind;
scanJSDocToken(): JsDocSyntaxKind;
scanJsDocToken(): JSDocSyntaxKind;
scan(): SyntaxKind;
getText(): string;
// Sets the text for the scanner to scan. An optional subrange starting point and length
Expand Down Expand Up @@ -886,7 +886,7 @@ namespace ts {
reScanJsxToken,
reScanLessThanToken,
scanJsxToken,
scanJSDocToken,
scanJsDocToken,
scan,
getText,
setText,
Expand Down Expand Up @@ -2050,7 +2050,7 @@ namespace ts {
}
}

function scanJSDocToken(): JsDocSyntaxKind {
function scanJsDocToken(): JSDocSyntaxKind {
startPos = tokenPos = pos;
tokenFlags = 0;
if (pos >= end) {
Expand Down Expand Up @@ -2093,12 +2093,7 @@ namespace ts {
case CharacterCodes.dot:
return token = SyntaxKind.DotToken;
case CharacterCodes.backtick:
while (pos < end && text.charCodeAt(pos) !== CharacterCodes.backtick) {
pos++;
}
tokenValue = text.substring(tokenPos + 1, pos);
pos++;
return token = SyntaxKind.NoSubstitutionTemplateLiteral;
return token = SyntaxKind.BacktickToken;
}

if (isIdentifierStart(ch, ScriptTarget.Latest)) {
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/types.ts
View file
Expand Up @@ -8,7 +8,7 @@ namespace ts {
end: number;
}

export type JsDocSyntaxKind =
export type JSDocSyntaxKind =
| SyntaxKind.EndOfFileToken
| SyntaxKind.WhitespaceTrivia
| SyntaxKind.AtToken
Expand All @@ -23,7 +23,7 @@ namespace ts {
| SyntaxKind.CommaToken
| SyntaxKind.DotToken
| SyntaxKind.Identifier
| SyntaxKind.NoSubstitutionTemplateLiteral
| SyntaxKind.BacktickToken
| SyntaxKind.Unknown
| KeywordSyntaxKind;

Expand Down Expand Up @@ -181,6 +181,8 @@ namespace ts {
QuestionToken,
ColonToken,
AtToken,
/** Only the JSDoc scanner produces BacktickToken. The normal scanner produces NoSubstitutionTemplateLiteral and related kinds. */
BacktickToken,
// Assignments
EqualsToken,
PlusEqualsToken,
Expand Down
4 changes: 2 additions & 2 deletions ...s/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType1.json
View file
Expand Up @@ -8,7 +8,7 @@
"0": {
"kind": "JSDocParameterTag",
"pos": 8,
"end": 29,
"end": 32,
"modifierFlagsCache": 0,
"transformFlags": 0,
"tagName": {
Expand Down Expand Up @@ -46,6 +46,6 @@
},
"length": 1,
"pos": 8,
"end": 29
"end": 32
}
}

0 comments on commit c3a9429

Please sign in to comment.