From 3f3df5e995ccbd8e07765e4f55f1f5c7a34ddfc7 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 5 Jul 2019 14:20:47 -0500 Subject: [PATCH 1/2] Added architecture and testing documentation --- docs/Architecture.md | 40 ++++++++++++---- docs/Dependencies.md | 64 +++++++++++++++++++++++++ docs/Development.md | 7 +-- docs/Testing.md | 20 ++++++++ src/conversion/convertConfig.ts | 17 +++++-- src/rules/converters/no-banned-terms.ts | 1 + src/rules/mergers/ban-types.ts | 1 + 7 files changed, 136 insertions(+), 14 deletions(-) create mode 100644 docs/Dependencies.md diff --git a/docs/Architecture.md b/docs/Architecture.md index 7116111d1..c27222891 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -2,13 +2,37 @@ ## CLI Architecture -- CLI usage starts in `bin/tslint-to-eslint-config`, which immediately calls `src/cli/main.ts`. -- CLI settings are parsed and read in `src/cli/runCli.ts`. -- Application logic is run by `src/conversion/convertConfig.ts`. +1. CLI usage starts in `bin/tslint-to-eslint-config`, which immediately calls `src/cli/main.ts`. +2. CLI settings are parsed and read in `src/cli/runCli.ts`. +3. Application logic is run by `src/conversion/convertConfig.ts`. -## Dependencies +## Configuration Conversion -Methods are created using a variant of [Dependency Injection](http://en.wikipedia.org/wiki/Dependency_Injection). -Any method with dependencies that should be stubbed out during tests, such as file system bindings, logging, or other methods, -takes in a first argument of name `dependencies`. -Its dependencies object is manually created in `src/cli/main.ts` and bound to the method with `bind`. +Within `src/conversion/convertConfig.ts`, the following steps occur: + +1. Existing configurations are read from disk +2. TSLint rules are converted into their ESLint configurations +3. ESLint configurations are simplified based on extended ESLint presets +4. The simplified configuration is written to the output config file +5. A summary of the results is printed to the user's console + +### Rule Converters + +Each TSLint rule should output at least one ESLint rule as the equivalent. +"Converters" for TSLint rules are located in `src/rules/converters/`, and keyed under their names by the map in `src/rules/converters.ts`. + +Each converter for a TSLint rule takes an arguments object for the rule, and returns an array of objects containing: + +- `ruleName`: Equivalent ESLint rule name that should be enabled +- `ruleArguments`: Any arguments for that ESLint rule + +Multiple objects must be supported because some general-use TSLint rules can only be represented by two or more ESLint rules. +For example, TSLint's `no-banned-terms` is represented by ESLint's `no-caller` and `no-eval`. + +### Rule Mergers + +It's possible that one ESLint rule will be output by multiple converters. +"Mergers" for those ESLint rules should take in two configurations to the same rule and output the equivalent single configuration. +These are located in `src/rules/mergers/`, and keyed under their names by the map in `src/rules/mergers.ts`. + +For example, `@typescript-eslint/ban-types` spreads both arguments' `types` members into one large `types` object. diff --git a/docs/Dependencies.md b/docs/Dependencies.md new file mode 100644 index 000000000..8323b1781 --- /dev/null +++ b/docs/Dependencies.md @@ -0,0 +1,64 @@ +# Dependencies + +Functions are created using a variant of [Dependency Injection](http://en.wikipedia.org/wiki/Dependency_Injection). +Any method with dependencies that should be stubbed out during tests, such as file system bindings, logging, or other functions, +takes in a first argument of name `dependencies`. +Its dependencies object is manually created in `src/cli/main.ts` and bound to the method with `bind`. + +## When to Use Dependencies + +Most functions don't need a `dependencies` object. +Only add one if something should be stubbed out during tests. + +## How to Use Dependencies + +Suppose your method `myMethod`, should take in a `fileSystem`, a `string`, and a `number`: + +1. Create a `MyMethodDependencies` type in `myMethod.ts`: + + ```ts + // ~~~/myMethod.ts + export type MyMethodDependencies = { + fileSystem: FileSystem; + }; + ``` + +2. Add `dependencies: MyMethodDependencies` as the first argument to `myMethod`: + + ```ts + // ~~~/myMethod.ts + export const myMethod = async ( + dependencies: MyMethodDependencies, + argumentOne: string, + argumentTwo: number, + ) => { + // ... + }; + ``` + +3. In `src/cli/main.ts`, create a `myMethodDependencies: MyMethodDependencies`: + + ```ts + // src/cli/main.ts + const myMethodDependencies: MyMethodDependencies = { + fileSystem, + }; + ``` + +4. In `src/cli/main.ts`, include `myMethod: bind(mymethod, myMethodDependencies)` in any dependencies object that requires `myMethod`: + + ```ts + // src/cli/main.ts + const otherMethodDependencies: OtherMethodDependencies = { + myMethod: bind(myMethod, myMethodDependencies), + }; + ``` + +5. In the types of any dependencies that include `myMethod`, add `myMethod: SansDependencies` to require the result of `bind`ing `myMethod`: + + ```ts + // ~~~/otherMethod.ts + export type OtherMethodDependencies = { + myMethod: SansDependencies; + }; + ``` diff --git a/docs/Development.md b/docs/Development.md index e35725807..776aee9e1 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -19,7 +19,8 @@ npm i Compile with `npm run tsc` and run tests with `npm run test:unit`. -## More Reading +## Further Reading -- [Architecture](./Architecture.md) -- [Testing](./Testing.md) +- [Architecture](./Architecture.md): How the general app structure operates +- [Dependencies](./Dependencies.md): How functions pass and receive static dependencies +- [Testing](./Testing.md): Unit and end-to-end tests diff --git a/docs/Testing.md b/docs/Testing.md index 0ff7b7d87..f7178a55f 100644 --- a/docs/Testing.md +++ b/docs/Testing.md @@ -18,3 +18,23 @@ Tests should include comments above each section of the "AAA" testing format: - `src/cli/main.ts` - `src/adapters/*.ts` - `src/**/*.stubs.ts` + +See [Dependencies](./Dependencies.md) for how static dependencies are stubbed out in functions. +That system is how functions can receive stubs and spies during unit tests. + +## End-to-End Tests + +```shell +npm run test:end-to-end +``` + +End-to-end tests that execute the `bin/tslint-to-eslint` command and validate outputs are generated from the directories in `test/tests/`. +Each directory there contains: + +- `.eslintrc.json`: `.gitignore`d output from the most recent test run +- `expected.json`: Expected output ESLint configuration +- `stderr.txt`: Any output written to the process `stderr` +- `stdout.txt`: Any output written to the process `stdout` +- `tslint.json`: Original TSLint configuration file to convert + +Within each directory, a test suite will execute `bin/tslint-to-eslint` and validate the outputs match what's on disk. diff --git a/src/conversion/convertConfig.ts b/src/conversion/convertConfig.ts index 81e8fbb9b..8091c965c 100644 --- a/src/conversion/convertConfig.ts +++ b/src/conversion/convertConfig.ts @@ -14,19 +14,27 @@ export type ConvertConfigDependencies = { writeConversionResults: SansDependencies; }; +/** + * Root-level driver to convert a TSLint configuration to ESLint. + * @see `Architecture.md` for documentation. + */ export const convertConfig = async ( dependencies: ConvertConfigDependencies, settings: TSLintToESLintSettings, ): Promise => { + // 1. Existing configurations are read const originalConfigurations = await dependencies.findOriginalConfigurations(settings); if (originalConfigurations.status !== ResultStatus.Succeeded) { return originalConfigurations; } + // 2. TSLint rules are converted into their ESLint configurations const ruleConversionResults = dependencies.convertRules( originalConfigurations.data.tslint.rules, ); - const mergedConfiguration = { + + // 3. ESLint configurations are simplified based on extended ESLint presets + const simplifiedConfiguration = { ...ruleConversionResults, ...(await dependencies.simplifyPackageRules( originalConfigurations.data.eslint, @@ -34,12 +42,15 @@ export const convertConfig = async ( )), }; + // 4. The simplified configuration is written to the output config file await dependencies.writeConversionResults( settings.config, - mergedConfiguration, + simplifiedConfiguration, originalConfigurations.data, ); - dependencies.reportConversionResults(mergedConfiguration); + + // 5. A summary of the results is printed to the user's console + dependencies.reportConversionResults(simplifiedConfiguration); return { status: ResultStatus.Succeeded, diff --git a/src/rules/converters/no-banned-terms.ts b/src/rules/converters/no-banned-terms.ts index 4e653cb90..8f7760dab 100644 --- a/src/rules/converters/no-banned-terms.ts +++ b/src/rules/converters/no-banned-terms.ts @@ -2,6 +2,7 @@ import { RuleConverter } from "../converter"; export const convertNoBannedTerms: RuleConverter = () => { return { + // This is mentioned in Architecture.md as a TSLint rule with two ESLint equivalents rules: [ { ruleName: "no-caller", diff --git a/src/rules/mergers/ban-types.ts b/src/rules/mergers/ban-types.ts index d5c06e338..e6597f810 100644 --- a/src/rules/mergers/ban-types.ts +++ b/src/rules/mergers/ban-types.ts @@ -5,6 +5,7 @@ export const mergeBanTypes: RuleMerger = (existingOptions, newOptions) => { return []; } + // This is mentioned in Architecture.md as an ESLint rule with a merger return [ { types: { From 87ac94914f6cb336a52388e8c96b2b13e0cbda47 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 6 Jul 2019 14:33:37 -0500 Subject: [PATCH 2/2] Documented overall conversion results --- docs/Architecture.md | 10 ++++++++++ src/rules/converter.ts | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/docs/Architecture.md b/docs/Architecture.md index c27222891..610fa0834 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -16,6 +16,10 @@ Within `src/conversion/convertConfig.ts`, the following steps occur: 4. The simplified configuration is written to the output config file 5. A summary of the results is printed to the user's console +### Conversion Results + +The overall configuration generated by steps 2-3 and printed in 4-5 contains the following information: + ### Rule Converters Each TSLint rule should output at least one ESLint rule as the equivalent. @@ -23,6 +27,12 @@ Each TSLint rule should output at least one ESLint rule as the equivalent. Each converter for a TSLint rule takes an arguments object for the rule, and returns an array of objects containing: +- `rules`: At least one equivalent ESLint rule and options +- `notices`: Any extra info that should be printed after conversion +- `plugins`: Any plugins that should now be installed if not already + +The `rules` output is an array of objects containing: + - `ruleName`: Equivalent ESLint rule name that should be enabled - `ruleArguments`: Any arguments for that ESLint rule diff --git a/src/rules/converter.ts b/src/rules/converter.ts index 77d63fb2a..7acee8fbe 100644 --- a/src/rules/converter.ts +++ b/src/rules/converter.ts @@ -37,6 +37,13 @@ export type ConversionResult = { * An ESLint rule equivalent to a previously enabled TSLint rule. */ export type ConvertedRuleChanges = { + /** + * Any arguments for that ESLint rule. + */ ruleArguments?: any[]; + + /** + * Equivalent ESLint rule name that should be enabled. + */ ruleName: string; };