From 439d8b0cb098873a9b4d8e273c0a9a26b3f58a45 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 4 Jul 2021 10:16:13 -0400 Subject: [PATCH] Added obsolete rule support to convertRules.ts --- docs/Architecture/Linters.md | 4 +- docs/Creating a Rule Converter.md | 6 +++ script/newConverter/index.js | 17 +++++---- script/newConverter/writeConverter.js | 20 ++++++---- script/newConverter/writeConverterTest.js | 18 +++++---- .../comments/convertFileComments.test.ts | 34 +++++++++++++++++ .../comments/replaceFileComments.ts | 2 +- .../configConversionResults.stubs.ts | 1 + .../lintConfigs/convertLintConfig.ts | 2 +- .../reportConfigConversionResults.test.ts | 38 +++++++++++++++++++ .../reportConfigConversionResults.ts | 5 +++ .../lintConfigs/rules/convertRules.test.ts | 16 ++++++++ .../lintConfigs/rules/convertRules.ts | 13 +++++-- .../lintConfigs/rules/ruleConverter.stubs.ts | 4 +- .../lintConfigs/rules/ruleConverter.ts | 4 +- .../lintConfigs/rules/ruleConverters.ts | 4 ++ .../import-destructuring-spacing.ts | 5 +++ .../ruleConverters/prefer-inline-decorator.ts | 5 +++ .../import-destructuring-spacing.test.ts | 11 ++++++ .../tests/prefer-inline-decorator.test.ts | 11 ++++++ src/reporting.ts | 16 ++++++++ 21 files changed, 203 insertions(+), 33 deletions(-) create mode 100644 src/converters/lintConfigs/rules/ruleConverters/import-destructuring-spacing.ts create mode 100644 src/converters/lintConfigs/rules/ruleConverters/prefer-inline-decorator.ts create mode 100644 src/converters/lintConfigs/rules/ruleConverters/tests/import-destructuring-spacing.test.ts create mode 100644 src/converters/lintConfigs/rules/ruleConverters/tests/prefer-inline-decorator.test.ts diff --git a/docs/Architecture/Linters.md b/docs/Architecture/Linters.md index 17a08c522..5c3046501 100644 --- a/docs/Architecture/Linters.md +++ b/docs/Architecture/Linters.md @@ -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. @@ -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 diff --git a/docs/Creating a Rule Converter.md b/docs/Creating a Rule Converter.md index 5f03e3ffe..1212e28d1 100644 --- a/docs/Creating a Rule Converter.md +++ b/docs/Creating a Rule Converter.md @@ -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 +``` diff --git a/script/newConverter/index.js b/script/newConverter/index.js index 4061a908d..a8db9c5bc 100644 --- a/script/newConverter/index.js +++ b/script/newConverter/index.js @@ -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 }); diff --git a/script/newConverter/writeConverter.js b/script/newConverter/writeConverter.js index 6c528346a..f65cc38d0 100644 --- a/script/newConverter/writeConverter.js +++ b/script/newConverter/writeConverter.js @@ -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(), ); diff --git a/script/newConverter/writeConverterTest.js b/script/newConverter/writeConverterTest.js index 03906a0cb..0ed3b76e9 100644 --- a/script/newConverter/writeConverterTest.js +++ b/script/newConverter/writeConverterTest.js @@ -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`, ` @@ -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(), diff --git a/src/converters/comments/convertFileComments.test.ts b/src/converters/comments/convertFileComments.test.ts index 1fb8dae1f..a87d2849d 100644 --- a/src/converters/comments/convertFileComments.test.ts +++ b/src/converters/comments/convertFileComments.test.ts @@ -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(), + new Map(), + ); + + // 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(` diff --git a/src/converters/comments/replaceFileComments.ts b/src/converters/comments/replaceFileComments.ts index 31e8e239f..6cf78413f 100644 --- a/src/converters/comments/replaceFileComments.ts +++ b/src/converters/comments/replaceFileComments.ts @@ -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); diff --git a/src/converters/lintConfigs/configConversionResults.stubs.ts b/src/converters/lintConfigs/configConversionResults.stubs.ts index 139f0b135..02faf4d8c 100644 --- a/src/converters/lintConfigs/configConversionResults.stubs.ts +++ b/src/converters/lintConfigs/configConversionResults.stubs.ts @@ -8,6 +8,7 @@ export const createEmptyConfigConversionResults = ( extensionRules: new Map(), failed: [], missing: [], + obsolete: new Set(), plugins: new Set(), ruleEquivalents: new Map(), ...overrides, diff --git a/src/converters/lintConfigs/convertLintConfig.ts b/src/converters/lintConfigs/convertLintConfig.ts index 14f3ced39..2d0a44f53 100644 --- a/src/converters/lintConfigs/convertLintConfig.ts +++ b/src/converters/lintConfigs/convertLintConfig.ts @@ -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, diff --git a/src/converters/lintConfigs/reporting/reportConfigConversionResults.test.ts b/src/converters/lintConfigs/reporting/reportConfigConversionResults.test.ts index 931206043..150ba15ad 100644 --- a/src/converters/lintConfigs/reporting/reportConfigConversionResults.test.ts +++ b/src/converters/lintConfigs/reporting/reportConfigConversionResults.test.ts @@ -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(); diff --git a/src/converters/lintConfigs/reporting/reportConfigConversionResults.ts b/src/converters/lintConfigs/reporting/reportConfigConversionResults.ts index 6161ddcf0..0b7a6de74 100644 --- a/src/converters/lintConfigs/reporting/reportConfigConversionResults.ts +++ b/src/converters/lintConfigs/reporting/reportConfigConversionResults.ts @@ -5,6 +5,7 @@ import { Logger } from "../../../adapters/logger"; import { logFailedConversions, logMissingConversionTarget, + logObsoleteRules, logSuccessfulConversions, } from "../../../reporting"; import { ESLintRuleOptions, TSLintRuleOptions } from "../rules/types"; @@ -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); } diff --git a/src/converters/lintConfigs/rules/convertRules.test.ts b/src/converters/lintConfigs/rules/convertRules.test.ts index d65b8244d..2df1e2633 100644 --- a/src/converters/lintConfigs/rules/convertRules.test.ts +++ b/src/converters/lintConfigs/rules/convertRules.test.ts @@ -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(), + ); + + // Assert + expect(Array.from(obsolete)).toEqual([tslintRule.ruleName]); + }); + it("marks a converted rule name as converted when a conversion has rules", () => { // Arrange const conversionResult = { diff --git a/src/converters/lintConfigs/rules/convertRules.ts b/src/converters/lintConfigs/rules/convertRules.ts index 031f60fa0..fe671cc4d 100644 --- a/src/converters/lintConfigs/rules/convertRules.ts +++ b/src/converters/lintConfigs/rules/convertRules.ts @@ -21,12 +21,13 @@ export type RuleConversionResults = { failed: ErrorSummary[]; missing: TSLintRuleOptions[]; plugins: Set; + obsolete: Set; ruleEquivalents: Map; }; /** * 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, @@ -36,6 +37,7 @@ export const convertRules = ( const converted = new Map(); const failed: ConversionError[] = []; const missing: TSLintRuleOptions[] = []; + const obsolete = new Set(); const plugins = new Set(); if (rawTslintRules !== undefined) { @@ -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); @@ -62,6 +64,11 @@ export const convertRules = ( continue; } + if (!conversion.rules) { + obsolete.add(tslintRule.ruleName); + continue; + } + const equivalents = new Set(); // 4. For each output rule equivalent given by the conversion: @@ -119,5 +126,5 @@ export const convertRules = ( } } - return { converted, failed, missing, plugins, ruleEquivalents }; + return { converted, failed, missing, obsolete, plugins, ruleEquivalents }; }; diff --git a/src/converters/lintConfigs/rules/ruleConverter.stubs.ts b/src/converters/lintConfigs/rules/ruleConverter.stubs.ts index ed16c2e57..c300cb209 100644 --- a/src/converters/lintConfigs/rules/ruleConverter.stubs.ts +++ b/src/converters/lintConfigs/rules/ruleConverter.stubs.ts @@ -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 })), }; }; }; diff --git a/src/converters/lintConfigs/rules/ruleConverter.ts b/src/converters/lintConfigs/rules/ruleConverter.ts index fc2f266b7..ea1f71b8f 100644 --- a/src/converters/lintConfigs/rules/ruleConverter.ts +++ b/src/converters/lintConfigs/rules/ruleConverter.ts @@ -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[]; }; /** diff --git a/src/converters/lintConfigs/rules/ruleConverters.ts b/src/converters/lintConfigs/rules/ruleConverters.ts index d91f13f94..4a89c025d 100644 --- a/src/converters/lintConfigs/rules/ruleConverters.ts +++ b/src/converters/lintConfigs/rules/ruleConverters.ts @@ -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"; @@ -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"; @@ -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], @@ -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], diff --git a/src/converters/lintConfigs/rules/ruleConverters/import-destructuring-spacing.ts b/src/converters/lintConfigs/rules/ruleConverters/import-destructuring-spacing.ts new file mode 100644 index 000000000..1d0a27a35 --- /dev/null +++ b/src/converters/lintConfigs/rules/ruleConverters/import-destructuring-spacing.ts @@ -0,0 +1,5 @@ +import { RuleConverter } from "../ruleConverter"; + +export const convertImportDestructuringSpacing: RuleConverter = () => { + return {}; +}; diff --git a/src/converters/lintConfigs/rules/ruleConverters/prefer-inline-decorator.ts b/src/converters/lintConfigs/rules/ruleConverters/prefer-inline-decorator.ts new file mode 100644 index 000000000..8d674d55e --- /dev/null +++ b/src/converters/lintConfigs/rules/ruleConverters/prefer-inline-decorator.ts @@ -0,0 +1,5 @@ +import { RuleConverter } from "../ruleConverter"; + +export const convertPreferInlineDecorator: RuleConverter = () => { + return {}; +}; diff --git a/src/converters/lintConfigs/rules/ruleConverters/tests/import-destructuring-spacing.test.ts b/src/converters/lintConfigs/rules/ruleConverters/tests/import-destructuring-spacing.test.ts new file mode 100644 index 000000000..c191fe533 --- /dev/null +++ b/src/converters/lintConfigs/rules/ruleConverters/tests/import-destructuring-spacing.test.ts @@ -0,0 +1,11 @@ +import { convertImportDestructuringSpacing } from "../import-destructuring-spacing"; + +describe(convertImportDestructuringSpacing, () => { + test("conversion without arguments", () => { + const result = convertImportDestructuringSpacing({ + ruleArguments: [], + }); + + expect(result).toEqual({}); + }); +}); diff --git a/src/converters/lintConfigs/rules/ruleConverters/tests/prefer-inline-decorator.test.ts b/src/converters/lintConfigs/rules/ruleConverters/tests/prefer-inline-decorator.test.ts new file mode 100644 index 000000000..aa62799ba --- /dev/null +++ b/src/converters/lintConfigs/rules/ruleConverters/tests/prefer-inline-decorator.test.ts @@ -0,0 +1,11 @@ +import { convertPreferInlineDecorator } from "../prefer-inline-decorator"; + +describe(convertPreferInlineDecorator, () => { + test("conversion without arguments", () => { + const result = convertPreferInlineDecorator({ + ruleArguments: [], + }); + + expect(result).toEqual({}); + }); +}); diff --git a/src/reporting.ts b/src/reporting.ts index 0c70ecf5b..4895e05a8 100644 --- a/src/reporting.ts +++ b/src/reporting.ts @@ -87,3 +87,19 @@ export const logMissingConversionTarget = ( ); logger.info.write(EOL); }; + +export const logObsoleteRules = (ruleNames: string[], logger: Logger) => { + const headline = + ruleNames.length === 1 + ? ` rule is obsolete and does not have an ESLint equivalent` + : ` rules are obsolete and do not have ESLint equivalents`; + + logger.stdout.write(chalk.magentaBright(`️${EOL}🦖 ${ruleNames.length}`)); + logger.stdout.write(chalk.magenta(`${headline}.`)); + logger.stdout.write(chalk.magentaBright(` 🦖${EOL}`)); + logger.stdout.write(chalk.magenta(` Check ${logger.debugFileName} for details.${EOL}`)); + + logger.info.write(ruleNames.map((ruleName) => ` * ${ruleName}${EOL}`).join("")); + + logger.info.write(EOL); +};