Skip to content

Commit

Permalink
Fix string literal completions when a partially-typed string fixes in…
Browse files Browse the repository at this point in the history 
…ference to a type parameter (#48410)

* Add failing test

* Fix string literal completions when a partially-typed string fixes inference to a type parameter
  • Loading branch information
@andrewbranch
andrewbranch committed Mar 29, 2022
1 parent bf7bfa1 commit 7ec7d6d
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 49 deletions.
90 changes: 52 additions & 38 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,16 @@ namespace ts {
}

const enum CheckMode {
Normal = 0, // Normal type checking
Contextual = 1 << 0, // Explicitly assigned contextual type, therefore not cacheable
Inferential = 1 << 1, // Inferential typing
SkipContextSensitive = 1 << 2, // Skip context sensitive function expressions
SkipGenericFunctions = 1 << 3, // Skip single signature generic functions
IsForSignatureHelp = 1 << 4, // Call resolution for purposes of signature help
RestBindingElement = 1 << 5, // Checking a type that is going to be used to determine the type of a rest binding element
// e.g. in `const { a, ...rest } = foo`, when checking the type of `foo` to determine the type of `rest`,
// we need to preserve generic types instead of substituting them for constraints
Normal = 0, // Normal type checking
Contextual = 1 << 0, // Explicitly assigned contextual type, therefore not cacheable
Inferential = 1 << 1, // Inferential typing
SkipContextSensitive = 1 << 2, // Skip context sensitive function expressions
SkipGenericFunctions = 1 << 3, // Skip single signature generic functions
IsForSignatureHelp = 1 << 4, // Call resolution for purposes of signature help
IsForStringLiteralArgumentCompletions = 1 << 5, // Do not infer from the argument currently being typed
RestBindingElement = 1 << 6, // Checking a type that is going to be used to determine the type of a rest binding element
// e.g. in `const { a, ...rest } = foo`, when checking the type of `foo` to determine the type of `rest`,
// we need to preserve generic types instead of substituting them for constraints
}

const enum SignatureCheckMode {
Expand Down Expand Up @@ -540,26 +541,10 @@ namespace ts {
if (!node) {
return undefined;
}
const containingCall = findAncestor(node, isCallLikeExpression);
const containingCallResolvedSignature = containingCall && getNodeLinks(containingCall).resolvedSignature;
if (contextFlags! & ContextFlags.Completions && containingCall) {
let toMarkSkip = node as Node;
do {
getNodeLinks(toMarkSkip).skipDirectInference = true;
toMarkSkip = toMarkSkip.parent;
} while (toMarkSkip && toMarkSkip !== containingCall);
getNodeLinks(containingCall).resolvedSignature = undefined;
}
const result = getContextualType(node, contextFlags);
if (contextFlags! & ContextFlags.Completions && containingCall) {
let toMarkSkip = node as Node;
do {
getNodeLinks(toMarkSkip).skipDirectInference = undefined;
toMarkSkip = toMarkSkip.parent;
} while (toMarkSkip && toMarkSkip !== containingCall);
getNodeLinks(containingCall).resolvedSignature = containingCallResolvedSignature;
if (contextFlags! & ContextFlags.Completions) {
return runWithInferenceBlockedFromSourceNode(node, () => getContextualType(node, contextFlags));
}
return result;
return getContextualType(node, contextFlags);
},
getContextualTypeForObjectLiteralElement: nodeIn => {
const node = getParseTreeNode(nodeIn, isObjectLiteralElementLike);
Expand All @@ -578,6 +563,8 @@ namespace ts {
getFullyQualifiedName,
getResolvedSignature: (node, candidatesOutArray, argumentCount) =>
getResolvedSignatureWorker(node, candidatesOutArray, argumentCount, CheckMode.Normal),
getResolvedSignatureForStringLiteralCompletions: (call, editingArgument, candidatesOutArray) =>
getResolvedSignatureWorker(call, candidatesOutArray, /*argumentCount*/ undefined, CheckMode.IsForStringLiteralArgumentCompletions, editingArgument),
getResolvedSignatureForSignatureHelp: (node, candidatesOutArray, argumentCount) =>
getResolvedSignatureWorker(node, candidatesOutArray, argumentCount, CheckMode.IsForSignatureHelp),
getExpandedParameters,
Expand Down Expand Up @@ -747,10 +734,36 @@ namespace ts {
getMemberOverrideModifierStatus,
};

function getResolvedSignatureWorker(nodeIn: CallLikeExpression, candidatesOutArray: Signature[] | undefined, argumentCount: number | undefined, checkMode: CheckMode): Signature | undefined {
function runWithInferenceBlockedFromSourceNode<T>(node: Node | undefined, fn: () => T): T {
const containingCall = findAncestor(node, isCallLikeExpression);
const containingCallResolvedSignature = containingCall && getNodeLinks(containingCall).resolvedSignature;
if (containingCall) {
let toMarkSkip = node!;
do {
getNodeLinks(toMarkSkip).skipDirectInference = true;
toMarkSkip = toMarkSkip.parent;
} while (toMarkSkip && toMarkSkip !== containingCall);
getNodeLinks(containingCall).resolvedSignature = undefined;
}
const result = fn();
if (containingCall) {
let toMarkSkip = node!;
do {
getNodeLinks(toMarkSkip).skipDirectInference = undefined;
toMarkSkip = toMarkSkip.parent;
} while (toMarkSkip && toMarkSkip !== containingCall);
getNodeLinks(containingCall).resolvedSignature = containingCallResolvedSignature;
}
return result;
}

function getResolvedSignatureWorker(nodeIn: CallLikeExpression, candidatesOutArray: Signature[] | undefined, argumentCount: number | undefined, checkMode: CheckMode, editingArgument?: Node): Signature | undefined {
const node = getParseTreeNode(nodeIn, isCallLikeExpression);
apparentArgumentCount = argumentCount;
const res = node ? getResolvedSignature(node, candidatesOutArray, checkMode) : undefined;
const res =
!node ? undefined :
editingArgument ? runWithInferenceBlockedFromSourceNode(editingArgument, () => getResolvedSignature(node, candidatesOutArray, checkMode)) :
getResolvedSignature(node, candidatesOutArray, checkMode);
apparentArgumentCount = undefined;
return res;
}
Expand Down Expand Up @@ -22664,7 +22677,7 @@ namespace ts {
const properties = getPropertiesOfObjectType(target);
for (const targetProp of properties) {
const sourceProp = getPropertyOfType(source, targetProp.escapedName);
if (sourceProp) {
if (sourceProp && !some(sourceProp.declarations, hasSkipDirectInferenceFlag)) {
inferFromTypes(getTypeOfSymbol(sourceProp), getTypeOfSymbol(targetProp));
}
}
Expand Down Expand Up @@ -29746,7 +29759,7 @@ namespace ts {

for (let i = 0; i < argCount; i++) {
const arg = args[i];
if (arg.kind !== SyntaxKind.OmittedExpression) {
if (arg.kind !== SyntaxKind.OmittedExpression && !(checkMode & CheckMode.IsForStringLiteralArgumentCompletions && hasSkipDirectInferenceFlag(arg))) {
const paramType = getTypeAtPosition(signature, i);
const argType = checkExpressionWithContextualType(arg, paramType, context, checkMode);
inferTypes(context.inferences, argType, paramType);
Expand Down Expand Up @@ -30486,7 +30499,7 @@ namespace ts {
}
}

return getCandidateForOverloadFailure(node, candidates, args, !!candidatesOutArray);
return getCandidateForOverloadFailure(node, candidates, args, !!candidatesOutArray, checkMode);

function addImplementationSuccessElaboration(failed: Signature, diagnostic: Diagnostic) {
const oldCandidatesForArgumentError = candidatesForArgumentError;
Expand Down Expand Up @@ -30600,14 +30613,15 @@ namespace ts {
candidates: Signature[],
args: readonly Expression[],
hasCandidatesOutArray: boolean,
checkMode: CheckMode,
): Signature {
Debug.assert(candidates.length > 0); // Else should not have called this.
checkNodeDeferred(node);
// Normally we will combine overloads. Skip this if they have type parameters since that's hard to combine.
// Don't do this if there is a `candidatesOutArray`,
// because then we want the chosen best candidate to be one of the overloads, not a combination.
return hasCandidatesOutArray || candidates.length === 1 || candidates.some(c => !!c.typeParameters)
? pickLongestCandidateSignature(node, candidates, args)
? pickLongestCandidateSignature(node, candidates, args, checkMode)
: createUnionOfSignaturesForOverloadFailure(candidates);
}

Expand Down Expand Up @@ -30661,7 +30675,7 @@ namespace ts {
return createSymbolWithType(first(sources), type);
}

function pickLongestCandidateSignature(node: CallLikeExpression, candidates: Signature[], args: readonly Expression[]): Signature {
function pickLongestCandidateSignature(node: CallLikeExpression, candidates: Signature[], args: readonly Expression[], checkMode: CheckMode): Signature {
// Pick the longest signature. This way we can get a contextual type for cases like:
// declare function f(a: { xa: number; xb: number; }, b: number);
// f({ |
Expand All @@ -30678,7 +30692,7 @@ namespace ts {
const typeArgumentNodes: readonly TypeNode[] | undefined = callLikeExpressionMayHaveTypeArguments(node) ? node.typeArguments : undefined;
const instantiated = typeArgumentNodes
? createSignatureInstantiation(candidate, getTypeArgumentsFromNodes(typeArgumentNodes, typeParameters, isInJSFile(node)))
: inferSignatureInstantiationForOverloadFailure(node, typeParameters, candidate, args);
: inferSignatureInstantiationForOverloadFailure(node, typeParameters, candidate, args, checkMode);
candidates[bestIndex] = instantiated;
return instantiated;
}
Expand All @@ -30694,9 +30708,9 @@ namespace ts {
return typeArguments;
}

function inferSignatureInstantiationForOverloadFailure(node: CallLikeExpression, typeParameters: readonly TypeParameter[], candidate: Signature, args: readonly Expression[]): Signature {
function inferSignatureInstantiationForOverloadFailure(node: CallLikeExpression, typeParameters: readonly TypeParameter[], candidate: Signature, args: readonly Expression[], checkMode: CheckMode): Signature {
const inferenceContext = createInferenceContext(typeParameters, candidate, /*flags*/ isInJSFile(node) ? InferenceFlags.AnyDefault : InferenceFlags.None);
const typeArgumentTypes = inferTypeArguments(node, candidate, args, CheckMode.SkipContextSensitive | CheckMode.SkipGenericFunctions, inferenceContext);
const typeArgumentTypes = inferTypeArguments(node, candidate, args, checkMode | CheckMode.SkipContextSensitive | CheckMode.SkipGenericFunctions, inferenceContext);
return createSignatureInstantiation(candidate, typeArgumentTypes);
}

Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4284,6 +4284,7 @@ namespace ts {
*/
getResolvedSignature(node: CallLikeExpression, candidatesOutArray?: Signature[], argumentCount?: number): Signature | undefined;
/* @internal */ getResolvedSignatureForSignatureHelp(node: CallLikeExpression, candidatesOutArray?: Signature[], argumentCount?: number): Signature | undefined;
/* @internal */ getResolvedSignatureForStringLiteralCompletions(call: CallLikeExpression, editingArgument: Node, candidatesOutArray: Signature[]): Signature | undefined;
/* @internal */ getExpandedParameters(sig: Signature): readonly (readonly Symbol[])[];
/* @internal */ hasEffectiveRestParameter(sig: Signature): boolean;
/* @internal */ containsArgumentsReference(declaration: SignatureDeclaration): boolean;
Expand Down
10 changes: 5 additions & 5 deletions src/harness/fourslashImpl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ namespace FourSlash {
// The position of the end of the current selection, or -1 if nothing is selected
public selectionEnd = -1;

public lastKnownMarker = "";
public lastKnownMarker: string | undefined;

// The file that's currently 'opened'
public activeFile!: FourSlashFile;
Expand Down Expand Up @@ -400,7 +400,7 @@ namespace FourSlash {
continue;
}
const memo = Utils.memoize(
(_version: number, _active: string, _caret: number, _selectEnd: number, _marker: string, ...args: any[]) => (ls[key] as Function)(...args),
(_version: number, _active: string, _caret: number, _selectEnd: number, _marker: string | undefined, ...args: any[]) => (ls[key] as Function)(...args),
(...args) => args.map(a => a && typeof a === "object" ? JSON.stringify(a) : a).join("|,|")
);
proxy[key] = (...args: any[]) => memo(
Expand Down Expand Up @@ -540,8 +540,8 @@ namespace FourSlash {
}

private messageAtLastKnownMarker(message: string) {
const locationDescription = this.lastKnownMarker ? this.lastKnownMarker : this.getLineColStringAtPosition(this.currentCaretPosition);
return `At ${locationDescription}: ${message}`;
const locationDescription = this.lastKnownMarker !== undefined ? this.lastKnownMarker : this.getLineColStringAtPosition(this.currentCaretPosition);
return `At marker '${locationDescription}': ${message}`;
}

private assertionMessageAtLastKnownMarker(msg: string) {
Expand Down Expand Up @@ -864,7 +864,7 @@ namespace FourSlash {
else {
for (const marker of toArray(options.marker)) {
this.goToMarker(marker);
this.verifyCompletionsWorker(options);
this.verifyCompletionsWorker({ ...options, marker });
}
}
}
Expand Down
19 changes: 13 additions & 6 deletions src/services/stringCompletions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,13 @@ namespace ts.Completions.StringCompletions {

case SyntaxKind.CallExpression:
case SyntaxKind.NewExpression:
case SyntaxKind.JsxAttribute:
if (!isRequireCallArgument(node) && !isImportCall(parent)) {
const argumentInfo = SignatureHelp.getArgumentInfoForCompletions(node, position, sourceFile);
const argumentInfo = SignatureHelp.getArgumentInfoForCompletions(parent.kind === SyntaxKind.JsxAttribute ? parent.parent : node, position, sourceFile);
// Get string literal completions from specialized signatures of the target
// i.e. declare function f(a: 'A');
// f("/*completion position*/")
return argumentInfo ? getStringLiteralCompletionsFromSignature(argumentInfo, typeChecker) : fromContextualType();
return argumentInfo ? getStringLiteralCompletionsFromSignature(argumentInfo.invocation, node, argumentInfo, typeChecker) : fromContextualType();
}
// falls through (is `require("")` or `require(""` or `import("")`)

Expand Down Expand Up @@ -257,15 +258,21 @@ namespace ts.Completions.StringCompletions {
type !== current && isLiteralTypeNode(type) && isStringLiteral(type.literal) ? type.literal.text : undefined);
}

function getStringLiteralCompletionsFromSignature(argumentInfo: SignatureHelp.ArgumentInfoForCompletions, checker: TypeChecker): StringLiteralCompletionsFromTypes {
function getStringLiteralCompletionsFromSignature(call: CallLikeExpression, arg: StringLiteralLike, argumentInfo: SignatureHelp.ArgumentInfoForCompletions, checker: TypeChecker): StringLiteralCompletionsFromTypes {
let isNewIdentifier = false;

const uniques = new Map<string, true>();
const candidates: Signature[] = [];
checker.getResolvedSignature(argumentInfo.invocation, candidates, argumentInfo.argumentCount);
const editingArgument = isJsxOpeningLikeElement(call) ? Debug.checkDefined(findAncestor(arg.parent, isJsxAttribute)) : arg;
checker.getResolvedSignatureForStringLiteralCompletions(call, editingArgument, candidates);
const types = flatMap(candidates, candidate => {
if (!signatureHasRestParameter(candidate) && argumentInfo.argumentCount > candidate.parameters.length) return;
const type = candidate.getTypeParameterAtPosition(argumentInfo.argumentIndex);
let type = candidate.getTypeParameterAtPosition(argumentInfo.argumentIndex);
if (isJsxOpeningLikeElement(call)) {
const propType = checker.getTypeOfPropertyOfType(type, (editingArgument as JsxAttribute).name.text);
if (propType) {
type = propType;
}
}
isNewIdentifier = isNewIdentifier || !!(type.flags & TypeFlags.String);
return getStringLiteralTypes(type, uniques);
});
Expand Down
23 changes: 23 additions & 0 deletions tests/cases/fourslash/completionsLiteralOverload.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/// <reference path="fourslash.ts" />

// @allowJs: true

// @Filename: /a.tsx
//// interface Events {
//// "": any;
//// drag: any;
//// dragenter: any;
//// }
//// declare function addListener<K extends keyof Events>(type: K, listener: (ev: Events[K]) => any): void;
////
//// declare function ListenerComponent<K extends keyof Events>(props: { type: K, onWhatever: (ev: Events[K]) => void }): JSX.Element;
////
//// addListener("/*ts*/");
//// (<ListenerComponent type="/*tsx*/" />);

// @Filename: /b.js
//// addListener("/*js*/");

verify.completions({ marker: ["ts", "tsx", "js"], exact: ["", "drag", "dragenter"] });
edit.insert("drag");
verify.completions({ marker: ["ts", "tsx", "js"], exact: ["", "drag", "dragenter"] });

0 comments on commit 7ec7d6d

Please sign in to comment.