Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Apr 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
29baeee
Set basic replace comment conversion
Oct 20, 2019
e84bb8c
Fix regex pattern and cli command name for convert comments
Oct 20, 2019
dcfce0c
Fix regex pattern condition to match rules without carriage return
Oct 20, 2019
300ebd3
Replace regex and use tslint functions to retrieve tslint comment data
Oct 21, 2019
fae2869
Switching rule handler
Dec 8, 2019
5dbd345
Merge branch 'master' into feature/opt-in-flag-convert-disable-comments
Dec 8, 2019
8cb8c06
Write file after switching rule comments
Dec 9, 2019
67b4d66
Merge branch 'master' into feature/opt-in-flag-convert-disable-comments
KingDarBoja Dec 26, 2019
2830122
Merge branch 'master' into feature/opt-in-flag-convert-disable-comments
KingDarBoja Mar 28, 2020
dd319f8
replace old path to rulesConverter
KingDarBoja Mar 28, 2020
f1bd492
Merge branch 'master' into feature/opt-in-flag-convert-disable-comments
JoshuaKGoldberg Apr 14, 2020
bacf9e8
Working replacements for single line changes
JoshuaKGoldberg Apr 25, 2020
35adb28
4am and it works
JoshuaKGoldberg Apr 25, 2020
63a3876
Revert .prettierrc testing change
JoshuaKGoldberg Apr 25, 2020
0456314
Love that nullish operator
JoshuaKGoldberg Apr 25, 2020
e61a434
Comments WIP
JoshuaKGoldberg Apr 25, 2020
5248447
Seems to be all working and tested now
JoshuaKGoldberg Apr 25, 2020
da1ace3
Merge branch 'master'
JoshuaKGoldberg Apr 25, 2020
425280a
Feature add: logged directive counts as a summary
JoshuaKGoldberg Apr 25, 2020
87e8953
Docs in CLI and README.md
JoshuaKGoldberg Apr 25, 2020
3098453
Merge branch 'master'
JoshuaKGoldberg Apr 26, 2020
fa7fcda
Filled in basic test for separateErrors
JoshuaKGoldberg Apr 26, 2020
2ec5b67
Normalized output per new norms
JoshuaKGoldberg Apr 26, 2020
bf6574f
Merge branch 'master' (and added --comments validation)
JoshuaKGoldberg Apr 27, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion docs/Architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
29 changes: 26 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand Down
6 changes: 0 additions & 6 deletions src/adapters/fileSystem.stub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
});
11 changes: 11 additions & 0 deletions src/adapters/globAsync.ts
Original file line number Diff line number Diff line change
@@ -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;
34 changes: 32 additions & 2 deletions src/cli/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -97,6 +117,10 @@ const findOriginalConfigurationsDependencies: FindOriginalConfigurationsDependen
mergeLintConfigurations,
};

const reportCommentResultsDependencies: ReportCommentResultsDependencies = {
logger: processLogger,
};

const choosePackageManagerDependencies: ChoosePackageManagerDependencies = {
fileSystem: fsFileSystem,
};
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down
4 changes: 4 additions & 0 deletions src/cli/runCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
88 changes: 88 additions & 0 deletions src/comments/convertComments.test.ts
Original file line number Diff line number Diff line change
@@ -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,
});
});
});
60 changes: 60 additions & 0 deletions src/comments/convertComments.ts
Original file line number Diff line number Diff line change
@@ -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,
};
};
Loading