From 5afc9835db053e623ec01067db151016eca6dba9 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 11 Sep 2020 14:03:12 -0400 Subject: [PATCH] Factored config rule conversions into comment conversions --- docs/Architecture.md | 13 ++++++- src/comments/convertComments.test.ts | 18 ++++----- src/comments/convertComments.ts | 5 ++- src/comments/convertFileComments.test.ts | 46 +++++++++++++++++++---- src/comments/convertFileComments.ts | 6 ++- src/comments/replaceFileComments.ts | 22 +++++++---- src/conversion/conversionResults.stubs.ts | 1 + src/conversion/convertLintConfig.test.ts | 44 +++++++++++----------- src/conversion/convertLintConfig.ts | 3 +- src/rules/convertRules.ts | 10 ++++- 10 files changed, 115 insertions(+), 53 deletions(-) diff --git a/docs/Architecture.md b/docs/Architecture.md index fab861c39..805024557 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -17,7 +17,7 @@ Within `src/conversion/convertLintConfig.ts`, the following steps occur: - 3a. If no output rules conflict with `eslint-config-prettier`, it's added in - 3b. Any ESLint rules that are configured the same as an extended preset are trimmed 4. The summarized configuration is written to the output config file -5. Files to transform comments in have source text rewritten using the same rule conversion logic +5. Files to transform comments in have source text rewritten using the cached rule conversion results 6. A summary of the results is printed to the user's console ### Conversion Results @@ -51,6 +51,17 @@ These are located in `src/rules/mergers/`, and keyed under their names by the ma For example, `@typescript-eslint/ban-types` spreads both arguments' `types` members into one large `types` object. +## Comment Conversion + +Comments are converted after rule conversion by `src/comments/convertComments.ts`. +Source files are parsed into TypeScript files by `src/comments/parseFileComments.ts`, which then extracts their comment nodes. +Those comments are parsed for TSLint rule disable or enable comments. + +Comments that match will be rewritten in their their file to their new ESLint rule equivalent in `src/comments/replaceFileComments.ts`, as determined by: + +1. First, if the `ruleEquivalents` cache received from configuration convertion has the TSLint rule's ESLint equivalents listed, those are used. +2. Failing that, a comment-specific `ruleCommentsCache` is populated with rules converted ad-hoc with no arguments. + ## Editor Configuration Conversion Editor lint configurations are converted by `src/editorSettings/convertEditorSettings.ts`. diff --git a/src/comments/convertComments.test.ts b/src/comments/convertComments.test.ts index ed17c7eb6..c2f750385 100644 --- a/src/comments/convertComments.test.ts +++ b/src/comments/convertComments.test.ts @@ -15,7 +15,7 @@ describe("convertComments", () => { const dependencies = createStubDependencies(); // Act - const result = await convertComments(dependencies, undefined); + const result = await convertComments(dependencies, undefined, new Map()); // Assert expect(result).toEqual({ @@ -29,7 +29,7 @@ describe("convertComments", () => { const dependencies = createStubDependencies(); // Act - const result = await convertComments(dependencies, true); + const result = await convertComments(dependencies, true, new Map()); // Assert expect(result).toEqual({ @@ -45,7 +45,7 @@ describe("convertComments", () => { }); // Act - const result = await convertComments(dependencies, true, { + const result = await convertComments(dependencies, true, new Map(), { files: ["src/a.ts"], }); @@ -61,7 +61,7 @@ describe("convertComments", () => { const dependencies = createStubDependencies(); // Act - const result = await convertComments(dependencies, true, { + const result = await convertComments(dependencies, true, new Map(), { include: ["src/*.ts"], }); @@ -77,7 +77,7 @@ describe("convertComments", () => { const dependencies = createStubDependencies(); // Act - const result = await convertComments(dependencies, true, { + const result = await convertComments(dependencies, true, new Map(), { exclude: ["src/b.ts"], include: ["src/*.ts"], }); @@ -94,7 +94,7 @@ describe("convertComments", () => { const dependencies = createStubDependencies(); // Act - const result = await convertComments(dependencies, []); + const result = await convertComments(dependencies, [], new Map()); // Assert expect(result).toEqual({ @@ -111,7 +111,7 @@ describe("convertComments", () => { }); // Act - const result = await convertComments(dependencies, ["*.ts"]); + const result = await convertComments(dependencies, ["*.ts"], new Map()); // Assert expect(result).toEqual({ @@ -128,7 +128,7 @@ describe("convertComments", () => { }); // Act - const result = await convertComments(dependencies, ["*.ts"]); + const result = await convertComments(dependencies, ["*.ts"], new Map()); // Assert expect(result).toEqual({ @@ -142,7 +142,7 @@ describe("convertComments", () => { const dependencies = createStubDependencies(); // Act - const result = await convertComments(dependencies, ["*.ts"]); + const result = await convertComments(dependencies, ["*.ts"], new Map()); // Assert expect(result).toEqual({ diff --git a/src/comments/convertComments.ts b/src/comments/convertComments.ts index 4bf8ccef8..6f26c70c3 100644 --- a/src/comments/convertComments.ts +++ b/src/comments/convertComments.ts @@ -15,6 +15,7 @@ export type ConvertCommentsDependencies = { export const convertComments = async ( dependencies: ConvertCommentsDependencies, filePathGlobs: true | string | string[] | undefined, + ruleEquivalents: Map, typescriptConfiguration?: TypeScriptConfiguration, ): Promise> => { let fromTypeScriptConfiguration: TypeScriptConfiguration | undefined; @@ -66,7 +67,7 @@ export const convertComments = async ( }; } - const ruleConversionCache = new Map(); + const ruleCommentsCache = new Map(); const uniqueGlobbedFilePaths = uniqueFromSources(...globbedFilePaths).filter( (filePathGlob) => !fromTypeScriptConfiguration?.exclude?.some((exclude) => @@ -77,7 +78,7 @@ export const convertComments = async ( const fileFailures = ( await Promise.all( uniqueGlobbedFilePaths.map(async (filePath) => - dependencies.convertFileComments(filePath, ruleConversionCache), + dependencies.convertFileComments(filePath, ruleCommentsCache, ruleEquivalents), ), ) ).filter(isError); diff --git a/src/comments/convertFileComments.test.ts b/src/comments/convertFileComments.test.ts index 808a25f3f..87f8dbd14 100644 --- a/src/comments/convertFileComments.test.ts +++ b/src/comments/convertFileComments.test.ts @@ -26,7 +26,7 @@ describe("convertFileComments", () => { const dependencies = createStubDependencies(readFileError); // Act - const result = await convertFileComments(dependencies, stubFileName, new Map()); + const result = await convertFileComments(dependencies, stubFileName, new Map(), new Map()); // Assert expect(result).toBe(readFileError); @@ -39,7 +39,7 @@ describe("convertFileComments", () => { `); // Act - await convertFileComments(dependencies, stubFileName, new Map()); + await convertFileComments(dependencies, stubFileName, new Map(), new Map()); // Assert expect(dependencies.fileSystem.writeFile).not.toHaveBeenCalled(); @@ -70,7 +70,7 @@ export const g = true; `); // Act - await convertFileComments(dependencies, stubFileName, new Map()); + await convertFileComments(dependencies, stubFileName, new Map(), new Map()); // Assert expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( @@ -110,7 +110,7 @@ export const b = true; `); // Act - await convertFileComments(dependencies, stubFileName, new Map()); + await convertFileComments(dependencies, stubFileName, new Map(), new Map()); // Assert expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( @@ -125,7 +125,7 @@ export const b = true; ); }); - it("re-uses a rule conversion from cache when it was already converted", async () => { + it("re-uses a rule conversion from conversion cache when it was already converted", async () => { // Arrange const dependencies = createStubDependencies(` /* tslint:disable:ts-a */ @@ -133,7 +133,37 @@ export const a = true; `); // Act - await convertFileComments(dependencies, stubFileName, new Map([["ts-a", "es-cached"]])); + await convertFileComments( + dependencies, + stubFileName, + new Map(), + new Map([["ts-a", ["es-cached"]]]), + ); + + // Assert + expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( + stubFileName, + ` +/* eslint-disable es-cached */ +export const a = true; +`, + ); + }); + + it("re-uses a rule conversion from comments cache when it was already converted", async () => { + // Arrange + const dependencies = createStubDependencies(` +/* tslint:disable:ts-a */ +export const a = true; +`); + + // Act + await convertFileComments( + dependencies, + stubFileName, + new Map([["ts-a", ["es-cached"]]]), + new Map(), + ); // Assert expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( @@ -153,7 +183,7 @@ export const a = true; `); // Act - await convertFileComments(dependencies, stubFileName, new Map()); + await convertFileComments(dependencies, stubFileName, new Map(), new Map()); // Assert expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( @@ -173,7 +203,7 @@ export const a = true; `); // Act - await convertFileComments(dependencies, stubFileName, new Map()); + await convertFileComments(dependencies, stubFileName, new Map(), new Map()); // Assert expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( diff --git a/src/comments/convertFileComments.ts b/src/comments/convertFileComments.ts index de129ebd5..b535b2d48 100644 --- a/src/comments/convertFileComments.ts +++ b/src/comments/convertFileComments.ts @@ -11,7 +11,8 @@ export type ConvertFileCommentsDependencies = { export const convertFileComments = async ( dependencies: ConvertFileCommentsDependencies, filePath: string, - ruleConversionCache: Map, + ruleCommentsCache: Map, + ruleEquivalents: Map, ) => { const fileContent = await dependencies.fileSystem.readFile(filePath); if (fileContent instanceof Error) { @@ -23,7 +24,8 @@ export const convertFileComments = async ( fileContent, comments, dependencies.converters, - ruleConversionCache, + ruleCommentsCache, + ruleEquivalents, ); return fileContent === newFileContent diff --git a/src/comments/replaceFileComments.ts b/src/comments/replaceFileComments.ts index 7ffd8566e..21350545b 100644 --- a/src/comments/replaceFileComments.ts +++ b/src/comments/replaceFileComments.ts @@ -9,23 +9,31 @@ export const replaceFileComments = ( content: string, comments: FileComment[], converters: Map, - ruleConversionCache: Map, + ruleCommentsCache: Map, + ruleEquivalents: Map, ) => { const getNewRuleLists = (ruleName: string) => { - if (ruleConversionCache.has(ruleName)) { - return ruleConversionCache.get(ruleName); + const cached = ruleEquivalents.get(ruleName) ?? ruleCommentsCache.get(ruleName); + if (cached !== undefined) { + return cached; } const converter = converters.get(ruleName); if (converter === undefined) { - ruleConversionCache.set(ruleName, undefined); + ruleCommentsCache.set(ruleName, []); return undefined; } const converted = converter({ ruleArguments: [] }); - return converted instanceof ConversionError - ? undefined - : converted.rules.map((conversion) => conversion.ruleName).join(", "); + if (converted instanceof ConversionError) { + return undefined; + } + + const equivalents = converted.rules.map((conversion) => conversion.ruleName); + + ruleCommentsCache.set(ruleName, equivalents); + + return equivalents.join(", "); }; for (const comment of [...comments].reverse()) { diff --git a/src/conversion/conversionResults.stubs.ts b/src/conversion/conversionResults.stubs.ts index b669023c4..69ea69bcb 100644 --- a/src/conversion/conversionResults.stubs.ts +++ b/src/conversion/conversionResults.stubs.ts @@ -10,6 +10,7 @@ export const createEmptyConversionResults = ( failed: [], missing: [], plugins: new Set(), + ruleEquivalents: new Map(), ...overrides, }); diff --git a/src/conversion/convertLintConfig.test.ts b/src/conversion/convertLintConfig.test.ts index cea6b4e04..419fe95ce 100644 --- a/src/conversion/convertLintConfig.test.ts +++ b/src/conversion/convertLintConfig.test.ts @@ -1,4 +1,5 @@ import { ResultStatus, FailedResult } from "../types"; +import { createEmptyConversionResults } from "./conversionResults.stubs"; import { convertLintConfig, ConvertLintConfigDependencies } from "./convertLintConfig"; const stubSettings = { @@ -7,28 +8,27 @@ const stubSettings = { const createStubDependencies = ( overrides: Partial = {}, -): ConvertLintConfigDependencies => ({ - convertComments: jest.fn(), - convertRules: jest.fn(), - findOriginalConfigurations: jest.fn().mockResolvedValue({ - data: createStubOriginalConfigurationsData(), - status: ResultStatus.Succeeded, - }), - reportCommentResults: jest.fn(), - reportConversionResults: jest.fn(), - summarizePackageRules: async (_configurations, data) => ({ - ...data, - converted: new Map(), - extends: [], - extensionRules: new Map(), - failed: [], - missing: [], - plugins: new Set(), - }), - logMissingPackages: jest.fn().mockReturnValue(Promise.resolve()), - writeConversionResults: jest.fn().mockReturnValue(Promise.resolve()), - ...overrides, -}); +): ConvertLintConfigDependencies => { + const ruleConversionResults = createEmptyConversionResults(); + + return { + convertComments: jest.fn(), + convertRules: jest.fn().mockReturnValue(ruleConversionResults), + findOriginalConfigurations: jest.fn().mockResolvedValue({ + data: createStubOriginalConfigurationsData(), + status: ResultStatus.Succeeded, + }), + reportCommentResults: jest.fn(), + reportConversionResults: jest.fn(), + summarizePackageRules: async (_configurations, data) => ({ + ...ruleConversionResults, + ...data, + }), + logMissingPackages: jest.fn().mockReturnValue(Promise.resolve()), + writeConversionResults: jest.fn().mockReturnValue(Promise.resolve()), + ...overrides, + }; +}; const createStubOriginalConfigurationsData = () => ({ tslint: { diff --git a/src/conversion/convertLintConfig.ts b/src/conversion/convertLintConfig.ts index 435caf7ed..6c5f1f1af 100644 --- a/src/conversion/convertLintConfig.ts +++ b/src/conversion/convertLintConfig.ts @@ -60,9 +60,10 @@ export const convertLintConfig = async ( }; } - // 5. Files to transform comments in have source text rewritten using the same rule conversion logic + // 5. Files to transform comments in have source text rewritten using the cached rule conversion results const commentsResult = await dependencies.convertComments( settings.comments, + ruleConversionResults.ruleEquivalents, originalConfigurations.data.typescript, ); diff --git a/src/rules/convertRules.ts b/src/rules/convertRules.ts index 29db1b5f7..0df35708f 100644 --- a/src/rules/convertRules.ts +++ b/src/rules/convertRules.ts @@ -18,6 +18,7 @@ export type RuleConversionResults = { failed: ErrorSummary[]; missing: TSLintRuleOptions[]; plugins: Set; + ruleEquivalents: Map; }; export const convertRules = ( @@ -28,6 +29,7 @@ export const convertRules = ( const failed: ConversionError[] = []; const missing: TSLintRuleOptions[] = []; const plugins = new Set(); + const ruleEquivalents = new Map(); if (rawTslintRules !== undefined) { for (const [ruleName, value] of Object.entries(rawTslintRules)) { @@ -47,7 +49,11 @@ export const convertRules = ( continue; } + const equivalents = new Set(); + for (const changes of conversion.rules) { + equivalents.add(changes.ruleName); + const existingConversion = converted.get(changes.ruleName); const newConversion = { ...changes, @@ -82,7 +88,9 @@ export const convertRules = ( plugins.add(newPlugin); } } + + ruleEquivalents.set(tslintRule.ruleName, Array.from(equivalents)); } } - return { converted, failed, missing, plugins }; + return { converted, failed, missing, plugins, ruleEquivalents }; };