Plugin warnings with code context, lots of refactoring

package.json

@@ -36,12 +36,14 @@
     "webpack-cli": "^3.3.2"
   },
   "dependencies": {
+    "@babel/code-frame": "^7.0.0",
     "@babel/plugin-syntax-class-properties": "^7.2.0",
     "@babel/plugin-syntax-decorators": "^7.2.0",
     "@babel/plugin-syntax-dynamic-import": "^7",
     "@babel/plugin-syntax-flow": "^7",
     "@babel/plugin-syntax-jsx": "^7",
     "@babel/types": "^7.4.4",
+    "@types/babel__code-frame": "^7.0.1",
     "@types/diff": "^4.0.1",
     "@types/glob": "^7.1.1",
     "@types/lodash": "^4.14.132",

src/cli/index.ts

@@ -56,9 +56,8 @@ const help: {
   dryRun: 'perform a trial run printing to stdout instead of writing a file',
   excludeDirs: 'list of recursively excluded directories',
   includePattern: 'set the glob pattern for input files',
-  replaceDecorators: 'replace class @decorators with ordinary function calls',
+  replaceDecorators: 'replace class @decorators with wrapped function calls to avoid TypeScript errors',
   replace: 'process files in-place. A new TS file will be created next to the original file otherwise.',
-  verbose: 'increase verbosity',
 };
 
 // Define the CLI interface
@@ -70,8 +69,7 @@ program
   .option('-e, --exclude-dirs <dirs ...>', help.excludeDirs, toArray, DEFAULT_EXCLUDE_DIRECTORIES)
   .option('-i, --include-pattern <pattern>', help.includePattern, DEFAULT_INCLUDE_PATTERN)
   .option('-r, --replace', help.replace)
-  .option('-D, --replace-decorators', help.replaceDecorators, false)
-  .option('-v, --verbose', help.verbose, false);
+  .option('-D, --replace-decorators', help.replaceDecorators, false);
 
 program.on('--help', () => {
   console.log('\nExamples:');

src/cli/runner.ts

@@ -1,13 +1,11 @@
 import { transformFileSync } from '@babel/core';
-import chalk from 'chalk';
 import { renameSync, statSync, writeFileSync, readFileSync } from 'fs';
 import glob, { IOptions as GlobOptions } from 'glob';
 import { extname, resolve } from 'path';
 
-import { logError, logPluginWarning, printRuler } from '../util/log';
+import { FileTypes } from '../plugin/util/file';
+import { logError, printRuler } from '../util/log';
 import { formatOutputCode } from '../plugin/util/format';
-import { Metrics, sortNumberMap } from '../plugin/util/metric';
-import { PluginWarnings } from '../plugin/util/warning';
 import { getTransformOptions } from '../plugin/options';
 
 import { ReflowOptions } from '../plugin/';
@@ -35,12 +33,12 @@ function getGlobOptions(options: GlobOptions, excludeDirs: string[]): GlobOption
 }
 
 export function transpileFiles(args: CommandLineArgs): string[] {
-  const { dryRun, replace, replaceDecorators, sources, verbose } = args;
+  const { dryRun, replace, replaceDecorators, sources } = args;
 
   const excludeDirs = args.excludeDirs || DEFAULT_EXCLUDE_DIRECTORIES;
   const includePattern = args.includePattern || DEFAULT_INCLUDE_PATTERN;
 
-  const pluginOptions: ReflowOptions = { replaceDecorators, verbose };
+  const pluginOptions: ReflowOptions = { replaceDecorators };
   const babelOptions = getTransformOptions({ pluginOptions });
 
   const writtenFiles: string[] = [];
@@ -53,15 +51,15 @@ export function transpileFiles(args: CommandLineArgs): string[] {
     const inputFiles = isDir ? glob.sync(includePattern, globOptions) : [resolve(source)];
 
     inputFiles.forEach(inputFile => {
-      console.log(chalk.magenta(`Transpiling ${inputFile}...`));
+      console.log(`Transpiling ${inputFile}...`);
       const out = transformFileSync(inputFile, babelOptions);
 
       if (out === null || !out.code) {
         logError(`Unable to transpile ${inputFile}`, 4);
       } else {
         const outputFile = process.env.DEBUG
           ? inputFile
-          : inputFile.replace(extname(inputFile), Metrics.fileTypes.get(inputFile) || '.ts');
+          : inputFile.replace(extname(inputFile), FileTypes.get(inputFile) || '.ts');
 
         const inputCode = String(readFileSync(inputFile));
         const formattedOutput = formatOutputCode(out.code, inputCode, pluginOptions);
@@ -79,12 +77,6 @@ export function transpileFiles(args: CommandLineArgs): string[] {
         }
       }
     });
-
-    PluginWarnings.getWarnings().forEach(logPluginWarning);
-
-    if (verbose && Metrics.typeCounter.getCounter().size) {
-      console.log(sortNumberMap(Metrics.typeCounter.getCounter()));
-    }
   });
 
   return writtenFiles;

src/plugin/__tests__/__fixtures__/formatting/index.test.ts

@@ -2,7 +2,7 @@ import { resolve } from 'path';
 
 import { ReflowOptions } from '../../..';
 import { getTransformOptions } from '../../../options';
-import { runFixtureTests } from '../..';
+import { runFixtureTests } from '../../../util/test-runner';
 
 const pluginOptions: ReflowOptions = {
   replaceDecorators: true,

src/plugin/__tests__/__fixtures__/optimizations/index.test.ts

@@ -2,7 +2,7 @@ import { resolve } from 'path';
 
 import { ReflowOptions } from '../../..';
 import { getTransformOptions } from '../../../options';
-import { runFixtureTests } from '../..';
+import { runFixtureTests } from '../../../util/test-runner';
 
 const pluginOptions: ReflowOptions = {
   replaceDecorators: true,

src/plugin/__tests__/__fixtures__/react/index.test.ts

@@ -1,7 +1,7 @@
 import { resolve } from 'path';
 
 import { getTransformOptions } from '../../../options';
-import { runFixtureTests } from '../..';
+import { runFixtureTests } from '../../../util/test-runner';
 
 describe('React', () => {
   runFixtureTests(

src/plugin/__tests__/__fixtures__/types/index.test.ts

@@ -1,7 +1,7 @@
 import { resolve } from 'path';
 
 import { getTransformOptions } from '../../../options';
-import { runFixtureTests } from '../..';
+import { runFixtureTests } from '../../../util/test-runner';
 
 describe('Types', () => {
   runFixtureTests(

src/plugin/converters/class.ts

@@ -7,34 +7,34 @@ import {
 } from '@babel/types';
 
 import { PluginPass } from '../types';
-import { PluginWarnings, WARNINGS } from '../util/warning';
+import { replaceClassDecorators } from '../optimizers/decorators';
+import { WARNINGS, logWarning } from '../util/warnings';
+import { BaseNode } from '../visitors/base';
 import {
   convertTypeParameterInstantiation,
   convertTypeParameterDeclaration,
 } from './type-parameter';
-import { replaceClassDecorators } from '../optimizers/decorators';
-import { BaseVisitorNodes } from '../visitors/base';
 
 export function convertClassDeclaration(
   path: NodePath<ClassDeclaration>,
-  state: PluginPass<BaseVisitorNodes>,
+  state: PluginPass<BaseNode>,
 ): NodePath<ClassDeclaration> {
   const { node } = path;
   const { body, superTypeParameters, typeParameters } = node;
 
   if (isTypeParameterInstantiation(superTypeParameters)) {
-    node.superTypeParameters = convertTypeParameterInstantiation(superTypeParameters);
+    node.superTypeParameters = convertTypeParameterInstantiation(superTypeParameters, state);
   }
 
   if (isTypeParameterDeclaration(typeParameters)) {
-    node.typeParameters = convertTypeParameterDeclaration(typeParameters);
+    node.typeParameters = convertTypeParameterDeclaration(typeParameters, state);
   }
 
   // Flow allows to specify the return type of constructor functions. This is
   // forbidden in TypeScript. So the the type annotation needs to be removed.
   body.body.forEach(elem => {
     if (isClassMethod(elem) && elem.kind === 'constructor' && elem.returnType) {
-      PluginWarnings.enable(WARNINGS.class.constructorReturnType);
+      logWarning(WARNINGS.class.constructorReturnType, state.file.code, node.loc);
       elem.returnType = null;
     }
   });

src/plugin/converters/declaration.ts

@@ -45,33 +45,34 @@ import {
 } from '@babel/types';
 
 import { UnexpectedError, NotImplementedError } from '../../util/error';
+import { ConverterState } from '../types';
 import { convertFlowType } from './flow-type';
 import { functionTypeParametersToIdentifiers } from './function';
+import { convertInterfaceDeclaration } from './interface';
 import {
   convertTypeParameterDeclaration,
   convertTypeParameterInstantiation,
 } from './type-parameter';
-import { convertInterfaceDeclaration } from './interface';
 
 function isConstructor(key: Identifier | StringLiteral): boolean {
   const methodName = isIdentifier(key) ? key.name : key.value;
 
   return methodName === 'constructor';
 }
 
-function createClassBody(node: ObjectTypeAnnotation): ClassBody {
+function createClassBody(node: ObjectTypeAnnotation, state: ConverterState): ClassBody {
   return classBody(
     node.properties.map(prop => {
       if (isObjectTypeProperty(prop)) {
         if (prop.method && isFunctionTypeAnnotation(prop.value)) {
           const { value, key } = prop;
 
-          const typeParameters = convertTypeParameterDeclaration(value.typeParameters);
-          const params = functionTypeParametersToIdentifiers(value.params) || [];
+          const typeParameters = convertTypeParameterDeclaration(value.typeParameters, state);
+          const params = functionTypeParametersToIdentifiers(value.params, state) || [];
 
           const returnType = isConstructor(key)
             ? null
-            : tsTypeAnnotation(convertFlowType(value.returnType));
+            : tsTypeAnnotation(convertFlowType(value.returnType, state));
 
           const method = tsDeclareMethod(null, key, typeParameters, params, returnType);
 
@@ -82,7 +83,7 @@ function createClassBody(node: ObjectTypeAnnotation): ClassBody {
         } else {
           const classProp = classProperty(prop.key);
 
-          classProp.typeAnnotation = tsTypeAnnotation(convertFlowType(prop.value));
+          classProp.typeAnnotation = tsTypeAnnotation(convertFlowType(prop.value, state));
           classProp.optional = prop.optional;
           classProp.static = prop.static;
 
@@ -95,17 +96,24 @@ function createClassBody(node: ObjectTypeAnnotation): ClassBody {
   );
 }
 
-export function convertDeclareClass(node: DeclareClass, declare: boolean = true): ClassDeclaration {
-  const classDec = classDeclaration(node.id, null, createClassBody(node.body));
+export function convertDeclareClass(
+  node: DeclareClass,
+  state: ConverterState,
+  declare: boolean = true,
+): ClassDeclaration {
+  const classDec = classDeclaration(node.id, null, createClassBody(node.body, state));
 
   classDec.declare = declare;
-  classDec.typeParameters = convertTypeParameterDeclaration(node.typeParameters);
+  classDec.typeParameters = convertTypeParameterDeclaration(node.typeParameters, state);
 
   node.extends &&
     node.extends.forEach(_extend => {
       if (isIdentifier(_extend.id)) {
         classDec.superClass = _extend.id;
-        classDec.superTypeParameters = convertTypeParameterInstantiation(_extend.typeParameters);
+        classDec.superTypeParameters = convertTypeParameterInstantiation(
+          _extend.typeParameters,
+          state,
+        );
       }
     });
 
@@ -114,6 +122,7 @@ export function convertDeclareClass(node: DeclareClass, declare: boolean = true)
 
 export function convertDeclareFunction(
   node: DeclareFunction,
+  state: ConverterState,
   declare: boolean = true,
 ): TSDeclareFunction {
   const { id } = node;
@@ -124,9 +133,9 @@ export function convertDeclareFunction(
     isFunctionTypeAnnotation(id.typeAnnotation.typeAnnotation)
   ) {
     const { typeAnnotation } = id.typeAnnotation;
-    const typeParameters = convertTypeParameterDeclaration(typeAnnotation.typeParameters);
-    const returnType = tsTypeAnnotation(convertFlowType(typeAnnotation.returnType));
-    const params = functionTypeParametersToIdentifiers(typeAnnotation.params) || [];
+    const typeParameters = convertTypeParameterDeclaration(typeAnnotation.typeParameters, state);
+    const returnType = tsTypeAnnotation(convertFlowType(typeAnnotation.returnType, state));
+    const params = functionTypeParametersToIdentifiers(typeAnnotation.params, state) || [];
 
     functionDec = tsDeclareFunction(id, typeParameters, params, returnType);
   } else {
@@ -140,39 +149,43 @@ export function convertDeclareFunction(
 
 export function convertDeclareInterface(
   node: DeclareInterface,
+  state: ConverterState,
   declare: boolean = true,
 ): TSInterfaceDeclaration {
-  const interfaceDec = convertInterfaceDeclaration(node);
+  const interfaceDec = convertInterfaceDeclaration(node, state);
   interfaceDec.declare = declare;
 
   return interfaceDec;
 }
 
-function convertModuleStatement(statement: Statement | Flow): Declaration {
+function convertModuleStatement(statement: Statement | Flow, state: ConverterState): Declaration {
   if (isDeclareClass(statement)) {
-    return convertDeclareClass(statement, false);
+    return convertDeclareClass(statement, state, false);
   }
 
   if (isDeclareFunction(statement)) {
-    return convertDeclareFunction(statement, false);
+    return convertDeclareFunction(statement, state, false);
   }
 
   if (isDeclareInterface(statement)) {
-    return convertDeclareInterface(statement, false);
+    return convertDeclareInterface(statement, state, false);
   }
 
   if (isDeclareVariable(statement)) {
     return variableDeclaration('var', [variableDeclarator(statement.id)]);
   }
 
   if (isInterfaceDeclaration(statement)) {
-    return convertInterfaceDeclaration(statement);
+    return convertInterfaceDeclaration(statement, state);
   }
 
   throw new NotImplementedError(`No conversion for ${statement.type}`);
 }
 
-export function convertDeclareModule(node: DeclareModule): TSModuleDeclaration {
+export function convertDeclareModule(
+  node: DeclareModule,
+  state: ConverterState,
+): TSModuleDeclaration {
   const statements = node.body.body.reduce<Statement[]>((acc, statement) => {
     if (isDeclareExportDeclaration(statement) && statement.declaration) {
       const { declaration, specifiers } = statement;
@@ -181,7 +194,9 @@ export function convertDeclareModule(node: DeclareModule): TSModuleDeclaration {
         // Arbitrary expressions are forbidden in export assignments in ambient
         // contexts in TS. So add a `_default` property, initialize it with the
         // Flow expression and export that by default (https://bit.ly/2Wi3b88).
-        const defaultExport = isFlowType(declaration) ? convertFlowType(declaration) : declaration;
+        const defaultExport = isFlowType(declaration)
+          ? convertFlowType(declaration, state)
+          : declaration;
         const defaultPropId = identifier('_default');
 
         defaultPropId.typeAnnotation = isTSType(defaultExport)
@@ -193,10 +208,12 @@ export function convertDeclareModule(node: DeclareModule): TSModuleDeclaration {
         acc.push(defaultProp);
         acc.push(exportDefaultDeclaration(identifier('_default')));
       } else {
-        acc.push(exportNamedDeclaration(convertModuleStatement(declaration), specifiers || []));
+        acc.push(
+          exportNamedDeclaration(convertModuleStatement(declaration, state), specifiers || []),
+        );
       }
     } else {
-      acc.push(convertModuleStatement(statement));
+      acc.push(convertModuleStatement(statement, state));
     }
 
     return acc;
@@ -208,9 +225,16 @@ export function convertDeclareModule(node: DeclareModule): TSModuleDeclaration {
   return moduleDec;
 }
 
-export function convertDeclareTypeAlias(node: DeclareTypeAlias): TSTypeAliasDeclaration {
-  const typeParameters = convertTypeParameterDeclaration(node.typeParameters);
-  const typeAliasDec = tsTypeAliasDeclaration(node.id, typeParameters, convertFlowType(node.right));
+export function convertDeclareTypeAlias(
+  node: DeclareTypeAlias,
+  state: ConverterState,
+): TSTypeAliasDeclaration {
+  const typeParameters = convertTypeParameterDeclaration(node.typeParameters, state);
+  const typeAliasDec = tsTypeAliasDeclaration(
+    node.id,
+    typeParameters,
+    convertFlowType(node.right, state),
+  );
 
   typeAliasDec.declare = true;