Skip to content

JSDoc:positional matching of destructured params #23307

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
Apr 10, 2018

Conversation

sandersn
Copy link
Member

  1. When looking up tags for a parameter whose name is a binding pattern, use the index of the parameter to get the type.
  2. When reporting errors for @param tags with no matching parameter name, do not report the error for tags whose index in the @param tag list matches the index of a parameter whose name is a binding pattern.

Fixes #19645

This is a small-ish change while we figure out how to make #18832 not hurt performance in the common case of Typescript code or non-destructuring parameters.

1. When looking up tags for a parameter whose name is a binding pattern, use
the index of the parameter to get the type.
2. When reporting errors for `@param` tags with no matching parameter
name, do not report the error for tags whose index in the `@param` tag list
matches the index of a parameter whose name is a binding pattern.
@sandersn sandersn requested review from mhegazy and a user April 10, 2018 17:26
@sandersn
Copy link
Member Author

Perf tests show everything about the same except Angular binding, which is slower. There's no change in the binder for this PR, so it makes me think the results for Angular when testing #18832 are not valid either.

}
// a binding pattern doesn't have a name, so it's not possible to match it a JSDoc parameter, which is identified by name
// return empty array for: incorrect binding patterns and JSDoc function syntax, which has un-named parameters
Copy link

Choose a reason for hiding this comment

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

An "incorrect" binding pattern is one at an index i which is greater than the number of @param tags?

@@ -21871,6 +21871,10 @@ namespace ts {
// and give a better error message when the host function mentions `arguments`
// but the tag doesn't have an array type
if (decl) {
const i = getJSDocTags(decl).filter(isJSDocParameterTag).indexOf(node);
Copy link

Choose a reason for hiding this comment

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

Maybe getParameterSymbolFromJSDoc should work for binding patterns too?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's no symbol on a binding pattern, though, so without adding something in the binder there's no benefit.

@sandersn sandersn merged commit 22919d5 into master Apr 10, 2018
@sandersn sandersn deleted the js/jsdoc-positional-matching-of-destructured-params branch April 10, 2018 19:48
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 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.

1 participant