Skip to content

Commit 6357c89

Browse files
gabrittogabritto
authored
Properly set hasAction for erasing modifiers in class member completions (#57643)
1 parent 881f449 commit 6357c89

12 files changed

+409
-88
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7019,6 +7019,10 @@
70197019
"category": "Message",
70207020
"code": 90060
70217021
},
7022+
"Update modifiers of '{0}'": {
7023+
"category": "Message",
7024+
"code": 90061
7025+
},
70227026

70237027
"Convert function to an ES2015 class": {
70247028
"category": "Message",

‎src/services/completions.ts

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,10 +1757,21 @@ function createCompletionEntry(
17571757
isClassLikeMemberCompletion(symbol, location, sourceFile)
17581758
) {
17591759
let importAdder;
1760-
const memberCompletionEntry = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, position, contextToken, formatContext);
1760+
const memberCompletionEntry = getEntryForMemberCompletion(
1761+
host,
1762+
program,
1763+
options,
1764+
preferences,
1765+
name,
1766+
symbol,
1767+
location,
1768+
position,
1769+
contextToken,
1770+
formatContext,
1771+
);
17611772
if (memberCompletionEntry) {
17621773
({ insertText, filterText, isSnippet, importAdder } = memberCompletionEntry);
1763-
if (importAdder?.hasFixes()) {
1774+
if (importAdder?.hasFixes() || memberCompletionEntry.eraseRange) {
17641775
hasAction = true;
17651776
source = CompletionSource.ClassMemberSnippet;
17661777
}
@@ -2068,6 +2079,7 @@ function getPresentModifiers(
20682079
let decorators: Decorator[] | undefined;
20692080
let contextMod;
20702081
const range: TextRange = { pos: position, end: position };
2082+
20712083
/*
20722084
Cases supported:
20732085
In
@@ -2083,23 +2095,41 @@ function getPresentModifiers(
20832095
}`
20842096
`contextToken` is ``override`` (as a keyword),
20852097
`contextToken.parent` is property declaration,
2086-
`location` is identifier ``m``,
2087-
`location.parent` is property declaration ``protected override m``,
2088-
`location.parent.parent` is class declaration ``class C { ... }``.
2098+
`location` is identifier ``m``.
20892099
*/
2090-
if (isPropertyDeclaration(contextToken.parent) && contextToken.parent.modifiers) {
2091-
modifiers |= modifiersToFlags(contextToken.parent.modifiers) & ModifierFlags.Modifier;
2092-
decorators = contextToken.parent.modifiers.filter(isDecorator) || [];
2093-
range.pos = Math.min(range.pos, contextToken.parent.modifiers.pos);
2094-
}
2095-
if (contextMod = isModifierLike(contextToken)) {
2100+
if (isPropertyDeclaration(contextToken.parent) && (contextMod = isModifierLike(contextToken))) {
2101+
if (contextToken.parent.modifiers) {
2102+
modifiers |= modifiersToFlags(contextToken.parent.modifiers) & ModifierFlags.Modifier;
2103+
decorators = contextToken.parent.modifiers.filter(isDecorator) || [];
2104+
range.pos = Math.min(...contextToken.parent.modifiers.map(n => n.getStart(sourceFile)));
2105+
}
20962106
const contextModifierFlag = modifierToFlag(contextMod);
20972107
if (!(modifiers & contextModifierFlag)) {
20982108
modifiers |= contextModifierFlag;
2099-
range.pos = Math.min(range.pos, contextToken.pos);
2109+
range.pos = Math.min(range.pos, contextToken.getStart(sourceFile));
2110+
}
2111+
/*
2112+
We have two cases:
2113+
1.
2114+
`class C {
2115+
modifier |
2116+
}`
2117+
`contextToken` is `modifier`,
2118+
and the range should be `modifier |`, ending at `position`,
2119+
and
2120+
2.
2121+
`class C {
2122+
modifier otherToken|
2123+
}`
2124+
`contextToken` is `modifier`,
2125+
`contextToken.parent.name` is `otherToken`,
2126+
and the range should be `modifier `, ending at the start of `otherToken`.
2127+
*/
2128+
if (contextToken.parent.name !== contextToken) {
2129+
range.end = contextToken.parent.name.getStart(sourceFile);
21002130
}
21012131
}
2102-
return { modifiers, decorators, range: range.pos !== position ? range : undefined };
2132+
return { modifiers, decorators, range: range.pos < range.end ? range : undefined };
21032133
}
21042134

21052135
function isModifierLike(node: Node): ModifierSyntaxKind | undefined {
@@ -2941,7 +2971,7 @@ function getCompletionEntryCodeActionsAndSourceDisplay(
29412971
contextToken,
29422972
formatContext,
29432973
)!;
2944-
if (importAdder || eraseRange) {
2974+
if (importAdder?.hasFixes() || eraseRange) {
29452975
const changes = textChanges.ChangeTracker.with(
29462976
{ host, formatContext, preferences },
29472977
tracker => {
@@ -2957,7 +2987,9 @@ function getCompletionEntryCodeActionsAndSourceDisplay(
29572987
sourceDisplay: undefined,
29582988
codeActions: [{
29592989
changes,
2960-
description: diagnosticToString([Diagnostics.Includes_imports_of_types_referenced_by_0, name]),
2990+
description: importAdder?.hasFixes() ?
2991+
diagnosticToString([Diagnostics.Includes_imports_of_types_referenced_by_0, name]) :
2992+
diagnosticToString([Diagnostics.Update_modifiers_of_0, name]),
29612993
}],
29622994
};
29632995
}

‎tests/cases/fourslash/completionsOverridingMethod0.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,8 @@ verify.completions({
267267
sortText: completion.SortText.LocationPriority,
268268
insertText: "static met(n: number): number {\n}",
269269
filterText: "met",
270+
hasAction: true,
271+
source: completion.CompletionSource.ClassMemberSnippet,
270272
}
271273
],
272274
});

‎tests/cases/fourslash/completionsOverridingMethod12.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ verify.completions({
3131
sortText: completion.SortText.LocationPriority,
3232
insertText: "abstract get P(): string;",
3333
filterText: "P",
34+
hasAction: true,
35+
source: completion.CompletionSource.ClassMemberSnippet,
3436
},
3537
],
3638
});
@@ -49,6 +51,33 @@ verify.completions({
4951
sortText: completion.SortText.LocationPriority,
5052
insertText: "abstract override get P(): string;",
5153
filterText: "P",
54+
hasAction: true,
55+
source: completion.CompletionSource.ClassMemberSnippet,
5256
},
5357
],
58+
});
59+
60+
verify.applyCodeActionFromCompletion("b", {
61+
preferences: {
62+
includeCompletionsWithInsertText: true,
63+
includeCompletionsWithSnippetText: false,
64+
includeCompletionsWithClassMemberSnippets: true,
65+
},
66+
name: "P",
67+
source: completion.CompletionSource.ClassMemberSnippet,
68+
description: "Update modifiers of 'P'",
69+
newFileContent:
70+
`abstract class A {
71+
public get P(): string {
72+
return "";
73+
}
74+
}
75+
76+
abstract class B extends A {
77+
abstract
78+
}
79+
80+
abstract class B1 extends A {
81+
82+
}`
5483
});

‎tests/cases/fourslash/completionsOverridingMethod18.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: a.ts
4+
// @newline: LF
5+
6+
//// declare function decorator(...args: any[]): any;
7+
8+
//// class DecoratorBase {
9+
//// protected foo(a: string): string;
10+
//// protected foo(a: number): number;
11+
//// protected foo(a: any): any {
12+
//// return a;
13+
//// }
14+
//// }
15+
16+
//// class DecoratorSub extends DecoratorBase {
17+
//// @decorator protected /**/
18+
//// }
19+
20+
verify.completions({
21+
marker: "",
22+
isNewIdentifierLocation: true,
23+
preferences: {
24+
includeCompletionsWithInsertText: true,
25+
includeCompletionsWithSnippetText: false,
26+
includeCompletionsWithClassMemberSnippets: true,
27+
},
28+
includes: [
29+
{
30+
name: "foo",
31+
sortText: completion.SortText.LocationPriority,
32+
insertText:
33+
`protected foo(a: string): string;
34+
protected foo(a: number): number;
35+
@decorator
36+
protected foo(a: any) {
37+
}`,
38+
filterText: "foo",
39+
replacementSpan: undefined,
40+
hasAction: true,
41+
source: completion.CompletionSource.ClassMemberSnippet,
42+
},
43+
]
44+
});
45+
46+
verify.applyCodeActionFromCompletion("", {
47+
preferences: {
48+
includeCompletionsWithInsertText: true,
49+
includeCompletionsWithSnippetText: false,
50+
includeCompletionsWithClassMemberSnippets: true,
51+
},
52+
name: "foo",
53+
source: completion.CompletionSource.ClassMemberSnippet,
54+
description: "Update modifiers of 'foo'",
55+
newFileContent:
56+
`declare function decorator(...args: any[]): any;
57+
class DecoratorBase {
58+
protected foo(a: string): string;
59+
protected foo(a: number): number;
60+
protected foo(a: any): any {
61+
return a;
62+
}
63+
}
64+
class DecoratorSub extends DecoratorBase {
65+
66+
}`
67+
});

‎tests/cases/fourslash/completionsOverridingMethod19.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: a.ts
4+
// @newline: LF
5+
//// class Base {
6+
//// method() {}
7+
//// protected prop = 1;
8+
//// }
9+
//// class E extends Base {
10+
//// protected notamodifier override /**/
11+
//// }
12+
13+
verify.completions({
14+
marker: "",
15+
isNewIdentifierLocation: true,
16+
preferences: {
17+
includeCompletionsWithInsertText: true,
18+
includeCompletionsWithSnippetText: false,
19+
includeCompletionsWithClassMemberSnippets: true,
20+
},
21+
includes: [
22+
{
23+
name: "method",
24+
sortText: completion.SortText.LocationPriority,
25+
insertText: "override method(): void {\n}",
26+
filterText: "method",
27+
replacementSpan: undefined,
28+
hasAction: true,
29+
source: completion.CompletionSource.ClassMemberSnippet,
30+
},
31+
{
32+
name: "prop",
33+
sortText: completion.SortText.LocationPriority,
34+
insertText: "protected override prop: number;",
35+
filterText: "prop",
36+
replacementSpan: undefined,
37+
hasAction: true,
38+
source: completion.CompletionSource.ClassMemberSnippet,
39+
},
40+
]
41+
},);
42+
43+
verify.applyCodeActionFromCompletion("", {
44+
preferences: {
45+
includeCompletionsWithInsertText: true,
46+
includeCompletionsWithSnippetText: false,
47+
includeCompletionsWithClassMemberSnippets: true,
48+
},
49+
name: "method",
50+
source: completion.CompletionSource.ClassMemberSnippet,
51+
description: "Update modifiers of 'method'",
52+
newFileContent:
53+
`class Base {
54+
method() {}
55+
protected prop = 1;
56+
}
57+
class E extends Base {
58+
protected notamodifier
59+
}`
60+
});

‎tests/cases/fourslash/completionsOverridingMethod20.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: a.ts
4+
// @newline: LF
5+
//// abstract class AFoo {
6+
//// abstract bar(): Promise<void>;
7+
//// }
8+
//// class Foo extends AFoo {
9+
//// async b/*a*/
10+
//// }
11+
12+
verify.completions({
13+
marker: "a",
14+
isNewIdentifierLocation: true,
15+
preferences: {
16+
includeCompletionsWithInsertText: true,
17+
includeCompletionsWithSnippetText: false,
18+
includeCompletionsWithClassMemberSnippets: true,
19+
},
20+
includes: [
21+
{
22+
name: "bar",
23+
sortText: completion.SortText.LocationPriority,
24+
insertText: "async bar(): Promise<void> {\n}",
25+
filterText: "bar",
26+
replacementSpan: undefined,
27+
hasAction: true,
28+
source: completion.CompletionSource.ClassMemberSnippet,
29+
},
30+
]
31+
});
32+
33+
verify.applyCodeActionFromCompletion("a", {
34+
preferences: {
35+
includeCompletionsWithInsertText: true,
36+
includeCompletionsWithSnippetText: false,
37+
includeCompletionsWithClassMemberSnippets: true,
38+
},
39+
name: "bar",
40+
source: completion.CompletionSource.ClassMemberSnippet,
41+
description: "Update modifiers of 'bar'",
42+
newFileContent:
43+
`abstract class AFoo {
44+
abstract bar(): Promise<void>;
45+
}
46+
class Foo extends AFoo {
47+
b
48+
}`
49+
});

‎tests/cases/fourslash/completionsOverridingMethod21.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: a.ts
4+
// @newline: LF
5+
//// abstract class AFoo {
6+
//// abstract bar(): Promise<void>;
7+
//// }
8+
//// class BFoo extends AFoo {
9+
//// async /*b*/
10+
//// }
11+
12+
verify.completions({
13+
marker: "b",
14+
isNewIdentifierLocation: true,
15+
preferences: {
16+
includeCompletionsWithInsertText: true,
17+
includeCompletionsWithSnippetText: false,
18+
includeCompletionsWithClassMemberSnippets: true,
19+
},
20+
includes: [
21+
{
22+
name: "bar",
23+
sortText: completion.SortText.LocationPriority,
24+
insertText: "async bar(): Promise<void> {\n}",
25+
filterText: "bar",
26+
replacementSpan: undefined,
27+
hasAction: true,
28+
source: completion.CompletionSource.ClassMemberSnippet,
29+
},
30+
]
31+
});
32+
verify.applyCodeActionFromCompletion("b", {
33+
preferences: {
34+
includeCompletionsWithInsertText: true,
35+
includeCompletionsWithSnippetText: false,
36+
includeCompletionsWithClassMemberSnippets: true,
37+
},
38+
name: "bar",
39+
source: completion.CompletionSource.ClassMemberSnippet,
40+
description: "Update modifiers of 'bar'",
41+
newFileContent:
42+
`abstract class AFoo {
43+
abstract bar(): Promise<void>;
44+
}
45+
class BFoo extends AFoo {
46+
47+
}`
48+
});

0 commit comments

Comments
 (0)