Merge pull request #47 from guor8lei/refactor/test-app

Refactor tests

README.md

@@ -2,22 +2,21 @@
 
 A CLI tool that automatically upgrades TypeScript codebases to conform to strict compiler flags using the `ts-morph` library and TypeScript compiler API.
 
-
-Table of Contents
-=================
-
-  * [Getting Started](#getting-started)
-      * [Usage](#usage)
-      * [Prerequisite](#prerequisite)
-      * [User Flow](#user-flow)
-  * [Developer and Maintenance Guide](#developer-and-maintenance-guide)
-      * [Supported Flags and Fixes](#supported-flags-and-fixes)
-      * [Unsupported Cases](#unsupported-cases)
-      * [Architecture](#architecture)
-      * [Testing](#testing)
-      * [Linting](#linting)
-  * [Read More](#read-more)
-  * [Source Code Headers](#source-code-headers)  
+## Table of Contents
+
+- [Getting Started](#getting-started)
+  - [Usage](#usage)
+  - [Prerequisite](#prerequisite)
+  - [User Flow](#user-flow)
+- [Developer and Maintenance Guide](#developer-and-maintenance-guide)
+  - [Supported Flags and Fixes](#supported-flags-and-fixes)
+  - [Removing Flag Fix Functionality](#removing-flag-fix-functionality)
+  - [Unsupported Cases](#unsupported-cases)
+  - [Architecture](#architecture)
+  - [Testing](#testing)
+  - [Linting](#linting)
+- [Read More](#read-more)
+- [Source Code Headers](#source-code-headers)
 
 ## Getting Started
 
@@ -33,35 +32,36 @@ cd typescript-flag-upgrade
 Then run the following command with the corresponding arguments:
 
 ```
-npm run dev -- -p <relative-path-to-tsconfig> [-i <relative-path-to-input-dir>]
+npm run dev -- -p <relative-path-to-tsconfig> [-i <relative-path-to-input-dir>] [-f]
 ```
 
-More specifically, the mandatory `-p` argument specifies the relative path to the `tsconfig.json` file of the TypeScript project to be upgraded. The tool will automatically retrieve all TypeScript files within the project based on the `tsconfig.json`. In the case you want to specify a separate or subdirectory to upgrade, you can specify the optional `-i` flag to override the input directory.
+More specifically, the mandatory `-p` argument specifies the relative path to the `tsconfig.json` file of the TypeScript project to be upgraded. The tool will automatically retrieve all TypeScript files within the project based on the `tsconfig.json`. In the case you want to specify a separate or subdirectory to upgrade, you can specify the optional `-i` flag to override the input directory. The `-f` optional flag specifies whether or not to format the output files.
 
 After running the command, the tool mutatively changes the input TypeScript files to fix for any errors that it found. Before each change, the tool inserts a comment of the form: `// typescript-flag-upgrade automated fix: --[compilerFlag]`.
 
-### Prerequisite
-Before running this tool, ensure that your TypeScript project is already compiling with no errors. This means that you should set any compiler flags that are causing errors to `false`. If the input TypeScript project has compiler errors, then this tool will error out and stop execution. After running the tool, reactivate the compiler flags that were originally causing the errors, and the number of errors should have decreased.
-
 For help, run:
 
 ```
 npm run dev -- --help
 ```
 
+### Prerequisite
+
+Before running this tool, ensure that your TypeScript project is already compiling with no errors. This means that you should set any compiler flags that are causing errors to `false`. If the input TypeScript project has compiler errors, then this tool will error out and stop execution. After running the tool, reactivate the compiler flags that were originally causing the errors, and the number of errors should have decreased.
+
 ### User Flow
 
 This tool was designed to be used iteratively as a way to semi-automatically fix and speed up the upgrading process for engineers:
 
 1. The engineer ensures that the TypeScript project is fully compiling with the flags set to false.
-2. The engineer runs the tool on the TypeScript project. During the run, the tool fixes as many errors as possible automatically. For errors that the tool is unable to fix automatically (see Supported Flags and Fixes), the tool logs a message to the engineer detailing the location of the error, as well as the relative priority of the error (based on how many other errors are associated with this location). 
+2. The engineer runs the tool on the TypeScript project. During the run, the tool fixes as many errors as possible automatically. For errors that the tool is unable to fix automatically (see Supported Flags and Fixes), the tool logs a message to the engineer detailing the location of the error, as well as the relative priority of the error (based on how many other errors are associated with this location).
 3. The engineer reads through the log to determine the highest priority errors and fixes to make.
-4. After fixing the highest priority errors, the engineer can re-run this tool, which will then propogate this fix other errors associated with the high-priority fixes that the engineer just made. 
+4. After fixing the highest priority errors, the engineer can re-run this tool, which will then propogate this fix other errors associated with the high-priority fixes that the engineer just made.
 5. This process can be repeated as a way to iteratively upgrade larger TypeScript codebases.
 
 ## Developer and Maintenance Guide
 
-This tool was designed, implemented, and tested within a period of 3 months, and there are many areas in which it can be improved.
+This tool was designed, implemented, and tested within a period of 3 months, and there are many areas in which it can be improved. Here are the current supported features of this tool, as well as unsupported cases.
 
 ### Supported Flags and Fixes
 
@@ -134,6 +134,13 @@ The engineer can then parse through these logs to determine which variables are
 
 In addition, because it was found that Jasmine testing files often containing multiple instances of variable run-time types not matching their compile-time types (caused by functions in the `TestBed` class, which have a compile-time return type of `any`). Also, in multiple test files, there were cases of accessing private methods and fields, which would only be allowed at compile-time for variables of type `any`. Because there are no easy solutions for these cases, the `NoImplicitAnyManipulator` skips fixes for all test files (any file ending in `_test.ts` or `.spec.ts`) in order to prevent making unwanted changes.
 
+### Removing Flag Fix Functionality
+
+In the case where the fixes for one of the flags is deemed as an additional burden, you can remove that particular flag fix functionality from tool by going through the following steps:
+
+1. In the constructor of `Runner`, remove the corresponding manipulator from the list of manipulators.
+2. In `createProject()` of `Runner`, update the default compiler flag options for the `setCompilerOptions` variable.
+
 ### Unsupported Cases
 
 This tool does not support more complex non-scalar types, such as (untyped or empty) arrays and objects. This is mainly due to the complexities of each of these data structures, which require hard-coding to take care of all cases. It was decided to leave these cases up to the engineer to fix with output logs as opposed to attempting to fix for all cases and potentially causing a runtime error.

src/index.ts

@@ -38,6 +38,13 @@ const args = yargs
       normalize: true,
       type: 'string',
     },
+    f: {
+      alias: 'format',
+      demandOption: false,
+      default: DEFAULT_ARGS.f,
+      description: 'Format emitted files',
+      type: 'boolean',
+    },
   })
   .usage('typescript-flag-upgrade -p <path-to-config> [-i <path-to-input-dir>]')
   .epilogue('Copyright 2020 Google LLC').argv;

src/runner.ts

@@ -39,6 +39,7 @@ export class Runner {
   private emitter: Emitter;
   private logger: Logger;
   private errorDetector: ErrorDetector;
+  private format: boolean | undefined;
 
   private manipulators: Manipulator[];
 
@@ -62,6 +63,7 @@ export class Runner {
     logger?: Logger
   ) {
     args = args || DEFAULT_ARGS;
+    this.format = args.f;
     this.parser = parser || new Parser();
     if (project) {
       this.project = project;
@@ -134,7 +136,7 @@ export class Runner {
     } while (errorsExist && !_.isEqual(errors, prevErrors));
 
     // Format and emit project.
-    this.emitter.format(modifiedSourceFilePaths, this.project);
+    if (this.format) this.emitter.format(modifiedSourceFilePaths, this.project);
     this.emitter.emit(this.project);
   }
 

src/types.ts

@@ -22,13 +22,16 @@ export type ArgumentOptions = {
   p: string;
   /** Override config and specify input directory */
   i?: string;
+  /** Format emitted files */
+  f?: boolean;
   _: string[];
   $0: string;
 };
 
 export const DEFAULT_ARGS = {
   /** Relative path to TypeScript config file */
   p: 'tsconfig.json',
+  f: false,
   _: [],
   $0: '',
 };

test/test_app/src/app/no_implicit_any/no_implicit_any_component.ts

@@ -30,20 +30,11 @@ export class NoImplicitAnyComponent implements OnInit {
 
   ngOnInit(): void {}
 
-  // Test: No information regarding variable type
-  // Fix: noInfo(name: any) {
+  // Test: Ignores when no information regarding variable type
   noInfo(name) {
     return name;
   }
 
-  // Test: Infer type from method signatures called by func
-  // Fix: callsTakeString(val: string) {
-  callsTakeString(val) {
-    this.takeString(val);
-  }
-
-  takeString(val: string) {}
-
   // Test: Infer type from method signatures calling func
   passString(val: string) {
     this.calledByPassString(val);
@@ -53,22 +44,6 @@ export class NoImplicitAnyComponent implements OnInit {
   // Fix: calledByPassString(val: string) {}
   calledByPassString(val) {}
 
-  // Test: Infer non-scalar type from method signatures called by func
-  // Fix: callsTakeObject(val: BasicInterface) {
-  callsTakeObject(val) {
-    this.takeObject(val);
-  }
-
-  takeObject(val: BasicInterface) {}
-
-  // Test: Infer non-scalar types from method signatures
-  passObject(val: BasicInterface) {
-    this.calledByPassObject(val);
-  }
-
-  // Fix: calledByPassObject(val: BasicInterface) {}
-  calledByPassObject(val) {}
-
   // Test: Concatenates multiple possible types
   passNum(val: number) {
     this.calledByPassNumAndPassString(val);
@@ -77,63 +52,51 @@ export class NoImplicitAnyComponent implements OnInit {
   // Fix: calledByPassString(val: string | number) {}
   calledByPassNumAndPassString(val) {}
 
-  // Test: Supports chaining
-  // Fix: callsCallsTakeString(val: string) {
-  callsCallsTakeString(val) {
-    this.callsTakeString(val);
+  // Test: Infer non-scalar types from method signatures calling func
+  passObject(val: BasicInterface) {
+    this.calledByPassObject(val);
   }
 
+  // Fix: calledByPassObject(val: BasicInterface) {}
+  calledByPassObject(val) {}
+
   // Test: Spans across multiple files
-  // Fix: callsTakeStringInSeparateClass(val: string) {
-  callsTakeStringInSeparateClass(val) {
-    new BasicClass().takeStringSeparateClass(val);
+  // Fix: callsReturnsStringInSeparateClass(val: string) {
+  callsReturnsStringInSeparateClass(val) {
+    val = new BasicClass().returnsString();
   }
 
-  // Fix: callsTakeStringInSeparateClass(val: BasicInterface) {
-  callsTakeObjectInSeparateClass(val) {
-    new BasicClass().takeObjectSeparateClass(val);
+  // Fix: callsReturnsObjectInSeparateClass(val: BasicInterface) {
+  callsReturnsObjectInSeparateClass(val) {
+    val = new BasicClass().returnsObject();
   }
 
-  // Lists
+  // Lists: UNSUPPORTED
 
   // Test: Assign any array to defined type array
   assignToDefinedArray() {
     let mustBeStringArray: string[];
-    // Fix: const anyArray: string[] = [];
     const anyArray = [];
     mustBeStringArray = anyArray;
   }
 
   assignToDefinedNonScalarArray() {
     let mustBeObjectArray: BasicInterface[];
-    // Fix: const anyArray: BasicInterface[] = [];
     const anyArray = [];
     mustBeObjectArray = anyArray;
   }
 
-  // Objects: For all object tests, possibility to leave smart comments detailing what the object structure should be
+  // Objects: UNSUPPORTED
 
   // Test: Accessing an empty object
   accessEmptyObjectProperty() {
-    // Fix: const emptyObject: any = {};
     const emptyObject = {};
     emptyObject['property'] = true;
   }
 
   // Test: Accessing a non-empty object
   accessNonEmptyObjectProperty() {
-    // Fix: const nonEmptyObject: any = { already: false };
     const nonEmptyObject = {already: false};
     nonEmptyObject['property'] = true;
   }
-
-  // Fix: printUnknownObject(unknownObject: any) {
-  printUnknownObject(unknownObject) {
-    console.log(unknownObject.foo + unknownObject.bar);
-  }
-
-  // Fix: printUnknownObjectWithProps(unknownObject: any) {
-  printUnknownObjectWithProps(unknownObject) {
-    Math.abs(unknownObject.baz);
-  }
 }

test/test_app/src/app/strict_null_checks/strict_null_checks_component.ts

@@ -87,26 +87,18 @@ export class StrictNullChecksComponent implements OnInit {
     n = null;
     u = undefined;
   }
-
-  // Test: Pass null value to function
-  // Fix: doesNotExpectNull(n: number | null) {}
-  doesNotExpectNull(n: number) {}
-
-  passesNullValue() {
-    this.doesNotExpectNull(null);
-  }
-
   // Test: Pass null and undefined value to function
-  // Fix: doesNotExpectNullUndefined(n: number | null | undefined) {}
   doesNotExpectNullUndefined(n: number) {}
 
   passesNullVariable() {
     const n: number | null = null;
+    // Fix: this.doesNotExpectNullUndefined(n!);
     this.doesNotExpectNullUndefined(n);
   }
 
   passesUndefinedVariable() {
     const n: number | undefined = undefined;
+    // Fix: this.doesNotExpectNullUndefined(n!);
     this.doesNotExpectNullUndefined(n);
   }
 
@@ -127,21 +119,20 @@ export class StrictNullChecksComponent implements OnInit {
     return 5;
   }
 
-  // Test: Add element to empty list
+  // UNSUPPORTED Test: Add element to empty list.
   addToEmptyList() {
-    // Fix: const emptyList: any[] = [];
     const emptyList = [];
     emptyList.push(5);
 
-    // Fix: const emptyListNonScalar: BasicClass[] = [];
     const emptyListNonScalar = [];
     emptyListNonScalar.push(new BasicClass());
   }
 
   // Test: Return null value but not in return type
-  // Fix: unexpectedlyReturnsNull(): number | null {
   unexpectedlyReturnsNull(): number {
-    return null;
+    const n: number | null = null;
+    // Fix: return n!;
+    return n;
   }
 
   // Test: Object is possibly null