[Experimental Feature] Opt in flag convert disable comments (#246)

* Set basic replace comment conversion

* Fix regex pattern and cli command name for convert comments

* Fix regex pattern condition to match rules without carriage return

* Replace regex and use tslint functions to retrieve tslint comment data

* Switching rule handler

Convert the input TSLint rule into equivalent ESLint

* Write file after switching rule comments

This is still a work in progress, need to check the start and end
position of rule comments to avoid overwriting the wrong lines.

* replace old path to rulesConverter

* Working replacements for single line changes

* 4am and it works

* Revert .prettierrc testing change

* Love that nullish operator

* Comments WIP

* Seems to be all working and tested now

* Feature add: logged directive counts as a summary

* Docs in CLI and README.md

* Filled in basic test for separateErrors

* Normalized output per new norms

Co-authored-by: Josh Goldberg <[email protected]>

Author: KingDarBoja

README.md

@@ -42,6 +42,7 @@ We **strongly** advise reading [docs/FAQs.md](./docs/FAQs.md) before planning yo
 
 Each of these flags is optional:
 
+-   **[`comments`](#comments)**: File glob path(s) to convert TSLint rule flags to ESLint within.
 -   **[`config`](#config)**: Path to print the generated ESLint configuration file to.
 -   **[`editor`](#editor)**: Path to an editor configuration file to convert linter settings within.
 -   **[`eslint`](#eslint)**: Path to an ESLint configuration file to read settings from.
@@ -50,6 +51,17 @@ Each of these flags is optional:
 -   **[`tslint`](#tslint)**: Path to a TSLint configuration file to read settings from.
 -   **[`typescript`](#typescript)**: Path to a TypeScript configuration file to read TypeScript compiler options from.
 
+#### `comments`
+
+```shell
+npx tslint-to-eslint-config --comments src/**/*.ts
+```
+
+_Default: none_
+
+File glob path(s) to convert [TSLint rule flags](https://palantir.github.io/tslint/usage/rule-flags) to [ESLint inline comments](https://eslint.org/docs/user-guide/configuring#disabling-rules-with-inline-comments) in.
+Comments such as `// tslint:disable: tslint-rule-name` will be converted to equivalents like `// eslint-disable eslint-rule-name`.
+
 #### `config`
 
 ```shell

docs/Architecture.md

@@ -16,7 +16,8 @@ Within `src/conversion/convertConfig.ts`, the following steps occur:
 3. ESLint configurations are simplified based on extended ESLint and TSLint presets
     - 3a. If no output rules conflict with `eslint-config-prettier`, it's added in
 4. The simplified configuration is written to the output config file
-5. A summary of the results is printed to the user's console
+5. Files to transform comments in have source text rewritten using the same rule conversion logic
+6. A summary of the results is printed to the user's console
 
 ### Conversion Results
 

package-lock.json

@@ -13,6 +13,7 @@
         "chalk": "4.0.0",
         "commander": "5.1.0",
         "eslint-config-prettier": "6.11.0",
+        "glob": "7.1.6",
         "strip-json-comments": "3.1.0",
         "tslint": "6.1.2",
         "typescript": "3.8.3"
@@ -23,6 +24,7 @@
         "@babel/plugin-proposal-optional-chaining": "7.9.0",
         "@babel/preset-env": "7.9.5",
         "@babel/preset-typescript": "7.9.0",
+        "@types/glob": "^7.1.1",
         "@types/jest": "25.2.1",
         "@types/node": "12.12.21",
         "@typescript-eslint/eslint-plugin": "2.29.0",

package.json

@@ -3,9 +3,3 @@ export const createStubFileSystem = ({ data = {}, exists = true } = {}) => ({
     readFile: jest.fn().mockReturnValue(Promise.resolve(data)),
     writeFile: jest.fn(),
 });
-
-export const createStubThrowingFileSystem = ({ err = "" } = {}) => ({
-    fileExists: jest.fn().mockRejectedValue(Promise.resolve(new Error(err))),
-    readFile: jest.fn().mockRejectedValue(Promise.resolve(new Error(err))),
-    writeFile: jest.fn().mockRejectedValue(Promise.resolve(new Error(err))),
-});

src/adapters/fileSystem.stub.ts

@@ -0,0 +1,11 @@
+import glob from "glob";
+
+export const globAsync = async (pattern: string) => {
+    return new Promise<string[] | Error>((resolve) => {
+        glob(pattern, (error, matches) => {
+            resolve(error ?? matches);
+        });
+    });
+};
+
+export type GlobAsync = typeof globAsync;

src/adapters/globAsync.ts

@@ -2,9 +2,15 @@ import { EOL } from "os";
 
 import { childProcessExec } from "../adapters/childProcessExec";
 import { fsFileSystem } from "../adapters/fsFileSystem";
+import { globAsync } from "../adapters/globAsync";
 import { nativeImporter } from "../adapters/nativeImporter";
 import { processLogger } from "../adapters/processLogger";
 import { bind } from "../binding";
+import { convertComments, ConvertCommentsDependencies } from "../comments/convertComments";
+import {
+    ConvertFileCommentsDependencies,
+    convertFileComments,
+} from "../comments/convertFileComments";
 import { convertConfig, ConvertConfigDependencies } from "../conversion/convertConfig";
 import {
     convertEditorConfig,
@@ -45,9 +51,13 @@ import { findTypeScriptConfiguration } from "../input/findTypeScriptConfiguratio
 import { importer, ImporterDependencies } from "../input/importer";
 import { mergeLintConfigurations } from "../input/mergeLintConfigurations";
 import {
-    ChoosePackageManagerDependencies,
     choosePackageManager,
+    ChoosePackageManagerDependencies,
 } from "../reporting/packages/choosePackageManager";
+import {
+    reportCommentResults,
+    ReportCommentResultsDependencies,
+} from "../reporting/reportCommentResults";
 import {
     logMissingPackages,
     LogMissingPackagesDependencies,
@@ -62,6 +72,16 @@ import { mergers } from "../rules/mergers";
 import { rulesConverters } from "../rules/rulesConverters";
 import { runCli, RunCliDependencies } from "./runCli";
 
+const convertFileCommentsDependencies: ConvertFileCommentsDependencies = {
+    converters: rulesConverters,
+    fileSystem: fsFileSystem,
+};
+
+const convertCommentsDependencies: ConvertCommentsDependencies = {
+    convertFileComments: bind(convertFileComments, convertFileCommentsDependencies),
+    globAsync,
+};
+
 const convertRulesDependencies: ConvertRulesDependencies = {
     converters: rulesConverters,
     mergers,
@@ -97,6 +117,10 @@ const findOriginalConfigurationsDependencies: FindOriginalConfigurationsDependen
     mergeLintConfigurations,
 };
 
+const reportCommentResultsDependencies: ReportCommentResultsDependencies = {
+    logger: processLogger,
+};
+
 const choosePackageManagerDependencies: ChoosePackageManagerDependencies = {
     fileSystem: fsFileSystem,
 };
@@ -124,12 +148,16 @@ const writeConversionResultsDependencies: WriteConversionResultsDependencies = {
     fileSystem: fsFileSystem,
 };
 
+const reportEditorSettingConversionResultsDependencies = {
+    logger: processLogger,
+};
+
 const convertEditorConfigDependencies: ConvertEditorConfigDependencies = {
     findEditorConfiguration: bind(findEditorConfiguration, findEditorConfigurationDependencies),
     convertEditorSettings: bind(convertEditorSettings, convertEditorSettingsDependencies),
     reportConversionResults: bind(
         reportEditorSettingConversionResults,
-        reportConversionResultsDependencies,
+        reportEditorSettingConversionResultsDependencies,
     ),
     writeConversionResults: bind(
         writeEditorConfigConversionResults,
@@ -138,12 +166,14 @@ const convertEditorConfigDependencies: ConvertEditorConfigDependencies = {
 };
 
 const convertConfigDependencies: ConvertConfigDependencies = {
+    convertComments: bind(convertComments, convertCommentsDependencies),
     convertRules: bind(convertRules, convertRulesDependencies),
     findOriginalConfigurations: bind(
         findOriginalConfigurations,
         findOriginalConfigurationsDependencies,
     ),
     logMissingPackages: bind(logMissingPackages, logMissingPackagesDependencies),
+    reportCommentResults: bind(reportCommentResults, reportCommentResultsDependencies),
     reportConversionResults: bind(reportConversionResults, reportConversionResultsDependencies),
     simplifyPackageRules: bind(simplifyPackageRules, simplifyPackageRulesDependencies),
     writeConversionResults: bind(writeConversionResults, writeConversionResultsDependencies),

src/cli/main.ts

@@ -19,6 +19,10 @@ export const runCli = async (
 ): Promise<ResultStatus> => {
     const command = new Command()
         .usage("[options] <file ...> --language [language]")
+        .option(
+            "--comments [files]",
+            "convert tslint:disable rule flags in globbed files (experimental)",
+        )
         .option("--config [config]", "eslint configuration file to output to")
         .option("--editor [editor]", "editor configuration file to convert")
         .option("--eslint [eslint]", "eslint configuration file to convert using")

src/cli/runCli.ts

@@ -0,0 +1,88 @@
+import { ResultStatus } from "../types";
+import { convertComments, ConvertCommentsDependencies } from "./convertComments";
+
+const createStubDependencies = (
+    overrides: Partial<ConvertCommentsDependencies> = {},
+): ConvertCommentsDependencies => ({
+    convertFileComments: jest.fn(),
+    globAsync: jest.fn().mockResolvedValue(["src/index.ts"]),
+    ...overrides,
+});
+
+describe("convertComments", () => {
+    it("returns an error when --comment is given as a boolean value", async () => {
+        // Arrange
+        const dependencies = createStubDependencies();
+
+        // Act
+        const result = await convertComments(dependencies, true);
+
+        // Assert
+        expect(result).toEqual({
+            errors: expect.arrayContaining([expect.any(Error)]),
+            status: ResultStatus.Failed,
+        });
+    });
+
+    it("returns an error when there are no file path globs", async () => {
+        // Arrange
+        const dependencies = createStubDependencies();
+
+        // Act
+        const result = await convertComments(dependencies, []);
+
+        // Assert
+        expect(result).toEqual({
+            errors: expect.arrayContaining([expect.any(Error)]),
+            status: ResultStatus.Failed,
+        });
+    });
+
+    it("returns the failure result when a file path glob fails", async () => {
+        // Arrange
+        const globAsyncError = new Error();
+        const dependencies = createStubDependencies({
+            globAsync: jest.fn().mockResolvedValueOnce(globAsyncError),
+        });
+
+        // Act
+        const result = await convertComments(dependencies, ["*.ts"]);
+
+        // Assert
+        expect(result).toEqual({
+            errors: [globAsyncError],
+            status: ResultStatus.Failed,
+        });
+    });
+
+    it("returns the failure result when a file conversion fails", async () => {
+        // Arrange
+        const fileConversionError = new Error();
+        const dependencies = createStubDependencies({
+            convertFileComments: jest.fn().mockResolvedValueOnce(fileConversionError),
+        });
+
+        // Act
+        const result = await convertComments(dependencies, ["*.ts"]);
+
+        // Assert
+        expect(result).toEqual({
+            errors: [fileConversionError],
+            status: ResultStatus.Failed,
+        });
+    });
+
+    it("returns a success result when all steps succeed", async () => {
+        // Arrange
+        const dependencies = createStubDependencies();
+
+        // Act
+        const result = await convertComments(dependencies, ["*.ts"]);
+
+        // Assert
+        expect(result).toEqual({
+            data: ["src/index.ts"],
+            status: ResultStatus.Succeeded,
+        });
+    });
+});

src/comments/convertComments.test.ts

@@ -0,0 +1,60 @@
+import { GlobAsync } from "../adapters/globAsync";
+import { SansDependencies } from "../binding";
+import { ResultStatus, ResultWithDataStatus } from "../types";
+import { separateErrors, uniqueFromSources, isError } from "../utils";
+import { convertFileComments } from "./convertFileComments";
+
+export type ConvertCommentsDependencies = {
+    convertFileComments: SansDependencies<typeof convertFileComments>;
+    globAsync: GlobAsync;
+};
+
+const noGlobsResult: ResultWithDataStatus<string[]> = {
+    errors: [new Error("--comment requires file path globs to be passed.")],
+    status: ResultStatus.Failed,
+};
+
+export const convertComments = async (
+    dependencies: ConvertCommentsDependencies,
+    filePathGlobs: true | string | string[] | undefined,
+): Promise<ResultWithDataStatus<string[]>> => {
+    if (filePathGlobs === true) {
+        return noGlobsResult;
+    }
+
+    const uniqueFilePathGlobs = uniqueFromSources(filePathGlobs);
+    if (uniqueFilePathGlobs.join("") === "") {
+        return noGlobsResult;
+    }
+
+    const [fileGlobErrors, globbedFilePaths] = separateErrors(
+        await Promise.all(uniqueFilePathGlobs.map(dependencies.globAsync)),
+    );
+    if (fileGlobErrors.length !== 0) {
+        return {
+            errors: fileGlobErrors,
+            status: ResultStatus.Failed,
+        };
+    }
+
+    const ruleConversionCache = new Map<string, string | undefined>();
+    const uniqueGlobbedFilePaths = uniqueFromSources(...globbedFilePaths);
+    const fileFailures = (
+        await Promise.all(
+            uniqueGlobbedFilePaths.map(async (filePath) =>
+                dependencies.convertFileComments(filePath, ruleConversionCache),
+            ),
+        )
+    ).filter(isError);
+    if (fileFailures.length !== 0) {
+        return {
+            errors: fileFailures,
+            status: ResultStatus.Failed,
+        };
+    }
+
+    return {
+        data: uniqueGlobbedFilePaths,
+        status: ResultStatus.Succeeded,
+    };
+};