Fixed some string literal argument completions depending on resolved signature#53996
Conversation
typescript-bot
added
the
For Uncommitted Bug
|
|
||
| for (let i = 0; i < argCount; i++) { | ||
| const arg = args[i]; | ||
| if (arg.kind !== SyntaxKind.OmittedExpression && !(checkMode & CheckMode.IsForStringLiteralArgumentCompletions && hasSkipDirectInferenceFlag(arg))) { |
There was a problem hiding this comment.
This change is quite nice (IMHO) because it eliminates the need for this special CheckMode almost entirely (in fact, the first commit in this PR does exactly that - it just doesn't fix the issue).
With this we can fully rely just on "node blocking" to alter the default inference behavior for the completion purposes.
| if (constraint) { | ||
| const instantiatedConstraint = instantiateType(constraint, context.nonFixingMapper); | ||
| if (!inferredType || !context.compareTypes(inferredType, getTypeWithThisArgument(instantiatedConstraint, inferredType))) { | ||
| if (!inferredType || inferredType === wildcardType || !context.compareTypes(inferredType, getTypeWithThisArgument(instantiatedConstraint, inferredType))) { |
There was a problem hiding this comment.
This change might be best reviewed in isolation by reviewing the first commit: 7ab8679
| // them now (and keeping do so for any subsequent candidates) and perform a second | ||
| // round of type inference and applicability checking for this particular candidate. | ||
| argCheckMode = checkMode & CheckMode.IsForStringLiteralArgumentCompletions; | ||
| argCheckMode = CheckMode.Normal; |
There was a problem hiding this comment.
Thanks to the introduced changes this is just a revert of another PR: https://github.com/microsoft/TypeScript/pull/48811/files . The patch from that other PR is not needed anymore since we now rely on wildcardType returned by checkExpressionWorker for blocked string literals
src/services/stringCompletions.ts
Outdated
| // i.e. declare function f(a: 'A'); | ||
| // f("/*completion position*/") | ||
| return argumentInfo && getStringLiteralCompletionsFromSignature(argumentInfo.invocation, node, argumentInfo, typeChecker) || fromContextualType(); | ||
| return argumentInfo && (getStringLiteralCompletionsFromSignature(argumentInfo.invocation, node, argumentInfo, typeChecker) || getStringLiteralCompletionsFromSignature(argumentInfo.invocation, node, argumentInfo, typeChecker, CheckMode.Normal)) || fromContextualType(); |
| getResolvedSignatureWorker(node, candidatesOutArray, argumentCount, CheckMode.Normal), | ||
| getResolvedSignatureForStringLiteralCompletions: (call, editingArgument, candidatesOutArray) => | ||
| runWithInferenceBlockedFromSourceNode(editingArgument, () => getResolvedSignatureWorker(call, candidatesOutArray, /*argumentCount*/ undefined, CheckMode.IsForStringLiteralArgumentCompletions)), | ||
| getResolvedSignatureForStringLiteralCompletions: (call, editingArgument, candidatesOutArray, checkMode = CheckMode.IsForStringLiteralArgumentCompletions) => { |
There was a problem hiding this comment.
After a second thought... this checkMode parameter could just be a boolean flag or something. We don't need this special CheckMode.IsForStringLiteralArgumentCompletions for anything now - it's only used to toggle the "inference blocking" behavior in this very function here. This is also reflected in the fact that I remove it from the checkMode that is passed to getResolvedSignatureWorker
| //// } | ||
| //// const parse = <def>(def: validate<def>) => def | ||
| //// const shallowExpression = parse("foo|/*ts*/") | ||
| //// const nestedExpression = parse({ prop: "foo|/*ts2*/" }) |
There was a problem hiding this comment.
it's worth pointing out here that this nestedExpression was already working OK, the problem was in the shallowExpression's case as that was using a completely different part of the algorithm (getStringLiteralCompletionsFromSignature, and not getContextualType~)
typescript-bot
removed
the
For Uncommitted Bug
|
@typescript-bot pack this |
|
Heya @jakebailey, I've started to run the tarball bundle task on this PR at 322f6cd. You can monitor the build here. |
|
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
…tion # Conflicts: # src/services/stringCompletions.ts
5 participants
fixes #53997