-
Notifications
You must be signed in to change notification settings - Fork 13k
Enhance type argument completions #62170
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
base: main
Are you sure you want to change the base?
Conversation
Previously, `getTypeArgumentConstraint` could only find constraints for type arguments of generic type instantiations. Now it additionally supports type arguments of the following expression kinds: - function calls - `new` expressions - tagged templates - JSX expressions - decorators - instantiation expressions
b9800de
to
d5bbf02
Compare
1 similar comment
@i-ayushh18 I don't know if this is intentional, but it seems that you have a bot which is spamming a bunch of nonsense comments across many pull requests. If it is intentional, please cut it out. |
Sorry for the thing,a bot was responsible for that,will suspend that at latest! |
I have tested this changes locally, and it works well for me. Looking forward to seeing it merged and released soon. |
return constraint && instantiateType(constraint, createTypeMapper(typeParameters, getEffectiveTypeArguments(typeReferenceNode, typeParameters))); | ||
let typeArgumentPosition; | ||
if ( | ||
"typeArguments" in node.parent && // eslint-disable-line local/no-in-operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We typically avoid these kinds of checks; is there no helper that achieves the same thing? Or at least, a switch case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was originally calling hasTypeArguments
here, but noticed that it's buggy and dropped it. For details, see the commit message of 53d3f29 and the commentary in #61758 (comment) (note that 53d3f29 isn't included in this branch).
However, some code change I made in the meantime must have sidestepped those issues, as I just tried hasTypeArguments
here again and everything seems copacetic.
I'll push a commit switching back to hasTypeArguments
, but since we're on the subject: should anything be done about its sketchiness? Maybe it's made moot by the Go rewrite (where a hasTypeArguments
equivalent would presumably work differently).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasTypeArguments
doesn't even exist in the Go codebase. Notably hasTypeArguments
is actually entirely unused in this repo, so I think nobody has ever even noticed it's broken and tried to fix it.
@@ -42624,6 +42624,48 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
return undefined; | |||
} | |||
|
|||
function getSignaturesFromCallLike(node: CallLikeExpression): readonly Signature[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels suspicious that this is the first time we'd need a func like this; there's nothing else that does this? Not just getResolvedSignature
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need the original un-instantiated generic signatures, but getResolvedSignature
always gives me concrete ones, even with CheckMode.SkipGenericFunctions
.
When asking for completions there is a (possibly-incomplete) type argument already present (the node looks something like f<{}>()
). I tried artificially clearing out the type argument list before calling getResolvedSignature
, but that still gives me concrete signatures (with type parameters substituted for their constraints). I traced the logic for regular function calls and in that case concretion occurs within resolveCall
(the call stack is getResolvedSignature
→ resolveSignature
→ resolveCallExpression
→ resolveCall
).
I searched for other existing utilities that would meet my needs but came up empty. It's entirely possible I missed something though; I wasn't sure exactly what to look for.
Maybe I could rename getSignaturesFromCallLike
to clarify how it's different from getResolvedSignature
. Something like getUninstantiatedSignatures
, perhaps? A docblock might also help. Let me know if you have any specific suggestions on these fronts.
#43526 implemented limited support for completions within type arguments, but only for type arguments of types (not values like generic function calls), and only for specific locations within the type literal (e.g. property names of object types). This pull request generalizes type argument completions such that suggestions now appear in many more scenarios, including:
f<…>()
)new
expressions (new Foo<…>()
)tag<…>`blah`
)<Component<…>/>
)@decorator<…> class {}
)f<…>
)Foo<{ x: … }>
)Foo<"…">
)Foo<[…]>
)Suggested completions are derived from the relevant type parameter's constraint.
This pull request subsumes #61758 and includes the additional enhancements mentioned in that pull request's description.
Fixes #61751.
Fixes #56299.
Fixes #52898.
Fixes #34771.