Skip to content

Handle JSDoc backticks in the parser, not scanner #30939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 18, 2019

Conversation

sandersn
Copy link
Member

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 @param tags.

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:

  1. Utilities: Add parseXXXJSDoc variants of most of the core parsing functions. Previously the jsdoc parser code would use the normal core parsing functions, which would (for example) produce template literals instead of backticks, and skip whitespace.
  2. Naming: Name the jsdoc parsing functions the same as the core parsing functions, but with the suffix "JSDoc". Previously it was an infix.

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 parseExpectedTokenJSDoc gets a jsdoc token, whereas parseExpectedToken gets 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

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
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

So this doesn't affect the @example indentation handling you fixed in #30838, huh?

@sandersn
Copy link
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.

@sandersn sandersn merged commit c3a9429 into master Apr 18, 2019
@sandersn sandersn deleted the restrict-jsdoc-backticks-scanning branch April 18, 2019 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSDoc does not strip leading * inside markdown code block
2 participants