Skip to content

Defaulted --comment globs to --typescript settings #707

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 2 commits into from
Sep 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 10 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,22 @@ Each of these flags is optional:
#### `comments`

```shell
npx tslint-to-eslint-config --comments 'src/**/*.ts'
npx tslint-to-eslint-config --comments
```

_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.
Indicates to convert from [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).
Comments such as `// tslint:disable: tslint-rule-name` will be converted to equivalents like `// eslint-disable eslint-rule-name`.

If passed without arguments, respects the `excludes`, `files`, and `includes` in your TypeScript configuration.

Alternately, you can specify which files to convert comments in as globs:

```shell
npx tslint-to-eslint-config --comments 'src/**/*.ts'
```

#### `config`

```shell
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"commander": "6.1.0",
"eslint-config-prettier": "6.11.0",
"glob": "7.1.6",
"minimatch": "^3.0.4",
"strip-json-comments": "3.1.1",
"tslint": "6.1.3",
"typescript": "4.0.2"
Expand All @@ -27,6 +28,7 @@
"@types/eslint-config-prettier": "6.11.0",
"@types/glob": "7.1.3",
"@types/jest": "26.0.13",
"@types/minimatch": "^3.0.3",
"@types/node": "12.12.21",
"@typescript-eslint/eslint-plugin": "4.1.0",
"@typescript-eslint/parser": "4.1.0",
Expand Down
5 changes: 1 addition & 4 deletions src/cli/runCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ export const runCli = async (
const command = new Command()
.storeOptionsAsProperties(false)
.usage("[options] <file ...> --language [language]")
.option(
"--comments [files]",
"convert tslint:disable rule flags in globbed files (experimental)",
)
.option("--comments [files]", "convert tslint:disable rule flags in 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
57 changes: 54 additions & 3 deletions src/comments/convertComments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const createStubDependencies = (
overrides: Partial<ConvertCommentsDependencies> = {},
): ConvertCommentsDependencies => ({
convertFileComments: jest.fn(),
globAsync: jest.fn().mockResolvedValue(["src/index.ts"]),
globAsync: jest.fn().mockResolvedValue(["src/a.ts", "src/b.ts"]),
...overrides,
});

Expand All @@ -24,7 +24,7 @@ describe("convertComments", () => {
});
});

it("returns an error when --comments is given as a boolean value", async () => {
it("returns an error when --comments is given as a boolean value without a TypeScript configuration", async () => {
// Arrange
const dependencies = createStubDependencies();

Expand All @@ -38,6 +38,57 @@ describe("convertComments", () => {
});
});

it("includes TypeScript files when --comments is given as a boolean value with a TypeScript files configuration", async () => {
// Arrange
const dependencies = createStubDependencies({
globAsync: jest.fn().mockResolvedValue(["src/a.ts"]),
});

// Act
const result = await convertComments(dependencies, true, {
files: ["src/a.ts"],
});

// Assert
expect(result).toEqual({
data: ["src/a.ts"],
status: ResultStatus.Succeeded,
});
});

it("includes TypeScript inclusions when --comments is given as a boolean value with a TypeScript include configuration", async () => {
// Arrange
const dependencies = createStubDependencies();

// Act
const result = await convertComments(dependencies, true, {
include: ["src/*.ts"],
});

// Assert
expect(result).toEqual({
data: ["src/a.ts", "src/b.ts"],
status: ResultStatus.Succeeded,
});
});

it("excludes TypeScript exclusions when --comments is given as a boolean value with a TypeScript excludes configuration", async () => {
// Arrange
const dependencies = createStubDependencies();

// Act
const result = await convertComments(dependencies, true, {
exclude: ["src/b.ts"],
include: ["src/*.ts"],
});

// Assert
expect(result).toEqual({
data: ["src/a.ts"],
status: ResultStatus.Succeeded,
});
});

it("returns an error when there are no file path globs", async () => {
// Arrange
const dependencies = createStubDependencies();
Expand Down Expand Up @@ -95,7 +146,7 @@ describe("convertComments", () => {

// Assert
expect(result).toEqual({
data: ["src/index.ts"],
data: ["src/a.ts", "src/b.ts"],
status: ResultStatus.Succeeded,
});
});
Expand Down
52 changes: 40 additions & 12 deletions src/comments/convertComments.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import minimatch from "minimatch";

import { GlobAsync } from "../adapters/globAsync";
import { SansDependencies } from "../binding";
import { TypeScriptConfiguration } from "../input/findTypeScriptConfiguration";
import { ResultStatus, ResultWithDataStatus } from "../types";
import { separateErrors, uniqueFromSources, isError } from "../utils";
import { convertFileComments } from "./convertFileComments";
Expand All @@ -9,29 +12,48 @@ export type ConvertCommentsDependencies = {
globAsync: GlobAsync;
};

const noGlobsResult: ResultWithDataStatus<string[]> = {
errors: [new Error("--comments requires file path globs to be passed.")],
status: ResultStatus.Failed,
};

export const convertComments = async (
dependencies: ConvertCommentsDependencies,
filePathGlobs: true | string | string[] | undefined,
typescriptConfiguration?: TypeScriptConfiguration,
): Promise<ResultWithDataStatus<string[] | undefined>> => {
let fromTypeScriptConfiguration: TypeScriptConfiguration | undefined;

if (filePathGlobs === true) {
if (!typescriptConfiguration) {
return {
errors: [
new Error(
"--comments indicated to convert files listed in a tsconfig.json, but one was not found on disk or specified by with --typescript.",
),
],
status: ResultStatus.Failed,
};
}

filePathGlobs = [
...(typescriptConfiguration.files ?? []),
...(typescriptConfiguration.include ?? []),
];
fromTypeScriptConfiguration = typescriptConfiguration;
}

if (filePathGlobs === undefined) {
return {
data: undefined,
status: ResultStatus.Succeeded,
};
}

if (filePathGlobs === true) {
return noGlobsResult;
}

const uniqueFilePathGlobs = uniqueFromSources(filePathGlobs);
if (uniqueFilePathGlobs.join("") === "") {
return noGlobsResult;
return {
errors: [
new Error(
"--comments found no files. Consider passing no globs to it, to default to all TypeScript files.",
),
],
status: ResultStatus.Failed,
};
}

const [fileGlobErrors, globbedFilePaths] = separateErrors(
Expand All @@ -45,7 +67,13 @@ export const convertComments = async (
}

const ruleConversionCache = new Map<string, string | undefined>();
const uniqueGlobbedFilePaths = uniqueFromSources(...globbedFilePaths);
const uniqueGlobbedFilePaths = uniqueFromSources(...globbedFilePaths).filter(
(filePathGlob) =>
!fromTypeScriptConfiguration?.exclude?.some((exclude) =>
minimatch(filePathGlob, exclude),
),
);

const fileFailures = (
await Promise.all(
uniqueGlobbedFilePaths.map(async (filePath) =>
Expand Down
5 changes: 4 additions & 1 deletion src/conversion/convertLintConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ export const convertLintConfig = async (
}

// 5. Files to transform comments in have source text rewritten using the same rule conversion logic
const commentsResult = await dependencies.convertComments(settings.comments);
const commentsResult = await dependencies.convertComments(
settings.comments,
originalConfigurations.data.typescript,
);

// 6. A summary of the results is printed to the user's console
await dependencies.reportConversionResults(settings.config, summarizedConfiguration);
Expand Down
4 changes: 4 additions & 0 deletions src/input/findTypeScriptConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export type TypeScriptConfiguration = {
lib?: string[];
target?: string;
};
exclude?: string[];
files?: string[];
include?: string[];
};

const defaultTypeScriptConfiguration = {
Expand All @@ -29,6 +32,7 @@ export const findTypeScriptConfiguration = async (
return rawConfiguration instanceof Error
? rawConfiguration
: {
...rawConfiguration,
compilerOptions: {
...defaultTypeScriptConfiguration.compilerOptions,
...rawConfiguration.compilerOptions,
Expand Down