From afae97ce6ec84f57daebc16491025f43857a4b92 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 3 Jan 2018 12:37:43 -0800 Subject: [PATCH 1/2] Refine extends-to-implements code fix 1. Retain surrounding trivia when swapping the keyword. 2. Insert commas at the full-starts, rather than starts, of existing keywords when merging with existing implements clauses. Fixes #18794 --- .../fixExtendsInterfaceBecomesImplements.ts | 15 +++++++++++++-- ...angeExtendsToImplementsAbstractModifier.ts | 2 +- ...xChangeExtendsToImplementsWithDecorator.ts | 2 +- ...eFixChangeExtendsToImplementsWithTrivia.ts | 19 +++++++++++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 tests/cases/fourslash/codeFixChangeExtendsToImplementsWithTrivia.ts diff --git a/src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts b/src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts index 446b0bddfab..cfa1a288ca8 100644 --- a/src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts +++ b/src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts @@ -27,11 +27,22 @@ namespace ts.codefix { } function doChanges(changes: textChanges.ChangeTracker, sourceFile: SourceFile, extendsToken: Node, heritageClauses: ReadonlyArray): void { - changes.replaceNode(sourceFile, extendsToken, createToken(SyntaxKind.ImplementsKeyword)); + changes.replaceRange(sourceFile, { pos: extendsToken.getStart(), end: extendsToken.end }, createToken(SyntaxKind.ImplementsKeyword)); // We replace existing keywords with commas. for (let i = 1; i < heritageClauses.length; i++) { const keywordToken = heritageClauses[i].getFirstToken()!; - changes.replaceNode(sourceFile, keywordToken, createToken(SyntaxKind.CommaToken)); + const keywordFullStart = keywordToken.getFullStart(); + changes.replaceRange(sourceFile, { pos: keywordFullStart, end: keywordFullStart }, createToken(SyntaxKind.CommaToken)); + + // Rough heuristic: delete trailing whitespace after keyword so that it's not excessive. + // (Trailing because leading might be indentation, which is more sensitive.) + const text = sourceFile.text; + let end = keywordToken.end; + while (end < text.length && ts.isWhiteSpaceSingleLine(text.charCodeAt(end))) { + end++; + } + + changes.deleteRange(sourceFile, { pos: keywordToken.getStart(), end }); } } } diff --git a/tests/cases/fourslash/codeFixChangeExtendsToImplementsAbstractModifier.ts b/tests/cases/fourslash/codeFixChangeExtendsToImplementsAbstractModifier.ts index 9bec3df4f41..0a7ae3e0e68 100644 --- a/tests/cases/fourslash/codeFixChangeExtendsToImplementsAbstractModifier.ts +++ b/tests/cases/fourslash/codeFixChangeExtendsToImplementsAbstractModifier.ts @@ -8,5 +8,5 @@ verify.codeFix({ description: "Change 'extends' to 'implements'", // TODO: GH#18794 - newRangeContent: "abstract class A implements I1 , I2", + newRangeContent: "abstract class A implements I1, I2", }); diff --git a/tests/cases/fourslash/codeFixChangeExtendsToImplementsWithDecorator.ts b/tests/cases/fourslash/codeFixChangeExtendsToImplementsWithDecorator.ts index 6940cd27830..e183677a31f 100644 --- a/tests/cases/fourslash/codeFixChangeExtendsToImplementsWithDecorator.ts +++ b/tests/cases/fourslash/codeFixChangeExtendsToImplementsWithDecorator.ts @@ -13,5 +13,5 @@ verify.codeFix({ description: "Change 'extends' to 'implements'", // TODO: GH#18794 - newRangeContent: "class A implements I1 , I2 { }", + newRangeContent: "class A implements I1, I2 { }", }); diff --git a/tests/cases/fourslash/codeFixChangeExtendsToImplementsWithTrivia.ts b/tests/cases/fourslash/codeFixChangeExtendsToImplementsWithTrivia.ts new file mode 100644 index 00000000000..ed7fa4c3b1d --- /dev/null +++ b/tests/cases/fourslash/codeFixChangeExtendsToImplementsWithTrivia.ts @@ -0,0 +1,19 @@ +/// + +//// interface I1 { } +//// interface I2 { } +//// interface I3 { } + +//// [|class MyClass /*A !*/ //B ! +//// /*C !*/ extends /*D !*/ I1 /*E !*/ //F ! +//// /*G !*/ implements /*H !*/ I2 /*I !*/, /*J !*/ I3 /*K !*/ //L !|] +//// { +//// } + +verify.codeFix({ + description: "Change 'extends' to 'implements'", + // TODO: GH#18794 + newRangeContent: `class MyClass /*A !*/ //B ! + /*C !*/ implements /*D !*/ I1, /*E !*/ //F ! + /*G !*/ /*H !*/ I2 /*I !*/, /*J !*/ I3 /*K !*/ //L !`, +}); From a961d7aa047aeecd7e14f206c6371d0c10ed24ec Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Thu, 4 Jan 2018 18:05:58 -0800 Subject: [PATCH 2/2] Only replace `implements` with a comma if the heritage clauses are sensible --- .../fixExtendsInterfaceBecomesImplements.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts b/src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts index cfa1a288ca8..d98ca556f47 100644 --- a/src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts +++ b/src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts @@ -28,21 +28,25 @@ namespace ts.codefix { function doChanges(changes: textChanges.ChangeTracker, sourceFile: SourceFile, extendsToken: Node, heritageClauses: ReadonlyArray): void { changes.replaceRange(sourceFile, { pos: extendsToken.getStart(), end: extendsToken.end }, createToken(SyntaxKind.ImplementsKeyword)); - // We replace existing keywords with commas. - for (let i = 1; i < heritageClauses.length; i++) { - const keywordToken = heritageClauses[i].getFirstToken()!; - const keywordFullStart = keywordToken.getFullStart(); - changes.replaceRange(sourceFile, { pos: keywordFullStart, end: keywordFullStart }, createToken(SyntaxKind.CommaToken)); + + // If there is already an implements clause, replace the implements keyword with a comma. + if (heritageClauses.length === 2 && + heritageClauses[0].token === SyntaxKind.ExtendsKeyword && + heritageClauses[1].token === SyntaxKind.ImplementsKeyword) { + + const implementsToken = heritageClauses[1].getFirstToken()!; + const implementsFullStart = implementsToken.getFullStart(); + changes.replaceRange(sourceFile, { pos: implementsFullStart, end: implementsFullStart }, createToken(SyntaxKind.CommaToken)); // Rough heuristic: delete trailing whitespace after keyword so that it's not excessive. // (Trailing because leading might be indentation, which is more sensitive.) const text = sourceFile.text; - let end = keywordToken.end; + let end = implementsToken.end; while (end < text.length && ts.isWhiteSpaceSingleLine(text.charCodeAt(end))) { end++; } - changes.deleteRange(sourceFile, { pos: keywordToken.getStart(), end }); + changes.deleteRange(sourceFile, { pos: implementsToken.getStart(), end }); } } }