Skip to content

Added obsolete rule support to convertRules.ts #1148

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 1 commit into from
Jul 6, 2021
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
4 changes: 2 additions & 2 deletions docs/Architecture/Linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Those are run by `src/converters/lintConfigs/rules/convertRules.ts`, which takes

1. The raw TSLint rule is converted to a standardized format.
2. The appropriate converter is run for the rule.
3. If the rule is missing or the conversion failed, this is marked.
3. If the rule is missing or obsolete, or the conversion failed, this is marked.
4. For each output rule equivalent given by the conversion:
* The output rule name is added to the TSLint rule's equivalency set.
* The TSLint rule's config severity is mapped to its ESLint equivalent.
Expand All @@ -34,7 +34,7 @@ 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
- `rules`: At least one equivalent ESLint rule and options, _or_ none if obsolete
- `notices`: Any extra info that should be printed after conversion
- `plugins`: Any plugins that should now be installed if not already

Expand Down
6 changes: 6 additions & 0 deletions docs/Creating a Rule Converter.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,9 @@ If the lint rule includes arguments, add the `--sameArguments` flag above to hav
```shell
node ./script/newConverter --eslint output-name --tslint input-name --sameArguments
```

If the original TSLint rule is obsolete and does not have an ESLint equivalent, you can omit `--eslint`:

```shell
node ./script/newConverter --tslint input-name
```
17 changes: 10 additions & 7 deletions script/newConverter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,20 @@ const { writeConverterTest } = require("./writeConverterTest");

const args = command.parse(process.argv).opts();

for (const arg of ["eslint", "tslint"]) {
if (!args[arg]) {
throw new Error(`Missing --${arg} option.`);
}
if (!args.tslint) {
throw new Error(`Missing --tslint option.`);
}

if (args.sameArguments && !args.eslint) {
throw new Error(`Cannot use --sameArguments without --eslint.`);
}

const tslintPascalCase = upperFirst(camelCase(args.tslint)).replace("A11Y", "A11y");
const plugins = args.eslint.includes("/")
? `
const plugins =
args.eslint && args.eslint.includes("/")
? `
plugins: ["${args.eslint.split("/")[0]}"],`
: "";
: "";

await rewriteConvertersMap({ args, tslintPascalCase });
await writeConverter({ args, plugins, tslintPascalCase });
Expand Down
20 changes: 12 additions & 8 deletions script/newConverter/writeConverter.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,23 @@ module.exports.writeConverter = async ({ args, plugins, tslintPascalCase }) => {
]
: ["", ""];

const body = args.eslint
? `{${plugins}
rules: [
{${ruleArguments}
ruleName: "${args.eslint}",
},
],
}`
: `({})`;

await fs.writeFile(
`./src/converters/lintConfigs/rules/ruleConverters/${args.tslint}.ts`,
`
import { RuleConverter } from "../ruleConverter";
import { RuleConverter } from "../ruleConverter";

export const convert${tslintPascalCase}: RuleConverter = (${functionArguments}) => {
return {${plugins}
rules: [
{${ruleArguments}
ruleName: "${args.eslint}",
},
],
};
return ${body};
};
`.trimLeft(),
);
Expand Down
18 changes: 11 additions & 7 deletions script/newConverter/writeConverterTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ module.exports.writeConverterTest = async ({ args, tslintPascalCase, plugins })
`
: "";

const body = args.eslint
? `
rules: [
{
ruleName: "${args.eslint}",
},
],
`
: "";

await fs.writeFile(
`./src/converters/lintConfigs/rules/ruleConverters/tests/${args.tslint}.test.ts`,
`
Expand All @@ -32,13 +42,7 @@ describe(convert${tslintPascalCase}, () => {
ruleArguments: [],
});

expect(result).toEqual({${plugins.replace("\n", "\n ")}
rules: [
{
ruleName: "${args.eslint}",
},
],
});
expect(result).toEqual({${plugins.replace("\n", "\n ")}${body}});
});${ruleArgumentsTest}
});
`.trimLeft(),
Expand Down
34 changes: 34 additions & 0 deletions src/converters/comments/convertFileComments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,40 @@ describe("convertFileComments", () => {
expect(dependencies.fileSystem.writeFile).not.toHaveBeenCalled();
});

it("ignores comment contents when an input rule is obsolete", async () => {
// Arrange
const dependencies = {
...createStubDependencies(`
// tslint:disable
export const a = true;

// tslint:disable:obsolete
export const b = true;
`),
converters: new Map([["obsolete", () => ({})]]),
};

// Act
await convertFileComments(
dependencies,
stubFileName,
new Map<string, string[]>(),
new Map<string, string[]>(),
);

// Assert
expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith(
stubFileName,
`
/* eslint-disable */
export const a = true;

/* eslint-disable */
export const b = true;
`,
);
});

it("parses TSLint directives to their matching ESLint directives", async () => {
// Arrange
const dependencies = createStubDependencies(`
Expand Down
2 changes: 1 addition & 1 deletion src/converters/comments/replaceFileComments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const replaceFileComments = (
return undefined;
}

const equivalents = converted.rules.map((conversion) => conversion.ruleName);
const equivalents = converted.rules?.map((conversion) => conversion.ruleName) ?? [];

ruleCommentsCache.set(ruleName, equivalents);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const createEmptyConfigConversionResults = (
extensionRules: new Map(),
failed: [],
missing: [],
obsolete: new Set(),
plugins: new Set(),
ruleEquivalents: new Map(),
...overrides,
Expand Down
2 changes: 1 addition & 1 deletion src/converters/lintConfigs/convertLintConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export type ConvertLintConfigDependencies = {

/**
* Root-level driver to convert a TSLint configuration to ESLint.
* @see `/docs/Architecture/Linting.md` for documentation.
* @see `/docs/Architecture/Linters.md` for documentation.
*/
export const convertLintConfig = async (
dependencies: ConvertLintConfigDependencies,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,44 @@ describe("reportConfigConversionResults", () => {
);
});

it("logs obsolete conversions when there is one obsolete conversion", async () => {
// Arrange
const logger = createStubLogger();
const conversionResults = createEmptyConfigConversionResults({
extends: basicExtends,
obsolete: new Set(["obsolete"]),
});

// Act
await reportConfigConversionResults({ logger }, ".eslintrc.js", conversionResults);

// Assert
expectEqualWrites(
logger.stdout.write,
`🦖 1 rule is obsolete and does not have an ESLint equivalent. 🦖`,
` Check ${logger.debugFileName} for details.`,
);
});

it("logs obsolete conversions when there are multiple obsolete conversions", async () => {
// Arrange
const logger = createStubLogger();
const conversionResults = createEmptyConfigConversionResults({
extends: basicExtends,
obsolete: new Set(["obsolete-a", "obsolete-b"]),
});

// Act
await reportConfigConversionResults({ logger }, ".eslintrc.js", conversionResults);

// Assert
expectEqualWrites(
logger.stdout.write,
`🦖 2 rules are obsolete and do not have ESLint equivalents. 🦖`,
` Check ${logger.debugFileName} for details.`,
);
});

it("logs a Prettier recommendation when extends doesn't include eslint-config-prettier", async () => {
// Arrange
const logger = createStubLogger();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Logger } from "../../../adapters/logger";
import {
logFailedConversions,
logMissingConversionTarget,
logObsoleteRules,
logSuccessfulConversions,
} from "../../../reporting";
import { ESLintRuleOptions, TSLintRuleOptions } from "../rules/types";
Expand Down Expand Up @@ -50,6 +51,10 @@ export const reportConfigConversionResults = async (
);
}

if (ruleConversionResults.obsolete.size !== 0) {
logObsoleteRules(Array.from(ruleConversionResults.obsolete), dependencies.logger);
}

if (!ruleConversionResults.extends.join("").includes("prettier")) {
logPrettierExtension(dependencies.logger);
}
Expand Down
16 changes: 16 additions & 0 deletions src/converters/lintConfigs/rules/convertRules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,22 @@ describe("convertRules", () => {
expect(failed).toEqual([conversionError]);
});

it("marks a converted rule as obsolete when it has no output rules", () => {
// Arrange
const { tslintRule, converters, mergers } = setupConversionEnvironment();
converters.set(tslintRule.ruleName, () => ({}));

// Act
const { obsolete } = convertRules(
{ ruleConverters: converters, ruleMergers: mergers },
{ [tslintRule.ruleName]: tslintRule },
new Map<string, string[]>(),
);

// Assert
expect(Array.from(obsolete)).toEqual([tslintRule.ruleName]);
});

it("marks a converted rule name as converted when a conversion has rules", () => {
// Arrange
const conversionResult = {
Expand Down
13 changes: 10 additions & 3 deletions src/converters/lintConfigs/rules/convertRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ export type RuleConversionResults = {
failed: ErrorSummary[];
missing: TSLintRuleOptions[];
plugins: Set<string>;
obsolete: Set<string>;
ruleEquivalents: Map<string, string[]>;
};

/**
* Converts raw TSLint rules to their ESLint equivalents.
* @see `/docs/Architecture/Linting.md` for documentation.
* @see `/docs/Architecture/Linters.md` for documentation.
*/
export const convertRules = (
dependencies: ConvertRulesDependencies,
Expand All @@ -36,6 +37,7 @@ export const convertRules = (
const converted = new Map<string, ESLintRuleOptions>();
const failed: ConversionError[] = [];
const missing: TSLintRuleOptions[] = [];
const obsolete = new Set<string>();
const plugins = new Set<string>();

if (rawTslintRules !== undefined) {
Expand All @@ -48,7 +50,7 @@ export const convertRules = (
// 2. The appropriate converter is run for the rule.
const conversion = convertRule(tslintRule, dependencies.ruleConverters);

// 3. If the rule is missing or the conversion failed, this is marked.
// 3. If the rule is missing or obsolete, or the conversion failed, this is marked.
if (conversion === undefined) {
if (tslintRule.ruleSeverity !== "off") {
missing.push(tslintRule);
Expand All @@ -62,6 +64,11 @@ export const convertRules = (
continue;
}

if (!conversion.rules) {
obsolete.add(tslintRule.ruleName);
continue;
}

const equivalents = new Set<string>();

// 4. For each output rule equivalent given by the conversion:
Expand Down Expand Up @@ -119,5 +126,5 @@ export const convertRules = (
}
}

return { converted, failed, missing, plugins, ruleEquivalents };
return { converted, failed, missing, obsolete, plugins, ruleEquivalents };
};
4 changes: 2 additions & 2 deletions src/converters/lintConfigs/rules/ruleConverter.stubs.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { ConversionError } from "../../../errors/conversionError";

export const createStubConverter = (result: ConversionError | string[]) => {
export const createStubConverter = (result?: ConversionError | string[]) => {
return () => {
return result instanceof ConversionError
? result
: {
rules: result.map((ruleName) => ({ ruleName })),
rules: result?.map((ruleName) => ({ ruleName })),
};
};
};
4 changes: 2 additions & 2 deletions src/converters/lintConfigs/rules/ruleConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ export type ConversionResult = {
plugins?: string[];

/**
* At least one equivalent ESLint rule and options.
* At least one equivalent ESLint rule and options, if not obsolete.
*/
rules: ConvertedRuleChanges[];
rules?: ConvertedRuleChanges[];
};

/**
Expand Down
4 changes: 4 additions & 0 deletions src/converters/lintConfigs/rules/ruleConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ import { convertFileNameCasing } from "./ruleConverters/file-name-casing";
import { convertForin } from "./ruleConverters/forin";
import { convertFunctionConstructor } from "./ruleConverters/function-constructor";
import { convertImportBlacklist } from "./ruleConverters/import-blacklist";
import { convertImportDestructuringSpacing } from "./ruleConverters/import-destructuring-spacing";
import { convertIncrementDecrement } from "./ruleConverters/increment-decrement";
import { convertIndent } from "./ruleConverters/indent";
import { convertInterfaceName } from "./ruleConverters/interface-name";
Expand Down Expand Up @@ -244,6 +245,7 @@ import { convertPreferConditionalExpression } from "./ruleConverters/prefer-cond
import { convertPreferConst } from "./ruleConverters/prefer-const";
import { convertPreferForOf } from "./ruleConverters/prefer-for-of";
import { convertPreferFunctionOverMethod } from "./ruleConverters/prefer-function-over-method";
import { convertPreferInlineDecorator } from "./ruleConverters/prefer-inline-decorator";
import { convertPreferObjectSpread } from "./ruleConverters/prefer-object-spread";
import { convertPreferReadonly } from "./ruleConverters/prefer-readonly";
import { convertPreferSwitch } from "./ruleConverters/prefer-switch";
Expand Down Expand Up @@ -309,6 +311,7 @@ export const ruleConverters = new Map([
["forin", convertForin],
["function-constructor", convertFunctionConstructor],
["import-blacklist", convertImportBlacklist],
["import-destructuring-spacing", convertImportDestructuringSpacing],
["increment-decrement", convertIncrementDecrement],
["indent", convertIndent],
["interface-name", convertInterfaceName],
Expand Down Expand Up @@ -478,6 +481,7 @@ export const ruleConverters = new Map([
["prefer-for-of", convertPreferForOf],
["prefer-function-over-method", convertPreferFunctionOverMethod],
["prefer-immediate-return", convertPreferImmediateReturn],
["prefer-inline-decorator", convertPreferInlineDecorator],
["prefer-object-spread", convertPreferObjectSpread],
["prefer-on-push-component-change-detection", convertPreferOnPushComponentChangeDetection],
["prefer-output-readonly", convertPreferOutputReadonly],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { RuleConverter } from "../ruleConverter";

export const convertImportDestructuringSpacing: RuleConverter = () => {
return {};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { RuleConverter } from "../ruleConverter";

export const convertPreferInlineDecorator: RuleConverter = () => {
return {};
};
Loading