Skip to content

Fix infinite loop in jsdoc parsing#17420

Merged
weswigham merged 3 commits into
microsoft:masterfrom
weswigham:more-general-parser-fix
Jul 26, 2017
Merged

Fix infinite loop in jsdoc parsing#17420
weswigham merged 3 commits into
microsoft:masterfrom
weswigham:more-general-parser-fix

Conversation

@weswigham
Copy link
Copy Markdown
Member

@weswigham weswigham commented Jul 26, 2017

I've included a test case demonstrating a (behaviorally) similar bug to those fixed in #17354, but in jsdoc. @sandersn and I were talking about how, after looking at it, it was likely we had a bug like this, so I looked into it and found a reproduction.
The fix generalizes the fix applied in #17354, by moving it into parseDelimitedList.

@weswigham weswigham requested review from a user, rbuckton and sandersn July 26, 2017 00:13
Comment thread src/compiler/parser.ts Outdated
parsingContext = saveParsingContext;
return result;

function checkZeroLengthNode(startPos: number) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be in its own function?

Comment thread src/compiler/parser.ts Outdated

function checkZeroLengthNode(startPos: number) {
if (startPos === scanner.getStartPos()) {
// What we're parsing isn't actually remotely recognizable as a parameter and we've consumed no tokens whatsoever
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment mentions parameters which may be too specific for this context.

@weswigham weswigham merged commit 5b77ef8 into microsoft:master Jul 26, 2017
@weswigham weswigham deleted the more-general-parser-fix branch July 26, 2017 17:13
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants