From 162446fef2107266f85897b22171cf1fb72edd2c Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 6 Apr 2018 16:13:09 -0700 Subject: [PATCH] Fix bug: don't insert a semicolon when inserting a FunctionDeclaration --- 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 daa0e27c05f0d..7b40f1bd68775 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -5618,6 +5618,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 40d3738f12f12..52ae6803ba359 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 0a48bcf622806..800b58e827422 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 89c1b54eec5f9..8d5922a8c6400 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3243,6 +3243,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 dbb4d6cc7bf46..99e13583660c6 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3298,6 +3298,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 841d43fef97e9..87206e588f225 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 6e88a371eb68a..780af267ac821 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 0000000000000..7aac7e7d42ab3 --- /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;`, +});