Properly set hasAction for erasing modifiers in class member completions (#57643)

This commit is contained in:
Gabriela Araujo Britto
2024-03-07 11:34:47 -08:00
committed by GitHub
parent 881f449a8a
commit 6357c89c52
12 changed files with 409 additions and 88 deletions

View File

@@ -7019,6 +7019,10 @@
"category": "Message",
"code": 90060
},
"Update modifiers of '{0}'": {
"category": "Message",
"code": 90061
},
"Convert function to an ES2015 class": {
"category": "Message",

View File

@@ -1757,10 +1757,21 @@ function createCompletionEntry(
isClassLikeMemberCompletion(symbol, location, sourceFile)
) {
let importAdder;
const memberCompletionEntry = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, position, contextToken, formatContext);
const memberCompletionEntry = getEntryForMemberCompletion(
host,
program,
options,
preferences,
name,
symbol,
location,
position,
contextToken,
formatContext,
);
if (memberCompletionEntry) {
({ insertText, filterText, isSnippet, importAdder } = memberCompletionEntry);
if (importAdder?.hasFixes()) {
if (importAdder?.hasFixes() || memberCompletionEntry.eraseRange) {
hasAction = true;
source = CompletionSource.ClassMemberSnippet;
}
@@ -2068,6 +2079,7 @@ function getPresentModifiers(
let decorators: Decorator[] | undefined;
let contextMod;
const range: TextRange = { pos: position, end: position };
/*
Cases supported:
In
@@ -2083,23 +2095,41 @@ function getPresentModifiers(
}`
`contextToken` is ``override`` (as a keyword),
`contextToken.parent` is property declaration,
`location` is identifier ``m``,
`location.parent` is property declaration ``protected override m``,
`location.parent.parent` is class declaration ``class C { ... }``.
`location` is identifier ``m``.
*/
if (isPropertyDeclaration(contextToken.parent) && contextToken.parent.modifiers) {
modifiers |= modifiersToFlags(contextToken.parent.modifiers) & ModifierFlags.Modifier;
decorators = contextToken.parent.modifiers.filter(isDecorator) || [];
range.pos = Math.min(range.pos, contextToken.parent.modifiers.pos);
}
if (contextMod = isModifierLike(contextToken)) {
if (isPropertyDeclaration(contextToken.parent) && (contextMod = isModifierLike(contextToken))) {
if (contextToken.parent.modifiers) {
modifiers |= modifiersToFlags(contextToken.parent.modifiers) & ModifierFlags.Modifier;
decorators = contextToken.parent.modifiers.filter(isDecorator) || [];
range.pos = Math.min(...contextToken.parent.modifiers.map(n => n.getStart(sourceFile)));
}
const contextModifierFlag = modifierToFlag(contextMod);
if (!(modifiers & contextModifierFlag)) {
modifiers |= contextModifierFlag;
range.pos = Math.min(range.pos, contextToken.pos);
range.pos = Math.min(range.pos, contextToken.getStart(sourceFile));
}
/*
We have two cases:
1.
`class C {
modifier |
}`
`contextToken` is `modifier`,
and the range should be `modifier |`, ending at `position`,
and
2.
`class C {
modifier otherToken|
}`
`contextToken` is `modifier`,
`contextToken.parent.name` is `otherToken`,
and the range should be `modifier `, ending at the start of `otherToken`.
*/
if (contextToken.parent.name !== contextToken) {
range.end = contextToken.parent.name.getStart(sourceFile);
}
}
return { modifiers, decorators, range: range.pos !== position ? range : undefined };
return { modifiers, decorators, range: range.pos < range.end ? range : undefined };
}
function isModifierLike(node: Node): ModifierSyntaxKind | undefined {
@@ -2941,7 +2971,7 @@ function getCompletionEntryCodeActionsAndSourceDisplay(
contextToken,
formatContext,
)!;
if (importAdder || eraseRange) {
if (importAdder?.hasFixes() || eraseRange) {
const changes = textChanges.ChangeTracker.with(
{ host, formatContext, preferences },
tracker => {
@@ -2957,7 +2987,9 @@ function getCompletionEntryCodeActionsAndSourceDisplay(
sourceDisplay: undefined,
codeActions: [{
changes,
description: diagnosticToString([Diagnostics.Includes_imports_of_types_referenced_by_0, name]),
description: importAdder?.hasFixes() ?
diagnosticToString([Diagnostics.Includes_imports_of_types_referenced_by_0, name]) :
diagnosticToString([Diagnostics.Update_modifiers_of_0, name]),
}],
};
}

View File

@@ -267,6 +267,8 @@ verify.completions({
sortText: completion.SortText.LocationPriority,
insertText: "static met(n: number): number {\n}",
filterText: "met",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
}
],
});

View File

@@ -31,6 +31,8 @@ verify.completions({
sortText: completion.SortText.LocationPriority,
insertText: "abstract get P(): string;",
filterText: "P",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
],
});
@@ -49,6 +51,33 @@ verify.completions({
sortText: completion.SortText.LocationPriority,
insertText: "abstract override get P(): string;",
filterText: "P",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
],
});
verify.applyCodeActionFromCompletion("b", {
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
name: "P",
source: completion.CompletionSource.ClassMemberSnippet,
description: "Update modifiers of 'P'",
newFileContent:
`abstract class A {
public get P(): string {
return "";
}
}
abstract class B extends A {
abstract
}
abstract class B1 extends A {
}`
});

View File

@@ -0,0 +1,67 @@
/// <reference path="fourslash.ts" />
// @Filename: a.ts
// @newline: LF
//// declare function decorator(...args: any[]): any;
//// class DecoratorBase {
//// protected foo(a: string): string;
//// protected foo(a: number): number;
//// protected foo(a: any): any {
//// return a;
//// }
//// }
//// class DecoratorSub extends DecoratorBase {
//// @decorator protected /**/
//// }
verify.completions({
marker: "",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "foo",
sortText: completion.SortText.LocationPriority,
insertText:
`protected foo(a: string): string;
protected foo(a: number): number;
@decorator
protected foo(a: any) {
}`,
filterText: "foo",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
]
});
verify.applyCodeActionFromCompletion("", {
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
name: "foo",
source: completion.CompletionSource.ClassMemberSnippet,
description: "Update modifiers of 'foo'",
newFileContent:
`declare function decorator(...args: any[]): any;
class DecoratorBase {
protected foo(a: string): string;
protected foo(a: number): number;
protected foo(a: any): any {
return a;
}
}
class DecoratorSub extends DecoratorBase {
}`
});

View File

@@ -0,0 +1,60 @@
/// <reference path="fourslash.ts" />
// @Filename: a.ts
// @newline: LF
//// class Base {
//// method() {}
//// protected prop = 1;
//// }
//// class E extends Base {
//// protected notamodifier override /**/
//// }
verify.completions({
marker: "",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "method",
sortText: completion.SortText.LocationPriority,
insertText: "override method(): void {\n}",
filterText: "method",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
{
name: "prop",
sortText: completion.SortText.LocationPriority,
insertText: "protected override prop: number;",
filterText: "prop",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
]
},);
verify.applyCodeActionFromCompletion("", {
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
name: "method",
source: completion.CompletionSource.ClassMemberSnippet,
description: "Update modifiers of 'method'",
newFileContent:
`class Base {
method() {}
protected prop = 1;
}
class E extends Base {
protected notamodifier
}`
});

View File

@@ -0,0 +1,49 @@
/// <reference path="fourslash.ts" />
// @Filename: a.ts
// @newline: LF
//// abstract class AFoo {
//// abstract bar(): Promise<void>;
//// }
//// class Foo extends AFoo {
//// async b/*a*/
//// }
verify.completions({
marker: "a",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "bar",
sortText: completion.SortText.LocationPriority,
insertText: "async bar(): Promise<void> {\n}",
filterText: "bar",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
]
});
verify.applyCodeActionFromCompletion("a", {
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
name: "bar",
source: completion.CompletionSource.ClassMemberSnippet,
description: "Update modifiers of 'bar'",
newFileContent:
`abstract class AFoo {
abstract bar(): Promise<void>;
}
class Foo extends AFoo {
b
}`
});

View File

@@ -0,0 +1,48 @@
/// <reference path="fourslash.ts" />
// @Filename: a.ts
// @newline: LF
//// abstract class AFoo {
//// abstract bar(): Promise<void>;
//// }
//// class BFoo extends AFoo {
//// async /*b*/
//// }
verify.completions({
marker: "b",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "bar",
sortText: completion.SortText.LocationPriority,
insertText: "async bar(): Promise<void> {\n}",
filterText: "bar",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
]
});
verify.applyCodeActionFromCompletion("b", {
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
name: "bar",
source: completion.CompletionSource.ClassMemberSnippet,
description: "Update modifiers of 'bar'",
newFileContent:
`abstract class AFoo {
abstract bar(): Promise<void>;
}
class BFoo extends AFoo {
}`
});

View File

@@ -0,0 +1,53 @@
/// <reference path="fourslash.ts" />
// Bug #57333
// @newline: LF
// @Filename: b.ts
//// export type C = { x: number }
//// export interface ExtShape {
//// $returnPromise(id: number): Promise<C>;
//// $return(id: number): C;
//// }
// @Filename: test.ts
//// import { ExtShape } from './b';
//// abstract class ExtBase implements ExtShape {
//// public $/**/
//// }
verify.completions({
marker: "",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "$returnPromise",
sortText: completion.SortText.LocationPriority,
insertText: "public \\$returnPromise(id: number): Promise<C> {\n}",
filterText: "$returnPromise",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
}
]
});
verify.applyCodeActionFromCompletion("", {
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
name: "$returnPromise",
source: completion.CompletionSource.ClassMemberSnippet,
description: "Includes imports of types referenced by '$returnPromise'",
newFileContent:
`import { C, ExtShape } from './b';
abstract class ExtBase implements ExtShape {
$
}`
});

View File

@@ -54,13 +54,17 @@ verify.completions({
name: "met",
sortText: completion.SortText.LocationPriority,
insertText: "abstract met(n: string): void;",
filterText: "met"
filterText: "met",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
{
name: "met2",
sortText: completion.SortText.LocationPriority,
insertText: "abstract met2(n: number): void;",
filterText: "met2"
filterText: "met2",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
}
],
});
@@ -78,13 +82,17 @@ verify.completions({
name: "met",
sortText: completion.SortText.LocationPriority,
insertText: "abstract met(n: string): void;",
filterText: "met"
filterText: "met",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
{
name: "met2",
sortText: completion.SortText.LocationPriority,
insertText: "abstract met2(n: number): void;",
filterText: "met2"
filterText: "met2",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
}
],
});

View File

@@ -24,28 +24,10 @@
//// override /*d*/
//// }
//// class E extends Base {
//// protected notamodifier override /*e*/
//// }
//// class f extends Base {
//// protected /*f*/
//// }
//// declare function decorator(...args: any[]): any;
//// class DecoratorBase {
//// protected foo(a: string): string;
//// protected foo(a: number): number;
//// protected foo(a: any): any {
//// return a;
//// }
//// }
//// class DecoratorSub extends DecoratorBase {
//// @decorator protected /*g*/
//// }
verify.completions(
{
marker: "a",
@@ -72,6 +54,8 @@ verify.completions(
insertText: "public abstract method(): void;",
filterText: "method",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
{
name: "prop",
@@ -79,6 +63,8 @@ verify.completions(
insertText: "public abstract prop: number;",
filterText: "prop",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
],
},
@@ -97,6 +83,8 @@ verify.completions(
insertText: "public override method(): void {\n}",
filterText: "method",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
{
name: "prop",
@@ -104,6 +92,8 @@ verify.completions(
insertText: "public override prop: number;",
filterText: "prop",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
]
},
@@ -122,6 +112,8 @@ verify.completions(
insertText: "override method(): void {\n}",
filterText: "method",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
{
name: "prop",
@@ -129,31 +121,8 @@ verify.completions(
insertText: "protected override prop: number;",
filterText: "prop",
replacementSpan: undefined,
},
]
},
{
marker: "e",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "method",
sortText: completion.SortText.LocationPriority,
insertText: "override method(): void {\n}",
filterText: "method",
replacementSpan: undefined,
},
{
name: "prop",
sortText: completion.SortText.LocationPriority,
insertText: "protected override prop: number;",
filterText: "prop",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
]
},
@@ -173,31 +142,9 @@ verify.completions(
insertText: "protected prop: number;",
filterText: "prop",
replacementSpan: undefined,
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
]
},
{
marker: "g",
isNewIdentifierLocation: true,
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
includes: [
{
name: "foo",
sortText: completion.SortText.LocationPriority,
insertText:
`protected foo(a: string): string;
protected foo(a: number): number;
@decorator
protected foo(a: any) {
}`,
filterText: "foo",
replacementSpan: undefined,
},
]
},
);
);

View File

@@ -28,6 +28,28 @@ verify.completions({
`abstract M<T>(t: T): void;
abstract M<T>(t: T, x: number): void;`,
filterText: "M",
hasAction: true,
source: completion.CompletionSource.ClassMemberSnippet,
},
],
});
});
verify.applyCodeActionFromCompletion("a", {
preferences: {
includeCompletionsWithInsertText: true,
includeCompletionsWithSnippetText: false,
includeCompletionsWithClassMemberSnippets: true,
},
name: "M",
source: completion.CompletionSource.ClassMemberSnippet,
description: "Update modifiers of 'M'",
newFileContent:
`abstract class Base {
abstract M<T>(t: T): void;
abstract M<T>(t: T, x: number): void;
}
abstract class Derived extends Base {
}`
});