Skip to content

Commit dba042d

Browse files
rbucktonrbuckton
authored
Add quick fix to add 'void' to Promise resolved without value (microsoft#40558)
* Add codefix to add 'void' to Promise resolved without value * Add specific error message in checker to reduce quick-fix time in editor
1 parent 7db9118 commit dba042d

20 files changed

+322
-18
lines changed

‎src/compiler/checker.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26708,6 +26708,22 @@ namespace ts {
2670826708
}
2670926709
}
2671026710

26711+
function isPromiseResolveArityError(node: CallLikeExpression) {
26712+
if (!isCallExpression(node) || !isIdentifier(node.expression)) return false;
26713+
26714+
const symbol = resolveName(node.expression, node.expression.escapedText, SymbolFlags.Value, undefined, undefined, false);
26715+
const decl = symbol?.valueDeclaration;
26716+
if (!decl || !isParameter(decl) || !isFunctionExpressionOrArrowFunction(decl.parent) || !isNewExpression(decl.parent.parent) || !isIdentifier(decl.parent.parent.expression)) {
26717+
return false;
26718+
}
26719+
26720+
const globalPromiseSymbol = getGlobalPromiseConstructorSymbol(/*reportErrors*/ false);
26721+
if (!globalPromiseSymbol) return false;
26722+
26723+
const constructorSymbol = getSymbolAtLocation(decl.parent.parent.expression, /*ignoreErrors*/ true);
26724+
return constructorSymbol === globalPromiseSymbol;
26725+
}
26726+
2671126727
function getArgumentArityError(node: CallLikeExpression, signatures: readonly Signature[], args: readonly Expression[]) {
2671226728
let min = Number.POSITIVE_INFINITY;
2671326729
let max = Number.NEGATIVE_INFINITY;
@@ -26740,9 +26756,15 @@ namespace ts {
2674026756
let spanArray: NodeArray<Node>;
2674126757
let related: DiagnosticWithLocation | undefined;
2674226758

26743-
const error = hasRestParameter || hasSpreadArgument ? hasRestParameter && hasSpreadArgument ? Diagnostics.Expected_at_least_0_arguments_but_got_1_or_more :
26744-
hasRestParameter ? Diagnostics.Expected_at_least_0_arguments_but_got_1 :
26745-
Diagnostics.Expected_0_arguments_but_got_1_or_more : Diagnostics.Expected_0_arguments_but_got_1;
26759+
const error = hasRestParameter || hasSpreadArgument ?
26760+
hasRestParameter && hasSpreadArgument ?
26761+
Diagnostics.Expected_at_least_0_arguments_but_got_1_or_more :
26762+
hasRestParameter ?
26763+
Diagnostics.Expected_at_least_0_arguments_but_got_1 :
26764+
Diagnostics.Expected_0_arguments_but_got_1_or_more :
26765+
paramRange === 1 && argCount === 0 && isPromiseResolveArityError(node) ?
26766+
Diagnostics.Expected_0_arguments_but_got_1_Did_you_forget_to_include_void_in_your_type_argument_to_Promise :
26767+
Diagnostics.Expected_0_arguments_but_got_1;
2674626768

2674726769
if (closestSignature && getMinArgumentCount(closestSignature) > argCount && closestSignature.declaration) {
2674826770
const paramDecl = closestSignature.declaration.parameters[closestSignature.thisParameter ? argCount + 1 : argCount];

‎src/compiler/diagnosticMessages.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3039,6 +3039,10 @@
30393039
"category": "Error",
30403040
"code": 2793
30413041
},
3042+
"Expected {0} arguments, but got {1}. Did you forget to include 'void' in your type argument to 'Promise'?": {
3043+
"category": "Error",
3044+
"code": 2794
3045+
},
30423046

30433047
"Import declaration '{0}' is using private name '{1}'.": {
30443048
"category": "Error",
@@ -5927,6 +5931,14 @@
59275931
"category": "Message",
59285932
"code": 95142
59295933
},
5934+
"Add 'void' to Promise resolved without a value": {
5935+
"category": "Message",
5936+
"code": 95143
5937+
},
5938+
"Add 'void' to all Promises resolved without a value": {
5939+
"category": "Message",
5940+
"code": 95144
5941+
},
59305942

59315943
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
59325944
"category": "Error",

‎src/harness/fourslashImpl.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -339,17 +339,23 @@ namespace FourSlash {
339339
this.languageServiceAdapterHost.addScript(fileName, file, /*isRootFile*/ true);
340340
}
341341
});
342-
if (!compilationOptions.noLib) {
343-
this.languageServiceAdapterHost.addScript(Harness.Compiler.defaultLibFileName,
344-
Harness.Compiler.getDefaultLibrarySourceFile()!.text, /*isRootFile*/ false);
345342

346-
compilationOptions.lib?.forEach(fileName => {
343+
if (!compilationOptions.noLib) {
344+
const seen = new Set<string>();
345+
const addSourceFile = (fileName: string) => {
346+
if (seen.has(fileName)) return;
347+
seen.add(fileName);
347348
const libFile = Harness.Compiler.getDefaultLibrarySourceFile(fileName);
348349
ts.Debug.assertIsDefined(libFile, `Could not find lib file '${fileName}'`);
349-
if (libFile) {
350-
this.languageServiceAdapterHost.addScript(fileName, libFile.text, /*isRootFile*/ false);
350+
this.languageServiceAdapterHost.addScript(fileName, libFile.text, /*isRootFile*/ false);
351+
if (!ts.some(libFile.libReferenceDirectives)) return;
352+
for (const directive of libFile.libReferenceDirectives) {
353+
addSourceFile(`lib.${directive.fileName}.d.ts`);
351354
}
352-
});
355+
};
356+
357+
addSourceFile(Harness.Compiler.defaultLibFileName);
358+
compilationOptions.lib?.forEach(addSourceFile);
353359
}
354360
}
355361

@@ -3878,7 +3884,7 @@ namespace FourSlash {
38783884
const testData = parseTestData(absoluteBasePath, content, absoluteFileName);
38793885
const state = new TestState(absoluteFileName, absoluteBasePath, testType, testData);
38803886
const actualFileName = Harness.IO.resolvePath(fileName) || absoluteFileName;
3881-
const output = ts.transpileModule(content, { reportDiagnostics: true, fileName: actualFileName, compilerOptions: { target: ts.ScriptTarget.ES2015, inlineSourceMap: true } });
3887+
const output = ts.transpileModule(content, { reportDiagnostics: true, fileName: actualFileName, compilerOptions: { target: ts.ScriptTarget.ES2015, inlineSourceMap: true, inlineSources: true } });
38823888
if (output.diagnostics!.length > 0) {
38833889
throw new Error(`Syntax error in ${absoluteBasePath}: ${output.diagnostics![0].messageText}`);
38843890
}
@@ -3888,7 +3894,7 @@ namespace FourSlash {
38883894
function runCode(code: string, state: TestState, fileName: string): void {
38893895
// Compile and execute the test
38903896
const generatedFile = ts.changeExtension(fileName, ".js");
3891-
const wrappedCode = `(function(test, goTo, plugins, verify, edit, debug, format, cancellation, classification, completion, verifyOperationIsCancelled) {${code}\n//# sourceURL=${generatedFile}\n})`;
3897+
const wrappedCode = `(function(test, goTo, plugins, verify, edit, debug, format, cancellation, classification, completion, verifyOperationIsCancelled) {${code}\n//# sourceURL=${ts.getBaseFileName(generatedFile)}\n})`;
38923898

38933899
type SourceMapSupportModule = typeof import("source-map-support") & {
38943900
// TODO(rbuckton): This is missing from the DT definitions and needs to be added.

‎src/services/codefixes/fixAddVoidToPromise.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/* @internal */
2+
namespace ts.codefix {
3+
const fixName = "addVoidToPromise";
4+
const fixId = "addVoidToPromise";
5+
const errorCodes = [
6+
Diagnostics.Expected_0_arguments_but_got_1_Did_you_forget_to_include_void_in_your_type_argument_to_Promise.code
7+
];
8+
registerCodeFix({
9+
errorCodes,
10+
fixIds: [fixId],
11+
getCodeActions(context) {
12+
const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span, context.program));
13+
if (changes.length > 0) {
14+
return [createCodeFixAction(fixName, changes, Diagnostics.Add_void_to_Promise_resolved_without_a_value, fixId, Diagnostics.Add_void_to_all_Promises_resolved_without_a_value)];
15+
}
16+
},
17+
getAllCodeActions(context: CodeFixAllContext) {
18+
return codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag, context.program, new Set()));
19+
}
20+
});
21+
22+
function makeChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, span: TextSpan, program: Program, seen?: Set<ParameterDeclaration>) {
23+
const node = getTokenAtPosition(sourceFile, span.start);
24+
if (!isIdentifier(node) || !isCallExpression(node.parent) || node.parent.expression !== node || node.parent.arguments.length !== 0) return;
25+
26+
const checker = program.getTypeChecker();
27+
const symbol = checker.getSymbolAtLocation(node);
28+
29+
// decl should be `new Promise((<decl>) => {})`
30+
const decl = symbol?.valueDeclaration;
31+
if (!decl || !isParameter(decl) || !isNewExpression(decl.parent.parent)) return;
32+
33+
// no need to make this change if we have already seen this parameter.
34+
if (seen?.has(decl)) return;
35+
seen?.add(decl);
36+
37+
const typeArguments = getEffectiveTypeArguments(decl.parent.parent);
38+
if (some(typeArguments)) {
39+
// append ` | void` to type argument
40+
const typeArgument = typeArguments[0];
41+
const needsParens = !isUnionTypeNode(typeArgument) && !isParenthesizedTypeNode(typeArgument) &&
42+
isParenthesizedTypeNode(factory.createUnionTypeNode([typeArgument, factory.createKeywordTypeNode(SyntaxKind.VoidKeyword)]).types[0]);
43+
if (needsParens) {
44+
changes.insertText(sourceFile, typeArgument.pos, "(");
45+
}
46+
changes.insertText(sourceFile, typeArgument.end, needsParens ? ") | void" : " | void");
47+
}
48+
else {
49+
// make sure the Promise is type is untyped (i.e., `unknown`)
50+
const signature = checker.getResolvedSignature(node.parent);
51+
const parameter = signature?.parameters[0];
52+
const parameterType = parameter && checker.getTypeOfSymbolAtLocation(parameter, decl.parent.parent);
53+
if (isInJSFile(decl)) {
54+
if (!parameterType || parameterType.flags & TypeFlags.AnyOrUnknown) {
55+
// give the expression a type
56+
changes.insertText(sourceFile, decl.parent.parent.end, `)`);
57+
changes.insertText(sourceFile, skipTrivia(sourceFile.text, decl.parent.parent.pos), `/** @type {Promise<void>} */(`);
58+
}
59+
}
60+
else {
61+
if (!parameterType || parameterType.flags & TypeFlags.Unknown) {
62+
// add `void` type argument
63+
changes.insertText(sourceFile, decl.parent.parent.expression.end, "<void>");
64+
}
65+
}
66+
}
67+
}
68+
69+
function getEffectiveTypeArguments(node: NewExpression) {
70+
if (isInJSFile(node)) {
71+
if (isParenthesizedExpression(node.parent)) {
72+
const jsDocType = getJSDocTypeTag(node.parent)?.typeExpression.type;
73+
if (jsDocType && isTypeReferenceNode(jsDocType) && isIdentifier(jsDocType.typeName) && idText(jsDocType.typeName) === "Promise") {
74+
return jsDocType.typeArguments;
75+
}
76+
}
77+
}
78+
else {
79+
return node.typeArguments;
80+
}
81+
}
82+
}

‎src/services/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
"codefixes/splitTypeOnlyImport.ts",
108108
"codefixes/convertConstToLet.ts",
109109
"codefixes/fixExpectedComma.ts",
110+
"codefixes/fixAddVoidToPromise.ts",
110111
"refactors/convertExport.ts",
111112
"refactors/convertImport.ts",
112113
"refactors/convertToOptionalChainExpression.ts",

‎tests/cases/fourslash/codeFixAddVoidToPromise.1.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
7+
////const p1 = new Promise(resolve => resolve());
8+
9+
verify.codeFix({
10+
errorCode: 2794,
11+
description: "Add 'void' to Promise resolved without a value",
12+
index: 0,
13+
newFileContent: `const p1 = new Promise<void>(resolve => resolve());`
14+
});

‎tests/cases/fourslash/codeFixAddVoidToPromise.2.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
7+
////const p2 = new Promise<number>(resolve => resolve());
8+
9+
verify.codeFix({
10+
errorCode: 2794,
11+
description: "Add 'void' to Promise resolved without a value",
12+
index: 0,
13+
newFileContent: `const p2 = new Promise<number | void>(resolve => resolve());`
14+
});

‎tests/cases/fourslash/codeFixAddVoidToPromise.3.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
7+
////const p3 = new Promise<number | string>(resolve => resolve());
8+
9+
verify.codeFix({
10+
errorCode: 2794,
11+
description: "Add 'void' to Promise resolved without a value",
12+
index: 0,
13+
newFileContent: `const p3 = new Promise<number | string | void>(resolve => resolve());`
14+
});

‎tests/cases/fourslash/codeFixAddVoidToPromise.4.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
7+
////const p4 = new Promise<{ x: number } & { y: string }>(resolve => resolve());
8+
9+
verify.codeFix({
10+
errorCode: 2794,
11+
description: "Add 'void' to Promise resolved without a value",
12+
index: 0,
13+
newFileContent: `const p4 = new Promise<({ x: number } & { y: string }) | void>(resolve => resolve());`
14+
});

‎tests/cases/fourslash/codeFixAddVoidToPromise.5.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
7+
////const p4: Promise<number> = new Promise(resolve => resolve());
8+
9+
verify.not.codeFixAvailable();

‎tests/cases/fourslash/codeFixAddVoidToPromiseJS.1.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
// @allowJS: true
7+
// @checkJS: true
8+
// @filename: main.js
9+
////const p1 = new Promise(resolve => resolve());
10+
11+
verify.codeFix({
12+
errorCode: 2794,
13+
description: "Add 'void' to Promise resolved without a value",
14+
index: 2,
15+
newFileContent: `const p1 = /** @type {Promise<void>} */(new Promise(resolve => resolve()));`
16+
});

‎tests/cases/fourslash/codeFixAddVoidToPromiseJS.2.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
// @allowJS: true
7+
// @checkJS: true
8+
// @filename: main.js
9+
////const p2 = /** @type {Promise<number>} */(new Promise(resolve => resolve()));
10+
11+
verify.codeFix({
12+
errorCode: 2794,
13+
description: "Add 'void' to Promise resolved without a value",
14+
index: 2,
15+
newFileContent: `const p2 = /** @type {Promise<number | void>} */(new Promise(resolve => resolve()));`
16+
});

‎tests/cases/fourslash/codeFixAddVoidToPromiseJS.3.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
// @allowJS: true
7+
// @checkJS: true
8+
// @filename: main.js
9+
////const p3 = /** @type {Promise<number | string>} */(new Promise(resolve => resolve()));
10+
11+
verify.codeFix({
12+
errorCode: 2794,
13+
description: "Add 'void' to Promise resolved without a value",
14+
index: 2,
15+
newFileContent: `const p3 = /** @type {Promise<number | string | void>} */(new Promise(resolve => resolve()));`
16+
});

‎tests/cases/fourslash/codeFixAddVoidToPromiseJS.4.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
// @allowJS: true
7+
// @checkJS: true
8+
// @filename: main.js
9+
////const p4 = /** @type {Promise<{ x: number } & { y: string }>} */(new Promise(resolve => resolve()));
10+
11+
verify.codeFix({
12+
errorCode: 2794,
13+
description: "Add 'void' to Promise resolved without a value",
14+
index: 2,
15+
newFileContent: `const p4 = /** @type {Promise<({ x: number } & { y: string }) | void>} */(new Promise(resolve => resolve()));`
16+
});

‎tests/cases/fourslash/codeFixAddVoidToPromiseJS.5.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
// @allowJS: true
7+
// @checkJS: true
8+
// @filename: main.js
9+
/////** @type {Promise<number>} */
10+
////const p2 = new Promise(resolve => resolve());
11+
12+
verify.not.codeFixAvailable("Add 'void' to Promise resolved without a value");

‎tests/cases/fourslash/codeFixAddVoidToPromiseJS_all.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @target: esnext
4+
// @lib: es2015
5+
// @strict: true
6+
// @allowJS: true
7+
// @checkJS: true
8+
// @filename: main.js
9+
////const p1 = new Promise(resolve => resolve());
10+
////const p2 = /** @type {Promise<number>} */(new Promise(resolve => resolve()));
11+
////const p3 = /** @type {Promise<number | string>} */(new Promise(resolve => resolve()));
12+
////const p4 = /** @type {Promise<{ x: number } & { y: string }>} */(new Promise(resolve => resolve()));
13+
14+
verify.codeFixAll({
15+
fixId: "addVoidToPromise",
16+
fixAllDescription: ts.Diagnostics.Add_void_to_all_Promises_resolved_without_a_value.message,
17+
newFileContent: `const p1 = /** @type {Promise<void>} */(new Promise(resolve => resolve()));
18+
const p2 = /** @type {Promise<number | void>} */(new Promise(resolve => resolve()));
19+
const p3 = /** @type {Promise<number | string | void>} */(new Promise(resolve => resolve()));
20+
const p4 = /** @type {Promise<({ x: number } & { y: string }) | void>} */(new Promise(resolve => resolve()));`
21+
});

0 commit comments

Comments
 (0)