From ef8eb0c876ccb71211eb61044ddba09a4010e30a Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Jan 2020 15:37:18 -0800 Subject: [PATCH] Fix contextually typed object literal completions where the object being edited affects its own inference (#36556) * Conditionally elide a parameter from contextual type signature calculation * Slightly different approach to forbid inference to specific expressions * Handle nested literals and mapped types correctly * Delete unused cache * Rename ContextFlags.BaseConstraint and related usage * Add tests from my PR * Update ContextFlags comment Co-Authored-By: Wesley Wigham * Update comments and fourslash triple slash refs Co-authored-by: Wesley Wigham --- src/compiler/checker.ts | 53 +++++++++++---- src/compiler/types.ts | 4 +- src/services/completions.ts | 22 +++---- .../completionsGenericIndexedAccess3.ts | 35 ++++++++++ .../completionsGenericIndexedAccess4.ts | 45 +++++++++++++ .../completionsGenericIndexedAccess5.ts | 30 +++++++++ .../completionsGenericIndexedAccess6.ts | 25 ++++++++ ...tionsObjectLiteralWithPartialConstraint.ts | 64 +++++++++++++++++++ 8 files changed, 253 insertions(+), 25 deletions(-) create mode 100644 tests/cases/fourslash/completionsGenericIndexedAccess3.ts create mode 100644 tests/cases/fourslash/completionsGenericIndexedAccess4.ts create mode 100644 tests/cases/fourslash/completionsGenericIndexedAccess5.ts create mode 100644 tests/cases/fourslash/completionsGenericIndexedAccess6.ts create mode 100644 tests/cases/fourslash/completionsObjectLiteralWithPartialConstraint.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d139935f42..ad3f3b71f2 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -467,7 +467,29 @@ namespace ts { getRootSymbols, getContextualType: (nodeIn: Expression, contextFlags?: ContextFlags) => { const node = getParseTreeNode(nodeIn, isExpression); - return node ? getContextualType(node, contextFlags) : undefined; + 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; + } + return result; }, getContextualTypeForObjectLiteralElement: nodeIn => { const node = getParseTreeNode(nodeIn, isObjectLiteralElementLike); @@ -17796,6 +17818,14 @@ namespace ts { undefined; } + function hasSkipDirectInferenceFlag(node: Node) { + return !!getNodeLinks(node).skipDirectInference; + } + + function isFromInferenceBlockedSource(type: Type) { + return !!(type.symbol && some(type.symbol.declarations, hasSkipDirectInferenceFlag)); + } + function inferTypes(inferences: InferenceInfo[], originalSource: Type, originalTarget: Type, priority: InferencePriority = 0, contravariant = false) { let symbolStack: Symbol[]; let visited: Map; @@ -17886,7 +17916,7 @@ namespace ts { // of inference. Also, we exclude inferences for silentNeverType (which is used as a wildcard // when constructing types from type parameters that had no inference candidates). if (getObjectFlags(source) & ObjectFlags.NonInferrableType || source === nonInferrableAnyType || source === silentNeverType || - (priority & InferencePriority.ReturnType && (source === autoType || source === autoArrayType))) { + (priority & InferencePriority.ReturnType && (source === autoType || source === autoArrayType)) || isFromInferenceBlockedSource(source)) { return; } const inference = getInferenceInfoForType(target); @@ -18190,7 +18220,7 @@ namespace ts { // type and then make a secondary inference from that type to T. We make a secondary inference // such that direct inferences to T get priority over inferences to Partial, for example. const inference = getInferenceInfoForType((constraintType).type); - if (inference && !inference.isFixed) { + if (inference && !inference.isFixed && !isFromInferenceBlockedSource(source)) { const inferredType = inferTypeForHomomorphicMappedType(source, target, constraintType); if (inferredType) { // We assign a lower priority to inferences made from types containing non-inferrable @@ -21449,19 +21479,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, contextFlags?: ContextFlags): Type | undefined { + function getContextualTypeForArgument(callTarget: CallLikeExpression, arg: Expression): 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, contextFlags); + return argIndex === -1 ? undefined : getContextualTypeForArgumentAtIndex(callTarget, argIndex); } - function getContextualTypeForArgumentAtIndex(callTarget: CallLikeExpression, argIndex: number, contextFlags?: ContextFlags): Type { + function getContextualTypeForArgumentAtIndex(callTarget: CallLikeExpression, argIndex: number): 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. - let signature = getNodeLinks(callTarget).resolvedSignature === resolvingSignature ? resolvingSignature : getResolvedSignature(callTarget); - if (contextFlags && contextFlags & ContextFlags.BaseConstraint && signature.target && !hasTypeArguments(callTarget)) { - signature = getBaseSignature(signature.target); - } + const signature = getNodeLinks(callTarget).resolvedSignature === resolvingSignature ? resolvingSignature : getResolvedSignature(callTarget); if (isJsxOpeningLikeElement(callTarget) && argIndex === 0) { return getEffectiveFirstArgumentForJsxSignature(signature, callTarget); @@ -21857,7 +21884,7 @@ namespace ts { } /* falls through */ case SyntaxKind.NewExpression: - return getContextualTypeForArgument(parent, node, contextFlags); + return getContextualTypeForArgument(parent, node); case SyntaxKind.TypeAssertionExpression: case SyntaxKind.AsExpression: return isConstTypeReference((parent).type) ? undefined : getTypeFromTypeNode((parent).type); @@ -21901,13 +21928,13 @@ namespace ts { } function getContextualJsxElementAttributesType(node: JsxOpeningLikeElement, contextFlags?: ContextFlags) { - if (isJsxOpeningElement(node) && node.parent.contextualType && contextFlags !== ContextFlags.BaseConstraint) { + if (isJsxOpeningElement(node) && node.parent.contextualType && contextFlags !== ContextFlags.Completions) { // Contextually applied type is moved from attributes up to the outer jsx attributes so when walking up from the children they get hit // _However_ to hit them from the _attributes_ we must look for them here; otherwise we'll used the declared type // (as below) instead! return node.parent.contextualType; } - return getContextualTypeForArgumentAtIndex(node, 0, contextFlags); + return getContextualTypeForArgumentAtIndex(node, 0); } function getEffectiveFirstArgumentForJsxSignature(signature: Signature, node: JsxOpeningLikeElement) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 7155ccb1d2..9b913c2f17 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3611,7 +3611,8 @@ namespace ts { None = 0, Signature = 1 << 0, // Obtaining contextual signature NoConstraints = 1 << 1, // Don't obtain type variable constraints - BaseConstraint = 1 << 2, // Use base constraint type for completions + Completions = 1 << 2, // Ignore inference to current node and parent nodes out to the containing call for completions + } // NOTE: If modifying this enum, must modify `TypeFormatFlags` too! @@ -4249,6 +4250,7 @@ namespace ts { outerTypeParameters?: TypeParameter[]; // Outer type parameters of anonymous object type instantiations?: Map; // Instantiations of generic type alias (undefined if non-generic) isExhaustive?: boolean; // Is node an exhaustive switch statement + skipDirectInference?: true; // Flag set by the API `getContextualType` call on a node when `Completions` is passed to force the checker to skip making inferences to a node's type } export const enum TypeFlags { diff --git a/src/services/completions.ts b/src/services/completions.ts index 84d1fb0d44..fdbc01ce50 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1280,8 +1280,8 @@ namespace ts.Completions { // Cursor is inside a JSX self-closing element or opening element const attrsType = jsxContainer && typeChecker.getContextualType(jsxContainer.attributes); if (!attrsType) return GlobalsSearch.Continue; - const baseType = jsxContainer && typeChecker.getContextualType(jsxContainer.attributes, ContextFlags.BaseConstraint); - symbols = filterJsxAttributes(getPropertiesForObjectExpression(attrsType, baseType, jsxContainer!.attributes, typeChecker), jsxContainer!.attributes.properties); + const completionsType = jsxContainer && typeChecker.getContextualType(jsxContainer.attributes, ContextFlags.Completions); + symbols = filterJsxAttributes(getPropertiesForObjectExpression(attrsType, completionsType, jsxContainer!.attributes, typeChecker), jsxContainer!.attributes.properties); setSortTextToOptionalMember(); completionKind = CompletionKind.MemberLike; isNewIdentifierLocation = false; @@ -1800,10 +1800,10 @@ namespace ts.Completions { if (objectLikeContainer.kind === SyntaxKind.ObjectLiteralExpression) { const instantiatedType = typeChecker.getContextualType(objectLikeContainer); - const baseType = instantiatedType && typeChecker.getContextualType(objectLikeContainer, ContextFlags.BaseConstraint); - if (!instantiatedType || !baseType) return GlobalsSearch.Fail; - isNewIdentifierLocation = hasIndexSignature(instantiatedType || baseType); - typeMembers = getPropertiesForObjectExpression(instantiatedType, baseType, objectLikeContainer, typeChecker); + const completionsType = instantiatedType && typeChecker.getContextualType(objectLikeContainer, ContextFlags.Completions); + if (!instantiatedType || !completionsType) return GlobalsSearch.Fail; + isNewIdentifierLocation = hasIndexSignature(instantiatedType || completionsType); + typeMembers = getPropertiesForObjectExpression(instantiatedType, completionsType, objectLikeContainer, typeChecker); existingMembers = objectLikeContainer.properties; } else { @@ -2549,10 +2549,10 @@ namespace ts.Completions { return jsdoc && jsdoc.tags && (rangeContainsPosition(jsdoc, position) ? findLast(jsdoc.tags, tag => tag.pos < position) : undefined); } - function getPropertiesForObjectExpression(contextualType: Type, baseConstrainedType: Type | undefined, obj: ObjectLiteralExpression | JsxAttributes, checker: TypeChecker): Symbol[] { - const hasBaseType = baseConstrainedType && baseConstrainedType !== contextualType; - const type = hasBaseType && !(baseConstrainedType!.flags & TypeFlags.AnyOrUnknown) - ? checker.getUnionType([contextualType, baseConstrainedType!]) + function getPropertiesForObjectExpression(contextualType: Type, completionsType: Type | undefined, obj: ObjectLiteralExpression | JsxAttributes, checker: TypeChecker): Symbol[] { + const hasCompletionsType = completionsType && completionsType !== contextualType; + const type = hasCompletionsType && !(completionsType!.flags & TypeFlags.AnyOrUnknown) + ? checker.getUnionType([contextualType, completionsType!]) : contextualType; const properties = type.isUnion() @@ -2564,7 +2564,7 @@ namespace ts.Completions { checker.isTypeInvalidDueToUnionDiscriminant(memberType, obj)))) : type.getApparentProperties(); - return hasBaseType ? properties.filter(hasDeclarationOtherThanSelf) : properties; + return hasCompletionsType ? properties.filter(hasDeclarationOtherThanSelf) : properties; // Filter out members whose only declaration is the object literal itself to avoid // self-fulfilling completions like: diff --git a/tests/cases/fourslash/completionsGenericIndexedAccess3.ts b/tests/cases/fourslash/completionsGenericIndexedAccess3.ts new file mode 100644 index 0000000000..730cdd8f63 --- /dev/null +++ b/tests/cases/fourslash/completionsGenericIndexedAccess3.ts @@ -0,0 +1,35 @@ +/// + +////interface CustomElements { +//// 'component-one': { +//// foo?: string; +//// }, +//// 'component-two': { +//// bar?: string; +//// } +////} +//// +////interface Options { +//// props: CustomElements[T]; +////} +//// +////declare function create(name: T, options: Options): void; +//// +////create('component-one', { props: { /*1*/ } }); +////create('component-two', { props: { /*2*/ } }); + +verify.completions({ + marker: "1", + exact: [{ + name: "foo", + sortText: completion.SortText.OptionalMember + }] +}); + +verify.completions({ + marker: "2", + exact: [{ + name: "bar", + sortText: completion.SortText.OptionalMember + }] +}); diff --git a/tests/cases/fourslash/completionsGenericIndexedAccess4.ts b/tests/cases/fourslash/completionsGenericIndexedAccess4.ts new file mode 100644 index 0000000000..0edeaedf98 --- /dev/null +++ b/tests/cases/fourslash/completionsGenericIndexedAccess4.ts @@ -0,0 +1,45 @@ +/// + +////interface CustomElements { +//// 'component-one': { +//// foo?: string; +//// }, +//// 'component-two': { +//// bar?: string; +//// } +////} +//// +////interface Options { +//// props: CustomElements[T]; +////} +//// +////declare function create(name: T, options: Options): void; +////declare function create(name: T, options: Options): void; +//// +////create('hello', { props: { /*1*/ } }) +////create('goodbye', { props: { /*2*/ } }) +////create('component-one', { props: { /*3*/ } }); + +verify.completions({ + marker: "1", + exact: [{ + name: "foo", + sortText: completion.SortText.OptionalMember + }] +}); + +verify.completions({ + marker: "2", + exact: [{ + name: "bar", + sortText: completion.SortText.OptionalMember + }] +}); + +verify.completions({ + marker: "3", + exact: [{ + name: "foo", + sortText: completion.SortText.OptionalMember + }] +}); diff --git a/tests/cases/fourslash/completionsGenericIndexedAccess5.ts b/tests/cases/fourslash/completionsGenericIndexedAccess5.ts new file mode 100644 index 0000000000..70ac6214d8 --- /dev/null +++ b/tests/cases/fourslash/completionsGenericIndexedAccess5.ts @@ -0,0 +1,30 @@ +/// + +////interface CustomElements { +//// 'component-one': { +//// foo?: string; +//// }, +//// 'component-two': { +//// bar?: string; +//// } +////} +//// +////interface Options { +//// props?: {} & { x: CustomElements[(T extends string ? T : never) & string][] }['x']; +////} +//// +////declare function f(k: T, options: Options): void; +//// +////f("component-one", { +//// props: [{ +//// /**/ +//// }] +////}) + +verify.completions({ + marker: "", + exact: [{ + name: "foo", + sortText: completion.SortText.OptionalMember + }] +}); diff --git a/tests/cases/fourslash/completionsGenericIndexedAccess6.ts b/tests/cases/fourslash/completionsGenericIndexedAccess6.ts new file mode 100644 index 0000000000..ae77524709 --- /dev/null +++ b/tests/cases/fourslash/completionsGenericIndexedAccess6.ts @@ -0,0 +1,25 @@ +/// + +// @Filename: component.tsx + +////interface CustomElements { +//// 'component-one': { +//// foo?: string; +//// }, +//// 'component-two': { +//// bar?: string; +//// } +////} +//// +////type Options = { kind: T } & Required<{ x: CustomElements[(T extends string ? T : never) & string] }['x']>; +//// +////declare function Component(props: Options): void; +//// +////const c = + +verify.completions({ + marker: "", + exact: [{ + name: "foo" + }] +}) diff --git a/tests/cases/fourslash/completionsObjectLiteralWithPartialConstraint.ts b/tests/cases/fourslash/completionsObjectLiteralWithPartialConstraint.ts new file mode 100644 index 0000000000..ebd47f3d9a --- /dev/null +++ b/tests/cases/fourslash/completionsObjectLiteralWithPartialConstraint.ts @@ -0,0 +1,64 @@ +/// + +////interface MyOptions { +//// hello?: boolean; +//// world?: boolean; +////} +////declare function bar(options?: Partial): void; +////bar({ hello: true, /*1*/ }); +//// +////interface Test { +//// keyPath?: string; +//// autoIncrement?: boolean; +////} +//// +////function test>(opt: T) { } +//// +////test({ +//// a: { +//// keyPath: 'x.y', +//// autoIncrement: true +//// }, +//// b: { +//// /*2*/ +//// } +////}); +////type Colors = { +//// rgb: { r: number, g: number, b: number }; +//// hsl: { h: number, s: number, l: number } +////}; +//// +////function createColor(kind: T, values: Colors[T]) { } +//// +////createColor('rgb', { +//// /*3*/ +////}); +//// +////declare function f(x: T, y: { a: U, b: V }[T]): void; +//// +////f('a', { +//// /*4*/ +////}); +//// +////declare function f2(x: T): void; +////f2({ +//// /*5*/ +////}); +//// +////type X = { a: { a }, b: { b } } +//// +////function f4(p: { kind: T } & X[T]) { } +//// +////f4({ +//// kind: "a", +//// /*6*/ +////}) + +verify.completions( + { marker: "1", exact: [{ name: "world", sortText: completion.SortText.OptionalMember }] }, + { marker: "2", exact: [{ name: "keyPath", sortText: completion.SortText.OptionalMember }, { name: "autoIncrement", sortText: completion.SortText.OptionalMember }] }, + { marker: "3", exact: ["r", "g", "b"] }, + { marker: "4", exact: [{ name: "a", sortText: completion.SortText.OptionalMember }] }, + { marker: "5", exact: [{ name: "x", sortText: completion.SortText.OptionalMember }] }, + { marker: "6", exact: ["a"] }, +); \ No newline at end of file