Skip to content
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

Always consider parameters in scope visible to node builder #58075

Merged
merged 7 commits into from
Apr 4, 2024

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Apr 4, 2024

Parameter symbols in scope resolve, thanks to recent changes, but don't always have a visible declaration (if their host signature isn't directly exported) - this doesn't matter in the node builder, since we're cloning that host signature in some way anyway, so if it's a parameter and we're looking at it, it aughta be visible.

@weswigham weswigham marked this pull request as ready for review April 4, 2024 17:57
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 4, 2024
sym.flags & SymbolFlags.FunctionScopedVariable
&& sym.valueDeclaration
) {
const parameter = sym.valueDeclaration && isParameterDeclaration(sym.valueDeclaration);
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that this will not work for binding elements. The original check was mean to walk up the binding elements. I this i lost something when I copied it over.

Copy link
Member Author

Choose a reason for hiding this comment

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

The confusingly-named isParameterDeclaration does not do what you think it does - it does walk binding elements.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we should really rename that to isPartOfParameterDeclaration. See also #52283

@weswigham weswigham merged commit f2bd592 into microsoft:main Apr 4, 2024
25 checks passed
@weswigham weswigham deleted the parameter-lookup-adjustments branch April 4, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants