Microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2025-11-18 17:21:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix modifier order for class member completions (#48066)

Browse Source 
* fix modifier order & tests

* remove empty replacement span from tests
This commit is contained in:
Gabriela Araujo Britto
2022-03-01 10:05:24 -08:00
committed by GitHub
parent e4fe50cca4
commit e64f04bd8c
14 changed files with 95 additions and 201 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/services/completions.ts
+16 -15
View File
@@ -768,7 +768,7 @@ namespace ts.Completions {
completionKind === CompletionKind.MemberLike &&
isClassLikeMemberCompletion(symbol, location)) {
let importAdder;
({ insertText, isSnippet, importAdder } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, contextToken, formatContext));
({ insertText, isSnippet, importAdder, replacementSpan } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, contextToken, formatContext));
if (importAdder?.hasFixes()) {
hasAction = true;
source = CompletionSource.ClassMemberSnippet;
@@ -894,13 +894,14 @@ namespace ts.Completions {
location: Node,
contextToken: Node | undefined,
formatContext: formatting.FormatContext | undefined,
): { insertText: string, isSnippet?: true, importAdder?: codefix.ImportAdder } {
): { insertText: string, isSnippet?: true, importAdder?: codefix.ImportAdder, replacementSpan?: TextSpan } {
const classLikeDeclaration = findAncestor(location, isClassLike);
if (!classLikeDeclaration) {
return { insertText: name };
}
let isSnippet: true | undefined;
let replacementSpan: TextSpan | undefined;
let insertText: string = name;
const checker = program.getTypeChecker();
@@ -932,10 +933,8 @@ namespace ts.Completions {
let modifiers = ModifierFlags.None;
// Whether the suggested member should be abstract.
// e.g. in `abstract class C { abstract | }`, we should offer abstract method signatures at position `|`.
// Note: We are relying on checking if the context token is `abstract`,
// since other visibility modifiers (e.g. `protected`) should come *before* `abstract`.
// However, that is not true for the e.g. `override` modifier, so this check has its limitations.
const isAbstract = contextToken && isModifierLike(contextToken) === SyntaxKind.AbstractKeyword;
const { modifiers: presentModifiers, span: modifiersSpan } = getPresentModifiers(contextToken);
const isAbstract = !!(presentModifiers & ModifierFlags.Abstract);
const completionNodes: Node[] = [];
codefix.addNewNodeForMemberSymbol(
symbol,
@@ -961,19 +960,14 @@ namespace ts.Completions {
requiredModifiers |= ModifierFlags.Override;
}
let presentModifiers = ModifierFlags.None;
if (!completionNodes.length) {
// Omit already present modifiers from the first completion node/signature.
if (contextToken) {
presentModifiers = getPresentModifiers(contextToken);
}
// Keep track of added missing required modifiers and modifiers already present.
// This is needed when we have overloaded signatures,
// so this callback will be called for multiple nodes/signatures,
// and we need to make sure the modifiers are uniform for all nodes/signatures.
modifiers = node.modifierFlagsCache | requiredModifiers | presentModifiers;
}
node = factory.updateModifiers(node, modifiers & (~presentModifiers));
node = factory.updateModifiers(node, modifiers);
completionNodes.push(node);
},
body,
@@ -981,6 +975,7 @@ namespace ts.Completions {
isAbstract);
if (completionNodes.length) {
replacementSpan = modifiersSpan;
// If we have access to formatting settings, we print the nodes using the emitter,
// and then format the printed text.
if (formatContext) {
@@ -1015,11 +1010,15 @@ namespace ts.Completions {
}
}
return { insertText, isSnippet, importAdder };
return { insertText, isSnippet, importAdder, replacementSpan };
}
function getPresentModifiers(contextToken: Node): ModifierFlags {
function getPresentModifiers(contextToken: Node | undefined): { modifiers: ModifierFlags, span?: TextSpan } {
if (!contextToken) {
return { modifiers: ModifierFlags.None };
}
let modifiers = ModifierFlags.None;
let span;
let contextMod;
/*
Cases supported:
@@ -1042,11 +1041,13 @@ namespace ts.Completions {
*/
if (contextMod = isModifierLike(contextToken)) {
modifiers |= modifierToFlag(contextMod);
span = createTextSpanFromNode(contextToken);
}
if (isPropertyDeclaration(contextToken.parent)) {
modifiers |= modifiersToFlags(contextToken.parent.modifiers);
span = createTextSpanFromNode(contextToken.parent);
}
return modifiers;
return { modifiers, span };
}
function isModifierLike(node: Node): ModifierSyntaxKind | undefined {
tests/cases/fourslash/completionsOverridingMethod.ts
+3 -52
View File
@@ -93,7 +93,7 @@
////
////class HSub extends HBase {
//// /*h1*/
//// static /*h2*/
//// [|static|] /*h2*/
////}
// @Filename: i.ts
@@ -124,11 +124,6 @@ verify.completions({
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "foo(param1: string, param2: boolean): Promise<void> {\n}",
}
],
@@ -146,11 +141,6 @@ verify.completions({
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "foo(a: string, b: string): string {\n}",
}
],
@@ -168,11 +158,6 @@ verify.completions({
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "foo(a: string): string {\n}",
}
],
@@ -190,11 +175,6 @@ verify.completions({
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "foo(a: string): string {\n}",
}
],
@@ -212,11 +192,6 @@ verify.completions({
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "foo(a: string): string {\n}",
}
],
@@ -234,11 +209,6 @@ verify.completions({
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "foo(a: string): string {\n}",
}
],
@@ -256,11 +226,6 @@ verify.completions({
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText:
`foo(a: string): string;
foo(a: undefined, b: number): string;
@@ -292,12 +257,8 @@ verify.completions({
{
name: "met",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "met(n: number): number {\n}",
replacementSpan: test.ranges()[0],
insertText: "static met(n: number): number {\n}",
}
],
});
@@ -314,21 +275,11 @@ verify.completions({
{
name: "met",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "met<T>(t: T): T {\n}",
},
{
name: "metcons",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "metcons<T extends string | number>(t: T): T {\n}",
}
],
tests/cases/fourslash/completionsOverridingMethod1.ts
-5
View File
@@ -25,11 +25,6 @@ verify.completions({
{
name: "foo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "override foo(a: string): void {\n}",
}
],
tests/cases/fourslash/completionsOverridingMethod10.ts
-15
View File
@@ -26,21 +26,11 @@ verify.completions({
{
name: "a",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "a: string;",
},
{
name: "b",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText:
`b(a: string): void {
}`,
@@ -48,11 +38,6 @@ verify.completions({
{
name: "c",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText:
`c(a: string): string;
c(a: number): number;
tests/cases/fourslash/completionsOverridingMethod11.ts
-15
View File
@@ -32,21 +32,11 @@ verify.completions({
{
name: "a",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "a: string",
},
{
name: "b",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText:
`b(a: string): void {
}`,
@@ -54,11 +44,6 @@ verify.completions({
{
name: "c",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText:
`c(a: string): string
c(a: number): number
tests/cases/fourslash/completionsOverridingMethod12.ts
+54
View File
@@ -0,0 +1,54 @@
/// <reference path="fourslash.ts" />
// @Filename: a.ts
// @newline: LF
// Case: modifier order
////abstract class A {
//// public get P(): string {
//// return "";
//// }
////}
////
////abstract class B extends A {
//// [|abstract|] /*a*/
////}
////
////abstract class B1 extends A {
//// [|abstract override|] /*b*/
////}
verify.completions({
marker: "a",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "P",
sortText: completion.SortText.LocationPriority,
replacementSpan: test.ranges()[0],
insertText: "public abstract get P(): string;",
},
],
});
verify.completions({
marker: "b",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "P",
sortText: completion.SortText.LocationPriority,
replacementSpan: test.ranges()[1],
insertText: "public abstract override get P(): string;",
},
],
});
tests/cases/fourslash/completionsOverridingMethod2.ts
-5
View File
@@ -23,11 +23,6 @@ verify.completions({
{
name: "$usd",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
isSnippet: true,
insertText: "\"\\$usd\"(a: number): number {\n $0\n}",
}
tests/cases/fourslash/completionsOverridingMethod3.ts
-5
View File
@@ -24,11 +24,6 @@ verify.completions({
{
name: "boo",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "boo(): string;",
}
],
tests/cases/fourslash/completionsOverridingMethod4.ts
-10
View File
@@ -43,21 +43,11 @@ verify.completions({
{
name: "hint",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "protected hint(): string {\n}",
},
{
name: "refuse",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "public refuse(): string {\n}",
}
],
tests/cases/fourslash/completionsOverridingMethod5.ts
+10 -36
View File
@@ -12,8 +12,8 @@
////
////abstract class Abc extends Ab {
//// /*a*/
//// abstract /*b*/
//// abstract m/*c*/
//// [|abstract|] /*b*/
//// [|abstract m|]/*c*/
////}
@@ -29,21 +29,11 @@ verify.completions({
{
name: "met",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "met(n: string): void {\n}",
},
{
name: "met2",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "met2(n: number): void {\n}",
}
],
@@ -61,22 +51,14 @@ verify.completions({
{
name: "met",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "met(n: string): void;",
replacementSpan: test.ranges()[0],
insertText: "abstract met(n: string): void;",
},
{
name: "met2",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "met2(n: number): void;",
replacementSpan: test.ranges()[0],
insertText: "abstract met2(n: number): void;",
}
],
});
@@ -93,22 +75,14 @@ verify.completions({
{
name: "met",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "met(n: string): void;",
replacementSpan: test.ranges()[1],
insertText: "abstract met(n: string): void;",
},
{
name: "met2",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "met2(n: number): void;",
replacementSpan: test.ranges()[1],
insertText: "abstract met2(n: number): void;",
}
],
});
tests/cases/fourslash/completionsOverridingMethod6.ts
+9 -21
View File
@@ -10,11 +10,11 @@
////}
////
////abstract class B extends A {
//// public abstract /*b*/
//// [|public abstract|] /*b*/
////}
////
////class C extends A {
//// public override m/*a*/
//// [|public override m|]/*a*/
////}
////
////interface D {
@@ -23,7 +23,7 @@
////}
////
////class E implements D {
//// public f/*c*/
//// [|public f|]/*c*/
////}
verify.completions({
@@ -38,12 +38,8 @@ verify.completions({
{
name: "method",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "method(): number {\n}",
replacementSpan: test.ranges()[1],
insertText: "public override method(): number {\n}",
},
],
});
@@ -60,12 +56,8 @@ verify.completions({
{
name: "method",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "method(): number;",
replacementSpan: test.ranges()[0],
insertText: "public abstract method(): number;",
},
],
});
@@ -82,13 +74,9 @@ verify.completions({
{
name: "fun",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
replacementSpan: test.ranges()[2],
insertText:
`fun(a: number): number;
`public fun(a: number): number;
public fun(a: undefined, b: string): number;
public fun(a: unknown, b?: unknown): number {
}`,
tests/cases/fourslash/completionsOverridingMethod7.ts
+3 -7
View File
@@ -9,7 +9,7 @@
////}
////
////abstract class Derived extends Base {
//// abstract /*a*/
//// [|abstract|] /*a*/
////}
verify.completions({
@@ -24,13 +24,9 @@ verify.completions({
{
name: "M",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
replacementSpan: test.ranges()[0],
insertText:
`M<T>(t: T): void;
`abstract M<T>(t: T): void;
abstract M<T>(t: T, x: number): void;`,
},
],
tests/cases/fourslash/completionsOverridingMethod8.ts
-5
View File
@@ -27,11 +27,6 @@ verify.completions({
includes: [{
name: "method",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "method(p: I): void {\n}",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
tests/cases/fourslash/completionsOverridingMethod9.ts
-10
View File
@@ -23,21 +23,11 @@ verify.completions({
{
name: "a",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "a?: number;"
},
{
name: "b",
sortText: completion.SortText.LocationPriority,
replacementSpan: {
fileName: "",
pos: 0,
end: 0,
},
insertText: "b(x: number): void {\n}"
},
],