From 76c6ee6e5362fdc894ace3884502e4881ddb0d6b Mon Sep 17 00:00:00 2001 From: iliashkolyar Date: Tue, 11 Sep 2018 08:04:09 +0300 Subject: [PATCH 1/4] Codefix: add quick fix for missing 'new' operator --- src/compiler/diagnosticMessages.json | 15 +++++++--- .../codefixes/fixAddMissingNewOperator.ts | 29 +++++++++++++++++++ src/services/tsconfig.json | 1 + tests/cases/fourslash/codeFixAddMissingNew.ts | 13 +++++++++ .../fourslash/codeFixAddMissingNew_all.ts | 18 ++++++++++++ 5 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 src/services/codefixes/fixAddMissingNewOperator.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingNew.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingNew_all.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 7a57b89d32..f4ba04df38 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4608,7 +4608,6 @@ "category": "Message", "code": 95062 }, - "Add missing enum member '{0}'": { "category": "Message", "code": 95063 @@ -4619,10 +4618,18 @@ }, "Convert to async function":{ "category": "Message", - "code": 95065 + "code": 95065 }, "Convert all to async functions": { - "category": "Message", - "code": 95066 + "category": "Message", + "code": 95066 + }, + "Add missing 'new' operator to caller": { + "category": "Message", + "code": 95067 + }, + "Add missing 'new' operator to all callers": { + "category": "Message", + "code": 95068 } } diff --git a/src/services/codefixes/fixAddMissingNewOperator.ts b/src/services/codefixes/fixAddMissingNewOperator.ts new file mode 100644 index 0000000000..b1eca15e18 --- /dev/null +++ b/src/services/codefixes/fixAddMissingNewOperator.ts @@ -0,0 +1,29 @@ +/* @internal */ +namespace ts.codefix { + const fixId = "addMissingNewOperator"; + const errorCodes = [Diagnostics.Value_of_type_0_is_not_callable_Did_you_mean_to_include_new.code]; + registerCodeFix({ + errorCodes, + getCodeActions(context) { + const { sourceFile, span } = context; + const identifierWithoutNew = getIdentifier(sourceFile, span.start); + const changes = textChanges.ChangeTracker.with(context, t => addMissingNewOperator(t, sourceFile, identifierWithoutNew)); + return [createCodeFixAction(fixId, changes, Diagnostics.Add_missing_new_operator_to_caller, fixId, Diagnostics.Add_missing_new_operator_to_all_callers)]; + }, + fixIds: [fixId], + getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => + addMissingNewOperator(changes, context.sourceFile, getIdentifier(diag.file, diag.start))), + }); + + function getIdentifier(sourceFile: SourceFile, pos: number): Identifier { + const token = getTokenAtPosition(sourceFile, pos); + Debug.assert(token.kind === SyntaxKind.Identifier); + Debug.assert(isCallExpression(token.parent)); + return token; + } + + function addMissingNewOperator(changes: textChanges.ChangeTracker, sourceFile: SourceFile, identifierWithoutNew: Identifier): void { + const newTypeNode = createNew(identifierWithoutNew, /*typeArguments*/ undefined, /*argumentsArray*/ undefined); + changes.replaceNode(sourceFile, identifierWithoutNew, newTypeNode); + } +} diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index fee6da0f8a..6ea6f09b9f 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -53,6 +53,7 @@ "codefixes/importFixes.ts", "codefixes/fixSpelling.ts", "codefixes/fixAddMissingMember.ts", + "codefixes/fixAddMissingNewOperator.ts", "codefixes/fixCannotFindModule.ts", "codefixes/fixClassDoesntImplementInheritedAbstractMember.ts", "codefixes/fixClassSuperMustPrecedeThisAccess.ts", diff --git a/tests/cases/fourslash/codeFixAddMissingNew.ts b/tests/cases/fourslash/codeFixAddMissingNew.ts new file mode 100644 index 0000000000..ee53938775 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingNew.ts @@ -0,0 +1,13 @@ +/// + +////class C { +////} +////var c = C(); + +verify.codeFix({ + description: "Add missing 'new' operator to caller", + index: 0, + newFileContent: `class C { +} +var c = new C();` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingNew_all.ts b/tests/cases/fourslash/codeFixAddMissingNew_all.ts new file mode 100644 index 0000000000..e8cca308c0 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingNew_all.ts @@ -0,0 +1,18 @@ +/// + +////class C { +//// constructor(num?: number) {} +////} +////var a = C(); +////var b = C(3); + +verify.codeFixAll({ + fixId: "addMissingNewOperator", + fixAllDescription: "Add missing 'new' operator to all callers", + newFileContent: +`class C { + constructor(num?: number) {} +} +var a = new C(); +var b = new C(3);` +}); From 8b14fb22ecf22aa5b95f66dee011c69686b458b4 Mon Sep 17 00:00:00 2001 From: iliashkolyar Date: Tue, 11 Sep 2018 13:56:51 +0300 Subject: [PATCH 2/4] Initial review --- src/compiler/diagnosticMessages.json | 4 ++-- .../codefixes/fixAddMissingNewOperator.ts | 19 ++++++++-------- tests/cases/fourslash/codeFixAddMissingNew.ts | 2 +- .../fourslash/codeFixAddMissingNew_all.ts | 2 +- .../codeFixAddMissingNew_all_arguments.ts | 22 +++++++++++++++++++ 5 files changed, 35 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingNew_all_arguments.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index f4ba04df38..2464b24a89 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4624,11 +4624,11 @@ "category": "Message", "code": 95066 }, - "Add missing 'new' operator to caller": { + "Add missing 'new' operator to call": { "category": "Message", "code": 95067 }, - "Add missing 'new' operator to all callers": { + "Add missing 'new' operator to all calls": { "category": "Message", "code": 95068 } diff --git a/src/services/codefixes/fixAddMissingNewOperator.ts b/src/services/codefixes/fixAddMissingNewOperator.ts index b1eca15e18..8b01ccd781 100644 --- a/src/services/codefixes/fixAddMissingNewOperator.ts +++ b/src/services/codefixes/fixAddMissingNewOperator.ts @@ -6,24 +6,23 @@ namespace ts.codefix { errorCodes, getCodeActions(context) { const { sourceFile, span } = context; - const identifierWithoutNew = getIdentifier(sourceFile, span.start); - const changes = textChanges.ChangeTracker.with(context, t => addMissingNewOperator(t, sourceFile, identifierWithoutNew)); - return [createCodeFixAction(fixId, changes, Diagnostics.Add_missing_new_operator_to_caller, fixId, Diagnostics.Add_missing_new_operator_to_all_callers)]; + const missingNewExpression = getMissingNewExpression(sourceFile, span.start); + const changes = textChanges.ChangeTracker.with(context, t => addMissingNewOperator(t, sourceFile, missingNewExpression)); + return [createCodeFixAction(fixId, changes, Diagnostics.Add_missing_new_operator_to_call, fixId, Diagnostics.Add_missing_new_operator_to_all_calls)]; }, fixIds: [fixId], getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => - addMissingNewOperator(changes, context.sourceFile, getIdentifier(diag.file, diag.start))), + addMissingNewOperator(changes, context.sourceFile, getMissingNewExpression(diag.file, diag.start))), }); - function getIdentifier(sourceFile: SourceFile, pos: number): Identifier { + function getMissingNewExpression(sourceFile: SourceFile, pos: number): Expression { const token = getTokenAtPosition(sourceFile, pos); - Debug.assert(token.kind === SyntaxKind.Identifier); Debug.assert(isCallExpression(token.parent)); - return token; + return token; } - function addMissingNewOperator(changes: textChanges.ChangeTracker, sourceFile: SourceFile, identifierWithoutNew: Identifier): void { - const newTypeNode = createNew(identifierWithoutNew, /*typeArguments*/ undefined, /*argumentsArray*/ undefined); - changes.replaceNode(sourceFile, identifierWithoutNew, newTypeNode); + function addMissingNewOperator(changes: textChanges.ChangeTracker, sourceFile: SourceFile, missingNewExpression: Expression): void { + const newTypeNode = createNew(missingNewExpression, /*typeArguments*/ undefined, /*argumentsArray*/ undefined); + changes.replaceNode(sourceFile, missingNewExpression, newTypeNode); } } diff --git a/tests/cases/fourslash/codeFixAddMissingNew.ts b/tests/cases/fourslash/codeFixAddMissingNew.ts index ee53938775..84ac58280e 100644 --- a/tests/cases/fourslash/codeFixAddMissingNew.ts +++ b/tests/cases/fourslash/codeFixAddMissingNew.ts @@ -5,7 +5,7 @@ ////var c = C(); verify.codeFix({ - description: "Add missing 'new' operator to caller", + description: "Add missing 'new' operator to call", index: 0, newFileContent: `class C { } diff --git a/tests/cases/fourslash/codeFixAddMissingNew_all.ts b/tests/cases/fourslash/codeFixAddMissingNew_all.ts index e8cca308c0..019f5b5a5a 100644 --- a/tests/cases/fourslash/codeFixAddMissingNew_all.ts +++ b/tests/cases/fourslash/codeFixAddMissingNew_all.ts @@ -8,7 +8,7 @@ verify.codeFixAll({ fixId: "addMissingNewOperator", - fixAllDescription: "Add missing 'new' operator to all callers", + fixAllDescription: "Add missing 'new' operator to all calls", newFileContent: `class C { constructor(num?: number) {} diff --git a/tests/cases/fourslash/codeFixAddMissingNew_all_arguments.ts b/tests/cases/fourslash/codeFixAddMissingNew_all_arguments.ts new file mode 100644 index 0000000000..130dfd28fd --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingNew_all_arguments.ts @@ -0,0 +1,22 @@ +/// + +////class C { +//// x?: T; +//// constructor(x: T) { this.x = x; } +////} +////let a = C(1, 2, 3); +////let b = C("hello"); +////let c = C(); + +verify.codeFixAll({ + fixId: "addMissingNewOperator", + fixAllDescription: "Add missing 'new' operator to all calls", + newFileContent: +`class C { + x?: T; + constructor(x: T) { this.x = x; } +} +let a = new C(1, 2, 3); +let b = new C("hello"); +let c = new C();` +}); From 7c15378376a3ae05e1e553a9af56ce63c10d22c6 Mon Sep 17 00:00:00 2001 From: iliashkolyar Date: Thu, 13 Sep 2018 08:30:43 +0300 Subject: [PATCH 3/4] InsertNode instead of replace + handling call expressions --- .../codefixes/fixAddMissingNewOperator.ts | 26 ++++++++++++------- tests/cases/fourslash/codeFixAddMissingNew.ts | 3 ++- .../cases/fourslash/codeFixAddMissingNew2.ts | 14 ++++++++++ .../cases/fourslash/codeFixAddMissingNew3.ts | 16 ++++++++++++ .../cases/fourslash/codeFixAddMissingNew4.ts | 18 +++++++++++++ 5 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingNew2.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingNew3.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingNew4.ts diff --git a/src/services/codefixes/fixAddMissingNewOperator.ts b/src/services/codefixes/fixAddMissingNewOperator.ts index 8b01ccd781..ce68659be7 100644 --- a/src/services/codefixes/fixAddMissingNewOperator.ts +++ b/src/services/codefixes/fixAddMissingNewOperator.ts @@ -6,23 +6,29 @@ namespace ts.codefix { errorCodes, getCodeActions(context) { const { sourceFile, span } = context; - const missingNewExpression = getMissingNewExpression(sourceFile, span.start); - const changes = textChanges.ChangeTracker.with(context, t => addMissingNewOperator(t, sourceFile, missingNewExpression)); + const changes = textChanges.ChangeTracker.with(context, t => addMissingNewOperator(t, sourceFile, span)); return [createCodeFixAction(fixId, changes, Diagnostics.Add_missing_new_operator_to_call, fixId, Diagnostics.Add_missing_new_operator_to_all_calls)]; }, fixIds: [fixId], getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => - addMissingNewOperator(changes, context.sourceFile, getMissingNewExpression(diag.file, diag.start))), + addMissingNewOperator(changes, context.sourceFile, diag)), }); - function getMissingNewExpression(sourceFile: SourceFile, pos: number): Expression { - const token = getTokenAtPosition(sourceFile, pos); - Debug.assert(isCallExpression(token.parent)); - return token; + function addMissingNewOperator(changes: textChanges.ChangeTracker, sourceFile: SourceFile, span: TextSpan): void { + const call = cast(findAncestorMatchingSpan(sourceFile, span), isCallExpression); + changes.insertNodeAt(sourceFile, span.start, createToken(SyntaxKind.NewKeyword), { suffix: " " }); + if (isCallExpression(call.expression)) { + changes.insertNodeAt(sourceFile, call.expression.getStart(sourceFile), createToken(SyntaxKind.OpenParenToken)); + changes.insertNodeAt(sourceFile, call.expression.end, createToken(SyntaxKind.CloseParenToken)); + } } - function addMissingNewOperator(changes: textChanges.ChangeTracker, sourceFile: SourceFile, missingNewExpression: Expression): void { - const newTypeNode = createNew(missingNewExpression, /*typeArguments*/ undefined, /*argumentsArray*/ undefined); - changes.replaceNode(sourceFile, missingNewExpression, newTypeNode); + function findAncestorMatchingSpan(sourceFile: SourceFile, span: TextSpan): Node { + let token = getTokenAtPosition(sourceFile, span.start); + const end = textSpanEnd(span); + while (token.end < end) { + token = token.parent; + } + return token; } } diff --git a/tests/cases/fourslash/codeFixAddMissingNew.ts b/tests/cases/fourslash/codeFixAddMissingNew.ts index 84ac58280e..ff1ce125c9 100644 --- a/tests/cases/fourslash/codeFixAddMissingNew.ts +++ b/tests/cases/fourslash/codeFixAddMissingNew.ts @@ -7,7 +7,8 @@ verify.codeFix({ description: "Add missing 'new' operator to call", index: 0, - newFileContent: `class C { + newFileContent: +`class C { } var c = new C();` }); diff --git a/tests/cases/fourslash/codeFixAddMissingNew2.ts b/tests/cases/fourslash/codeFixAddMissingNew2.ts new file mode 100644 index 0000000000..a90b1c580d --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingNew2.ts @@ -0,0 +1,14 @@ +/// + +////class C { +////} +////let x = (() => C)()(); + +verify.codeFix({ + description: "Add missing 'new' operator to call", + index: 0, + newFileContent: +`class C { +} +let x = new ((() => C)())();` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingNew3.ts b/tests/cases/fourslash/codeFixAddMissingNew3.ts new file mode 100644 index 0000000000..c420d0446b --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingNew3.ts @@ -0,0 +1,16 @@ +/// + +////class C { +////} +////let x = [C]; +////let a = x[0](); + +verify.codeFix({ + description: "Add missing 'new' operator to call", + index: 0, + newFileContent: +`class C { +} +let x = [C]; +let a = new x[0]();` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingNew4.ts b/tests/cases/fourslash/codeFixAddMissingNew4.ts new file mode 100644 index 0000000000..e216b28bf5 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingNew4.ts @@ -0,0 +1,18 @@ +/// + +////class C { +////} +////class D { +////} +////let x = (true ? C : D)(); + +verify.codeFix({ + description: "Add missing 'new' operator to call", + index: 0, + newFileContent: +`class C { +} +class D { +} +let x = new (true ? C : D)();` +}); From 6bd9b766b38939a5fdcfd732dbcd690eb6e30e0a Mon Sep 17 00:00:00 2001 From: iliashkolyar Date: Sun, 7 Oct 2018 22:19:09 +0300 Subject: [PATCH 4/4] Code review - remove 'isCallExpression' check --- .../codefixes/fixAddMissingNewOperator.ts | 8 +++--- .../cases/fourslash/codeFixAddMissingNew5.ts | 25 +++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingNew5.ts diff --git a/src/services/codefixes/fixAddMissingNewOperator.ts b/src/services/codefixes/fixAddMissingNewOperator.ts index ce68659be7..5acaf6eb82 100644 --- a/src/services/codefixes/fixAddMissingNewOperator.ts +++ b/src/services/codefixes/fixAddMissingNewOperator.ts @@ -16,11 +16,9 @@ namespace ts.codefix { function addMissingNewOperator(changes: textChanges.ChangeTracker, sourceFile: SourceFile, span: TextSpan): void { const call = cast(findAncestorMatchingSpan(sourceFile, span), isCallExpression); - changes.insertNodeAt(sourceFile, span.start, createToken(SyntaxKind.NewKeyword), { suffix: " " }); - if (isCallExpression(call.expression)) { - changes.insertNodeAt(sourceFile, call.expression.getStart(sourceFile), createToken(SyntaxKind.OpenParenToken)); - changes.insertNodeAt(sourceFile, call.expression.end, createToken(SyntaxKind.CloseParenToken)); - } + const newExpression = createNew(call.expression, call.typeArguments, call.arguments); + + changes.replaceNode(sourceFile, call, newExpression); } function findAncestorMatchingSpan(sourceFile: SourceFile, span: TextSpan): Node { diff --git a/tests/cases/fourslash/codeFixAddMissingNew5.ts b/tests/cases/fourslash/codeFixAddMissingNew5.ts new file mode 100644 index 0000000000..ecc5509a18 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingNew5.ts @@ -0,0 +1,25 @@ +/// + +////class C { +////} +//// +////function foo() { +//// return C; +////} +//// +////foo()!(); + + +verify.codeFix({ + description: "Add missing 'new' operator to call", + index: 0, + newFileContent: +`class C { +} + +function foo() { + return C; +} + +new (foo()!)();` +});