From 659dc03f6869477e5422f32dd54f94bae39df1fc Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 27 Mar 2018 19:36:54 -0700 Subject: [PATCH] completions: isNewIdentifierLocation = false for string literal where all legal values are known (#22933) --- .../fixClassIncorrectlyImplementsInterface.ts | 4 +-- src/services/completions.ts | 32 ++++++++++++------- ...nForQuotedPropertyInPropertyAssignment1.ts | 4 +-- ...nForQuotedPropertyInPropertyAssignment2.ts | 4 +-- ...nForQuotedPropertyInPropertyAssignment3.ts | 4 +-- .../fourslash/completionForStringLiteral2.ts | 15 +++------ .../fourslash/completionForStringLiteral3.ts | 10 +----- .../fourslash/completionForStringLiteral7.ts | 7 ++-- .../completionListInvalidMemberNames.ts | 3 +- .../completionsJsPropertyAssignment.ts | 3 +- .../fourslash/completionsJsdocTypeTagCast.ts | 2 +- ...letionsStringLiteral_fromTypeConstraint.ts | 3 +- tests/cases/fourslash/completionsUnion.ts | 3 +- 13 files changed, 42 insertions(+), 52 deletions(-) diff --git a/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts b/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts index 695bbdda4b0..91815edb775 100644 --- a/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts +++ b/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts @@ -48,10 +48,10 @@ namespace ts.codefix { const classType = checker.getTypeAtLocation(classDeclaration); - if (!checker.getIndexTypeOfType(classType, IndexKind.Number)) { + if (!classType.getNumberIndexType()) { createMissingIndexSignatureDeclaration(implementedType, IndexKind.Number); } - if (!checker.getIndexTypeOfType(classType, IndexKind.String)) { + if (!classType.getStringIndexType()) { createMissingIndexSignatureDeclaration(implementedType, IndexKind.String); } diff --git a/src/services/completions.ts b/src/services/completions.ts index c0f11d516b9..ebc3674129e 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -86,11 +86,11 @@ namespace ts.Completions { case StringLiteralCompletionKind.Properties: { const entries: CompletionEntry[] = []; getCompletionEntriesFromSymbols(completion.symbols, entries, sourceFile, sourceFile, checker, ScriptTarget.ESNext, log, CompletionKind.String, preferences); // Target will not be used, so arbitrary - return { isGlobalCompletion: false, isMemberCompletion: true, isNewIdentifierLocation: true, entries }; + return { isGlobalCompletion: false, isMemberCompletion: true, isNewIdentifierLocation: completion.hasIndexSignature, entries }; } case StringLiteralCompletionKind.Types: { const entries = completion.types.map(type => ({ name: type.value, kindModifiers: ScriptElementKindModifier.none, kind: ScriptElementKind.typeElement, sortText: "0" })); - return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: true, entries }; + return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries }; } default: return Debug.assertNever(completion); @@ -360,9 +360,14 @@ namespace ts.Completions { } const enum StringLiteralCompletionKind { Paths, Properties, Types } + interface StringLiteralCompletionsFromProperties { + readonly kind: StringLiteralCompletionKind.Properties; + readonly symbols: ReadonlyArray; + readonly hasIndexSignature: boolean; + } type StringLiteralCompletion = | { readonly kind: StringLiteralCompletionKind.Paths, readonly paths: ReadonlyArray } - | { readonly kind: StringLiteralCompletionKind.Properties, readonly symbols: ReadonlyArray } + | StringLiteralCompletionsFromProperties | { readonly kind: StringLiteralCompletionKind.Types, readonly types: ReadonlyArray }; function getStringLiteralCompletionEntries(sourceFile: SourceFile, node: StringLiteralLike, position: number, typeChecker: TypeChecker, compilerOptions: CompilerOptions, host: LanguageServiceHost): StringLiteralCompletion | undefined { switch (node.parent.kind) { @@ -377,7 +382,7 @@ namespace ts.Completions { // bar: string; // } // let x: Foo["/*completion position*/"] - return { kind: StringLiteralCompletionKind.Properties, symbols: typeChecker.getTypeFromTypeNode((node.parent.parent as IndexedAccessTypeNode).objectType).getApparentProperties() }; + return stringLiteralCompletionsFromProperties(typeChecker.getTypeFromTypeNode((node.parent.parent as IndexedAccessTypeNode).objectType)); default: return undefined; } @@ -396,8 +401,7 @@ namespace ts.Completions { // foo({ // '/*completion position*/' // }); - const type = typeChecker.getContextualType(node.parent.parent); - return { kind: StringLiteralCompletionKind.Properties, symbols: type && type.getApparentProperties() }; + return stringLiteralCompletionsFromProperties(typeChecker.getContextualType(node.parent.parent)); } return fromContextualType(); @@ -410,7 +414,7 @@ namespace ts.Completions { // } // let a: A; // a['/*completion position*/'] - return { kind: StringLiteralCompletionKind.Properties, symbols: typeChecker.getTypeAtLocation(expression).getApparentProperties() }; + return stringLiteralCompletionsFromProperties(typeChecker.getTypeAtLocation(expression)); } return undefined; } @@ -454,6 +458,10 @@ namespace ts.Completions { } } + function stringLiteralCompletionsFromProperties(type: Type | undefined): StringLiteralCompletionsFromProperties | undefined { + return type && { kind: StringLiteralCompletionKind.Properties, symbols: type.getApparentProperties(), hasIndexSignature: hasIndexSignature(type) }; + } + function getStringLiteralTypes(type: Type, typeChecker: TypeChecker, uniques = createMap()): ReadonlyArray { if (type && type.flags & TypeFlags.TypeParameter) { type = type.getConstraint(); @@ -1051,7 +1059,7 @@ namespace ts.Completions { } function addTypeProperties(type: Type): void { - isNewIdentifierLocation = !!type.getStringIndexType() || !!type.getNumberIndexType(); + isNewIdentifierLocation = hasIndexSignature(type); if (isSourceFileJavaScript(sourceFile)) { // In javascript files, for union types, we don't just get the members that @@ -1454,11 +1462,9 @@ namespace ts.Completions { let existingMembers: ReadonlyArray; if (objectLikeContainer.kind === SyntaxKind.ObjectLiteralExpression) { - // We are completing on contextual types, but may also include properties - // other than those within the declared type. - isNewIdentifierLocation = true; const typeForObject = typeChecker.getContextualType(objectLikeContainer); if (!typeForObject) return GlobalsSearch.Fail; + isNewIdentifierLocation = hasIndexSignature(typeForObject); typeMembers = getPropertiesForCompletion(typeForObject, typeChecker, /*isForAccess*/ false); existingMembers = objectLikeContainer.properties; } @@ -2247,4 +2253,8 @@ namespace ts.Completions { function isFromObjectTypeDeclaration(node: Node): boolean { return node.parent && (isClassElement(node.parent) || isTypeElement(node.parent)) && isObjectTypeDeclaration(node.parent.parent); } + + function hasIndexSignature(type: Type): boolean { + return !!type.getStringIndexType() || !!type.getNumberIndexType(); + } } diff --git a/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment1.ts b/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment1.ts index ab218cea93d..1b1cbe9a4df 100644 --- a/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment1.ts +++ b/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment1.ts @@ -13,5 +13,5 @@ //// '/*1*/': '' //// } -verify.completionsAt("0", ["jspm", '"jspm:browser"', '"jspm:dev"', '"jspm:node"'], { isNewIdentifierLocation: true }); -verify.completionsAt("1", ["jspm", "jspm:browser", "jspm:dev", "jspm:node"], { isNewIdentifierLocation: true }); +verify.completionsAt("0", ["jspm", '"jspm:browser"', '"jspm:dev"', '"jspm:node"']); +verify.completionsAt("1", ["jspm", "jspm:browser", "jspm:dev", "jspm:node"]); diff --git a/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment2.ts b/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment2.ts index 66ba4ada241..669e58bdac1 100644 --- a/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment2.ts +++ b/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment2.ts @@ -19,5 +19,5 @@ //// } //// } -verify.completionsAt("0", ["jspm", '"jspm:browser"', '"jspm:dev"', '"jspm:node"'], { isNewIdentifierLocation: true }); -verify.completionsAt("1", ["jspm", "jspm:browser", "jspm:dev", "jspm:node"], { isNewIdentifierLocation: true }); +verify.completionsAt("0", ["jspm", '"jspm:browser"', '"jspm:dev"', '"jspm:node"']); +verify.completionsAt("1", ["jspm", "jspm:browser", "jspm:dev", "jspm:node"]); diff --git a/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment3.ts b/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment3.ts index 1ab9aa4a8cf..7878750c6fa 100644 --- a/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment3.ts +++ b/tests/cases/fourslash/completionForQuotedPropertyInPropertyAssignment3.ts @@ -15,5 +15,5 @@ //// '/*1*/': "" //// } -verify.completionsAt("0", ["jspm", '"jspm:browser"'], { isNewIdentifierLocation: true }); -verify.completionsAt("1", ["jspm", "jspm:browser"], { isNewIdentifierLocation: true }); +verify.completionsAt("0", ["jspm", '"jspm:browser"']); +verify.completionsAt("1", ["jspm", "jspm:browser"]); diff --git a/tests/cases/fourslash/completionForStringLiteral2.ts b/tests/cases/fourslash/completionForStringLiteral2.ts index 9b00208c729..2bc783f7e53 100644 --- a/tests/cases/fourslash/completionForStringLiteral2.ts +++ b/tests/cases/fourslash/completionForStringLiteral2.ts @@ -5,16 +5,11 @@ //// bar: 0, //// "some other name": 1 ////}; +////declare const p: { [s: string]: any, a: number }; //// ////o["/*1*/bar"]; -////o["/*2*/ +////o["/*2*/ ; +////p["/*3*/"]; -goTo.marker('1'); -verify.completionListContains("foo"); -verify.completionListAllowsNewIdentifier(); -verify.completionListCount(3); - -goTo.marker('2'); -verify.completionListContains("some other name"); -verify.completionListAllowsNewIdentifier(); -verify.completionListCount(3); +verify.completionsAt(["1", "2"], ["foo", "bar", "some other name"]); +verify.completionsAt("3", ["a"], { isNewIdentifierLocation: true }); diff --git a/tests/cases/fourslash/completionForStringLiteral3.ts b/tests/cases/fourslash/completionForStringLiteral3.ts index 238ba74b4d8..7b4713a81a7 100644 --- a/tests/cases/fourslash/completionForStringLiteral3.ts +++ b/tests/cases/fourslash/completionForStringLiteral3.ts @@ -9,12 +9,4 @@ //// ////f("/*2*/ -goTo.marker('1'); -verify.completionListContains("A"); -verify.completionListAllowsNewIdentifier(); -verify.completionListCount(3); - -goTo.marker('2'); -verify.completionListContains("A"); -verify.completionListAllowsNewIdentifier(); -verify.completionListCount(3); +verify.completionsAt(["1", "2"], ["A", "B", "C"]); diff --git a/tests/cases/fourslash/completionForStringLiteral7.ts b/tests/cases/fourslash/completionForStringLiteral7.ts index 6b7bead2a66..23a12f9ea4b 100644 --- a/tests/cases/fourslash/completionForStringLiteral7.ts +++ b/tests/cases/fourslash/completionForStringLiteral7.ts @@ -5,8 +5,5 @@ ////function f(x: T, ...args: U[]) { }; ////f("/*1*/", "/*2*/", "/*3*/"); -// TODO: GH#22907 -const options = { isNewIdentifierLocation: true }; -verify.completionsAt("1", ["foo", "bar"], options); -verify.completionsAt("2", ["oof", "rab"], options); -verify.completionsAt("3", ["oof", "rab"], options); +verify.completionsAt("1", ["foo", "bar"]); +verify.completionsAt(["2", "3"], ["oof", "rab"]); diff --git a/tests/cases/fourslash/completionListInvalidMemberNames.ts b/tests/cases/fourslash/completionListInvalidMemberNames.ts index 291ea945b29..3dc54274e83 100644 --- a/tests/cases/fourslash/completionListInvalidMemberNames.ts +++ b/tests/cases/fourslash/completionListInvalidMemberNames.ts @@ -14,8 +14,7 @@ ////x[|./*a*/|]; ////x["/*b*/"]; -// TODO: GH#22907 -verify.completionsAt("b", ["foo ", "bar", "break", "any", "#", "$", "b", "1b"], { isNewIdentifierLocation: true }); +verify.completionsAt("b", ["foo ", "bar", "break", "any", "#", "$", "b", "1b"]); const replacementSpan = test.ranges()[0]; verify.completionsAt("a", [ diff --git a/tests/cases/fourslash/completionsJsPropertyAssignment.ts b/tests/cases/fourslash/completionsJsPropertyAssignment.ts index fa758188ea9..c7e8a9ad268 100644 --- a/tests/cases/fourslash/completionsJsPropertyAssignment.ts +++ b/tests/cases/fourslash/completionsJsPropertyAssignment.ts @@ -7,5 +7,4 @@ ////const x = { p: "x" }; ////x.p = "/**/"; -// TODO: GH#22907 -verify.completionsAt("", ["x", "y"], { isNewIdentifierLocation: true }); +verify.completionsAt("", ["x", "y"]); diff --git a/tests/cases/fourslash/completionsJsdocTypeTagCast.ts b/tests/cases/fourslash/completionsJsdocTypeTagCast.ts index 8cd78103e34..822069feb53 100644 --- a/tests/cases/fourslash/completionsJsdocTypeTagCast.ts +++ b/tests/cases/fourslash/completionsJsdocTypeTagCast.ts @@ -4,4 +4,4 @@ // @Filename: /a.js ////const x = /** @type {{ s: string }} */ ({ /**/ }); -verify.completionsAt("", ["s", "x"], { isNewIdentifierLocation: true }); +verify.completionsAt("", ["s", "x"]); diff --git a/tests/cases/fourslash/completionsStringLiteral_fromTypeConstraint.ts b/tests/cases/fourslash/completionsStringLiteral_fromTypeConstraint.ts index 18941eb35ea..cb3f1a31fe2 100644 --- a/tests/cases/fourslash/completionsStringLiteral_fromTypeConstraint.ts +++ b/tests/cases/fourslash/completionsStringLiteral_fromTypeConstraint.ts @@ -3,5 +3,4 @@ ////interface Foo { foo: string; bar: string; } ////type T = Pick; -// TODO: GH#22907 -verify.completionsAt("", ["foo", "bar"], { isNewIdentifierLocation: true }); +verify.completionsAt("", ["foo", "bar"]); diff --git a/tests/cases/fourslash/completionsUnion.ts b/tests/cases/fourslash/completionsUnion.ts index 4ee701dddbe..d9d1d3c792f 100644 --- a/tests/cases/fourslash/completionsUnion.ts +++ b/tests/cases/fourslash/completionsUnion.ts @@ -7,5 +7,4 @@ // We specifically filter out any array-like types. // Private members will be excluded by `createUnionOrIntersectionProperty`. -// TODO: GH#22907 -verify.completionsAt("", ["x"], { isNewIdentifierLocation: true }); +verify.completionsAt("", ["x"]);