diff --git a/.eslintrc.js b/.eslintrc.js index 25128a2..c13aacc 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -79,6 +79,8 @@ module.exports = { "@typescript-eslint/no-empty-function": "off", "@typescript-eslint/explicit-function-return-type": "off", "@typescript-eslint/no-var-requires": "off", - "@typescript-eslint/no-use-before-define": "off" + "@typescript-eslint/no-use-before-define": "off", + "no-useless-constructor": "off", + "@typescript-eslint/no-useless-constructor": "error" } } diff --git a/packages/ts-migrate-plugins/src/plugins/add-conversions.ts b/packages/ts-migrate-plugins/src/plugins/add-conversions.ts index 383b364..7e6e316 100644 --- a/packages/ts-migrate-plugins/src/plugins/add-conversions.ts +++ b/packages/ts-migrate-plugins/src/plugins/add-conversions.ts @@ -3,6 +3,7 @@ import { Plugin } from 'ts-migrate-server'; import { isDiagnosticWithLinePosition } from '../utils/type-guards'; import getTokenAtPosition from './utils/token-pos'; import { AnyAliasOptions, validateAnyAliasOptions } from '../utils/validateOptions'; +import UpdateTracker from './utils/update'; type Options = AnyAliasOptions; @@ -16,20 +17,16 @@ const supportedDiagnostics = new Set([ const addConversionsPlugin: Plugin = { name: 'add-conversions', - run({ fileName, sourceFile, text, options, getLanguageService }) { + run({ fileName, sourceFile, options, getLanguageService }) { // Filter out diagnostics we care about. const diags = getLanguageService() .getSemanticDiagnostics(fileName) .filter(isDiagnosticWithLinePosition) .filter((diag) => supportedDiagnostics.has(diag.code)); - const result = ts.transform(sourceFile, [addConversionsTransformerFactory(diags, options)]); - const newSourceFile = result.transformed[0]; - if (newSourceFile === sourceFile) { - return text; - } - const printer = ts.createPrinter(); - return printer.printFile(newSourceFile); + const updates = new UpdateTracker(sourceFile); + ts.transform(sourceFile, [addConversionsTransformerFactory(updates, diags, options)]); + return updates.apply(); }, validate: validateAnyAliasOptions, @@ -38,6 +35,7 @@ const addConversionsPlugin: Plugin = { export default addConversionsPlugin; const addConversionsTransformerFactory = ( + updates: UpdateTracker, diags: ts.DiagnosticWithLocation[], { anyAlias }: Options, ) => (context: ts.TransformationContext) => { @@ -70,16 +68,113 @@ const addConversionsTransformerFactory = ( }) .filter((node): node is ts.Expression => node !== null), ); - return ts.visitNode(file, visit); + visit(file); + return file; }; - function visit(origNode: ts.Node): ts.Node { + function visit(origNode: ts.Node): ts.Node | undefined { const needsConversion = nodesToConvert.has(origNode); - const node = ts.visitEachChild(origNode, visit, context); - if (!needsConversion) { - return node; + let node = ts.visitEachChild(origNode, visit, context); + if (node === origNode && !needsConversion) { + return origNode; + } + + if (needsConversion) { + node = factory.createAsExpression(node as ts.Expression, anyType); + } + + if (shouldReplace(node)) { + replaceNode(origNode, node); + return origNode; } - return factory.createAsExpression(node as ts.Expression, anyType); + return node; + } + + // Nodes that have one expression child called "expression". + type ExpressionChild = + | ts.DoStatement + | ts.IfStatement + | ts.SwitchStatement + | ts.WithStatement + | ts.WhileStatement; + + /** + * For nodes that contain both expression and statement children, only + * replace the direct expression children. The statements have already + * been replaced at a lower level and replacing them again can produce + * duplicate statements or invalid syntax. + */ + function replaceNode(origNode: ts.Node, newNode: ts.Node): void { + switch (origNode.kind) { + case ts.SyntaxKind.DoStatement: + case ts.SyntaxKind.IfStatement: + case ts.SyntaxKind.SwitchStatement: + case ts.SyntaxKind.WithStatement: + case ts.SyntaxKind.WhileStatement: + updates.replaceNode( + (origNode as ExpressionChild).expression, + (newNode as ExpressionChild).expression, + ); + break; + + case ts.SyntaxKind.ForStatement: + updates.replaceNode( + (origNode as ts.ForStatement).initializer, + (newNode as ts.ForStatement).initializer, + ); + updates.replaceNode( + (origNode as ts.ForStatement).condition, + (newNode as ts.ForStatement).condition, + ); + updates.replaceNode( + (origNode as ts.ForStatement).incrementor, + (newNode as ts.ForStatement).incrementor, + ); + break; + + case ts.SyntaxKind.ForInStatement: + case ts.SyntaxKind.ForOfStatement: + updates.replaceNode( + (origNode as ts.ForInOrOfStatement).expression, + (newNode as ts.ForInOrOfStatement).expression, + ); + updates.replaceNode( + (origNode as ts.ForInOrOfStatement).initializer, + (newNode as ts.ForInOrOfStatement).initializer, + ); + break; + + default: + updates.replaceNode(origNode, newNode); + break; + } } }; + +/** + * Determines whether a node is eligible to be replaced. + * + * Replacing only the expression may produce invalid syntax due to missing parentheses. + * There is still some risk of losing whitespace if the expression is contained within + * an if statement condition or other construct that can contain blocks. + */ +function shouldReplace(node: ts.Node): boolean { + if (isStatement(node)) { + return true; + } + switch (node.kind) { + case ts.SyntaxKind.CaseClause: + case ts.SyntaxKind.ClassDeclaration: + case ts.SyntaxKind.EnumMember: + case ts.SyntaxKind.HeritageClause: + case ts.SyntaxKind.SourceFile: // In case we missed any other case. + return true; + default: + return false; + } +} + +function isStatement(node: ts.Node): node is ts.Statement { + return ts.SyntaxKind.FirstStatement <= node.kind && node.kind <= ts.SyntaxKind.LastStatement; +} diff --git a/packages/ts-migrate-plugins/src/plugins/jsdoc.ts b/packages/ts-migrate-plugins/src/plugins/jsdoc.ts index 38c0725..c4c0020 100644 --- a/packages/ts-migrate-plugins/src/plugins/jsdoc.ts +++ b/packages/ts-migrate-plugins/src/plugins/jsdoc.ts @@ -7,6 +7,7 @@ import { anyAliasProperty, createValidate, } from '../utils/validateOptions'; +import UpdateTracker from './utils/update'; type TypeMap = Record; @@ -66,14 +67,10 @@ const optionProperties: Properties = { const jsDocPlugin: Plugin = { name: 'jsdoc', - run({ sourceFile, text, options }) { - const result = ts.transform(sourceFile, [jsDocTransformerFactory(options)]); - const newSourceFile = result.transformed[0]; - if (newSourceFile === sourceFile) { - return text; - } - const printer = ts.createPrinter(); - return printer.printFile(newSourceFile); + run({ sourceFile, options }) { + const updates = new UpdateTracker(sourceFile); + ts.transform(sourceFile, [jsDocTransformerFactory(updates, options)]); + return updates.apply(); }, validate: createValidate(optionProperties), @@ -81,31 +78,28 @@ const jsDocPlugin: Plugin = { export default jsDocPlugin; -const jsDocTransformerFactory = ({ - annotateReturns, - anyAlias, - typeMap: optionsTypeMap, -}: Options) => (context: ts.TransformationContext) => { +const jsDocTransformerFactory = ( + updates: UpdateTracker, + { annotateReturns, anyAlias, typeMap: optionsTypeMap }: Options, +) => (context: ts.TransformationContext) => { const { factory } = context; const anyType = anyAlias ? factory.createTypeReferenceNode(anyAlias, undefined) : factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword); const typeMap: TypeMap = { ...defaultTypeMap, ...optionsTypeMap }; - - return (file: ts.SourceFile) => ts.visitNode(file, visit); - - function visit(origNode: ts.Node): ts.Node { - const node = ts.visitEachChild(origNode, visit, context); - if (ts.isFunctionLike(node)) { - return visitFunctionLike(node, ts.isClassDeclaration(origNode.parent)); + return (file: ts.SourceFile) => { + visit(file); + return file; + }; + + function visit(origNode: ts.Node): void { + origNode.forEachChild(visit); + if (ts.isFunctionLike(origNode)) { + visitFunctionLike(origNode, ts.isClassDeclaration(origNode.parent)); } - return node; } - function visitFunctionLike( - node: ts.SignatureDeclaration, - insideClass: boolean, - ): ts.SignatureDeclaration { + function visitFunctionLike(node: ts.SignatureDeclaration, insideClass: boolean): void { const modifiers = ts.isMethodDeclaration(node) && insideClass ? modifiersFromJSDoc(node, factory) @@ -117,90 +111,27 @@ const jsDocTransformerFactory = ({ parameters === node.parameters && returnType === node.type ) { - return node; + return; + } + + const newModifiers = modifiers ? factory.createNodeArray(modifiers) : undefined; + if (newModifiers) { + if (node.modifiers) { + updates.replaceNodes(node.modifiers, newModifiers); + } else { + const pos = node.name!.getStart(); + updates.insertNodes(pos, newModifiers); + } } - const newModifiers = factory.createNodeArray(modifiers); const newParameters = factory.createNodeArray(parameters); - const newType = returnType; + const addParens = + ts.isArrowFunction(node) && node.getFirstToken()?.kind !== ts.SyntaxKind.OpenParenToken; + updates.replaceNodes(node.parameters, newParameters, addParens); - switch (node.kind) { - case ts.SyntaxKind.FunctionDeclaration: - return factory.updateFunctionDeclaration( - node, - node.decorators, - newModifiers, - node.asteriskToken, - node.name, - node.typeParameters, - newParameters, - newType, - node.body, - ); - case ts.SyntaxKind.MethodDeclaration: - return factory.updateMethodDeclaration( - node, - node.decorators, - newModifiers, - node.asteriskToken, - node.name, - node.questionToken, - node.typeParameters, - newParameters, - newType, - node.body, - ); - case ts.SyntaxKind.Constructor: - return factory.updateConstructorDeclaration( - node, - node.decorators, - newModifiers, - newParameters, - node.body, - ); - case ts.SyntaxKind.GetAccessor: - return factory.updateGetAccessorDeclaration( - node, - node.decorators, - newModifiers, - node.name, - newParameters, - newType, - node.body, - ); - case ts.SyntaxKind.SetAccessor: - return factory.updateSetAccessorDeclaration( - node, - node.decorators, - newModifiers, - node.name, - newParameters, - node.body, - ); - case ts.SyntaxKind.FunctionExpression: - return factory.updateFunctionExpression( - node, - newModifiers, - node.asteriskToken, - node.name, - node.typeParameters, - newParameters, - newType, - node.body, - ); - case ts.SyntaxKind.ArrowFunction: - return factory.updateArrowFunction( - node, - newModifiers, - node.typeParameters, - newParameters, - newType, - node.equalsGreaterThanToken, - node.body, - ); - default: - // Should be impossible. - return node; + const newType = returnType; + if (newType) { + updates.addReturnAnnotation(node, newType); } } diff --git a/packages/ts-migrate-plugins/src/plugins/utils/update.ts b/packages/ts-migrate-plugins/src/plugins/utils/update.ts new file mode 100644 index 0000000..2f18e28 --- /dev/null +++ b/packages/ts-migrate-plugins/src/plugins/utils/update.ts @@ -0,0 +1,104 @@ +import ts from 'typescript'; +import updateSourceText, { SourceTextUpdate } from '../../utils/updateSourceText'; + +/** + * Tracks updates to a ts.SourceFile as text changes. + * This is useful to preserve as much of the original whitespace in the source + * file as possible. Re-printing the entire file causes blank lines to be lost. + * + * See: https://github.com/microsoft/TypeScript/issues/843 + */ +class UpdateTracker { + private updates: SourceTextUpdate[] = []; + + private printer = ts.createPrinter(); + + constructor(private sourceFile: ts.SourceFile) {} + + private insert(pos: number, text: string): void { + this.updates.push({ + kind: 'insert', + index: pos, + text, + }); + } + + /** + * Adds a return type annotation to a function. + * replaceNode would require reprinting the entire function body, losing all whitespace details. + */ + public addReturnAnnotation(node: ts.SignatureDeclaration, type: ts.TypeNode): void { + const paren = node + .getChildren(this.sourceFile) + .find((node) => node.kind === ts.SyntaxKind.CloseParenToken); + let pos; + if (paren) { + pos = paren.pos + 1; + } else { + // Must be an arrow function with single parameter and no parentheses. + // Add parentheses. + pos = node.parameters.end; + const [param] = node.parameters; + this.insert(param.getStart(), '('); + this.insert(pos, ')'); + } + const text = this.printer.printNode(ts.EmitHint.Unspecified, type, this.sourceFile); + this.insert(pos, `: ${text}`); + } + + public insertNodes(pos: number, nodes: ts.NodeArray): void { + const text = this.printer.printList(ts.ListFormat.SpaceAfterList, nodes, this.sourceFile); + this.insert(pos, text); + } + + private replace(pos: number, length: number, text: string): void { + this.updates.push({ + kind: 'replace', + index: pos, + length, + text, + }); + } + + public replaceNode(oldNode: ts.Node | undefined, newNode: ts.Node | undefined): void { + if (oldNode && newNode && oldNode !== newNode) { + const printedNextNode = this.printer.printNode( + ts.EmitHint.Unspecified, + newNode, + this.sourceFile, + ); + const text = oldNode + .getFullText(this.sourceFile) + .replace(/^(\s*)[^]*?(\s*)$/, `$1${printedNextNode}$2`); + this.updates.push({ + kind: 'replace', + index: oldNode.pos, + length: oldNode.end - oldNode.pos, + text, + }); + } + } + + public replaceNodes( + oldNodes: ts.NodeArray, + newNodes: ts.NodeArray, + addParens = false, + ): void { + if (oldNodes !== newNodes) { + const listFormat = addParens ? ts.ListFormat.Parenthesis : ts.ListFormat.CommaListElements; + const printedNextNode = this.printer.printList(listFormat, newNodes, this.sourceFile); + const prevText = this.sourceFile.text.substring(oldNodes.pos, oldNodes.end); + const text = prevText.replace(/^(\s*)[^]*?(\s*)$/, `$1${printedNextNode}$2`); + this.replace(oldNodes.pos, oldNodes.end - oldNodes.pos, text); + } + } + + /** + * Returns the result of applying all tracked changes to the source file. + */ + public apply(): string { + return updateSourceText(this.sourceFile.text, this.updates); + } +} + +export default UpdateTracker; diff --git a/packages/ts-migrate-plugins/tests/src/add-conversions.test.ts b/packages/ts-migrate-plugins/tests/src/add-conversions.test.ts index 2821a2b..1988727 100644 --- a/packages/ts-migrate-plugins/tests/src/add-conversions.test.ts +++ b/packages/ts-migrate-plugins/tests/src/add-conversions.test.ts @@ -4,7 +4,22 @@ import addConversionsPlugin from '../../src/plugins/add-conversions'; describe('add-conversions plugin', () => { const text = `\ const a = {}; +const b = {}; + a.b = 1; +a.b = b.c; + +if (a.b) { + b.c = 1; +} + +class C extends a.b { +} + +enum E { + A = a.b +} + console.log(a.c); `; @@ -13,7 +28,22 @@ console.log(a.c); expect(result).toBe(`\ const a = {}; +const b = {}; + (a as any).b = 1; +(a as any).b = (b as any).c; + +if ((a as any).b) { + (b as any).c = 1; +} + +class C extends (a as any).b { +} + +enum E { + A = (a as any).b +} + console.log((a as any).c); `); }); @@ -25,7 +55,22 @@ console.log((a as any).c); expect(result).toBe(`\ const a = {}; +const b = {}; + (a as $TSFixMe).b = 1; +(a as $TSFixMe).b = (b as $TSFixMe).c; + +if ((a as $TSFixMe).b) { + (b as $TSFixMe).c = 1; +} + +class C extends (a as $TSFixMe).b { +} + +enum E { + A = (a as $TSFixMe).b +} + console.log((a as $TSFixMe).c); `); }); diff --git a/packages/ts-migrate-plugins/tests/src/jsdoc.test.ts b/packages/ts-migrate-plugins/tests/src/jsdoc.test.ts index 2192a3e..71dfc9d 100644 --- a/packages/ts-migrate-plugins/tests/src/jsdoc.test.ts +++ b/packages/ts-migrate-plugins/tests/src/jsdoc.test.ts @@ -14,9 +14,9 @@ function B(b) {} expect(result).toBe(`\ /** @param a {?} */ -function A(a: any) { } +function A(a: any) {} /** @param b {*} */ -function B(b: any) { } +function B(b: any) {} `); }); @@ -32,9 +32,9 @@ function B(b) {} expect(result).toBe(`\ /** @arg a {Number} */ -function A(a: number) { } +function A(a: number) {} /** @argument b {Number} */ -function B(b: number) { } +function B(b: number) {} `); }); @@ -56,15 +56,15 @@ function E(e) {} expect(result).toBe(`\ /** @param a {Number} */ -function A(a: number) { } +function A(a: number) {} /** @param b {String} */ -function B(b: string) { } +function B(b: string) {} /** @param c {Boolean} */ -function C(c: boolean) { } +function C(c: boolean) {} /** @param d {Object} */ -function D(d: object) { } +function D(d: object) {} /** @param e {date} */ -function E(e: Date) { } +function E(e: Date) {} `); }); @@ -84,13 +84,13 @@ function D(d) {} expect(result).toBe(`\ /** @param a {Number} */ -function A(a: number) { } +function A(a: number) {} /** @param b {String} */ -function B(b: string) { } +function B(b: string) {} /** @param c {Boolean} */ -function C(c: boolean) { } +function C(c: boolean) {} /** @param d {Object} */ -function D(d: object) { } +function D(d: object) {} `); }); @@ -104,7 +104,7 @@ function A(a) {} expect(result).toBe(`\ /** @param a {?Number} */ -function A(a: number | null) { } +function A(a: number | null) {} `); }); @@ -118,7 +118,7 @@ function A(a) {} expect(result).toBe(`\ /** @param a {!Number} */ -function A(a: number) { } +function A(a: number) {} `); }); @@ -138,13 +138,13 @@ function D(d = 1) {} expect(result).toBe(`\ /** @param a {Number=} */ -function A(a?: number) { } +function A(a?: number) {} /** @param [b] {Number} */ -function B(b?: number) { } +function B(b?: number) {} /** @param [c] {Object} */ -function C({ c }: object) { } +function C({ c }: object) {} /** @param [d] {Number} */ -function D(d: number = 1) { } +function D(d: number = 1) {} `); }); @@ -164,13 +164,13 @@ function D(d) {} expect(result).toBe(`\ /** @param a {Array} */ -function A(a: Array) { } +function A(a: Array) {} /** @param b {Array} */ -function B(b: Array) { } +function B(b: Array) {} /** @param c {Array.} */ -function C(c: Array) { } +function C(c: Array) {} /** @param d {String[]} */ -function D(d: string[]) { } +function D(d: string[]) {} `); }); @@ -186,9 +186,9 @@ function B(b) {} expect(result).toBe(`\ /** @param a {Object} */ -function A(a: { [n: number]: any; }) { } +function A(a: { [n: number]: any; }) {} /** @param b {Object} */ -function B(b: { [s: string]: any; }) { } +function B(b: { [s: string]: any; }) {} `); }); @@ -208,13 +208,13 @@ function D(d) {} expect(result).toBe(`\ /** @param a {function(number)} */ -function A(a: (arg0: number) => any) { } +function A(a: (arg0: number) => any) {} /** @param b {function(): number} */ -function B(b: () => number) { } +function B(b: () => number) {} /** @param c {function(this: number)} */ -function C(c: (this: number) => any) { } +function C(c: (this: number) => any) {} /** @param d {function(...number)} */ -function D(d: (...rest: number[]) => any) { } +function D(d: (...rest: number[]) => any) {} `); }); @@ -257,7 +257,7 @@ function Destructured({ a, b }) {} function Project(employee: { name: string; department?: string; -}) { } +}) {} /** * @param {Object} employee * @param employee.name @@ -266,7 +266,7 @@ function Project(employee: { function NoTypes(employee: { name: any; department: any; -}) { } +}) {} /** * @param {Object} employee * @param {Object} employee.name @@ -276,7 +276,7 @@ function DeepNesting(employee: { name: { first: string; }; -}) { } +}) {} /** * @param {Object} param * @param {String} param.a @@ -285,7 +285,7 @@ function DeepNesting(employee: { function Destructured({ a, b }: { a: string; b: number; -}) { } +}) {} `); }); @@ -299,7 +299,7 @@ function A(a) {} expect(result).toBe(`\ /** @param a {Undeclared} */ -function A(a: Undeclared) { } +function A(a: Undeclared) {} `); }); @@ -319,7 +319,7 @@ function A(a, b, c) {} * @param a {number} * @param b {string} */ -function A(a: number, b: string, c) { } +function A(a: number, b: string, c) {} `); }); @@ -349,7 +349,7 @@ function A() {} expect(result).toBe(`\ /** @return {number} */ -function A(): number { } +function A(): number {} `); }); @@ -383,8 +383,8 @@ class C { expect(result).toBe(`\ class C { - /** @param a {number} */ - A(a: number) { } + /** @param a {number} */ + A(a: number) {} } `); }); @@ -413,20 +413,20 @@ class C { expect(result).toBe(`\ class C { - /** @private */ - private A() { } - /** @protected */ - protected B() { } - /** @public */ - public C() { } - /** - * @private - * @protected - * @public - */ - private D() { } - /** @public */ - private E() { } + /** @private */ + private A() {} + /** @protected */ + protected B() {} + /** @public */ + public C() {} + /** + * @private + * @protected + * @public + */ + private D() {} + /** @public */ + private E() {} } `); }); @@ -453,16 +453,16 @@ const O = { expect(result).toBe(`\ const O = { - /** @param a {number} */ - A(a: number) { }, - /** @return {string} */ - B(): string { }, - /** @private */ - C() { }, - /** @param a {number} */ - D: (a: number) => { }, - /** @return {string} */ - E: (): string => { } + /** @param a {number} */ + A(a: number) {}, + /** @return {string} */ + B(): string {}, + /** @private */ + C() {}, + /** @param a {number} */ + D: (a: number) => {}, + /** @return {string} */ + E: (): string => {} }; `); }); @@ -483,11 +483,11 @@ window.c = function(c) {}; expect(result).toBe(`\ /** @param a {number} */ -const A = function (a: number) { }; +const A = function(a: number) {}; /** @return {string} */ -const B = function (): string { }; +const B = function(): string {}; /** @param c {number} */ -window.c = function (c: number) { }; +window.c = function(c: number) {}; `); }); @@ -534,9 +534,9 @@ function() { const result = jsDocPlugin.run(mockPluginParams({ text, fileName: 'file.tsx' })); expect(result).toBe(`\ -function () { +function() { /** @param a {number} */ - function A(a: number) { } + function A(a: number) {} } `); });