Skip to content

Don't capture type parameters defined inside the extraction range with "Extract to function" #53543

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 2 commits into from
Mar 28, 2023

Conversation

MariaSolOs
Copy link
Contributor

Fixes #51827

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Mar 27, 2023
@@ -1547,13 +1548,13 @@ function getContainingVariableDeclarationIfInList(node: Node, scope: Scope) {
}
}

function getFirstDeclaration(type: Type): Declaration | undefined {
function getFirstDeclarationBeforePosition(type: Type, position: number): Declaration | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

What exactly was the problem? What is the thing that was captured before a specific position?

Copy link
Contributor Author

@MariaSolOs MariaSolOs Mar 27, 2023

Choose a reason for hiding this comment

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

The problem was that type variables declared inside the extraction range were used in the type parameter list of the new function, which isn't needed because the types will be declared in the function's body instead. So with this change I'm only including the type parameters in scope that are declared before the extraction's range.

As for the respective crash, I'm not too sure. I debugged this a bit with @sandersn a few weeks ago and we suspect that there's an issue with the printer inserting trivia nodes in the incorrect places.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, so this is a really just a proxy/hack for saying "get me the free (captured) type variables" - because any captured/outer type parameter must occur strictly before the function itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly.

@DanielRosenwasser DanielRosenwasser merged commit 6e44db7 into microsoft:main Mar 28, 2023
@MariaSolOs MariaSolOs deleted the extract-to-func branch March 28, 2023 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Extract to function" refactoring triggers an assertion failure
3 participants