Skip to content

Retry querying string completions from the inferred type if the original completions request doesn't return anything #52875

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
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
16 changes: 12 additions & 4 deletions src/services/stringCompletions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ function getStringLiteralCompletionEntries(sourceFile: SourceFile, node: StringL
// });
return stringLiteralCompletionsForObjectLiteral(typeChecker, parent.parent);
}
return fromContextualType();
return fromContextualType() || fromContextualType(ContextFlags.None);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's unfortunate that this involves querying the contextual type twice (with different flags). However, it's super hard to distinguish the cases in which the completions should be returned from the constraint (that can be returned from the first requests since it blocks the inference) or if they should be returned based on the inferred type (second request without the inference block).

I assume here that this second request is somewhat cheap since it should often just look through the existing~ contextual type. Perhaps there is a way to limit situations in which this second request is even done. I wasn't sure what would be the best condition for that though. Essentially, I only want to do this request when the node is within a potential inference context (nested in a call-like).


case SyntaxKind.ElementAccessExpression: {
const { expression, argumentExpression } = parent as ElementAccessExpression;
Expand Down Expand Up @@ -432,16 +432,24 @@ function getStringLiteralCompletionEntries(sourceFile: SourceFile, node: StringL
return { kind: StringLiteralCompletionKind.Paths, paths: getStringLiteralCompletionsFromModuleNames(sourceFile, node, compilerOptions, host, typeChecker, preferences) };
case SyntaxKind.CaseClause:
const tracker = newCaseClauseTracker(typeChecker, (parent as CaseClause).parent.clauses);
const literals = fromContextualType().types.filter(literal => !tracker.hasValue(literal.value));
const contextualTypes = fromContextualType();
if (!contextualTypes) {
return;
}
const literals = contextualTypes.types.filter(literal => !tracker.hasValue(literal.value));
return { kind: StringLiteralCompletionKind.Types, types: literals, isNewIdentifier: false };
default:
return fromContextualType();
}

function fromContextualType(): StringLiteralCompletionsFromTypes {
function fromContextualType(contextFlags: ContextFlags = ContextFlags.Completions): StringLiteralCompletionsFromTypes | undefined {
// Get completion for string literal from string literal type
// i.e. var x: "hi" | "hello" = "/*completion position*/"
return { kind: StringLiteralCompletionKind.Types, types: getStringLiteralTypes(getContextualTypeFromParent(node, typeChecker, ContextFlags.Completions)), isNewIdentifier: false };
const types = getStringLiteralTypes(getContextualTypeFromParent(node, typeChecker, contextFlags));
if (!types.length) {
return;
}
return { kind: StringLiteralCompletionKind.Types, types, isNewIdentifier: false };
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/completionPreferredSuggestions1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@
verify.completions({ marker: "1", includes: ["a", "b", "c"] });
verify.completions({ marker: "2", includes: ["0", "1", "2"], isNewIdentifierLocation: true });
verify.completions({ marker: "3", includes: ["a", "b", "c"] });
verify.completions({ marker: "4", excludes: ["a", "b", "c"] });
verify.completions({ marker: "4", exact: [] });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This case wasn't actually returning any meaningful completions, see the playground here. It's just that empty actualCompletions were received here but with the current change in this PR I just don't return any completions here. I strongly believe that both are OK, it's just that excludes in the verify.completions call imply that there should be some completions (even if empty). It's purely a test setup/helper logic and not something that affects conumers.

verify.completions({ marker: "5", includes: ["a", "b", "c"] });
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/// <reference path="fourslash.ts" />

// @Filename: /a.tsx
//// declare function test<T>(a: {
//// [K in keyof T]: {
//// b?: keyof T;
//// };
//// }): void;
////
//// test({
//// foo: {},
//// bar: {
//// b: "/*ts*/",
//// },
//// });

verify.completions({ marker: ["ts"], exact: ["foo", "bar"] });
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/// <reference path="fourslash.ts" />

// @Filename: /a.tsx
//// type Values<T> = T[keyof T];
////
//// type GetStates<T> = T extends { states: object } ? T["states"] : never;
////
//// type IsNever<T> = [T] extends [never] ? 1 : 0;
////
//// type GetIds<T, Gathered extends string = never> = IsNever<T> extends 1
//// ? Gathered
//// : "id" extends keyof T
//// ? GetIds<Values<GetStates<T>>, Gathered | `#${T["id"] & string}`>
//// : GetIds<Values<GetStates<T>>, Gathered>;
////
//// type StateConfig<
//// TStates extends Record<string, StateConfig> = Record<
//// string,
//// StateConfig<any>
//// >,
//// TIds extends string = string
//// > = {
//// id?: string;
//// initial?: keyof TStates & string;
//// states?: {
//// [K in keyof TStates]: StateConfig<GetStates<TStates[K]>, TIds>;
//// };
//// on?: Record<string, TIds | `.${keyof TStates & string}`>;
//// };
////
//// declare function createMachine<const T extends StateConfig<GetStates<T>, GetIds<T>>>(
//// config: T
//// ): void;
////
//// createMachine({
//// initial: "child",
//// states: {
//// child: {
//// initial: "foo",
//// states: {
//// foo: {
//// id: "wow_deep_id",
//// },
//// },
//// },
//// },
//// on: {
//// EV: "/*ts*/",
//// },
//// });

verify.completions({ marker: ["ts"], exact: ["#wow_deep_id", ".child"] });