Include all type parameters in completions within type parameters' constraints#56543
Conversation
| } | ||
|
|
||
| function getVariableOrParameterDeclaration(contextToken: Node | undefined, location: Node) { | ||
| function getClosestSymbolDeclaration(contextToken: Node | undefined, location: Node) { |
There was a problem hiding this comment.
since Parameter has a specific meaning and commonly doesn't include TypeParameter I decided to rename this (as this function might return both of those types)
| variableOrParameterDeclaration.parent.typeParameters; | ||
| if (symbolDeclarationPos >= variableOrParameterDeclaration.pos && parameters && symbolDeclarationPos < parameters.end) { | ||
| return false; | ||
| if (closestSymbolDeclaration && symbolDeclaration) { |
There was a problem hiding this comment.
The only real change here is the addition of isInTypeParameterDefault check. I decided to refactor this slightly though as I find handling the parameters and typeParameters cases through those separate (although similar) code paths cleaner/more readable
src/services/completions.ts
Outdated
| && isInTypeParameterDefault(contextToken) | ||
| && !isInferTypeNode(closestSymbolDeclaration.parent) | ||
| ) { | ||
| const typeParameters = closestSymbolDeclaration.parent.typeParameters; |
There was a problem hiding this comment.
I believe that those are always available in this codepath - but I kept this as nullable to stay within the safety bounds just in case
|
@iisaduan would it be possible for you to take a quick look at this? |
iisaduan
left a comment
There was a problem hiding this comment.
Thanks for the reminder! Change generally looks good, but the self-completion is invalid, so we should continue filtering it out.
There was a problem hiding this comment.
function test<First extends First, Second>(a: First, b: Second) {} and type A1<K extends K, L> = K are both circular definitions and are invalid code, so we should filter the current parameter out. I can't think of a situation where First extends First is the first part of some valid parameter and not offered by the completion for the valid case
There was a problem hiding this comment.
Ok, I addressed this - could you take another look? :)
fixes #56474