Skip to content

Commit 659dc03

Browse files
author
Andy
authored
completions: isNewIdentifierLocation = false for string literal where all legal values are known (microsoft#22933)
1 parent 6118f21 commit 659dc03

13 files changed

Lines changed: 42 additions & 52 deletions

‎src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,10 @@ namespace ts.codefix {
4848

4949
const classType = checker.getTypeAtLocation(classDeclaration);
5050

51-
if (!checker.getIndexTypeOfType(classType, IndexKind.Number)) {
51+
if (!classType.getNumberIndexType()) {
5252
createMissingIndexSignatureDeclaration(implementedType, IndexKind.Number);
5353
}
54-
if (!checker.getIndexTypeOfType(classType, IndexKind.String)) {
54+
if (!classType.getStringIndexType()) {
5555
createMissingIndexSignatureDeclaration(implementedType, IndexKind.String);
5656
}
5757

‎src/services/completions.ts‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,11 @@ namespace ts.Completions {
8686
case StringLiteralCompletionKind.Properties: {
8787
const entries: CompletionEntry[] = [];
8888
getCompletionEntriesFromSymbols(completion.symbols, entries, sourceFile, sourceFile, checker, ScriptTarget.ESNext, log, CompletionKind.String, preferences); // Target will not be used, so arbitrary
89-
return { isGlobalCompletion: false, isMemberCompletion: true, isNewIdentifierLocation: true, entries };
89+
return { isGlobalCompletion: false, isMemberCompletion: true, isNewIdentifierLocation: completion.hasIndexSignature, entries };
9090
}
9191
case StringLiteralCompletionKind.Types: {
9292
const entries = completion.types.map(type => ({ name: type.value, kindModifiers: ScriptElementKindModifier.none, kind: ScriptElementKind.typeElement, sortText: "0" }));
93-
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: true, entries };
93+
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries };
9494
}
9595
default:
9696
return Debug.assertNever(completion);
@@ -360,9 +360,14 @@ namespace ts.Completions {
360360
}
361361

362362
const enum StringLiteralCompletionKind { Paths, Properties, Types }
363+
interface StringLiteralCompletionsFromProperties {
364+
readonly kind: StringLiteralCompletionKind.Properties;
365+
readonly symbols: ReadonlyArray<Symbol>;
366+
readonly hasIndexSignature: boolean;
367+
}
363368
type StringLiteralCompletion =
364369
| { readonly kind: StringLiteralCompletionKind.Paths, readonly paths: ReadonlyArray<PathCompletions.PathCompletion> }
365-
| { readonly kind: StringLiteralCompletionKind.Properties, readonly symbols: ReadonlyArray<Symbol> }
370+
| StringLiteralCompletionsFromProperties
366371
| { readonly kind: StringLiteralCompletionKind.Types, readonly types: ReadonlyArray<StringLiteralType> };
367372
function getStringLiteralCompletionEntries(sourceFile: SourceFile, node: StringLiteralLike, position: number, typeChecker: TypeChecker, compilerOptions: CompilerOptions, host: LanguageServiceHost): StringLiteralCompletion | undefined {
368373
switch (node.parent.kind) {
@@ -377,7 +382,7 @@ namespace ts.Completions {
377382
// bar: string;
378383
// }
379384
// let x: Foo["/*completion position*/"]
380-
return { kind: StringLiteralCompletionKind.Properties, symbols: typeChecker.getTypeFromTypeNode((node.parent.parent as IndexedAccessTypeNode).objectType).getApparentProperties() };
385+
return stringLiteralCompletionsFromProperties(typeChecker.getTypeFromTypeNode((node.parent.parent as IndexedAccessTypeNode).objectType));
381386
default:
382387
return undefined;
383388
}
@@ -396,8 +401,7 @@ namespace ts.Completions {
396401
// foo({
397402
// '/*completion position*/'
398403
// });
399-
const type = typeChecker.getContextualType(node.parent.parent);
400-
return { kind: StringLiteralCompletionKind.Properties, symbols: type && type.getApparentProperties() };
404+
return stringLiteralCompletionsFromProperties(typeChecker.getContextualType(node.parent.parent));
401405
}
402406
return fromContextualType();
403407

@@ -410,7 +414,7 @@ namespace ts.Completions {
410414
// }
411415
// let a: A;
412416
// a['/*completion position*/']
413-
return { kind: StringLiteralCompletionKind.Properties, symbols: typeChecker.getTypeAtLocation(expression).getApparentProperties() };
417+
return stringLiteralCompletionsFromProperties(typeChecker.getTypeAtLocation(expression));
414418
}
415419
return undefined;
416420
}
@@ -454,6 +458,10 @@ namespace ts.Completions {
454458
}
455459
}
456460

461+
function stringLiteralCompletionsFromProperties(type: Type | undefined): StringLiteralCompletionsFromProperties | undefined {
462+
return type && { kind: StringLiteralCompletionKind.Properties, symbols: type.getApparentProperties(), hasIndexSignature: hasIndexSignature(type) };
463+
}
464+
457465
function getStringLiteralTypes(type: Type, typeChecker: TypeChecker, uniques = createMap<true>()): ReadonlyArray<StringLiteralType> {
458466
if (type && type.flags & TypeFlags.TypeParameter) {
459467
type = type.getConstraint();
@@ -1051,7 +1059,7 @@ namespace ts.Completions {
10511059
}
10521060

10531061
function addTypeProperties(type: Type): void {
1054-
isNewIdentifierLocation = !!type.getStringIndexType() || !!type.getNumberIndexType();
1062+
isNewIdentifierLocation = hasIndexSignature(type);
10551063

10561064
if (isSourceFileJavaScript(sourceFile)) {
10571065
// In javascript files, for union types, we don't just get the members that
@@ -1454,11 +1462,9 @@ namespace ts.Completions {
14541462
let existingMembers: ReadonlyArray<Declaration>;
14551463

14561464
if (objectLikeContainer.kind === SyntaxKind.ObjectLiteralExpression) {
1457-
// We are completing on contextual types, but may also include properties
1458-
// other than those within the declared type.
1459-
isNewIdentifierLocation = true;
14601465
const typeForObject = typeChecker.getContextualType(objectLikeContainer);
14611466
if (!typeForObject) return GlobalsSearch.Fail;
1467+
isNewIdentifierLocation = hasIndexSignature(typeForObject);
14621468
typeMembers = getPropertiesForCompletion(typeForObject, typeChecker, /*isForAccess*/ false);
14631469
existingMembers = objectLikeContainer.properties;
14641470
}
@@ -2247,4 +2253,8 @@ namespace ts.Completions {
22472253
function isFromObjectTypeDeclaration(node: Node): boolean {
22482254
return node.parent && (isClassElement(node.parent) || isTypeElement(node.parent)) && isObjectTypeDeclaration(node.parent.parent);
22492255
}
2256+
2257+
function hasIndexSignature(type: Type): boolean {
2258+
return !!type.getStringIndexType() || !!type.getNumberIndexType();
2259+
}
22502260
}

‎tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment1.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@
1313
//// '/*1*/': ''
1414
//// }
1515

16-
verify.completionsAt("0", ["jspm", '"jspm:browser"', '"jspm:dev"', '"jspm:node"'], { isNewIdentifierLocation: true });
17-
verify.completionsAt("1", ["jspm", "jspm:browser", "jspm:dev", "jspm:node"], { isNewIdentifierLocation: true });
16+
verify.completionsAt("0", ["jspm", '"jspm:browser"', '"jspm:dev"', '"jspm:node"']);
17+
verify.completionsAt("1", ["jspm", "jspm:browser", "jspm:dev", "jspm:node"]);

‎tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment2.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,5 @@
1919
//// }
2020
//// }
2121

22-
verify.completionsAt("0", ["jspm", '"jspm:browser"', '"jspm:dev"', '"jspm:node"'], { isNewIdentifierLocation: true });
23-
verify.completionsAt("1", ["jspm", "jspm:browser", "jspm:dev", "jspm:node"], { isNewIdentifierLocation: true });
22+
verify.completionsAt("0", ["jspm", '"jspm:browser"', '"jspm:dev"', '"jspm:node"']);
23+
verify.completionsAt("1", ["jspm", "jspm:browser", "jspm:dev", "jspm:node"]);

‎tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment3.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@
1515
//// '/*1*/': ""
1616
//// }
1717

18-
verify.completionsAt("0", ["jspm", '"jspm:browser"'], { isNewIdentifierLocation: true });
19-
verify.completionsAt("1", ["jspm", "jspm:browser"], { isNewIdentifierLocation: true });
18+
verify.completionsAt("0", ["jspm", '"jspm:browser"']);
19+
verify.completionsAt("1", ["jspm", "jspm:browser"]);

‎tests/cases/fourslash/completionForStringLiteral2.ts‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,11 @@
55
//// bar: 0,
66
//// "some other name": 1
77
////};
8+
////declare const p: { [s: string]: any, a: number };
89
////
910
////o["/*1*/bar"];
10-
////o["/*2*/
11+
////o["/*2*/ ;
12+
////p["/*3*/"];
1113

12-
goTo.marker('1');
13-
verify.completionListContains("foo");
14-
verify.completionListAllowsNewIdentifier();
15-
verify.completionListCount(3);
16-
17-
goTo.marker('2');
18-
verify.completionListContains("some other name");
19-
verify.completionListAllowsNewIdentifier();
20-
verify.completionListCount(3);
14+
verify.completionsAt(["1", "2"], ["foo", "bar", "some other name"]);
15+
verify.completionsAt("3", ["a"], { isNewIdentifierLocation: true });

‎tests/cases/fourslash/completionForStringLiteral3.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,4 @@
99
////
1010
////f("/*2*/
1111

12-
goTo.marker('1');
13-
verify.completionListContains("A");
14-
verify.completionListAllowsNewIdentifier();
15-
verify.completionListCount(3);
16-
17-
goTo.marker('2');
18-
verify.completionListContains("A");
19-
verify.completionListAllowsNewIdentifier();
20-
verify.completionListCount(3);
12+
verify.completionsAt(["1", "2"], ["A", "B", "C"]);

‎tests/cases/fourslash/completionForStringLiteral7.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,5 @@
55
////function f(x: T, ...args: U[]) { };
66
////f("/*1*/", "/*2*/", "/*3*/");
77

8-
// TODO: GH#22907
9-
const options = { isNewIdentifierLocation: true };
10-
verify.completionsAt("1", ["foo", "bar"], options);
11-
verify.completionsAt("2", ["oof", "rab"], options);
12-
verify.completionsAt("3", ["oof", "rab"], options);
8+
verify.completionsAt("1", ["foo", "bar"]);
9+
verify.completionsAt(["2", "3"], ["oof", "rab"]);

‎tests/cases/fourslash/completionListInvalidMemberNames.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@
1414
////x[|./*a*/|];
1515
////x["/*b*/"];
1616

17-
// TODO: GH#22907
18-
verify.completionsAt("b", ["foo ", "bar", "break", "any", "#", "$", "b", "1b"], { isNewIdentifierLocation: true });
17+
verify.completionsAt("b", ["foo ", "bar", "break", "any", "#", "$", "b", "1b"]);
1918

2019
const replacementSpan = test.ranges()[0];
2120
verify.completionsAt("a", [

‎tests/cases/fourslash/completionsJsPropertyAssignment.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,4 @@
77
////const x = { p: "x" };
88
////x.p = "/**/";
99

10-
// TODO: GH#22907
11-
verify.completionsAt("", ["x", "y"], { isNewIdentifierLocation: true });
10+
verify.completionsAt("", ["x", "y"]);

0 commit comments

Comments
 (0)