Skip to content

Commit bb3a7ae

Browse files
a-tarasyuka-tarasyuk
authoredAug 26, 2022
fix(50415): Language server debug failure - Did not expect GetAccessor to have an Identifier in its trivia (#50470)
* fix(50415): clone props for get/set accessors * add additional tests * create helpers to create name, body, modifiers, typeName * cleanup
·
v5.8.2v4.9-beta
1 parent 3557092 commit bb3a7ae

File tree

6 files changed

+490
-16
lines changed

6 files changed

+490
-16
lines changed
 

‎src/compiler/factory/nodeFactory.ts

+2
Original file line numberDiff line numberDiff line change
@@ -6823,6 +6823,7 @@ namespace ts {
68236823
constantValue,
68246824
helpers,
68256825
startsOnNewLine,
6826+
snippetElement,
68266827
} = sourceEmitNode;
68276828
if (!destEmitNode) destEmitNode = {} as EmitNode;
68286829
// We are using `.slice()` here in case `destEmitNode.leadingComments` is pushed to later.
@@ -6839,6 +6840,7 @@ namespace ts {
68396840
}
68406841
}
68416842
if (startsOnNewLine !== undefined) destEmitNode.startsOnNewLine = startsOnNewLine;
6843+
if (snippetElement !== undefined) destEmitNode.snippetElement = snippetElement;
68426844
return destEmitNode;
68436845
}
68446846

‎src/services/codefixes/helpers.ts

+32-16
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,8 @@ namespace ts.codefix {
7979
* In such cases, we assume the declaration to be a `PropertySignature`.
8080
*/
8181
const kind = declaration?.kind ?? SyntaxKind.PropertySignature;
82-
const name = getSynthesizedDeepClone(getNameOfDeclaration(declaration), /*includeTrivia*/ false) as PropertyName;
82+
const declarationName = getNameOfDeclaration(declaration) as PropertyName;
8383
const visibilityModifier = createVisibilityModifier(declaration ? getEffectiveModifierFlags(declaration) : ModifierFlags.None);
84-
const modifiers = visibilityModifier ? factory.createNodeArray([visibilityModifier]) : undefined;
8584
const type = checker.getWidenedType(checker.getTypeOfSymbolAtLocation(symbol, enclosingDeclaration));
8685
const optional = !!(symbol.flags & SymbolFlags.Optional);
8786
const ambient = !!(enclosingDeclaration.flags & NodeFlags.Ambient) || isAmbient;
@@ -100,8 +99,8 @@ namespace ts.codefix {
10099
}
101100
}
102101
addClassElement(factory.createPropertyDeclaration(
103-
modifiers,
104-
declaration ? name : symbol.getName(),
102+
createModifiers(visibilityModifier),
103+
declaration ? createName(declarationName) : symbol.getName(),
105104
optional && (preserveOptional & PreserveOptionalFlags.Property) ? factory.createToken(SyntaxKind.QuestionToken) : undefined,
106105
typeNode,
107106
/*initializer*/ undefined));
@@ -124,21 +123,21 @@ namespace ts.codefix {
124123
for (const accessor of orderedAccessors) {
125124
if (isGetAccessorDeclaration(accessor)) {
126125
addClassElement(factory.createGetAccessorDeclaration(
127-
modifiers,
128-
name,
126+
createModifiers(visibilityModifier),
127+
createName(declarationName),
129128
emptyArray,
130-
typeNode,
131-
ambient ? undefined : body || createStubbedMethodBody(quotePreference)));
129+
createTypeNode(typeNode),
130+
createBody(body, quotePreference, ambient)));
132131
}
133132
else {
134133
Debug.assertNode(accessor, isSetAccessorDeclaration, "The counterpart to a getter should be a setter");
135134
const parameter = getSetAccessorValueParameter(accessor);
136135
const parameterName = parameter && isIdentifier(parameter.name) ? idText(parameter.name) : undefined;
137136
addClassElement(factory.createSetAccessorDeclaration(
138-
modifiers,
139-
name,
140-
createDummyParameters(1, [parameterName], [typeNode], 1, /*inJs*/ false),
141-
ambient ? undefined : body || createStubbedMethodBody(quotePreference)));
137+
createModifiers(visibilityModifier),
138+
createName(declarationName),
139+
createDummyParameters(1, [parameterName], [createTypeNode(typeNode)], 1, /*inJs*/ false),
140+
createBody(body, quotePreference, ambient)));
142141
}
143142
}
144143
break;
@@ -161,23 +160,23 @@ namespace ts.codefix {
161160
if (declarations.length === 1) {
162161
Debug.assert(signatures.length === 1, "One declaration implies one signature");
163162
const signature = signatures[0];
164-
outputMethod(quotePreference, signature, modifiers, name, ambient ? undefined : body || createStubbedMethodBody(quotePreference));
163+
outputMethod(quotePreference, signature, createModifiers(visibilityModifier), createName(declarationName), createBody(body, quotePreference, ambient));
165164
break;
166165
}
167166

168167
for (const signature of signatures) {
169168
// Ensure nodes are fresh so they can have different positions when going through formatting.
170-
outputMethod(quotePreference, signature, getSynthesizedDeepClones(modifiers, /*includeTrivia*/ false), getSynthesizedDeepClone(name, /*includeTrivia*/ false));
169+
outputMethod(quotePreference, signature, createModifiers(visibilityModifier), createName(declarationName));
171170
}
172171

173172
if (!ambient) {
174173
if (declarations.length > signatures.length) {
175174
const signature = checker.getSignatureFromDeclaration(declarations[declarations.length - 1] as SignatureDeclaration)!;
176-
outputMethod(quotePreference, signature, modifiers, name, body || createStubbedMethodBody(quotePreference));
175+
outputMethod(quotePreference, signature, createModifiers(visibilityModifier), createName(declarationName), createBody(body, quotePreference));
177176
}
178177
else {
179178
Debug.assert(declarations.length === signatures.length, "Declarations and signatures should match count");
180-
addClassElement(createMethodImplementingSignatures(checker, context, enclosingDeclaration, signatures, name, optional && !!(preserveOptional & PreserveOptionalFlags.Method), modifiers, quotePreference, body));
179+
addClassElement(createMethodImplementingSignatures(checker, context, enclosingDeclaration, signatures, createName(declarationName), optional && !!(preserveOptional & PreserveOptionalFlags.Method), createModifiers(visibilityModifier), quotePreference, body));
181180
}
182181
}
183182
break;
@@ -187,6 +186,23 @@ namespace ts.codefix {
187186
const method = createSignatureDeclarationFromSignature(SyntaxKind.MethodDeclaration, context, quotePreference, signature, body, name, modifiers, optional && !!(preserveOptional & PreserveOptionalFlags.Method), enclosingDeclaration, importAdder) as MethodDeclaration;
188187
if (method) addClassElement(method);
189188
}
189+
190+
function createName(node: PropertyName) {
191+
return getSynthesizedDeepClone(node, /*includeTrivia*/ false);
192+
}
193+
194+
function createModifiers(modifier: Modifier | undefined) {
195+
return modifier ? factory.createNodeArray([modifier]) : undefined;
196+
}
197+
198+
function createBody(block: Block | undefined, quotePreference: QuotePreference, ambient?: boolean) {
199+
return ambient ? undefined :
200+
getSynthesizedDeepClone(block, /*includeTrivia*/ false) || createStubbedMethodBody(quotePreference);
201+
}
202+
203+
function createTypeNode(typeNode: TypeNode | undefined) {
204+
return getSynthesizedDeepClone(typeNode, /*includeTrivia*/ false);
205+
}
190206
}
191207

192208
export function createSignatureDeclarationFromSignature(

‎tests/baselines/reference/completionsOverridingMethod15.baseline

+211
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
[
2+
{
3+
"marker": {
4+
"fileName": "/tests/cases/fourslash/completionsOverridingMethod15.ts",
5+
"position": 89,
6+
"name": ""
7+
},
8+
"completionList": {
9+
"flags": 0,
10+
"isGlobalCompletion": false,
11+
"isMemberCompletion": true,
12+
"isNewIdentifierLocation": true,
13+
"entries": [
14+
{
15+
"name": "abstract",
16+
"kind": "keyword",
17+
"kindModifiers": "",
18+
"sortText": "15",
19+
"displayParts": [
20+
{
21+
"text": "abstract",
22+
"kind": "keyword"
23+
}
24+
]
25+
},
26+
{
27+
"name": "async",
28+
"kind": "keyword",
29+
"kindModifiers": "",
30+
"sortText": "15",
31+
"displayParts": [
32+
{
33+
"text": "async",
34+
"kind": "keyword"
35+
}
36+
]
37+
},
38+
{
39+
"name": "constructor",
40+
"kind": "keyword",
41+
"kindModifiers": "",
42+
"sortText": "15",
43+
"displayParts": [
44+
{
45+
"text": "constructor",
46+
"kind": "keyword"
47+
}
48+
]
49+
},
50+
{
51+
"name": "declare",
52+
"kind": "keyword",
53+
"kindModifiers": "",
54+
"sortText": "15",
55+
"displayParts": [
56+
{
57+
"text": "declare",
58+
"kind": "keyword"
59+
}
60+
]
61+
},
62+
{
63+
"name": "get",
64+
"kind": "keyword",
65+
"kindModifiers": "",
66+
"sortText": "15",
67+
"displayParts": [
68+
{
69+
"text": "get",
70+
"kind": "keyword"
71+
}
72+
]
73+
},
74+
{
75+
"name": "override",
76+
"kind": "keyword",
77+
"kindModifiers": "",
78+
"sortText": "15",
79+
"displayParts": [
80+
{
81+
"text": "override",
82+
"kind": "keyword"
83+
}
84+
]
85+
},
86+
{
87+
"name": "private",
88+
"kind": "keyword",
89+
"kindModifiers": "",
90+
"sortText": "15",
91+
"displayParts": [
92+
{
93+
"text": "private",
94+
"kind": "keyword"
95+
}
96+
]
97+
},
98+
{
99+
"name": "protected",
100+
"kind": "keyword",
101+
"kindModifiers": "",
102+
"sortText": "15",
103+
"displayParts": [
104+
{
105+
"text": "protected",
106+
"kind": "keyword"
107+
}
108+
]
109+
},
110+
{
111+
"name": "public",
112+
"kind": "keyword",
113+
"kindModifiers": "",
114+
"sortText": "15",
115+
"displayParts": [
116+
{
117+
"text": "public",
118+
"kind": "keyword"
119+
}
120+
]
121+
},
122+
{
123+
"name": "readonly",
124+
"kind": "keyword",
125+
"kindModifiers": "",
126+
"sortText": "15",
127+
"displayParts": [
128+
{
129+
"text": "readonly",
130+
"kind": "keyword"
131+
}
132+
]
133+
},
134+
{
135+
"name": "set",
136+
"kind": "keyword",
137+
"kindModifiers": "",
138+
"sortText": "15",
139+
"displayParts": [
140+
{
141+
"text": "set",
142+
"kind": "keyword"
143+
}
144+
]
145+
},
146+
{
147+
"name": "static",
148+
"kind": "keyword",
149+
"kindModifiers": "",
150+
"sortText": "15",
151+
"displayParts": [
152+
{
153+
"text": "static",
154+
"kind": "keyword"
155+
}
156+
]
157+
},
158+
{
159+
"name": "foo",
160+
"kind": "property",
161+
"kindModifiers": "declare",
162+
"sortText": "17",
163+
"insertText": "get foo(): any {\n}\nset foo(value: any) {\n}",
164+
"displayParts": [
165+
{
166+
"text": "(",
167+
"kind": "punctuation"
168+
},
169+
{
170+
"text": "property",
171+
"kind": "text"
172+
},
173+
{
174+
"text": ")",
175+
"kind": "punctuation"
176+
},
177+
{
178+
"text": " ",
179+
"kind": "space"
180+
},
181+
{
182+
"text": "B",
183+
"kind": "className"
184+
},
185+
{
186+
"text": ".",
187+
"kind": "punctuation"
188+
},
189+
{
190+
"text": "foo",
191+
"kind": "propertyName"
192+
},
193+
{
194+
"text": ":",
195+
"kind": "punctuation"
196+
},
197+
{
198+
"text": " ",
199+
"kind": "space"
200+
},
201+
{
202+
"text": "any",
203+
"kind": "keyword"
204+
}
205+
],
206+
"documentation": []
207+
}
208+
]
209+
}
210+
}
211+
]

‎tests/baselines/reference/completionsOverridingMethod16.baseline

+211
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
[
2+
{
3+
"marker": {
4+
"fileName": "/tests/cases/fourslash/completionsOverridingMethod16.ts",
5+
"position": 89,
6+
"name": ""
7+
},
8+
"completionList": {
9+
"flags": 0,
10+
"isGlobalCompletion": false,
11+
"isMemberCompletion": true,
12+
"isNewIdentifierLocation": true,
13+
"entries": [
14+
{
15+
"name": "abstract",
16+
"kind": "keyword",
17+
"kindModifiers": "",
18+
"sortText": "15",
19+
"displayParts": [
20+
{
21+
"text": "abstract",
22+
"kind": "keyword"
23+
}
24+
]
25+
},
26+
{
27+
"name": "async",
28+
"kind": "keyword",
29+
"kindModifiers": "",
30+
"sortText": "15",
31+
"displayParts": [
32+
{
33+
"text": "async",
34+
"kind": "keyword"
35+
}
36+
]
37+
},
38+
{
39+
"name": "constructor",
40+
"kind": "keyword",
41+
"kindModifiers": "",
42+
"sortText": "15",
43+
"displayParts": [
44+
{
45+
"text": "constructor",
46+
"kind": "keyword"
47+
}
48+
]
49+
},
50+
{
51+
"name": "declare",
52+
"kind": "keyword",
53+
"kindModifiers": "",
54+
"sortText": "15",
55+
"displayParts": [
56+
{
57+
"text": "declare",
58+
"kind": "keyword"
59+
}
60+
]
61+
},
62+
{
63+
"name": "get",
64+
"kind": "keyword",
65+
"kindModifiers": "",
66+
"sortText": "15",
67+
"displayParts": [
68+
{
69+
"text": "get",
70+
"kind": "keyword"
71+
}
72+
]
73+
},
74+
{
75+
"name": "override",
76+
"kind": "keyword",
77+
"kindModifiers": "",
78+
"sortText": "15",
79+
"displayParts": [
80+
{
81+
"text": "override",
82+
"kind": "keyword"
83+
}
84+
]
85+
},
86+
{
87+
"name": "private",
88+
"kind": "keyword",
89+
"kindModifiers": "",
90+
"sortText": "15",
91+
"displayParts": [
92+
{
93+
"text": "private",
94+
"kind": "keyword"
95+
}
96+
]
97+
},
98+
{
99+
"name": "protected",
100+
"kind": "keyword",
101+
"kindModifiers": "",
102+
"sortText": "15",
103+
"displayParts": [
104+
{
105+
"text": "protected",
106+
"kind": "keyword"
107+
}
108+
]
109+
},
110+
{
111+
"name": "public",
112+
"kind": "keyword",
113+
"kindModifiers": "",
114+
"sortText": "15",
115+
"displayParts": [
116+
{
117+
"text": "public",
118+
"kind": "keyword"
119+
}
120+
]
121+
},
122+
{
123+
"name": "readonly",
124+
"kind": "keyword",
125+
"kindModifiers": "",
126+
"sortText": "15",
127+
"displayParts": [
128+
{
129+
"text": "readonly",
130+
"kind": "keyword"
131+
}
132+
]
133+
},
134+
{
135+
"name": "set",
136+
"kind": "keyword",
137+
"kindModifiers": "",
138+
"sortText": "15",
139+
"displayParts": [
140+
{
141+
"text": "set",
142+
"kind": "keyword"
143+
}
144+
]
145+
},
146+
{
147+
"name": "static",
148+
"kind": "keyword",
149+
"kindModifiers": "",
150+
"sortText": "15",
151+
"displayParts": [
152+
{
153+
"text": "static",
154+
"kind": "keyword"
155+
}
156+
]
157+
},
158+
{
159+
"name": "foo",
160+
"kind": "property",
161+
"kindModifiers": "declare",
162+
"sortText": "17",
163+
"insertText": "set foo(value: any) {\n}\nget foo(): any {\n}",
164+
"displayParts": [
165+
{
166+
"text": "(",
167+
"kind": "punctuation"
168+
},
169+
{
170+
"text": "property",
171+
"kind": "text"
172+
},
173+
{
174+
"text": ")",
175+
"kind": "punctuation"
176+
},
177+
{
178+
"text": " ",
179+
"kind": "space"
180+
},
181+
{
182+
"text": "B",
183+
"kind": "className"
184+
},
185+
{
186+
"text": ".",
187+
"kind": "punctuation"
188+
},
189+
{
190+
"text": "foo",
191+
"kind": "propertyName"
192+
},
193+
{
194+
"text": ":",
195+
"kind": "punctuation"
196+
},
197+
{
198+
"text": " ",
199+
"kind": "space"
200+
},
201+
{
202+
"text": "any",
203+
"kind": "keyword"
204+
}
205+
],
206+
"documentation": []
207+
}
208+
]
209+
}
210+
}
211+
]

‎tests/cases/fourslash/completionsOverridingMethod15.ts

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @newline: LF
4+
////declare class B {
5+
//// get foo(): any;
6+
//// set foo(value: any);
7+
////}
8+
////class A extends B {
9+
//// /**/
10+
////}
11+
12+
goTo.marker("");
13+
verify.baselineCompletions({
14+
includeCompletionsWithInsertText: true,
15+
includeCompletionsWithSnippetText: false,
16+
includeCompletionsWithClassMemberSnippets: true,
17+
});

‎tests/cases/fourslash/completionsOverridingMethod16.ts

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @newline: LF
4+
////declare class B {
5+
//// set foo(value: any);
6+
//// get foo(): any;
7+
////}
8+
////class A extends B {
9+
//// /**/
10+
////}
11+
12+
goTo.marker("");
13+
verify.baselineCompletions({
14+
includeCompletionsWithInsertText: true,
15+
includeCompletionsWithSnippetText: false,
16+
includeCompletionsWithClassMemberSnippets: true,
17+
});

0 commit comments

Comments
 (0)
Please sign in to comment.