Skip to content

JSDoc:positional matching of destructured params#23307

Merged
sandersn merged 3 commits into
masterfrom
js/jsdoc-positional-matching-of-destructured-params
Apr 10, 2018
Merged

JSDoc:positional matching of destructured params#23307
sandersn merged 3 commits into
masterfrom
js/jsdoc-positional-matching-of-destructured-params

Conversation

@sandersn
Copy link
Copy Markdown
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 a user and mhegazy April 10, 2018 17:26
@sandersn
Copy link
Copy Markdown
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.

Comment thread src/compiler/utilities.ts Outdated
}
}
// 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
Copy Markdown

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?

Comment thread src/compiler/checker.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
Copy Markdown

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