Merge pull request #127 from edsrzf/ts-preserve-space

Preserve blank lines in plugins using TypeScript transforms

.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"
     }
 }

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<Options> = {
   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<Options> = {
 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;
+}

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<string, TypeOptions>;
 
@@ -66,46 +67,39 @@ const optionProperties: Properties = {
 const jsDocPlugin: Plugin<Options> = {
   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),
 };
 
 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);
     }
   }