Constructor function methods:Add two missing tag lookups #47742
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. During name resolution, `@param` and `@return` tags should walk up through the jsdoc comment and then jump to the host function. Previously they did not, which would cause them to not resolve type parameters bound in the scope of a host that was not a sibling of the comment. The example from #46618 is a prototype method: ```js /** * @template {T} * @param {T} t */ C.prototype.m = function (t) { } ``` 2. During name resolution, prototype methods are supposed to resolve types both from the host function's location and from the containing class' location. The containing class lookup happens in a separate call to `resolveName`. Previously, the code that finds the containing class only worked for the above style of comment, which is on the outer ExpressionStatement, but not for the below style, which is on the function expression itself: ```js C.prototype.m = /** * @template {T} * @param {T} t */ function (t) { } ```
typescript-bot
added
Author: Team
For Milestone Bug
sandersn
commented
Feb 4, 2022
| isExpressionStatement(host) && | ||
| isBinaryExpression(host.expression) && | ||
| getAssignmentDeclarationKind(host.expression) === AssignmentDeclarationKind.PrototypeProperty) { | ||
| // X.prototype.m = /** @param {K} p */ function () { } <-- look for K on X's declaration |
There was a problem hiding this comment.
This comment was wrong before. The new code implements this.
gabritto
approved these changes
Feb 7, 2022
| const symbol = getSymbolOfNode(host.expression.left); | ||
| if (symbol) { | ||
| return getDeclarationOfJSPrototypeContainer(symbol); | ||
| } | ||
| } | ||
| if (host && isFunctionExpression(host) && isPrototypePropertyAssignment(host.parent) && isExpressionStatement(host.parent.parent)) { | ||
| // X.prototype.m = /** @param {K} p */ function () { } <-- look for K on X's declaration |
There was a problem hiding this comment.
not sure if this is something we have to worry about here in this function, but what happens if we have conflicting tags with the same name in the top-level and nested scopes?
There was a problem hiding this comment.
The nearest tag to the parameter, the one on the function expression, resolves first. You can also put a @type on the parameter itself, and that resolves before any @param tag.
Conflicting tags in the same comment should resolve to the first tag.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@paramand@returntags should walk up through the jsdoc comment and then jump to the host function. Previously they did not, which would cause them to not resolve type parameters bound in the scope of a host that was not a sibling of the comment. The example from Failing to resolve multiple JSDOC @template tags #46618 is a prototype method:resolveName. Previously, the code that finds the containing class only worked for the above style of comment, which is on the outer ExpressionStatement, but not for the below style, which is on the function expression itself:Fixes #46618