Skip to content

Commit ecef2dc

Browse files
author
Andy
authored
Improve testing of code fixes, and improve diagnostic messages (#18742)
* Improve testing of code fixes, and improve diagnostic messages * Disambiguate `newFileContent` from `newRangeContent`
1 parent a655592 commit ecef2dc

15 files changed

+231
-75
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3689,6 +3689,10 @@
36893689
"category": "Message",
36903690
"code": 90026
36913691
},
3692+
"Declare static property '{0}'.": {
3693+
"category": "Message",
3694+
"code": 90027
3695+
},
36923696

36933697
"Convert function to an ES2015 class": {
36943698
"category": "Message",

‎src/harness/fourslash.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2337,6 +2337,39 @@ namespace FourSlash {
23372337
}
23382338
}
23392339

2340+
public verifyCodeFix(options: FourSlashInterface.VerifyCodeFixOptions) {
2341+
const fileName = this.activeFile.fileName;
2342+
const actions = this.getCodeFixActions(fileName, options.errorCode);
2343+
let index = options.index;
2344+
if (index === undefined) {
2345+
if (!(actions && actions.length === 1)) {
2346+
this.raiseError(`Should find exactly one codefix, but ${actions ? actions.length : "none"} found. ${actions ? actions.map(a => `${Harness.IO.newLine()} "${a.description}"`) : ""}`);
2347+
}
2348+
index = 0;
2349+
}
2350+
else {
2351+
if (!(actions && actions.length >= index + 1)) {
2352+
this.raiseError(`Should find at least ${index + 1} codefix(es), but ${actions ? actions.length : "none"} found.`);
2353+
}
2354+
}
2355+
2356+
const action = actions[index];
2357+
2358+
assert.equal(action.description, options.description);
2359+
2360+
for (const change of action.changes) {
2361+
this.applyEdits(change.fileName, change.textChanges, /*isFormattingEdit*/ false);
2362+
}
2363+
2364+
if (options.newFileContent) {
2365+
assert(!options.newRangeContent);
2366+
this.verifyCurrentFileContent(options.newFileContent);
2367+
}
2368+
else {
2369+
this.verifyRangeIs(options.newRangeContent, /*includeWhitespace*/ true);
2370+
}
2371+
}
2372+
23402373
/**
23412374
* Rerieves a codefix satisfying the parameters, or undefined if no such codefix is found.
23422375
* @param fileName Path to file where error should be retrieved from.
@@ -3716,6 +3749,10 @@ namespace FourSlashInterface {
37163749
this.state.verifySpanOfEnclosingComment(this.negative, onlyMultiLineDiverges);
37173750
}
37183751

3752+
public codeFix(options: FourSlashInterface.VerifyCodeFixOptions) {
3753+
this.state.verifyCodeFix(options);
3754+
}
3755+
37193756
public codeFixAvailable() {
37203757
this.state.verifyCodeFixAvailable(this.negative);
37213758
}
@@ -4359,4 +4396,13 @@ namespace FourSlashInterface {
43594396
export interface CompletionsAtOptions {
43604397
isNewIdentifierLocation?: boolean;
43614398
}
4399+
4400+
export interface VerifyCodeFixOptions {
4401+
description: string;
4402+
// One of these should be defined.
4403+
newFileContent?: string;
4404+
newRangeContent?: string;
4405+
errorCode?: number;
4406+
index?: number;
4407+
}
43624408
}

‎src/services/codefixes/fixAddMissingMember.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,9 @@ namespace ts.codefix {
156156
const propertyChangeTracker = textChanges.ChangeTracker.fromContext(context);
157157
propertyChangeTracker.insertNodeAfter(classDeclarationSourceFile, classOpenBrace, property, { suffix: context.newLineCharacter });
158158

159-
(actions || (actions = [])).push({
160-
description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Declare_property_0), [tokenName]),
159+
const diag = makeStatic ? Diagnostics.Declare_static_property_0 : Diagnostics.Declare_property_0;
160+
actions = append(actions, {
161+
description: formatStringFromArgs(getLocaleSpecificMessage(diag), [tokenName]),
161162
changes: propertyChangeTracker.getChanges()
162163
});
163164

@@ -197,11 +198,9 @@ namespace ts.codefix {
197198

198199
const methodDeclarationChangeTracker = textChanges.ChangeTracker.fromContext(context);
199200
methodDeclarationChangeTracker.insertNodeAfter(classDeclarationSourceFile, classOpenBrace, methodDeclaration, { suffix: context.newLineCharacter });
201+
const diag = makeStatic ? Diagnostics.Declare_static_method_0 : Diagnostics.Declare_method_0;
200202
return {
201-
description: formatStringFromArgs(getLocaleSpecificMessage(makeStatic ?
202-
Diagnostics.Declare_method_0 :
203-
Diagnostics.Declare_static_method_0),
204-
[tokenName]),
203+
description: formatStringFromArgs(getLocaleSpecificMessage(diag), [tokenName]),
205204
changes: methodDeclarationChangeTracker.getChanges()
206205
};
207206
}

‎tests/cases/fourslash/codeFixAddForgottenThis01.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
//// |]}
88
////}
99

10-
verify.rangeAfterCodeFix(`
10+
verify.codeFix({
11+
description: "Add 'this.' to unresolved variable.",
12+
newRangeContent: `
1113
this.foo = 10;
12-
`, /*includeWhitespace*/ true);
14+
`
15+
});

‎tests/cases/fourslash/codeFixAddForgottenThis02.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,7 @@
66
//// bar() { [|foo = 10|] };
77
////}
88

9-
verify.rangeAfterCodeFix("this.foo = 10");
9+
verify.codeFix({
10+
description: "Add 'this.' to unresolved variable.",
11+
newRangeContent: "this.foo = 10",
12+
});

‎tests/cases/fourslash/codeFixAddMissingMember.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
/// <reference path='fourslash.ts' />
22

3-
////[|class C {
3+
////class C {
44
//// method() {
55
//// this.foo = 10;
66
//// }
7-
////}|]
7+
////}
88

9-
verify.rangeAfterCodeFix(`class C {
10-
foo: number;
9+
verify.codeFix({
10+
description: "Declare property 'foo'.",
11+
index: 0,
12+
// TODO: GH#18445
13+
newFileContent: `class C {
14+
foo: number;\r
1115
method() {
1216
this.foo = 10;
1317
}
14-
}`, /*includeWhiteSpace*/false, /*errorCode*/ undefined, /*index*/ 0);
18+
}`
19+
});

‎tests/cases/fourslash/codeFixAddMissingMember2.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
/// <reference path='fourslash.ts' />
22

3-
////[|class C {
3+
////class C {
44
//// method() {
55
//// this.foo = 10;
66
//// }
7-
////}|]
7+
////}
88

9-
verify.rangeAfterCodeFix(`class C {
10-
[x:string]: number;
9+
verify.codeFix({
10+
description: "Add index signature for property 'foo'.",
11+
index: 1,
12+
// TODO: GH#18445
13+
newFileContent: `class C {
14+
[x: string]: number;\r
1115
method() {
1216
this.foo = 10;
1317
}
14-
}`, /*includeWhiteSpace*/false, /*errorCode*/ undefined, /*index*/ 1);
18+
}`
19+
});

‎tests/cases/fourslash/codeFixAddMissingMember3.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
/// <reference path='fourslash.ts' />
22

3-
////[|class C {
3+
////class C {
44
//// static method() {
55
//// this.foo = 10;
66
//// }
7-
////}|]
7+
////}
88

9-
verify.rangeAfterCodeFix(`class C {
10-
static foo: number;
9+
verify.codeFix({
10+
description: "Declare static property 'foo'.",
11+
index: 0,
12+
// TODO: GH#18445
13+
newFileContent: `class C {
14+
static foo: number;\r
1115
static method() {
1216
this.foo = 10;
1317
}
14-
}`, /*includeWhiteSpace*/false, /*errorCode*/ undefined, /*index*/ 0);
18+
}`
19+
});

‎tests/cases/fourslash/codeFixAddMissingMember4.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,25 @@
44
// @allowJs: true
55

66
// @Filename: a.js
7-
////[|class C {
7+
////class C {
88
//// constructor() {
99
//// }
1010
//// method() {
1111
//// this.foo === 10;
1212
//// }
13-
////}|]
13+
////}
1414

15-
verify.rangeAfterCodeFix(`class C {
15+
verify.codeFix({
16+
description: "Initialize property 'foo' in the constructor.",
17+
index: 0,
18+
// TODO: GH#18741 and GH#18445
19+
newFileContent: `class C {
1620
constructor() {
17-
this.foo = undefined;
21+
\r
22+
this.foo = undefined;\r
1823
}
1924
method() {
2025
this.foo === 10;
2126
}
22-
}`, /*includeWhiteSpace*/false, /*errorCode*/ undefined, /*index*/ 0);
27+
}`
28+
});

‎tests/cases/fourslash/codeFixAddMissingMember5.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,20 @@
44
// @allowJs: true
55

66
// @Filename: a.js
7-
////[|class C {
7+
////class C {
88
//// static method() {
99
//// ()=>{ this.foo === 10 };
1010
//// }
1111
////}
12-
////|]
1312

14-
verify.getAndApplyCodeFix(/*errorCode*/ undefined, /*index*/ 0);
15-
verify.currentFileContentIs(`class C {
13+
verify.codeFix({
14+
description: "Initialize static property 'foo'.",
15+
index: 0,
16+
// TODO: GH#18743 and GH#18445
17+
newFileContent: `class C {
1618
static method() {
1719
()=>{ this.foo === 10 };
1820
}
19-
}
20-
C.foo = undefined;` + "\r\n"); // TODO: GH#18445
21+
}C.foo = undefined;\r
22+
`
23+
});

0 commit comments

Comments
 (0)