From 7f72630a9175beebab1b74e1d3e683c52a95b652 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 7 Jun 2018 14:21:55 -0700 Subject: [PATCH 1/4] Don't add completions from a discriminated union type when the discriminant doesn't match --- src/compiler/checker.ts | 1 + src/compiler/types.ts | 1 + src/services/completions.ts | 40 +++++++++++++------ .../completionsDiscriminatedUnion.ts | 7 ++++ 4 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 tests/cases/fourslash/completionsDiscriminatedUnion.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 62607797679ea..f9cc34d4d05f5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -109,6 +109,7 @@ namespace ts { getDeclaredTypeOfSymbol, getPropertiesOfType, getPropertyOfType: (type, name) => getPropertyOfType(type, escapeLeadingUnderscores(name)), + getTypeOfPropertyOfType: (type, name) => getTypeOfPropertyOfType(type, escapeLeadingUnderscores(name)), getIndexInfoOfType, getSignaturesOfType, getIndexTypeOfType, diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 3a19762f0a996..2a389153f3482 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2883,6 +2883,7 @@ namespace ts { getDeclaredTypeOfSymbol(symbol: Symbol): Type; getPropertiesOfType(type: Type): Symbol[]; getPropertyOfType(type: Type, propertyName: string): Symbol | undefined; + /* @internal */ getTypeOfPropertyOfType(type: Type, propertyName: string): Type | undefined; getIndexInfoOfType(type: Type, kind: IndexKind): IndexInfo | undefined; getSignaturesOfType(type: Type, kind: SignatureKind): Signature[]; getIndexTypeOfType(type: Type, kind: IndexKind): Type | undefined; diff --git a/src/services/completions.ts b/src/services/completions.ts index d83e86ab4ad6a..911723b18a285 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1082,7 +1082,7 @@ namespace ts.Completions { // each individual type has. This is because we're going to add all identifiers // anyways. So we might as well elevate the members that were at least part // of the individual types to a higher status since we know what they are. - symbols.push(...getPropertiesForCompletion(type, typeChecker, /*isForAccess*/ true)); + symbols.push(...getPropertiesForCompletion(type, typeChecker)); } else { for (const symbol of type.getApparentProperties()) { @@ -1198,7 +1198,7 @@ namespace ts.Completions { if (preferences.includeCompletionsWithInsertText && scopeNode.kind !== SyntaxKind.SourceFile) { const thisType = typeChecker.tryGetThisTypeAt(scopeNode); if (thisType) { - for (const symbol of getPropertiesForCompletion(thisType, typeChecker, /*isForAccess*/ true)) { + for (const symbol of getPropertiesForCompletion(thisType, typeChecker)) { symbolToOriginInfoMap[getSymbolId(symbol)] = { type: "this-type" }; symbols.push(symbol); } @@ -1515,7 +1515,7 @@ namespace ts.Completions { const typeForObject = typeChecker.getContextualType(objectLikeContainer); if (!typeForObject) return GlobalsSearch.Fail; isNewIdentifierLocation = hasIndexSignature(typeForObject); - typeMembers = getPropertiesForCompletion(typeForObject, typeChecker, /*isForAccess*/ false); + typeMembers = getPropertiesForObjectExpression(typeForObject, objectLikeContainer, typeChecker); existingMembers = objectLikeContainer.properties; } else { @@ -2160,19 +2160,35 @@ namespace ts.Completions { } } + function getPropertiesForObjectExpression(contextualType: Type, obj: ObjectLiteralExpression, checker: TypeChecker): Symbol[] { + return contextualType.isUnion() + ? checker.getAllPossiblePropertiesOfTypes(contextualType.types.filter(memberType => isAllowedUnionTypeForObjectExpression(memberType, obj, checker))) + : contextualType.getApparentProperties(); + } + function isAllowedUnionTypeForObjectExpression(contextualType: Type, obj: ObjectLiteralExpression, checker: TypeChecker): boolean { + // If we're providing completions for an object literal, skip primitive, array-like, or callable types since those shouldn't be implemented by object literals. + return !(contextualType.flags & TypeFlags.Primitive || checker.isArrayLikeType(contextualType) || typeHasCallOrConstructSignatures(contextualType, checker)) && + // In `{ kind: "a", | }`, we only want completions from contextual types where `kind` is "a". + obj.properties.every(property => { + const name = property.name && getTextOfPropertyName(property.name); + const expected = name === undefined ? undefined : checker.getTypeOfPropertyOfType(contextualType, unescapeLeadingUnderscores(name)); + if (expected && expected.isLiteral()) { + const actual = checker.getTypeAtLocation(property); + // Apparently two literal types for the same literal are still not equal. + return !actual || actual.isLiteral() && actual.value === expected.value; + } + return true; + }); + } + /** * Gets all properties on a type, but if that type is a union of several types, * excludes array-like types or callable/constructable types. */ - function getPropertiesForCompletion(type: Type, checker: TypeChecker, isForAccess: boolean): Symbol[] { - if (!(type.isUnion())) { - return Debug.assertEachDefined(type.getApparentProperties(), "getApparentProperties() should all be defined"); - } - - // If we're providing completions for an object literal, skip primitive, array-like, or callable types since those shouldn't be implemented by object literals. - const filteredTypes = isForAccess ? type.types : type.types.filter(memberType => - !(memberType.flags & TypeFlags.Primitive || checker.isArrayLikeType(memberType) || typeHasCallOrConstructSignatures(memberType, checker))); - return Debug.assertEachDefined(checker.getAllPossiblePropertiesOfTypes(filteredTypes), "getAllPossiblePropertiesOfTypes() should all be defined"); + function getPropertiesForCompletion(type: Type, checker: TypeChecker): Symbol[] { + return type.isUnion() + ? Debug.assertEachDefined(checker.getAllPossiblePropertiesOfTypes(type.types), "getAllPossiblePropertiesOfTypes() should all be defined") + : Debug.assertEachDefined(type.getApparentProperties(), "getApparentProperties() should all be defined"); } /** diff --git a/tests/cases/fourslash/completionsDiscriminatedUnion.ts b/tests/cases/fourslash/completionsDiscriminatedUnion.ts new file mode 100644 index 0000000000000..ff6346e909f09 --- /dev/null +++ b/tests/cases/fourslash/completionsDiscriminatedUnion.ts @@ -0,0 +1,7 @@ +/// + +////interface A { kind: "a"; a: number; } +////interface B { kind: "b"; b: number; } +////const c: A | B = { kind: "a", /**/ }; + +verify.completions({ marker: "", exact: ["a"] }); From c6997a972c4ff9bd31337fc1f7003b9c1bf75735 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 27 Jun 2018 16:43:11 -0700 Subject: [PATCH 2/4] Move code to checker --- src/compiler/checker.ts | 18 ++++++++++++++++++ src/compiler/types.ts | 5 +++++ src/services/completions.ts | 22 ++++++---------------- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 636ea4237c867..51909973c1038 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -292,6 +292,7 @@ namespace ts { getNeverType: () => neverType, isSymbolAccessible, isArrayLikeType, + isTypeInvalidDueToUnionDiscriminant, getAllPossiblePropertiesOfTypes, getSuggestionForNonexistentProperty: (node, type) => getSuggestionForNonexistentProperty(node, type), getSuggestionForNonexistentSymbol: (location, name, meaning) => getSuggestionForNonexistentSymbol(location, escapeLeadingUnderscores(name), meaning), @@ -6747,6 +6748,19 @@ namespace ts { getPropertiesOfObjectType(type); } + function isTypeInvalidDueToUnionDiscriminant(contextualType: Type, obj: ObjectLiteralExpression): boolean { + return obj.properties.some(property => { + const name = property.name && getTextOfPropertyName(property.name); + const expected = name === undefined ? undefined : getTypeOfPropertyOfType(contextualType, name); + if (expected && typeIsLiteralType(expected)) { + const actual = getTypeOfNode(property); + // Apparently two literal types for the same literal are still not equal. + return !!actual && (!typeIsLiteralType(actual) || actual.value !== expected.value); + } + return false; + }); + } + function getAllPossiblePropertiesOfTypes(types: ReadonlyArray): Symbol[] { const unionType = getUnionType(types); if (!(unionType.flags & TypeFlags.Union)) { @@ -29012,4 +29026,8 @@ namespace ts { export const IntrinsicClassAttributes = "IntrinsicClassAttributes" as __String; // tslint:enable variable-name } + + function typeIsLiteralType(type: Type): type is LiteralType { + return !!(type.flags & TypeFlags.Literal); + } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 58e6de0406cd6..8043826ddaad8 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3047,6 +3047,11 @@ namespace ts { /* @internal */ getTypeCount(): number; /* @internal */ isArrayLikeType(type: Type): boolean; + /** + * True if `contextualType` should not be considered for completions because + * e.g. it specifies `kind: "a"` and obj has `kind: "b"`. + */ + /* @internal */ isTypeInvalidDueToUnionDiscriminant(contextualType: Type, obj: ObjectLiteralExpression): boolean; /** * For a union, will include a property if it's defined in *any* of the member types. * So for `{ a } | { b }`, this will include both `a` and `b`. diff --git a/src/services/completions.ts b/src/services/completions.ts index f2ccfab5107b3..4578f062abaa9 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2165,24 +2165,14 @@ namespace ts.Completions { function getPropertiesForObjectExpression(contextualType: Type, obj: ObjectLiteralExpression, checker: TypeChecker): Symbol[] { return contextualType.isUnion() - ? checker.getAllPossiblePropertiesOfTypes(contextualType.types.filter(memberType => isAllowedUnionTypeForObjectExpression(memberType, obj, checker))) + ? checker.getAllPossiblePropertiesOfTypes(contextualType.types.filter(memberType => + // If we're providing completions for an object literal, skip primitive, array-like, or callable types since those shouldn't be implemented by object literals. + !(memberType.flags & TypeFlags.Primitive || + checker.isArrayLikeType(memberType) || + typeHasCallOrConstructSignatures(memberType, checker) || + checker.isTypeInvalidDueToUnionDiscriminant(memberType, obj)))) : contextualType.getApparentProperties(); } - function isAllowedUnionTypeForObjectExpression(contextualType: Type, obj: ObjectLiteralExpression, checker: TypeChecker): boolean { - // If we're providing completions for an object literal, skip primitive, array-like, or callable types since those shouldn't be implemented by object literals. - return !(contextualType.flags & TypeFlags.Primitive || checker.isArrayLikeType(contextualType) || typeHasCallOrConstructSignatures(contextualType, checker)) && - // In `{ kind: "a", | }`, we only want completions from contextual types where `kind` is "a". - obj.properties.every(property => { - const name = property.name && getTextOfPropertyName(property.name); - const expected = name === undefined ? undefined : checker.getTypeOfPropertyOfType(contextualType, unescapeLeadingUnderscores(name)); - if (expected && expected.isLiteral()) { - const actual = checker.getTypeAtLocation(property); - // Apparently two literal types for the same literal are still not equal. - return !actual || actual.isLiteral() && actual.value === expected.value; - } - return true; - }); - } /** * Gets all properties on a type, but if that type is a union of several types, From 51666317b7788456181c14c96eb9e93fd6281778 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 28 Jun 2018 10:01:26 -0700 Subject: [PATCH 3/4] Update API (#24966) --- tests/baselines/reference/api/tsserverlibrary.d.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index a0624e4f087ed..47b8c7e45560b 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2566,6 +2566,7 @@ declare namespace ts { getDeclaredTypeOfSymbol(symbol: Symbol): Type; getPropertiesOfType(type: Type): Symbol[]; getPropertyOfType(type: Type, propertyName: string): Symbol | undefined; + getTypeOfPropertyOfType(type: Type, propertyName: string): Type | undefined; getIndexInfoOfType(type: Type, kind: IndexKind): IndexInfo | undefined; getSignaturesOfType(type: Type, kind: SignatureKind): ReadonlyArray; getIndexTypeOfType(type: Type, kind: IndexKind): Type | undefined; @@ -2708,6 +2709,11 @@ declare namespace ts { getSymbolCount(): number; getTypeCount(): number; isArrayLikeType(type: Type): boolean; + /** + * True if `contextualType` should not be considered for completions because + * e.g. it specifies `kind: "a"` and obj has `kind: "b"`. + */ + isTypeInvalidDueToUnionDiscriminant(contextualType: Type, obj: ObjectLiteralExpression): boolean; /** * For a union, will include a property if it's defined in *any* of the member types. * So for `{ a } | { b }`, this will include both `a` and `b`. From 27f1a79ee4a9ca30d48b5a8d4333388c130b328d Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 3 Jul 2018 16:45:52 -0700 Subject: [PATCH 4/4] Use isTypeIdenticalTo --- src/compiler/checker.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 51909973c1038..16594682a6368 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6754,8 +6754,7 @@ namespace ts { const expected = name === undefined ? undefined : getTypeOfPropertyOfType(contextualType, name); if (expected && typeIsLiteralType(expected)) { const actual = getTypeOfNode(property); - // Apparently two literal types for the same literal are still not equal. - return !!actual && (!typeIsLiteralType(actual) || actual.value !== expected.value); + return !!actual && !isTypeIdenticalTo(actual, expected); } return false; });