Skip to content

Fix parsing of parenthesized JSDoc parameters #25799

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 3 commits into from
Jul 19, 2018

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Jul 19, 2018

Parenthesis can start a jsdoc function parameter since it is just a type, and parenthesis can start a type:

/** @type {function(((string))): void} */

However, this is not legal in other parameter lists:

function x((((a))): string) { }

This change makes jsdoc function parameter lists parse differently than normal parameter lists by allowing parenthesis as a start character of jsdoc parameters.

Fixes #25779
Fixes #25800

Parenthesis can start a jsdoc function parameter since it is just a
type, and parenthesis can start a type:

```js
/** @type {function(((string))): void} */
```

However, this is not legal in other parameter lists:

```ts
function x((((a))): string) { }
```

This change makes jsdoc function parameter lists parse differently than
normal parameter lists by allowing parenthesis as a start character of
jsdoc parameters.
@sandersn
Copy link
Member Author

I added a commit that also fixes #25800 and shows that use of parenthesised types in nested jsdoc functions works.

@sandersn sandersn requested review from mhegazy and a user July 19, 2018 16:29
// @strict: true
// @Filename: paren.js
/** @type {function((string), function((string)): string): string} */
var x = s => s.toString()
Copy link

Choose a reason for hiding this comment

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

Would be nice to give the arrow function 2 parameters to verify that the contextual type is correct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I forgot to copy over the newer version of my in-editor test buffer. Fixed.

return isStartOfParameter();
return isStartOfParameter(/*isJSDocParameter*/ false);
case ParsingContext.JSDocParameters:
return isStartOfParameter(/*isJSDocParameter*/ true);
Copy link

Choose a reason for hiding this comment

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

Does anything bad happen if we just always treat ( as the start of a parameter? It's a parse error anyway, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me try it ...

Copy link
Member Author

Choose a reason for hiding this comment

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

It disrupts parsing quite a bit. I'm not sure why, but it appears to cause type parameter lists to be parsed as parameter lists in arrow functions, and it also breaks function expressions inside object literal property assignments.

@sandersn sandersn merged commit 1cedab1 into master Jul 19, 2018
@sandersn sandersn deleted the fix-parsing-parenthesised-jsdoc-parameters branch July 19, 2018 19:50
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.

2 participants