Repository navigation
Handle JSDoc backticks in the parser, not scanner - #30939
Merged
Nathan Shively-Sanders (sandersn) merged 4 commits intoApr 18, 2019
Merged
Conversation
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.
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
Nathan Shively-Sanders (sandersn)
requested review from
Andrew Branch (andrewbranch) and
Sheetal Nandi (sheetalkamat)
April 15, 2019 18:20
Andrew Branch (andrewbranch)
approved these changes
Apr 18, 2019
Andrew Branch (andrewbranch)
left a comment
Andrew Branch (andrewbranch)
left a comment
Member
There was a problem hiding this comment.
So this doesn't affect the @example indentation handling you fixed in #30838, huh?
Member
Author
|
Nope, those examples didn't have backticks. And, uh, I guess the code there was able to handle the indentation in the new examples in this PR. |
Nathan Shively-Sanders (sandersn)
deleted the
restrict-jsdoc-backticks-scanning
branch
April 18, 2019 16:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the jsdoc scanner had ad-hoc handling of backticks that was similar in structure to the normal scanner's handling of template literals, but much simpler. But the only use of these simplified template literals was to allow non-standard markdown-quoting of names in
@paramtags.This PR changes the jsdoc scanner to produce backticks instead of template literal tokens. This is a bigger change, but it (1) reflects that backticks aren't special except for markdown-quoted param names (2) avoids treating large swatches of jsdoc text as template literals, which causes code fences to break jsdoc asterisk handling in #23517.
Since this is a code improvement, I also improved a couple of other JSDoc parsing things:
Note that there are dependencies between subsequent calls to parseExpected/parseOptional/et al. Each of these functions, when they succeed, ask the scanner for the next token, but
parseExpectedTokenJSDocgets a jsdoc token, whereasparseExpectedTokengets a normal token. Then the next function that inspects the current token will see that jsdoc/normal token.These dependencies are non-local and non-obvious. But I'm not sure sure of the best way to get rid of them, so I'm leaving them for now.
Fixes #23517