From ce5d22fef00830329cb1d3a9d83f9584ae1ecd28 Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 9 Apr 2018 15:03:24 -0700 Subject: [PATCH] Fix bug: don't insert a semicolon when inserting a FunctionDeclaration (#23240) --- src/compiler/utilities.ts | 4 ++++ src/services/completions.ts | 2 +- src/services/textChanges.ts | 12 +++++++----- .../baselines/reference/api/tsserverlibrary.d.ts | 1 + tests/baselines/reference/api/typescript.d.ts | 1 + .../textChanges/insertNodeAfterInClass1.js | 2 +- ...tNodeInInterfaceAfterNodeWithoutSeparator2.js | 2 +- .../convertFunctionToEs6ClassNoSemicolon.ts | 16 ++++++++++++++++ 8 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 tests/cases/fourslash/convertFunctionToEs6ClassNoSemicolon.ts diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index a73efb55eef..8e6b3f8ae39 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -5629,6 +5629,10 @@ namespace ts { || kind === SyntaxKind.MissingDeclaration; } + export function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement { + return isTypeElement(node) || isClassElement(node); + } + export function isObjectLiteralElementLike(node: Node): node is ObjectLiteralElementLike { const kind = node.kind; return kind === SyntaxKind.PropertyAssignment diff --git a/src/services/completions.ts b/src/services/completions.ts index 40d3738f12f..52ae6803ba3 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2248,7 +2248,7 @@ namespace ts.Completions { // TODO: GH#19856 Would like to return `node is Node & { parent: (ClassElement | TypeElement) & { parent: ObjectTypeDeclaration } }` but then compilation takes > 10 minutes function isFromObjectTypeDeclaration(node: Node): boolean { - return node.parent && (isClassElement(node.parent) || isTypeElement(node.parent)) && isObjectTypeDeclaration(node.parent.parent); + return node.parent && isClassOrTypeElement(node.parent) && isObjectTypeDeclaration(node.parent.parent); } function hasIndexSignature(type: Type): boolean { diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 0a48bcf6228..800b58e8274 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -447,10 +447,7 @@ namespace ts.textChanges { } public insertNodeAfter(sourceFile: SourceFile, after: Node, newNode: Node): this { - if (isStatementButNotDeclaration(after) || - after.kind === SyntaxKind.PropertyDeclaration || - after.kind === SyntaxKind.PropertySignature || - after.kind === SyntaxKind.MethodSignature) { + if (needSemicolonBetween(after, newNode)) { // check if previous statement ends with semicolon // if not - insert semicolon to preserve the code from changing the meaning due to ASI if (sourceFile.text.charCodeAt(after.end - 1) !== CharacterCodes.semicolon) { @@ -465,7 +462,7 @@ namespace ts.textChanges { if (isClassDeclaration(node) || isModuleDeclaration(node)) { return { prefix: this.newLineCharacter, suffix: this.newLineCharacter }; } - else if (isStatement(node) || isClassElement(node) || isTypeElement(node)) { + else if (isStatement(node) || isClassOrTypeElement(node)) { return { suffix: this.newLineCharacter }; } else if (isVariableDeclaration(node)) { @@ -893,4 +890,9 @@ namespace ts.textChanges { export function isValidLocationToAddComment(sourceFile: SourceFile, position: number) { return !isInComment(sourceFile, position) && !isInString(sourceFile, position) && !isInTemplateString(sourceFile, position); } + + function needSemicolonBetween(a: Node, b: Node): boolean { + return (isPropertySignature(a) || isPropertyDeclaration(a)) && isClassOrTypeElement(b) && b.name.kind === SyntaxKind.ComputedPropertyName + || isStatementButNotDeclaration(a) && isStatementButNotDeclaration(b); // TODO: only if b would start with a `(` or `[` + } } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index a990c0af274..f602e872c14 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3247,6 +3247,7 @@ declare namespace ts { function isClassLike(node: Node): node is ClassLikeDeclaration; function isAccessor(node: Node): node is AccessorDeclaration; function isTypeElement(node: Node): node is TypeElement; + function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement; function isObjectLiteralElementLike(node: Node): node is ObjectLiteralElementLike; /** * Node test that determines whether a node is a valid type node. diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 5b6ef18b680..94fd684daee 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3302,6 +3302,7 @@ declare namespace ts { function isClassLike(node: Node): node is ClassLikeDeclaration; function isAccessor(node: Node): node is AccessorDeclaration; function isTypeElement(node: Node): node is TypeElement; + function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement; function isObjectLiteralElementLike(node: Node): node is ObjectLiteralElementLike; /** * Node test that determines whether a node is a valid type node. diff --git a/tests/baselines/reference/textChanges/insertNodeAfterInClass1.js b/tests/baselines/reference/textChanges/insertNodeAfterInClass1.js index 841d43fef97..87206e588f2 100644 --- a/tests/baselines/reference/textChanges/insertNodeAfterInClass1.js +++ b/tests/baselines/reference/textChanges/insertNodeAfterInClass1.js @@ -7,6 +7,6 @@ class A { ===MODIFIED=== class A { - x; + x a: boolean; } diff --git a/tests/baselines/reference/textChanges/insertNodeInInterfaceAfterNodeWithoutSeparator2.js b/tests/baselines/reference/textChanges/insertNodeInInterfaceAfterNodeWithoutSeparator2.js index 6e88a371eb6..780af267ac8 100644 --- a/tests/baselines/reference/textChanges/insertNodeInInterfaceAfterNodeWithoutSeparator2.js +++ b/tests/baselines/reference/textChanges/insertNodeInInterfaceAfterNodeWithoutSeparator2.js @@ -7,6 +7,6 @@ interface A { ===MODIFIED=== interface A { - x(); + x() [1]: any; } diff --git a/tests/cases/fourslash/convertFunctionToEs6ClassNoSemicolon.ts b/tests/cases/fourslash/convertFunctionToEs6ClassNoSemicolon.ts new file mode 100644 index 00000000000..7aac7e7d42a --- /dev/null +++ b/tests/cases/fourslash/convertFunctionToEs6ClassNoSemicolon.ts @@ -0,0 +1,16 @@ +/// + +// @allowJs: true + +// @Filename: /a.js +////var C = function() { this.x = 0; } +////0; + +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: +`class C { + constructor() { this.x = 0; } +} +0;`, +});