Skip to content

Fix parsing of parenthesized JSDoc parameters#25799

Merged
sandersn merged 3 commits into
masterfrom
fix-parsing-parenthesised-jsdoc-parameters
Jul 19, 2018
Merged

Fix parsing of parenthesized JSDoc parameters#25799
sandersn merged 3 commits into
masterfrom
fix-parsing-parenthesised-jsdoc-parameters

Conversation

@sandersn
Copy link
Copy Markdown
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
Copy Markdown
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 a user and mhegazy July 19, 2018 16:29
// @strict: true
// @Filename: paren.js
/** @type {function((string), function((string)): string): string} */
var x = s => s.toString()
Copy link
Copy Markdown

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
Copy Markdown
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.

Comment thread src/compiler/parser.ts
return isStartOfParameter();
return isStartOfParameter(/*isJSDocParameter*/ false);
case ParsingContext.JSDocParameters:
return isStartOfParameter(/*isJSDocParameter*/ true);
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
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.

2 participants