Skip to content

Fixes #30507 #32100

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

Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 28 additions & 32 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,6 @@ namespace ts {
IsForSignatureHelp = 1 << 4, // Call resolution for purposes of signature help
}

const enum ContextFlags {
None = 0,
Signature = 1 << 0, // Obtaining contextual signature
}

const enum AccessFlags {
None = 0,
NoIndexSignatures = 1 << 0,
Expand Down Expand Up @@ -446,9 +441,9 @@ namespace ts {
},
getAugmentedPropertiesOfType,
getRootSymbols,
getContextualType: nodeIn => {
getContextualType: (nodeIn: Expression, contextFlags?: ContextFlags) => {
const node = getParseTreeNode(nodeIn, isExpression);
return node ? getContextualType(node) : undefined;
return node ? getContextualType(node, contextFlags) : undefined;
},
getContextualTypeForObjectLiteralElement: nodeIn => {
const node = getParseTreeNode(nodeIn, isObjectLiteralElementLike);
Expand Down Expand Up @@ -16156,7 +16151,7 @@ namespace ts {
return getWidenedType(unwidenedType);
}

function getInferredType(context: InferenceContext, index: number): Type {
function getInferredType(context: InferenceContext, index: number, contextFlags?: ContextFlags): Type {
const inference = context.inferences[index];
if (!inference.inferredType) {
let inferredType: Type | undefined;
Expand Down Expand Up @@ -16201,7 +16196,8 @@ namespace ts {
const constraint = getConstraintOfTypeParameter(inference.typeParameter);
if (constraint) {
const instantiatedConstraint = instantiateType(constraint, context.nonFixingMapper);
if (!inferredType || !context.compareTypes(inferredType, getTypeWithThisArgument(instantiatedConstraint, inferredType))) {
const isCompletionContext = contextFlags && (contextFlags & ContextFlags.Completion);
if (!inferredType || isCompletionContext || !context.compareTypes(inferredType, getTypeWithThisArgument(instantiatedConstraint, inferredType))) {
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem like the right place to do this - rather than passing the flag all the way thru signature resolution, if this flag is set, we should just look at the "base signature" (getBaseSignature) rather than starting inference at all, I think. Then again, if we did that we wouldn't get an instantiated constraint which reports more detailed information on circular constraints. Then again again, I'm pretty sure as-is this is caching into the resolvedSignature of a node, which this very much is not - which means requesting completions poisons the cache with incorrect types for later checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback @weswigham! Do you have a suggestion how we still could fix the autocompletion for this? This was, unfortunately, the only way I found (with my limited knowledge of the codebase).

Maybe introducing a new method as an alternative to getContextualType just to be used by the completion service?

Copy link
Member

@weswigham weswigham Sep 16, 2019

Choose a reason for hiding this comment

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

I think getContextualType is still right, we just need to avoid using any resolved signature caches.

inference.inferredType = inferredType = instantiatedConstraint;
}
}
Expand All @@ -16214,10 +16210,10 @@ namespace ts {
return isInJavaScriptFile ? anyType : unknownType;
}

function getInferredTypes(context: InferenceContext): Type[] {
function getInferredTypes(context: InferenceContext, contextFlags?: ContextFlags): Type[] {
const result: Type[] = [];
for (let i = 0; i < context.inferences.length; i++) {
result.push(getInferredType(context, i));
result.push(getInferredType(context, i, contextFlags));
}
return result;
}
Expand Down Expand Up @@ -18925,16 +18921,16 @@ namespace ts {
}

// In a typed function call, an argument or substitution expression is contextually typed by the type of the corresponding parameter.
function getContextualTypeForArgument(callTarget: CallLikeExpression, arg: Expression): Type | undefined {
function getContextualTypeForArgument(callTarget: CallLikeExpression, arg: Expression, contextFlags?: ContextFlags): Type | undefined {
const args = getEffectiveCallArguments(callTarget);
const argIndex = args.indexOf(arg); // -1 for e.g. the expression of a CallExpression, or the tag of a TaggedTemplateExpression
return argIndex === -1 ? undefined : getContextualTypeForArgumentAtIndex(callTarget, argIndex);
return argIndex === -1 ? undefined : getContextualTypeForArgumentAtIndex(callTarget, argIndex, contextFlags);
}

function getContextualTypeForArgumentAtIndex(callTarget: CallLikeExpression, argIndex: number): Type {
function getContextualTypeForArgumentAtIndex(callTarget: CallLikeExpression, argIndex: number, contextFlags?: ContextFlags): Type {
// If we're already in the process of resolving the given signature, don't resolve again as
// that could cause infinite recursion. Instead, return anySignature.
const signature = getNodeLinks(callTarget).resolvedSignature === resolvingSignature ? resolvingSignature : getResolvedSignature(callTarget);
const signature = getNodeLinks(callTarget).resolvedSignature === resolvingSignature ? resolvingSignature : getResolvedSignature(callTarget, /*candidatesOutArray*/ undefined, /*checkMode*/ undefined, contextFlags);
if (isJsxOpeningLikeElement(callTarget) && argIndex === 0) {
return getEffectiveFirstArgumentForJsxSignature(signature, callTarget);
}
Expand Down Expand Up @@ -19324,7 +19320,7 @@ namespace ts {
}
/* falls through */
case SyntaxKind.NewExpression:
return getContextualTypeForArgument(<CallExpression | NewExpression>parent, node);
return getContextualTypeForArgument(<CallExpression | NewExpression>parent, node, contextFlags);
case SyntaxKind.TypeAssertionExpression:
case SyntaxKind.AsExpression:
return isConstTypeReference((<AssertionExpression>parent).type) ? undefined : getTypeFromTypeNode((<AssertionExpression>parent).type);
Expand Down Expand Up @@ -21426,7 +21422,7 @@ namespace ts {
return getInferredTypes(context);
}

function inferTypeArguments(node: CallLikeExpression, signature: Signature, args: readonly Expression[], checkMode: CheckMode, context: InferenceContext): Type[] {
function inferTypeArguments(node: CallLikeExpression, signature: Signature, args: readonly Expression[], checkMode: CheckMode, context: InferenceContext, contextFlags?: ContextFlags): Type[] {
if (isJsxOpeningLikeElement(node)) {
return inferJsxTypeArguments(node, signature, checkMode, context);
}
Expand Down Expand Up @@ -21492,7 +21488,7 @@ namespace ts {
inferTypes(context.inferences, spreadType, restType);
}

return getInferredTypes(context);
return getInferredTypes(context, contextFlags);
}

function getArrayifiedType(type: Type) {
Expand Down Expand Up @@ -21928,7 +21924,7 @@ namespace ts {
return createDiagnosticForNodeArray(getSourceFileOfNode(node), typeArguments, Diagnostics.Expected_0_type_arguments_but_got_1, belowArgCount === -Infinity ? aboveArgCount : belowArgCount, argCount);
}

function resolveCall(node: CallLikeExpression, signatures: readonly Signature[], candidatesOutArray: Signature[] | undefined, checkMode: CheckMode, fallbackError?: DiagnosticMessage): Signature {
function resolveCall(node: CallLikeExpression, signatures: readonly Signature[], candidatesOutArray: Signature[] | undefined, checkMode: CheckMode, fallbackError?: DiagnosticMessage, contextFlags?: ContextFlags): Signature {
const isTaggedTemplate = node.kind === SyntaxKind.TaggedTemplateExpression;
const isDecorator = node.kind === SyntaxKind.Decorator;
const isJsxOpeningOrSelfClosingElement = isJsxOpeningLikeElement(node);
Expand Down Expand Up @@ -22017,7 +22013,7 @@ namespace ts {
result = chooseOverload(candidates, subtypeRelation, signatureHelpTrailingComma);
}
if (!result) {
result = chooseOverload(candidates, assignableRelation, signatureHelpTrailingComma);
result = chooseOverload(candidates, assignableRelation, signatureHelpTrailingComma, contextFlags);
}
if (result) {
return result;
Expand Down Expand Up @@ -22109,7 +22105,7 @@ namespace ts {

return produceDiagnostics || !args ? resolveErrorCall(node) : getCandidateForOverloadFailure(node, candidates, args, !!candidatesOutArray);

function chooseOverload(candidates: Signature[], relation: Map<RelationComparisonResult>, signatureHelpTrailingComma = false) {
function chooseOverload(candidates: Signature[], relation: Map<RelationComparisonResult>, signatureHelpTrailingComma = false, contextFlags?: ContextFlags) {
candidatesForArgumentError = undefined;
candidateForArgumentArityError = undefined;
candidateForTypeArgumentError = undefined;
Expand Down Expand Up @@ -22146,7 +22142,7 @@ namespace ts {
}
else {
inferenceContext = createInferenceContext(candidate.typeParameters, candidate, /*flags*/ isInJSFile(node) ? InferenceFlags.AnyDefault : InferenceFlags.None);
typeArgumentTypes = inferTypeArguments(node, candidate, args, argCheckMode | CheckMode.SkipGenericFunctions, inferenceContext);
typeArgumentTypes = inferTypeArguments(node, candidate, args, argCheckMode | CheckMode.SkipGenericFunctions, inferenceContext, contextFlags);
argCheckMode |= inferenceContext.flags & InferenceFlags.SkippedGenericFunction ? CheckMode.SkipGenericFunctions : CheckMode.Normal;
}
checkCandidate = getSignatureInstantiation(candidate, typeArgumentTypes, isInJSFile(candidate.declaration), inferenceContext && inferenceContext.inferredTypeParameters);
Expand Down Expand Up @@ -22316,7 +22312,7 @@ namespace ts {
return maxParamsIndex;
}

function resolveCallExpression(node: CallExpression, candidatesOutArray: Signature[] | undefined, checkMode: CheckMode): Signature {
function resolveCallExpression(node: CallExpression, candidatesOutArray: Signature[] | undefined, checkMode: CheckMode, contextFlags?: ContextFlags): Signature {
if (node.expression.kind === SyntaxKind.SuperKeyword) {
const superType = checkSuperExpression(node.expression);
if (isTypeAny(superType)) {
Expand Down Expand Up @@ -22412,7 +22408,7 @@ namespace ts {
error(node, Diagnostics.Value_of_type_0_is_not_callable_Did_you_mean_to_include_new, typeToString(funcType));
return resolveErrorCall(node);
}
return resolveCall(node, callSignatures, candidatesOutArray, checkMode);
return resolveCall(node, callSignatures, candidatesOutArray, checkMode, /*fallbackError*/ undefined, contextFlags);
}

function isGenericFunctionReturningFunction(signature: Signature) {
Expand All @@ -22430,7 +22426,7 @@ namespace ts {
!numCallSignatures && !numConstructSignatures && !(apparentFuncType.flags & (TypeFlags.Union | TypeFlags.Never)) && isTypeAssignableTo(funcType, globalFunctionType);
}

function resolveNewExpression(node: NewExpression, candidatesOutArray: Signature[] | undefined, checkMode: CheckMode): Signature {
function resolveNewExpression(node: NewExpression, candidatesOutArray: Signature[] | undefined, checkMode: CheckMode, contextFlags?: ContextFlags): Signature {
if (node.arguments && languageVersion < ScriptTarget.ES5) {
const spreadIndex = getSpreadArgumentIndex(node.arguments);
if (spreadIndex >= 0) {
Expand Down Expand Up @@ -22483,7 +22479,7 @@ namespace ts {
return resolveErrorCall(node);
}

return resolveCall(node, constructSignatures, candidatesOutArray, checkMode);
return resolveCall(node, constructSignatures, candidatesOutArray, checkMode, /*fallbackError*/ undefined, contextFlags);
}

// If expressionType's apparent type is an object type with no construct signatures but
Expand All @@ -22492,7 +22488,7 @@ namespace ts {
// operation is Any. It is an error to have a Void this type.
const callSignatures = getSignaturesOfType(expressionType, SignatureKind.Call);
if (callSignatures.length) {
const signature = resolveCall(node, callSignatures, candidatesOutArray, checkMode);
const signature = resolveCall(node, callSignatures, candidatesOutArray, checkMode, /*fallbackError*/ undefined, contextFlags);
if (!noImplicitAny) {
if (signature.declaration && !isJSConstructor(signature.declaration) && getReturnTypeOfSignature(signature) !== voidType) {
error(node, Diagnostics.Only_a_void_function_can_be_called_with_the_new_keyword);
Expand Down Expand Up @@ -22838,12 +22834,12 @@ namespace ts {
signature.parameters.length < getDecoratorArgumentCount(decorator, signature));
}

function resolveSignature(node: CallLikeExpression, candidatesOutArray: Signature[] | undefined, checkMode: CheckMode): Signature {
function resolveSignature(node: CallLikeExpression, candidatesOutArray: Signature[] | undefined, checkMode: CheckMode, contextFlags?: ContextFlags): Signature {
switch (node.kind) {
case SyntaxKind.CallExpression:
return resolveCallExpression(node, candidatesOutArray, checkMode);
return resolveCallExpression(node, candidatesOutArray, checkMode, contextFlags);
case SyntaxKind.NewExpression:
return resolveNewExpression(node, candidatesOutArray, checkMode);
return resolveNewExpression(node, candidatesOutArray, checkMode, contextFlags);
case SyntaxKind.TaggedTemplateExpression:
return resolveTaggedTemplateExpression(node, candidatesOutArray, checkMode);
case SyntaxKind.Decorator:
Expand All @@ -22862,7 +22858,7 @@ namespace ts {
* the function will fill it up with appropriate candidate signatures
* @return a signature of the call-like expression or undefined if one can't be found
*/
function getResolvedSignature(node: CallLikeExpression, candidatesOutArray?: Signature[] | undefined, checkMode?: CheckMode): Signature {
function getResolvedSignature(node: CallLikeExpression, candidatesOutArray?: Signature[] | undefined, checkMode?: CheckMode, contextFlags?: ContextFlags): Signature {
const links = getNodeLinks(node);
// If getResolvedSignature has already been called, we will have cached the resolvedSignature.
// However, it is possible that either candidatesOutArray was not passed in the first time,
Expand All @@ -22873,7 +22869,7 @@ namespace ts {
return cached;
}
links.resolvedSignature = resolvingSignature;
const result = resolveSignature(node, candidatesOutArray, checkMode || CheckMode.Normal);
const result = resolveSignature(node, candidatesOutArray, checkMode || CheckMode.Normal, contextFlags);
// When CheckMode.SkipGenericFunctions is set we use resolvingSignature to indicate that call
// resolution should be deferred.
if (result !== resolvingSignature) {
Expand Down
9 changes: 9 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3220,8 +3220,10 @@ namespace ts {

getFullyQualifiedName(symbol: Symbol): string;
getAugmentedPropertiesOfType(type: Type): Symbol[];

getRootSymbols(symbol: Symbol): readonly Symbol[];
getContextualType(node: Expression): Type | undefined;
/* @internal */ getContextualType(node: Expression, contextFlags?: ContextFlags): Type | undefined; // eslint-disable-line @typescript-eslint/unified-signatures
/* @internal */ getContextualTypeForObjectLiteralElement(element: ObjectLiteralElementLike): Type | undefined;
/* @internal */ getContextualTypeForArgumentAtIndex(call: CallLikeExpression, argIndex: number): Type | undefined;
/* @internal */ getContextualTypeForJsxAttribute(attribute: JsxAttribute | JsxSpreadAttribute): Type | undefined;
Expand Down Expand Up @@ -3379,6 +3381,13 @@ namespace ts {
Subtype
}

/* @internal */
export const enum ContextFlags {
None = 0,
Signature = 1 << 0, // Obtaining contextual signature
Completion = 1 << 1, // Obtaining constraint type for completion
}

// NOTE: If modifying this enum, must modify `TypeFormatFlags` too!
export const enum NodeBuilderFlags {
None = 0,
Expand Down
2 changes: 1 addition & 1 deletion src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1501,7 +1501,7 @@ namespace ts.Completions {
let existingMembers: readonly Declaration[] | undefined;

if (objectLikeContainer.kind === SyntaxKind.ObjectLiteralExpression) {
const typeForObject = typeChecker.getContextualType(objectLikeContainer);
const typeForObject = typeChecker.getContextualType(objectLikeContainer, ContextFlags.Completion);
if (!typeForObject) return GlobalsSearch.Fail;
isNewIdentifierLocation = hasIndexSignature(typeForObject);
typeMembers = getPropertiesForObjectExpression(typeForObject, objectLikeContainer, typeChecker);
Expand Down
11 changes: 11 additions & 0 deletions tests/cases/fourslash/completionsWithGenericStringLiteral.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path="fourslash.ts" />
// @strict: true

//// declare function get<T, K extends keyof T>(obj: T, key: K): T[K];
//// get({ hello: 123, world: 456 }, "/**/");

verify.completions({
marker: "",
includes: ['hello', 'world']
});

19 changes: 19 additions & 0 deletions tests/cases/fourslash/completionsWithOptionalPropertiesGeneric.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/// <reference path="fourslash.ts" />
// @strict: true

//// interface MyOptions {
//// hello?: boolean;
//// world?: boolean;
//// }
//// declare function bar<T extends MyOptions>(options?: Partial<T>): void;
//// bar({ hello, /*1*/ });

verify.completions({
marker: '1',
includes: [
{
sortText: completion.SortText.OptionalMember,
name: 'world'
},
]
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/// <reference path="fourslash.ts" />
// @strict: true

//// interface Options {
//// someFunction?: () => string
//// anotherFunction?: () => string
//// }
////
//// export class Clazz<T extends Options> {
//// constructor(public a: T) {}
//// }
////
//// new Clazz({ /*1*/ })

verify.completions({
marker: '1',
includes: [
{
sortText: completion.SortText.OptionalMember,
name: 'someFunction'
},
{
sortText: completion.SortText.OptionalMember,
name: 'anotherFunction'
},
]
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/// <reference path="fourslash.ts" />
// @strict: true

//// interface DeepOptions {
//// another?: boolean;
//// }
//// interface MyOptions {
//// hello?: boolean;
//// world?: boolean;
//// deep?: DeepOptions
//// }
//// declare function bar<T extends MyOptions>(options?: Partial<T>): void;
//// bar({ deep: {/*1*/} });

verify.completions({
marker: '1',
includes: [
{
sortText: completion.SortText.OptionalMember,
name: 'another'
},
]
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/// <reference path="fourslash.ts" />
// @strict: true

//// interface Foo {
//// a_a: boolean;
//// a_b: boolean;
//// a_c: boolean;
//// b_a: boolean;
//// }
//// function partialFoo<T extends Partial<Foo>>(t: T) {return t}
//// partialFoo({ /*1*/ });

verify.completions({
marker: '1',
includes: [
{
sortText: completion.SortText.OptionalMember,
name: 'a_a'
},
{
sortText: completion.SortText.OptionalMember,
name: 'a_b'
},
{
sortText: completion.SortText.OptionalMember,
name: 'a_c'
},
{
sortText: completion.SortText.OptionalMember,
name: 'b_a'
},
]
})