Skip to content

Improve some completions on generic object literals #34855

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

Merged
merged 5 commits into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21160,9 +21160,9 @@ namespace ts {
if (isJsxOpeningLikeElement(callTarget) && argIndex === 0) {
return getEffectiveFirstArgumentForJsxSignature(signature, callTarget);
}
if (contextFlags && contextFlags & ContextFlags.Completion && signature.target) {
if (contextFlags && contextFlags & ContextFlags.BaseConstraint && signature.target && !hasTypeArguments(callTarget)) {
const baseSignature = getBaseSignature(signature.target);
return intersectTypes(getTypeAtPosition(signature, argIndex), getTypeAtPosition(baseSignature, argIndex));
return getTypeAtPosition(baseSignature, argIndex);
}
return getTypeAtPosition(signature, argIndex);
}
Expand Down
15 changes: 11 additions & 4 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,13 @@ namespace ts {
| JSDocOptionalType
| JSDocVariadicType;

export type HasTypeArguments =
| CallExpression
| NewExpression
| TaggedTemplateExpression
| JsxOpeningElement
| JsxSelfClosingElement;

export type HasInitializer =
| HasExpressionInitializer
| ForStatement
Expand Down Expand Up @@ -3534,10 +3541,10 @@ namespace ts {

/* @internal */
export const enum ContextFlags {
None = 0,
Signature = 1 << 0, // Obtaining contextual signature
NoConstraints = 1 << 1, // Don't obtain type variable constraints
Completion = 1 << 2, // Obtaining constraint type for completion
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
}

// NOTE: If modifying this enum, must modify `TypeFormatFlags` too!
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2433,6 +2433,10 @@ namespace ts {
return (node as ParameterDeclaration).dotDotDotToken !== undefined || !!type && type.kind === SyntaxKind.JSDocVariadicType;
}

export function hasTypeArguments(node: Node): node is HasTypeArguments {
return !!(node as HasTypeArguments).typeArguments;
}

export const enum AssignmentKind {
None, Definite, Compound
}
Expand Down
23 changes: 14 additions & 9 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1279,7 +1279,7 @@ 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;
symbols = filterJsxAttributes(getPropertiesForObjectExpression(attrsType, jsxContainer!.attributes, typeChecker), jsxContainer!.attributes.properties);
symbols = filterJsxAttributes(getPropertiesForObjectExpression(attrsType, /*baseType*/ undefined, jsxContainer!.attributes, typeChecker), jsxContainer!.attributes.properties);
setSortTextToOptionalMember();
completionKind = CompletionKind.MemberLike;
isNewIdentifierLocation = false;
Expand Down Expand Up @@ -1795,10 +1795,11 @@ namespace ts.Completions {
let existingMembers: readonly Declaration[] | undefined;

if (objectLikeContainer.kind === SyntaxKind.ObjectLiteralExpression) {
const typeForObject = typeChecker.getContextualType(objectLikeContainer, ContextFlags.Completion);
if (!typeForObject) return GlobalsSearch.Fail;
isNewIdentifierLocation = hasIndexSignature(typeForObject);
typeMembers = getPropertiesForObjectExpression(typeForObject, objectLikeContainer, typeChecker);
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);
existingMembers = objectLikeContainer.properties;
}
else {
Expand Down Expand Up @@ -2535,15 +2536,19 @@ namespace ts.Completions {
return jsdoc && jsdoc.tags && (rangeContainsPosition(jsdoc, position) ? findLast(jsdoc.tags, tag => tag.pos < position) : undefined);
}

function getPropertiesForObjectExpression(contextualType: Type, obj: ObjectLiteralExpression | JsxAttributes, checker: TypeChecker): Symbol[] {
return contextualType.isUnion()
? checker.getAllPossiblePropertiesOfTypes(contextualType.types.filter(memberType =>
function getPropertiesForObjectExpression(contextualType: Type, baseConstrainedType: Type | undefined, obj: ObjectLiteralExpression | JsxAttributes, checker: TypeChecker): Symbol[] {
const type = baseConstrainedType && !(baseConstrainedType.flags & TypeFlags.AnyOrUnknown)
? checker.getUnionType([contextualType, baseConstrainedType])
: contextualType;

return type.isUnion()
? checker.getAllPossiblePropertiesOfTypes(type.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();
: type.getApparentProperties();
}

/**
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ declare namespace ts {
}
export type HasJSDoc = ParameterDeclaration | CallSignatureDeclaration | ConstructSignatureDeclaration | MethodSignature | PropertySignature | ArrowFunction | ParenthesizedExpression | SpreadAssignment | ShorthandPropertyAssignment | PropertyAssignment | FunctionExpression | LabeledStatement | ExpressionStatement | VariableStatement | FunctionDeclaration | ConstructorDeclaration | MethodDeclaration | PropertyDeclaration | AccessorDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | EnumMember | EnumDeclaration | ModuleDeclaration | ImportEqualsDeclaration | IndexSignatureDeclaration | FunctionTypeNode | ConstructorTypeNode | JSDocFunctionType | ExportDeclaration | EndOfFileToken;
export type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
export type HasTypeArguments = CallExpression | NewExpression | TaggedTemplateExpression | JsxOpeningElement | JsxSelfClosingElement;
export type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
export type HasExpressionInitializer = VariableDeclaration | ParameterDeclaration | BindingElement | PropertySignature | PropertyDeclaration | PropertyAssignment | EnumMember;
export interface NodeArray<T extends Node> extends ReadonlyArray<T>, TextRange {
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ declare namespace ts {
}
export type HasJSDoc = ParameterDeclaration | CallSignatureDeclaration | ConstructSignatureDeclaration | MethodSignature | PropertySignature | ArrowFunction | ParenthesizedExpression | SpreadAssignment | ShorthandPropertyAssignment | PropertyAssignment | FunctionExpression | LabeledStatement | ExpressionStatement | VariableStatement | FunctionDeclaration | ConstructorDeclaration | MethodDeclaration | PropertyDeclaration | AccessorDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | EnumMember | EnumDeclaration | ModuleDeclaration | ImportEqualsDeclaration | IndexSignatureDeclaration | FunctionTypeNode | ConstructorTypeNode | JSDocFunctionType | ExportDeclaration | EndOfFileToken;
export type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
export type HasTypeArguments = CallExpression | NewExpression | TaggedTemplateExpression | JsxOpeningElement | JsxSelfClosingElement;
export type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
export type HasExpressionInitializer = VariableDeclaration | ParameterDeclaration | BindingElement | PropertySignature | PropertyDeclaration | PropertyAssignment | EnumMember;
export interface NodeArray<T extends Node> extends ReadonlyArray<T>, TextRange {
Expand Down
22 changes: 22 additions & 0 deletions tests/cases/fourslash/completionsConditionalMember.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/// <reference path="fourslash.ts" />

////declare function f<T extends string>(
//// p: { a: T extends 'foo' ? { x: string } : { y: string } }
////): void;
////
////f<'foo'>({ a: { /*1*/ } });
////f<string>({ a: { /*2*/ } });
Copy link
Member

Choose a reason for hiding this comment

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

Why no tests without type arguments? The new code seems to apply exclusively in this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

completionsGenericIndexedAccess2 is without type arguments, which tests the primary way that #33937 was a regression. Almost all the existing tests from #33937 / #32100 are without type arguments, but they weren’t complex enough to fail. This test was to ensure that the new clever stuff doesn’t apply when there is an explicit type argument.


verify.completions({
marker: '1',
exact: [{
name: 'x'
}]
});

verify.completions({
marker: '2',
exact: [{
name: 'y'
}]
});
18 changes: 18 additions & 0 deletions tests/cases/fourslash/completionsGenericIndexedAccess1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path="fourslash.ts" />

// #34825

////interface Sample {
//// addBook: { name: string, year: number }
////}
////
////export declare function testIt<T>(method: T[keyof T]): any
////testIt<Sample>({ /**/ });

verify.completions({
marker: '',
exact: [
{ name: 'name' },
{ name: 'year' },
]
});
34 changes: 34 additions & 0 deletions tests/cases/fourslash/completionsGenericIndexedAccess2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/// <reference path="fourslash.ts" />

// #34825

////export type GetMethodsForType<T, G extends string> = { [K in keyof T]:
//// T[K] extends () => any ? { name: K, group: G, } : T[K] extends (s: infer U) => any ? { name: K, group: G, payload: U } : never }[keyof T];
////
////
////class Sample {
//// count = 0;
//// books: { name: string, year: number }[] = []
//// increment() {
//// this.count++
//// this.count++
//// }
////
//// addBook(book: Sample["books"][0]) {
//// this.books.push(book)
//// }
////}
////export declare function testIt<T, G extends string>(): (input: any, method: GetMethodsForType<T, G>) => any
////
////
////const t = testIt<Sample, "Sample">()
////
////const i = t(null, { name: "addBook", group: "Sample", payload: { /**/ } })

verify.completions({
marker: '',
exact: [
{ name: 'name' },
{ name: 'year' },
]
});