From 29baeeeb5b28c6fac2e059ae0fd29f5202576dc5 Mon Sep 17 00:00:00 2001 From: KingDarBoja Date: Sat, 19 Oct 2019 22:30:07 -0500 Subject: [PATCH 01/17] Set basic replace comment conversion --- src/adapters/fileSystem.ts | 5 ++++ src/adapters/fsFileSystem.ts | 10 ++++++- src/cli/runCli.ts | 1 + src/conversion/convertConfig.ts | 11 +++++++ src/rules/convertComments.ts | 53 +++++++++++++++++++++++++++++++++ src/types.ts | 5 ++++ 6 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 src/rules/convertComments.ts diff --git a/src/adapters/fileSystem.ts b/src/adapters/fileSystem.ts index 2b5f825da..61c19e5df 100644 --- a/src/adapters/fileSystem.ts +++ b/src/adapters/fileSystem.ts @@ -1,5 +1,10 @@ +import * as fs from "fs"; + +export type ReadDirOptions = { encoding?: string | null; withFileTypes: true }; + export type FileSystem = { fileExists: (filePath: string) => Promise; readFile: (filePath: string) => Promise; writeFile: (filePath: string, contents: string) => Promise; + readDir: (dirPath: string, options: ReadDirOptions) => Promise; }; diff --git a/src/adapters/fsFileSystem.ts b/src/adapters/fsFileSystem.ts index 9740bcde9..3fa5b35a9 100644 --- a/src/adapters/fsFileSystem.ts +++ b/src/adapters/fsFileSystem.ts @@ -1,9 +1,10 @@ import * as fs from "fs"; import { promisify } from "util"; -import { FileSystem } from "./fileSystem"; +import { FileSystem, ReadDirOptions } from "./fileSystem"; const readFile = promisify(fs.readFile); +const readDir = promisify(fs.readdir); const writeFile = promisify(fs.writeFile); export const fsFileSystem: FileSystem = { @@ -28,4 +29,11 @@ export const fsFileSystem: FileSystem = { return error; } }, + readDir: async (dirPath: string, options: ReadDirOptions) => { + try { + return readDir(dirPath, options); + } catch (error) { + return error; + } + }, }; diff --git a/src/cli/runCli.ts b/src/cli/runCli.ts index 147891d02..ca2efac76 100644 --- a/src/cli/runCli.ts +++ b/src/cli/runCli.ts @@ -24,6 +24,7 @@ export const runCli = async ( .option("--package [package]", "package configuration file to convert using") .option("--tslint [tslint]", "tslint configuration file to convert using") .option("--typescript [typescript]", "typescript configuration file to convert using") + .option("-c, --convert ", "convert all tslint:disable comments into eslint-disable") .option("-V --version", "output the package version"); const parsedArgv = { diff --git a/src/conversion/convertConfig.ts b/src/conversion/convertConfig.ts index a6308d039..a429ac3ae 100644 --- a/src/conversion/convertConfig.ts +++ b/src/conversion/convertConfig.ts @@ -5,12 +5,14 @@ import { findOriginalConfigurations } from "../input/findOriginalConfigurations" import { reportConversionResults } from "../reporting/reportConversionResults"; import { convertRules } from "../rules/convertRules"; import { ResultStatus, ResultWithStatus, TSLintToESLintSettings } from "../types"; +import { convertComments } from "../rules/convertComments"; export type ConvertConfigDependencies = { convertRules: SansDependencies; findOriginalConfigurations: SansDependencies; reportConversionResults: SansDependencies; simplifyPackageRules: SansDependencies; + convertComments: SansDependencies; writeConversionResults: SansDependencies; }; @@ -55,6 +57,15 @@ export const convertConfig = async ( }; } + // 4.B Convert comments. + const fileCommentsError = await dependencies.convertComments(); + if (fileCommentsError !== undefined) { + return { + errors: [fileCommentsError], + status: ResultStatus.Failed, + }; + } + // 5. A summary of the results is printed to the user's console dependencies.reportConversionResults(simplifiedConfiguration); diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts new file mode 100644 index 000000000..be6737b97 --- /dev/null +++ b/src/rules/convertComments.ts @@ -0,0 +1,53 @@ +import { FileSystem } from "../adapters/fileSystem"; +import { isError } from "../utils"; +import { getCommentAtPosition } from "tsutils"; +import * as ts from "typescript"; + +// Check https://github.com/Microsoft/TypeScript/issues/21049 + +export type ConvertCommentsResultsDependencies = { + fileSystem: Pick; +}; + +export const convertComments = async (dependencies: ConvertCommentsResultsDependencies) => { + const filenames = await dependencies.fileSystem.readDir("", { withFileTypes: true }); + if (!isError(filenames)) { + const filteredFilenames: string[] = filenames + .filter(fileEnt => fileEnt.isFile()) + .map(fileEnt => fileEnt.name); + for (const filename of filteredFilenames) { + const fileContent: string | Error = await dependencies.fileSystem.readFile(filename); + if (!isError(fileContent)) { + replaceComments(filename, fileContent); + } + } + } + return Promise.resolve(); +}; + +const replaceComments = (fileName: string, fileContent: string) => { + const tslintRegex: RegExp = new RegExp( + "/[/*]s*tslint:(enable|disable)((?:-next)?-line)?s*?(?:|*/)", + ); + const sourceFile: ts.SourceFile = ts.createSourceFile( + fileName, + fileContent, + ts.ScriptTarget.ES2015, + /*setParentNodes */ true, + ); + + for ( + let match = tslintRegex.exec(fileContent); + match !== null; + match = tslintRegex.exec(fileContent) + ) { + const comment: ts.CommentRange | undefined = getCommentAtPosition(sourceFile, match.index); + if ( + comment === undefined || + comment.pos !== match.index || + comment.end !== match.index + match[0].length + ) + continue; + console.log(match); + } +}; diff --git a/src/types.ts b/src/types.ts index 474eb3f46..0de9709d3 100644 --- a/src/types.ts +++ b/src/types.ts @@ -23,6 +23,11 @@ export type TSLintToESLintSettings = { * Original TypeScript configuration file path, such as `tsconfig.json`. */ typescript?: string; + + /** + * Opt-in flag to convert `tslint:disable` comments to `eslint-disable`. + */ + convert?: boolean; }; export type TSLintToESLintResult = ResultWithStatus; From e84bb8c9206d207efb6f90354ca019e94595d6bf Mon Sep 17 00:00:00 2001 From: KingDarBoja Date: Sun, 20 Oct 2019 15:38:31 -0500 Subject: [PATCH 02/17] Fix regex pattern and cli command name for convert comments --- src/cli/main.ts | 6 ++++ src/cli/runCli.ts | 2 +- src/conversion/convertConfig.test.ts | 1 + src/rules/convertComments.ts | 41 +++++++++++++++++++++------- src/types.ts | 2 +- 5 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/cli/main.ts b/src/cli/main.ts index c6a3535f1..00c5f8358 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -36,6 +36,7 @@ import { converters } from "../rules/converters"; import { convertRules } from "../rules/convertRules"; import { mergers } from "../rules/mergers"; import { runCli, RunCliDependencies } from "./runCli"; +import { convertComments, ConvertCommentsResultsDependencies } from "../rules/convertComments"; const convertRulesDependencies = { converters, @@ -67,6 +68,10 @@ const simplifyPackageRulesDependencies: SimplifyPackageRulesDependencies = { retrieveExtendsValues: bind(retrieveExtendsValues, retrieveExtendsValuesDependencies), }; +const convertCommentsResultsDependencies: ConvertCommentsResultsDependencies = { + fileSystem: fsFileSystem, +}; + const writeConversionResultsDependencies: WriteConversionResultsDependencies = { fileSystem: fsFileSystem, }; @@ -79,6 +84,7 @@ const convertConfigDependencies: ConvertConfigDependencies = { ), reportConversionResults: bind(reportConversionResults, reportConversionResultsDependencies), simplifyPackageRules: bind(simplifyPackageRules, simplifyPackageRulesDependencies), + convertComments: bind(convertComments, convertCommentsResultsDependencies), writeConversionResults: bind(writeConversionResults, writeConversionResultsDependencies), }; diff --git a/src/cli/runCli.ts b/src/cli/runCli.ts index ca2efac76..995ebdaf9 100644 --- a/src/cli/runCli.ts +++ b/src/cli/runCli.ts @@ -24,7 +24,7 @@ export const runCli = async ( .option("--package [package]", "package configuration file to convert using") .option("--tslint [tslint]", "tslint configuration file to convert using") .option("--typescript [typescript]", "typescript configuration file to convert using") - .option("-c, --convert ", "convert all tslint:disable comments into eslint-disable") + .option("-c, --convertComments", "convert all tslint:disable comments into eslint-disable") .option("-V --version", "output the package version"); const parsedArgv = { diff --git a/src/conversion/convertConfig.test.ts b/src/conversion/convertConfig.test.ts index aadc25758..9330895b9 100644 --- a/src/conversion/convertConfig.test.ts +++ b/src/conversion/convertConfig.test.ts @@ -15,6 +15,7 @@ const createStubDependencies = ( }), reportConversionResults: jest.fn(), simplifyPackageRules: async (_configurations, data) => data, + convertComments: jest.fn(), writeConversionResults: jest.fn().mockReturnValue(Promise.resolve()), ...overrides, }); diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts index be6737b97..68d94d013 100644 --- a/src/rules/convertComments.ts +++ b/src/rules/convertComments.ts @@ -10,24 +10,39 @@ export type ConvertCommentsResultsDependencies = { }; export const convertComments = async (dependencies: ConvertCommentsResultsDependencies) => { - const filenames = await dependencies.fileSystem.readDir("", { withFileTypes: true }); + // TODO: Remove console logs + console.log("Started"); + const filenames = await dependencies.fileSystem.readDir("./", { withFileTypes: true }); if (!isError(filenames)) { const filteredFilenames: string[] = filenames .filter(fileEnt => fileEnt.isFile()) .map(fileEnt => fileEnt.name); + // TODO: Remove console logs + console.log("Filenames filtered"); + console.log(filteredFilenames); for (const filename of filteredFilenames) { const fileContent: string | Error = await dependencies.fileSystem.readFile(filename); if (!isError(fileContent)) { - replaceComments(filename, fileContent); + const writeFileRes = await replaceComments(dependencies, filename, fileContent); + if (isError(writeFileRes)) { + return Error("Failed to convert file comments"); + } } } + return undefined; + } else { + return Error("Failed to convert file comments"); } - return Promise.resolve(); }; -const replaceComments = (fileName: string, fileContent: string) => { +const replaceComments = async ( + dependencies: ConvertCommentsResultsDependencies, + fileName: string, + fileContent: string, +) => { + // TODO: Include rules on the regexp to properly satisfy comment.end !== match.index + match[0].length check const tslintRegex: RegExp = new RegExp( - "/[/*]s*tslint:(enable|disable)((?:-next)?-line)?s*?(?:|*/)", + /\/[/*]\s*(tslint):(enable|disable)((?:-next)?-line)?\s*?(?:|\*\/)/gm, ); const sourceFile: ts.SourceFile = ts.createSourceFile( fileName, @@ -35,19 +50,25 @@ const replaceComments = (fileName: string, fileContent: string) => { ts.ScriptTarget.ES2015, /*setParentNodes */ true, ); - for ( - let match = tslintRegex.exec(fileContent); + let match = tslintRegex.exec(sourceFile.text); match !== null; - match = tslintRegex.exec(fileContent) + match = tslintRegex.exec(sourceFile.text) ) { + // TODO: Remove console logs + console.log("Match"); + console.log(match); + console.log(match[0].length); const comment: ts.CommentRange | undefined = getCommentAtPosition(sourceFile, match.index); + console.log("comment"); + console.log(comment); if ( comment === undefined || comment.pos !== match.index || comment.end !== match.index + match[0].length - ) + ) { continue; - console.log(match); + } } + return await dependencies.fileSystem.writeFile(fileName, fileContent); }; diff --git a/src/types.ts b/src/types.ts index 0de9709d3..49be3400b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -27,7 +27,7 @@ export type TSLintToESLintSettings = { /** * Opt-in flag to convert `tslint:disable` comments to `eslint-disable`. */ - convert?: boolean; + convertComments?: boolean; }; export type TSLintToESLintResult = ResultWithStatus; From dcfce0cfe6710d5c3ee87bb6b35a70ee4bbd70f8 Mon Sep 17 00:00:00 2001 From: KingDarBoja Date: Sun, 20 Oct 2019 16:54:38 -0500 Subject: [PATCH 03/17] Fix regex pattern condition to match rules without carriage return --- src/rules/convertComments.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts index 68d94d013..8fb9b218f 100644 --- a/src/rules/convertComments.ts +++ b/src/rules/convertComments.ts @@ -40,9 +40,8 @@ const replaceComments = async ( fileName: string, fileContent: string, ) => { - // TODO: Include rules on the regexp to properly satisfy comment.end !== match.index + match[0].length check const tslintRegex: RegExp = new RegExp( - /\/[/*]\s*(tslint):(enable|disable)((?:-next)?-line)?\s*?(?:|\*\/)/gm, + /\/[\/\*]\s*(tslint):(enable|disable)((?:-next)?-line)?(?::\s)?(:?(?:(?:[\w-]+\/)*[\w-]+\s*\s*)*(?:(?:[\w-]+\/)*[\w-]+\s*)*[\w-]+)?\s*?(?:$|\*\/)/gm, ); const sourceFile: ts.SourceFile = ts.createSourceFile( fileName, @@ -69,6 +68,7 @@ const replaceComments = async ( ) { continue; } + console.log("Passed Check"); } return await dependencies.fileSystem.writeFile(fileName, fileContent); }; From 300ebd3547e1334f35639adeacd36d8db8a2eaa4 Mon Sep 17 00:00:00 2001 From: KingDarBoja Date: Sun, 20 Oct 2019 23:50:21 -0500 Subject: [PATCH 04/17] Replace regex and use tslint functions to retrieve tslint comment data --- src/rules/convertComments.ts | 121 +++++++++++++++++++++++++++-------- 1 file changed, 95 insertions(+), 26 deletions(-) diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts index 8fb9b218f..20557abe8 100644 --- a/src/rules/convertComments.ts +++ b/src/rules/convertComments.ts @@ -1,14 +1,17 @@ import { FileSystem } from "../adapters/fileSystem"; import { isError } from "../utils"; -import { getCommentAtPosition } from "tsutils"; +import * as utils from "tsutils"; import * as ts from "typescript"; -// Check https://github.com/Microsoft/TypeScript/issues/21049 - +// Inspirated by: +// - https://github.com/Microsoft/TypeScript/issues/21049 +// - https://github.com/palantir/tslint/blob/master/src/enableDisableRules.ts export type ConvertCommentsResultsDependencies = { fileSystem: Pick; }; +const tslintRegex: RegExp = new RegExp(/\s*tslint:(enable|disable)(?:-(line|next-line))?(:|\s|$)/g); + export const convertComments = async (dependencies: ConvertCommentsResultsDependencies) => { // TODO: Remove console logs console.log("Started"); @@ -35,40 +38,106 @@ export const convertComments = async (dependencies: ConvertCommentsResultsDepend } }; +type Modifier = "line" | "next-line" | undefined; +function parseComment( + commentText: string, +): { rulesList: string[] | "all"; isEnabled: boolean; modifier: Modifier } | undefined { + const match = tslintRegex.exec(commentText); + if (match === null) { + return undefined; + } + + // remove everything matched by the previous regex to get only the specified rules + // split at whitespaces + // filter empty items coming from whitespaces at start, at end or empty list + let rulesList: string[] | "all" = splitOnSpaces(commentText.substr(match[0].length)); + if (rulesList.length === 0 && match[3] === ":") { + // nothing to do here: an explicit separator was specified but no rules to switch + return undefined; + } + if (rulesList.length === 0 || rulesList.indexOf("all") !== -1) { + // if list is empty we default to all enabled rules + // if `all` is specified we ignore the other rules and take all enabled rules + rulesList = "all"; + } + + return { rulesList, isEnabled: match[1] === "enable", modifier: match[2] as Modifier }; +} + +function splitOnSpaces(str: string): string[] { + return str.split(/\s+/).filter(s => s !== ""); +} + const replaceComments = async ( dependencies: ConvertCommentsResultsDependencies, fileName: string, fileContent: string, ) => { - const tslintRegex: RegExp = new RegExp( - /\/[\/\*]\s*(tslint):(enable|disable)((?:-next)?-line)?(?::\s)?(:?(?:(?:[\w-]+\/)*[\w-]+\s*\s*)*(?:(?:[\w-]+\/)*[\w-]+\s*)*[\w-]+)?\s*?(?:$|\*\/)/gm, - ); const sourceFile: ts.SourceFile = ts.createSourceFile( fileName, fileContent, ts.ScriptTarget.ES2015, /*setParentNodes */ true, ); - for ( - let match = tslintRegex.exec(sourceFile.text); - match !== null; - match = tslintRegex.exec(sourceFile.text) - ) { - // TODO: Remove console logs - console.log("Match"); - console.log(match); - console.log(match[0].length); - const comment: ts.CommentRange | undefined = getCommentAtPosition(sourceFile, match.index); - console.log("comment"); - console.log(comment); - if ( - comment === undefined || - comment.pos !== match.index || - comment.end !== match.index + match[0].length - ) { - continue; + // I assume that the token enum is `SingleLineCommentTrivia` to catch /* and // tokens. + // If not, it is `MultiLineCommentTrivia` as this method does check if it is a comment... + // or that's what I think it does. + utils.forEachComment(sourceFile, (fullText, comment) => { + const commentText = + comment.kind === ts.SyntaxKind.SingleLineCommentTrivia + ? fullText.substring(comment.pos + 2, comment.end) + : fullText.substring(comment.pos + 2, comment.end - 2); + + const parsed = parseComment(commentText); + if (parsed !== undefined) { + const { modifier } = parsed; + const switchRange = getSwitchRange(modifier, comment, sourceFile); + if (switchRange !== undefined) { + console.log("---------- COMMENT TEXT -----------"); + console.log(commentText); + console.log("PARSED DATA"); + console.log(parsed); + console.log("SWITCH RANGE"); + console.log(switchRange); + } } - console.log("Passed Check"); - } + }); return await dependencies.fileSystem.writeFile(fileName, fileContent); }; + +function getSwitchRange( + modifier: Modifier, + range: ts.TextRange, + sourceFile: ts.SourceFile, +): ts.TextRange | undefined { + const lineStarts = sourceFile.getLineStarts(); + + switch (modifier) { + case "line": + return { + // start at the beginning of the line where comment starts + pos: getStartOfLinePosition(range.pos), + // end at the beginning of the line following the comment + end: getStartOfLinePosition(range.end, 1), + }; + case "next-line": + // start at the beginning of the line following the comment + const pos = getStartOfLinePosition(range.end, 1); + if (pos === -1) { + // no need to switch anything, there is no next line + return undefined; + } + // end at the beginning of the line following the next line + return { pos, end: getStartOfLinePosition(range.end, 2) }; + default: + // switch rule for the rest of the file + // start at the current position, but skip end position + return { pos: range.pos, end: -1 }; + } + + /** Returns -1 for last line. */ + function getStartOfLinePosition(position: number, lineOffset = 0): number { + const line = ts.getLineAndCharacterOfPosition(sourceFile, position).line + lineOffset; + return line >= lineStarts.length ? -1 : lineStarts[line]; + } +} From fae2869c1d9aa97c25c1c6b2007a5967038d723a Mon Sep 17 00:00:00 2001 From: KingDarBoja Date: Sun, 8 Dec 2019 18:40:03 -0500 Subject: [PATCH 05/17] Switching rule handler Convert the input TSLint rule into equivalent ESLint --- src/rules/convertComments.ts | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts index 20557abe8..1c5ebc3ee 100644 --- a/src/rules/convertComments.ts +++ b/src/rules/convertComments.ts @@ -2,6 +2,9 @@ import { FileSystem } from "../adapters/fileSystem"; import { isError } from "../utils"; import * as utils from "tsutils"; import * as ts from "typescript"; +import { converters } from "./converters"; +import { formatRawTslintRule } from "./formatRawTslintRule"; +import { ConversionError } from "../errors/conversionError"; // Inspirated by: // - https://github.com/Microsoft/TypeScript/issues/21049 @@ -90,7 +93,7 @@ const replaceComments = async ( const parsed = parseComment(commentText); if (parsed !== undefined) { - const { modifier } = parsed; + const { rulesList, isEnabled, modifier } = parsed; const switchRange = getSwitchRange(modifier, comment, sourceFile); if (switchRange !== undefined) { console.log("---------- COMMENT TEXT -----------"); @@ -99,12 +102,32 @@ const replaceComments = async ( console.log(parsed); console.log("SWITCH RANGE"); console.log(switchRange); + const rulesToSwitch = + rulesList === "all" + ? Array.from(converters.keys()) + : rulesList.filter(ruleKey => converters.has(ruleKey)); + for (const ruleToSwitch of rulesToSwitch) { + switchRulename(ruleToSwitch, isEnabled, switchRange.pos, switchRange.end); + } } } }); return await dependencies.fileSystem.writeFile(fileName, fileContent); }; +function switchRulename(ruleName: string, _isEnable: boolean, _start: number, _end: number): void { + const tslintRuleConverter = converters.get(ruleName); + if (tslintRuleConverter) { + const tslintRule = formatRawTslintRule(ruleName, { ruleName }); + const conversion = tslintRuleConverter(tslintRule); + if (!(conversion instanceof ConversionError)) { + const eslintRules = conversion.rules.map(r => r.ruleName); + console.log(`Rulename: ${ruleName}`); + console.log(eslintRules); + } + } +} + function getSwitchRange( modifier: Modifier, range: ts.TextRange, From 8cb8c067a93df4ba7b5392a7adde41a4135b4a47 Mon Sep 17 00:00:00 2001 From: KingDarBoja Date: Sun, 8 Dec 2019 19:52:46 -0500 Subject: [PATCH 06/17] Write file after switching rule comments This is still a work in progress, need to check the start and end position of rule comments to avoid overwriting the wrong lines. --- src/adapters/fileSystem.ts | 1 + src/adapters/fsFileSystem.ts | 8 ++++++ src/rules/convertComments.ts | 49 +++++++++++++++++++++++++++++------- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/adapters/fileSystem.ts b/src/adapters/fileSystem.ts index 61c19e5df..d63ee4982 100644 --- a/src/adapters/fileSystem.ts +++ b/src/adapters/fileSystem.ts @@ -6,5 +6,6 @@ export type FileSystem = { fileExists: (filePath: string) => Promise; readFile: (filePath: string) => Promise; writeFile: (filePath: string, contents: string) => Promise; + writeFileSync: (filePath: string, contents: string) => undefined; readDir: (dirPath: string, options: ReadDirOptions) => Promise; }; diff --git a/src/adapters/fsFileSystem.ts b/src/adapters/fsFileSystem.ts index 3fa5b35a9..de1e7538e 100644 --- a/src/adapters/fsFileSystem.ts +++ b/src/adapters/fsFileSystem.ts @@ -6,6 +6,7 @@ import { FileSystem, ReadDirOptions } from "./fileSystem"; const readFile = promisify(fs.readFile); const readDir = promisify(fs.readdir); const writeFile = promisify(fs.writeFile); +const writeFileSync = fs.writeFileSync; export const fsFileSystem: FileSystem = { fileExists: async (filePath: string) => { @@ -29,6 +30,13 @@ export const fsFileSystem: FileSystem = { return error; } }, + writeFileSync: (filePath: string, contents: string) => { + try { + return writeFileSync(filePath, contents); + } catch (error) { + return error; + } + }, readDir: async (dirPath: string, options: ReadDirOptions) => { try { return readDir(dirPath, options); diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts index 1c5ebc3ee..5886ea73e 100644 --- a/src/rules/convertComments.ts +++ b/src/rules/convertComments.ts @@ -1,7 +1,7 @@ import { FileSystem } from "../adapters/fileSystem"; import { isError } from "../utils"; import * as utils from "tsutils"; -import * as ts from "typescript"; +import ts from "typescript"; import { converters } from "./converters"; import { formatRawTslintRule } from "./formatRawTslintRule"; import { ConversionError } from "../errors/conversionError"; @@ -10,7 +10,7 @@ import { ConversionError } from "../errors/conversionError"; // - https://github.com/Microsoft/TypeScript/issues/21049 // - https://github.com/palantir/tslint/blob/master/src/enableDisableRules.ts export type ConvertCommentsResultsDependencies = { - fileSystem: Pick; + fileSystem: Pick; }; const tslintRegex: RegExp = new RegExp(/\s*tslint:(enable|disable)(?:-(line|next-line))?(:|\s|$)/g); @@ -71,8 +71,14 @@ function splitOnSpaces(str: string): string[] { return str.split(/\s+/).filter(s => s !== ""); } +interface IReplacement { + start: number; + end: number; + replacementText: string; +} + const replaceComments = async ( - dependencies: ConvertCommentsResultsDependencies, + _dependencies: ConvertCommentsResultsDependencies, fileName: string, fileContent: string, ) => { @@ -86,14 +92,15 @@ const replaceComments = async ( // If not, it is `MultiLineCommentTrivia` as this method does check if it is a comment... // or that's what I think it does. utils.forEachComment(sourceFile, (fullText, comment) => { + const replacements: IReplacement[] = []; const commentText = comment.kind === ts.SyntaxKind.SingleLineCommentTrivia - ? fullText.substring(comment.pos + 2, comment.end) - : fullText.substring(comment.pos + 2, comment.end - 2); + ? fullText.substring(comment.pos + 3, comment.end) + : fullText.substring(comment.pos + 3, comment.end - 2); const parsed = parseComment(commentText); if (parsed !== undefined) { - const { rulesList, isEnabled, modifier } = parsed; + const { rulesList, modifier } = parsed; const switchRange = getSwitchRange(modifier, comment, sourceFile); if (switchRange !== undefined) { console.log("---------- COMMENT TEXT -----------"); @@ -107,15 +114,37 @@ const replaceComments = async ( ? Array.from(converters.keys()) : rulesList.filter(ruleKey => converters.has(ruleKey)); for (const ruleToSwitch of rulesToSwitch) { - switchRulename(ruleToSwitch, isEnabled, switchRange.pos, switchRange.end); + const transformedRules = switchRule(ruleToSwitch); + if (transformedRules) { + replacements.push({ + start: switchRange.pos, + end: switchRange.end, + replacementText: transformedRules.join(" "), + }); + } } } } + // Reverse the replacement list + replacements.reverse(); + + const newText = getNewText(fileContent, replacements); + console.log("************** NEW FILE BEING WRITTEN ! **************"); + console.log(newText); + // dependencies.fileSystem.writeFileSync(fileName, newText); }); - return await dependencies.fileSystem.writeFile(fileName, fileContent); + return true; }; -function switchRulename(ruleName: string, _isEnable: boolean, _start: number, _end: number): void { +function getNewText(sourceText: string, replacementsInReverse: IReplacement[]) { + for (const { start, end, replacementText } of replacementsInReverse) { + sourceText = sourceText.slice(0, start) + replacementText + sourceText.slice(end); + } + + return sourceText; +} + +function switchRule(ruleName: string): string[] | null { const tslintRuleConverter = converters.get(ruleName); if (tslintRuleConverter) { const tslintRule = formatRawTslintRule(ruleName, { ruleName }); @@ -124,8 +153,10 @@ function switchRulename(ruleName: string, _isEnable: boolean, _start: number, _e const eslintRules = conversion.rules.map(r => r.ruleName); console.log(`Rulename: ${ruleName}`); console.log(eslintRules); + return eslintRules; } } + return null; } function getSwitchRange( From dd319f8b057fb3b0f367a4ad7b5cfc81645bf7f0 Mon Sep 17 00:00:00 2001 From: KingDarBoja Date: Sat, 28 Mar 2020 14:00:46 -0500 Subject: [PATCH 07/17] replace old path to rulesConverter --- src/cli/runCli.ts | 2 +- src/rules/convertComments.ts | 32 ++++++++++++++++++-------------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/cli/runCli.ts b/src/cli/runCli.ts index fbd20292f..524eac356 100644 --- a/src/cli/runCli.ts +++ b/src/cli/runCli.ts @@ -25,7 +25,7 @@ export const runCli = async ( .option("--tslint [tslint]", "tslint configuration file to convert using") .option("--typescript [typescript]", "typescript configuration file to convert using") .option("--editor [editor]", "editor configuration file to convert") - .option("-c --convertComments", "convert all tslint:disable comments into eslint-disable") + .option("-C --convertComments", "convert all tslint:disable comments into eslint-disable") .option("-V --version", "output the package version"); const parsedArgv = { diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts index 5886ea73e..bc00fb438 100644 --- a/src/rules/convertComments.ts +++ b/src/rules/convertComments.ts @@ -2,7 +2,7 @@ import { FileSystem } from "../adapters/fileSystem"; import { isError } from "../utils"; import * as utils from "tsutils"; import ts from "typescript"; -import { converters } from "./converters"; +import { rulesConverters } from "./rulesConverters"; import { formatRawTslintRule } from "./formatRawTslintRule"; import { ConversionError } from "../errors/conversionError"; @@ -13,7 +13,7 @@ export type ConvertCommentsResultsDependencies = { fileSystem: Pick; }; -const tslintRegex: RegExp = new RegExp(/\s*tslint:(enable|disable)(?:-(line|next-line))?(:|\s|$)/g); +const tslintRegex = new RegExp(/\s*tslint:(enable|disable)(?:-(line|next-line))?(:|\s|$)/g); export const convertComments = async (dependencies: ConvertCommentsResultsDependencies) => { // TODO: Remove console logs @@ -36,9 +36,8 @@ export const convertComments = async (dependencies: ConvertCommentsResultsDepend } } return undefined; - } else { - return Error("Failed to convert file comments"); } + return Error("Failed to convert file comments"); }; type Modifier = "line" | "next-line" | undefined; @@ -58,7 +57,7 @@ function parseComment( // nothing to do here: an explicit separator was specified but no rules to switch return undefined; } - if (rulesList.length === 0 || rulesList.indexOf("all") !== -1) { + if (rulesList.length === 0 || rulesList.includes("all")) { // if list is empty we default to all enabled rules // if `all` is specified we ignore the other rules and take all enabled rules rulesList = "all"; @@ -71,11 +70,11 @@ function splitOnSpaces(str: string): string[] { return str.split(/\s+/).filter(s => s !== ""); } -interface IReplacement { +type IReplacement = { start: number; end: number; replacementText: string; -} +}; const replaceComments = async ( _dependencies: ConvertCommentsResultsDependencies, @@ -103,16 +102,17 @@ const replaceComments = async ( const { rulesList, modifier } = parsed; const switchRange = getSwitchRange(modifier, comment, sourceFile); if (switchRange !== undefined) { - console.log("---------- COMMENT TEXT -----------"); + // Extra log to check what is going on + console.log("----------- COMMENT TEXT -----------"); console.log(commentText); - console.log("PARSED DATA"); + console.log("----------- PARSED DATA -----------"); console.log(parsed); - console.log("SWITCH RANGE"); + console.log("----------- SWITCH RANGE -----------"); console.log(switchRange); const rulesToSwitch = rulesList === "all" - ? Array.from(converters.keys()) - : rulesList.filter(ruleKey => converters.has(ruleKey)); + ? Array.from(rulesConverters.keys()) + : rulesList.filter(ruleKey => rulesConverters.has(ruleKey)); for (const ruleToSwitch of rulesToSwitch) { const transformedRules = switchRule(ruleToSwitch); if (transformedRules) { @@ -129,9 +129,13 @@ const replaceComments = async ( replacements.reverse(); const newText = getNewText(fileContent, replacements); + // Check the output before writing to file. + console.log(""); console.log("************** NEW FILE BEING WRITTEN ! **************"); console.log(newText); - // dependencies.fileSystem.writeFileSync(fileName, newText); + // Write the file with the changes. + // At the moment, + _dependencies.fileSystem.writeFileSync(fileName, newText); }); return true; }; @@ -145,7 +149,7 @@ function getNewText(sourceText: string, replacementsInReverse: IReplacement[]) { } function switchRule(ruleName: string): string[] | null { - const tslintRuleConverter = converters.get(ruleName); + const tslintRuleConverter = rulesConverters.get(ruleName); if (tslintRuleConverter) { const tslintRule = formatRawTslintRule(ruleName, { ruleName }); const conversion = tslintRuleConverter(tslintRule); From bacf9e8455714934f546989aba5354dfc2f58ebe Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 02:11:01 -0400 Subject: [PATCH 08/17] Working replacements for single line changes --- src/rules/convertComments.ts | 65 +++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts index bc00fb438..fbd4b7744 100644 --- a/src/rules/convertComments.ts +++ b/src/rules/convertComments.ts @@ -17,16 +17,16 @@ const tslintRegex = new RegExp(/\s*tslint:(enable|disable)(?:-(line|next-line))? export const convertComments = async (dependencies: ConvertCommentsResultsDependencies) => { // TODO: Remove console logs - console.log("Started"); const filenames = await dependencies.fileSystem.readDir("./", { withFileTypes: true }); if (!isError(filenames)) { const filteredFilenames: string[] = filenames - .filter(fileEnt => fileEnt.isFile()) - .map(fileEnt => fileEnt.name); + .filter((fileEnt) => fileEnt.isFile()) + .map((fileEnt) => fileEnt.name); // TODO: Remove console logs - console.log("Filenames filtered"); - console.log(filteredFilenames); for (const filename of filteredFilenames) { + // todo: cli should pass glob/wildcard + if (!filename.includes("index")) continue; + const fileContent: string | Error = await dependencies.fileSystem.readFile(filename); if (!isError(fileContent)) { const writeFileRes = await replaceComments(dependencies, filename, fileContent); @@ -40,7 +40,9 @@ export const convertComments = async (dependencies: ConvertCommentsResultsDepend return Error("Failed to convert file comments"); }; -type Modifier = "line" | "next-line" | undefined; +type CommentKind = ts.SyntaxKind.MultiLineCommentTrivia | ts.SyntaxKind.SingleLineCommentTrivia; + +type Modifier = "line" | "next-line" | undefined; // todo: split "line" into "block-start" and "block-end"? function parseComment( commentText: string, ): { rulesList: string[] | "all"; isEnabled: boolean; modifier: Modifier } | undefined { @@ -67,7 +69,7 @@ function parseComment( } function splitOnSpaces(str: string): string[] { - return str.split(/\s+/).filter(s => s !== ""); + return str.split(/\s+/).filter((s) => s !== ""); } type IReplacement = { @@ -92,34 +94,34 @@ const replaceComments = async ( // or that's what I think it does. utils.forEachComment(sourceFile, (fullText, comment) => { const replacements: IReplacement[] = []; - const commentText = - comment.kind === ts.SyntaxKind.SingleLineCommentTrivia - ? fullText.substring(comment.pos + 3, comment.end) - : fullText.substring(comment.pos + 3, comment.end - 2); + const commentText = (comment.kind === ts.SyntaxKind.SingleLineCommentTrivia + ? fullText.substring(comment.pos + 2, comment.end) + : fullText.substring(comment.pos + 2, comment.end - 2) + ).trim(); const parsed = parseComment(commentText); if (parsed !== undefined) { const { rulesList, modifier } = parsed; const switchRange = getSwitchRange(modifier, comment, sourceFile); + console.log({ modifier, comment }); if (switchRange !== undefined) { // Extra log to check what is going on - console.log("----------- COMMENT TEXT -----------"); - console.log(commentText); - console.log("----------- PARSED DATA -----------"); - console.log(parsed); - console.log("----------- SWITCH RANGE -----------"); - console.log(switchRange); const rulesToSwitch = rulesList === "all" ? Array.from(rulesConverters.keys()) - : rulesList.filter(ruleKey => rulesConverters.has(ruleKey)); + : rulesList.filter((ruleKey) => rulesConverters.has(ruleKey)); + console.log({ rulesToSwitch }); for (const ruleToSwitch of rulesToSwitch) { const transformedRules = switchRule(ruleToSwitch); if (transformedRules) { replacements.push({ - start: switchRange.pos, - end: switchRange.end, - replacementText: transformedRules.join(" "), + start: comment.pos, + end: comment.end, + replacementText: createReplacementText( + comment.kind, + modifier, + transformedRules, + ), }); } } @@ -130,9 +132,6 @@ const replaceComments = async ( const newText = getNewText(fileContent, replacements); // Check the output before writing to file. - console.log(""); - console.log("************** NEW FILE BEING WRITTEN ! **************"); - console.log(newText); // Write the file with the changes. // At the moment, _dependencies.fileSystem.writeFileSync(fileName, newText); @@ -140,7 +139,21 @@ const replaceComments = async ( return true; }; +function createReplacementText( + commentKind: CommentKind, + _modifier: Modifier, + transformedRules: string[], +) { + const directive = "eslint-disable-next-line"; // todo: map from modifier + const [left, right] = + commentKind === ts.SyntaxKind.SingleLineCommentTrivia ? ["// ", ""] : ["/* ", " */"]; + + // todo: no transformed rules? + return [left, directive, " ", transformedRules.join(" "), right].join(""); +} + function getNewText(sourceText: string, replacementsInReverse: IReplacement[]) { + console.log({ replacementsInReverse }); for (const { start, end, replacementText } of replacementsInReverse) { sourceText = sourceText.slice(0, start) + replacementText + sourceText.slice(end); } @@ -154,9 +167,7 @@ function switchRule(ruleName: string): string[] | null { const tslintRule = formatRawTslintRule(ruleName, { ruleName }); const conversion = tslintRuleConverter(tslintRule); if (!(conversion instanceof ConversionError)) { - const eslintRules = conversion.rules.map(r => r.ruleName); - console.log(`Rulename: ${ruleName}`); - console.log(eslintRules); + const eslintRules = conversion.rules.map((r) => r.ruleName); return eslintRules; } } From 35adb284e18e5761235cdf7d848250b36d394307 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 03:58:01 -0400 Subject: [PATCH 09/17] 4am and it works --- .prettierrc | 3 +- docs/Architecture.md | 3 +- package-lock.json | 29 +++- package.json | 2 + src/adapters/fileSystem.ts | 6 - src/adapters/fsFileSystem.ts | 18 +-- src/adapters/globAsync.ts | 11 ++ src/cli/main.ts | 31 +++- src/cli/runCli.ts | 4 +- src/comments/convertComments.ts | 50 +++++++ src/comments/convertFileComments.ts | 25 ++++ src/comments/parseFileComments.ts | 66 +++++++++ src/comments/replaceFileComments.ts | 51 +++++++ src/conversion/convertConfig.test.ts | 21 ++- src/conversion/convertConfig.ts | 20 +-- src/rules/convertComments.ts | 212 --------------------------- src/types.ts | 10 +- src/utils.ts | 15 +- 18 files changed, 306 insertions(+), 271 deletions(-) create mode 100644 src/adapters/globAsync.ts create mode 100644 src/comments/convertComments.ts create mode 100644 src/comments/convertFileComments.ts create mode 100644 src/comments/parseFileComments.ts create mode 100644 src/comments/replaceFileComments.ts delete mode 100644 src/rules/convertComments.ts diff --git a/.prettierrc b/.prettierrc index 11a639ca7..4d63829ac 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,5 +1,6 @@ { + "arrowParens": "avoid", "printWidth": 100, "tabWidth": 4, "trailingComma": "all" -} \ No newline at end of file +} diff --git a/docs/Architecture.md b/docs/Architecture.md index ec2224a31..dc69d2b6c 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -15,7 +15,8 @@ Within `src/conversion/convertConfig.ts`, the following steps occur: 2. TSLint rules are converted into their ESLint configurations 3. ESLint configurations are simplified based on extended ESLint and TSLint presets 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 diff --git a/package-lock.json b/package-lock.json index 9d59cc7e8..635805cae 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3203,6 +3203,23 @@ "integrity": "sha512-OCutwjDZ4aFS6PB1UZ988C4YgwlBHJd6wCeQqaLdmadZ/7e+w79+hbMUFC1QXDNCmdyoRfAFdm0RypzwR+Qpag==", "dev": true }, + "@types/events": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@types/events/-/events-3.0.0.tgz", + "integrity": "sha512-EaObqwIvayI5a8dCzhFrjKzVwKLxjoG9T6Ppd5CEo07LRKfQ8Yokw54r5+Wq7FaBQ+yXRvQAYPrHwya1/UFt9g==", + "dev": true + }, + "@types/glob": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/@types/glob/-/glob-7.1.1.tgz", + "integrity": "sha512-1Bh06cbWJUHMC97acuD6UMG29nMt0Aqz1vF3guLfG+kHHJhy3AyohZFFxYk2f7Q1SQIrNwvncxAE0N/9s70F2w==", + "dev": true, + "requires": { + "@types/events": "*", + "@types/minimatch": "*", + "@types/node": "*" + } + }, "@types/istanbul-lib-coverage": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.1.tgz", @@ -3244,6 +3261,12 @@ "integrity": "sha512-8+KAKzEvSUdeo+kmqnKrqgeE+LcA0tjYWFY7RPProVYwnqDjukzO+3b6dLD56rYX5TdWejnEOLJYOIeh4CXKuA==", "dev": true }, + "@types/minimatch": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz", + "integrity": "sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA==", + "dev": true + }, "@types/node": { "version": "12.12.21", "resolved": "https://registry.npmjs.org/@types/node/-/node-12.12.21.tgz", @@ -5074,9 +5097,9 @@ } }, "glob": { - "version": "7.1.4", - "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.4.tgz", - "integrity": "sha512-hkLPepehmnKk41pUGm3sYxoFs/umurYfYJCerbXEyFIWcAzvpipAgVkBqqT9RBKMGjnq6kMuyYwha6csxbiM1A==", + "version": "7.1.6", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.6.tgz", + "integrity": "sha512-LwaxwyZ72Lk7vZINtNNrywX0ZuLyStrdDtabefZKAY5ZGJhVtgdznluResxNmPitE0SAO+O26sWTHeKSI2wMBA==", "requires": { "fs.realpath": "^1.0.0", "inflight": "^1.0.4", diff --git a/package.json b/package.json index 21f1c89ca..89f058ee1 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ "dependencies": { "chalk": "4.0.0", "commander": "5.0.0", + "glob": "^7.1.6", "strip-json-comments": "3.1.0", "tslint": "6.1.1", "typescript": "3.8.3" @@ -22,6 +23,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.28.0", diff --git a/src/adapters/fileSystem.ts b/src/adapters/fileSystem.ts index d63ee4982..2b5f825da 100644 --- a/src/adapters/fileSystem.ts +++ b/src/adapters/fileSystem.ts @@ -1,11 +1,5 @@ -import * as fs from "fs"; - -export type ReadDirOptions = { encoding?: string | null; withFileTypes: true }; - export type FileSystem = { fileExists: (filePath: string) => Promise; readFile: (filePath: string) => Promise; writeFile: (filePath: string, contents: string) => Promise; - writeFileSync: (filePath: string, contents: string) => undefined; - readDir: (dirPath: string, options: ReadDirOptions) => Promise; }; diff --git a/src/adapters/fsFileSystem.ts b/src/adapters/fsFileSystem.ts index de1e7538e..9740bcde9 100644 --- a/src/adapters/fsFileSystem.ts +++ b/src/adapters/fsFileSystem.ts @@ -1,12 +1,10 @@ import * as fs from "fs"; import { promisify } from "util"; -import { FileSystem, ReadDirOptions } from "./fileSystem"; +import { FileSystem } from "./fileSystem"; const readFile = promisify(fs.readFile); -const readDir = promisify(fs.readdir); const writeFile = promisify(fs.writeFile); -const writeFileSync = fs.writeFileSync; export const fsFileSystem: FileSystem = { fileExists: async (filePath: string) => { @@ -30,18 +28,4 @@ export const fsFileSystem: FileSystem = { return error; } }, - writeFileSync: (filePath: string, contents: string) => { - try { - return writeFileSync(filePath, contents); - } catch (error) { - return error; - } - }, - readDir: async (dirPath: string, options: ReadDirOptions) => { - try { - return readDir(dirPath, options); - } catch (error) { - return error; - } - }, }; diff --git a/src/adapters/globAsync.ts b/src/adapters/globAsync.ts new file mode 100644 index 000000000..fb53eb1f2 --- /dev/null +++ b/src/adapters/globAsync.ts @@ -0,0 +1,11 @@ +import glob from "glob"; + +export const globAsync = async (pattern: string) => { + return new Promise(resolve => { + glob(pattern, (error, matches) => { + resolve(error || matches); + }); + }); +}; + +export type GlobAsync = typeof globAsync; diff --git a/src/cli/main.ts b/src/cli/main.ts index 53865c834..6e7a2efd7 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -2,10 +2,20 @@ 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 { + replaceFileComments, + ReplaceFileCommentsDependencies, +} from "../comments/replaceFileComments"; import { convertEditorConfig, ConvertEditorConfigDependencies, @@ -50,7 +60,20 @@ import { convertRules, ConvertRulesDependencies } from "../rules/convertRules"; import { mergers } from "../rules/mergers"; import { rulesConverters } from "../rules/rulesConverters"; import { runCli, RunCliDependencies } from "./runCli"; -import { convertComments, ConvertCommentsResultsDependencies } from "../rules/convertComments"; + +const replaceFileCommentsDependencies: ReplaceFileCommentsDependencies = { + converters: rulesConverters, +}; + +const convertFileCommentsDependencies: ConvertFileCommentsDependencies = { + fileSystem: fsFileSystem, + replaceFileComments: bind(replaceFileComments, replaceFileCommentsDependencies), +}; + +const convertCommentsDependencies: ConvertCommentsDependencies = { + convertFileComments: bind(convertFileComments, convertFileCommentsDependencies), + globAsync, +}; const convertRulesDependencies: ConvertRulesDependencies = { converters: rulesConverters, @@ -100,10 +123,6 @@ const simplifyPackageRulesDependencies: SimplifyPackageRulesDependencies = { retrieveExtendsValues: bind(retrieveExtendsValues, retrieveExtendsValuesDependencies), }; -const convertCommentsResultsDependencies: ConvertCommentsResultsDependencies = { - fileSystem: fsFileSystem, -}; - const writeConversionResultsDependencies: WriteConversionResultsDependencies = { fileSystem: fsFileSystem, }; @@ -122,6 +141,7 @@ const convertEditorConfigDependencies: ConvertEditorConfigDependencies = { }; const convertConfigDependencies: ConvertConfigDependencies = { + convertComments: bind(convertComments, convertCommentsDependencies), convertRules: bind(convertRules, convertRulesDependencies), findOriginalConfigurations: bind( findOriginalConfigurations, @@ -129,7 +149,6 @@ const convertConfigDependencies: ConvertConfigDependencies = { ), reportConversionResults: bind(reportConversionResults, reportConversionResultsDependencies), simplifyPackageRules: bind(simplifyPackageRules, simplifyPackageRulesDependencies), - convertComments: bind(convertComments, convertCommentsResultsDependencies), writeConversionResults: bind(writeConversionResults, writeConversionResultsDependencies), }; diff --git a/src/cli/runCli.ts b/src/cli/runCli.ts index 524eac356..d4c079daa 100644 --- a/src/cli/runCli.ts +++ b/src/cli/runCli.ts @@ -19,13 +19,13 @@ export const runCli = async ( ): Promise => { const command = new Command() .usage("[options] --language [language]") + .option("--comments [files]", "convert tslint:disable comments in matching files") .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") .option("--package [package]", "package configuration file to convert using") .option("--tslint [tslint]", "tslint configuration file to convert using") .option("--typescript [typescript]", "typescript configuration file to convert using") - .option("--editor [editor]", "editor configuration file to convert") - .option("-C --convertComments", "convert all tslint:disable comments into eslint-disable") .option("-V --version", "output the package version"); const parsedArgv = { diff --git a/src/comments/convertComments.ts b/src/comments/convertComments.ts new file mode 100644 index 000000000..d498f39cc --- /dev/null +++ b/src/comments/convertComments.ts @@ -0,0 +1,50 @@ +import { GlobAsync } from "../adapters/globAsync"; +import { SansDependencies } from "../binding"; +import { ResultStatus, ResultWithStatus } from "../types"; +import { separateErrors, uniqueFromSources, isError } from "../utils"; +import { convertFileComments } from "./convertFileComments"; + +export type ConvertCommentsDependencies = { + convertFileComments: SansDependencies; + globAsync: GlobAsync; +}; + +export const convertComments = async ( + dependencies: ConvertCommentsDependencies, + filePathGlobs: string | string[] | undefined, +): Promise => { + if (filePathGlobs === undefined) { + return { + status: ResultStatus.Succeeded, + }; + } + + const [fileGlobErrors, globbedFilePaths] = separateErrors( + await Promise.all(uniqueFromSources(filePathGlobs).map(dependencies.globAsync)), + ); + if (fileGlobErrors.length !== 0) { + return { + errors: fileGlobErrors, + status: ResultStatus.Failed, + }; + } + + const ruleConversionCache = new Map(); + const fileFailures = ( + await Promise.all( + uniqueFromSources(...globbedFilePaths).map(async filePath => + dependencies.convertFileComments(filePath, ruleConversionCache), + ), + ) + ).filter(isError); + if (fileFailures.length !== 0) { + return { + errors: fileFailures, + status: ResultStatus.Failed, + }; + } + + return { + status: ResultStatus.Succeeded, + }; +}; diff --git a/src/comments/convertFileComments.ts b/src/comments/convertFileComments.ts new file mode 100644 index 000000000..12e5866e2 --- /dev/null +++ b/src/comments/convertFileComments.ts @@ -0,0 +1,25 @@ +import { FileSystem } from "../adapters/fileSystem"; +import { SansDependencies } from "../binding"; +import { parseFileComments } from "./parseFileComments"; +import { replaceFileComments } from "./replaceFileComments"; + +export type ConvertFileCommentsDependencies = { + fileSystem: Pick; + replaceFileComments: SansDependencies; +}; + +export const convertFileComments = async ( + dependencies: ConvertFileCommentsDependencies, + filePath: string, + ruleConversionCache: Map, +) => { + const content = await dependencies.fileSystem.readFile(filePath); + if (content instanceof Error) { + return content; + } + + const comments = parseFileComments(filePath, content); + const newFileContent = dependencies.replaceFileComments(content, comments, ruleConversionCache); + + return await dependencies.fileSystem.writeFile(filePath, newFileContent); +}; diff --git a/src/comments/parseFileComments.ts b/src/comments/parseFileComments.ts new file mode 100644 index 000000000..35b00ab81 --- /dev/null +++ b/src/comments/parseFileComments.ts @@ -0,0 +1,66 @@ +import * as utils from "tsutils"; +import * as ts from "typescript"; + +export type FileComment = { + commentKind: ts.CommentKind; + directive: TSLintDirective; + end: number; + pos: number; + ruleNames: string[]; +}; + +export type TSLintDirective = + | "tslint:disable" + | "tslint:disable-next-line" + | "tslint:enable" + | "tslint:enable-next-line"; + +export const parseFileComments = (filePath: string, content: string) => { + // See https://github.com/Microsoft/TypeScript/issues/21049 + const sourceFile = ts.createSourceFile( + filePath, + content, + ts.ScriptTarget.Latest, + /*setParentNodes */ true, + ); + const directives: FileComment[] = []; + + utils.forEachComment(sourceFile, (fullText, comment) => { + const parsedComment = parseFileComment(fullText, comment); + if (parsedComment !== undefined) { + directives.push(parsedComment); + } + }); + + return directives; +}; + +/** + * @see https://github.com/palantir/tslint/blob/master/src/enableDisableRules.ts + */ +const tslintRegex = new RegExp(/tslint:(enable|disable)(?:-(line|next-line))?(:|\s|$)/g); + +const parseFileComment = (fullText: string, comment: ts.CommentRange): FileComment | undefined => { + const commentText = (comment.kind === ts.SyntaxKind.SingleLineCommentTrivia + ? fullText.substring(comment.pos + 2, comment.end) + : fullText.substring(comment.pos + 2, comment.end - 2) + ).trim(); + const match = tslintRegex.exec(commentText); + if (match === null) { + return undefined; + } + + return { + commentKind: comment.kind, + directive: match[0].replace("e:", "e") as TSLintDirective, + end: comment.end, + pos: comment.pos, + ruleNames: commentText.slice(match[0].length).split(/\s+/).map(trimColons).filter(Boolean), + }; +}; + +const trimColons = (text: string) => + text + .replace(/^(:|\s)*/, "") + .replace(/(:|\s)*$/, "") + .trim(); diff --git a/src/comments/replaceFileComments.ts b/src/comments/replaceFileComments.ts new file mode 100644 index 000000000..813ae470e --- /dev/null +++ b/src/comments/replaceFileComments.ts @@ -0,0 +1,51 @@ +import * as ts from "typescript"; + +import { RuleConverter } from "../rules/converter"; +import { FileComment } from "./parseFileComments"; +import { uniqueFromSources } from "../utils"; +import { ConversionError } from "../errors/conversionError"; + +export type ReplaceFileCommentsDependencies = { + converters: Map; +}; + +export const replaceFileComments = ( + dependencies: ReplaceFileCommentsDependencies, + content: string, + comments: FileComment[], + ruleConversionCache: Map, +) => { + const getNewRuleLists = (ruleName: string) => { + if (ruleConversionCache.has(ruleName)) { + return ruleConversionCache.get(ruleName); + } + + const converter = dependencies.converters.get(ruleName); + if (converter === undefined) { + ruleConversionCache.set(ruleName, undefined); + return undefined; + } + + const converted = converter({ ruleArguments: [] }); + return converted instanceof ConversionError + ? undefined + : converted.rules.map(conversion => conversion.ruleName).join(", "); + }; + + for (const comment of [...comments].reverse()) { + const directive = comment.directive.replace("tslint:", "eslint-"); + const ruleLists = uniqueFromSources(comment.ruleNames.map(getNewRuleLists)).filter(Boolean); + const [left, right] = + comment.commentKind === ts.SyntaxKind.SingleLineCommentTrivia + ? ["// ", ""] + : ["/* ", " */"]; + + content = [ + content.slice(0, comment.pos), + `${left}${directive} ${ruleLists.join(", ")}${right}`, + content.slice(comment.end), + ].join(""); + } + + return content; +}; diff --git a/src/conversion/convertConfig.test.ts b/src/conversion/convertConfig.test.ts index 2fd728a5e..747f5ac87 100644 --- a/src/conversion/convertConfig.test.ts +++ b/src/conversion/convertConfig.test.ts @@ -8,6 +8,7 @@ const stubSettings = { const createStubDependencies = ( overrides: Partial = {}, ): ConvertConfigDependencies => ({ + convertComments: jest.fn(), convertRules: jest.fn(), findOriginalConfigurations: jest.fn().mockResolvedValue({ data: createStubOriginalConfigurationsData(), @@ -19,7 +20,6 @@ const createStubDependencies = ( converted: new Map(), failed: [], }), - convertComments: jest.fn(), writeConversionResults: jest.fn().mockReturnValue(Promise.resolve()), ...overrides, }); @@ -69,7 +69,24 @@ describe("convertConfig", () => { }); }); - it("returns a successful result when finding the original configurations succeeds", async () => { + it("returns the failure result when converting comments fails", async () => { + // Arrange + const convertCommentsError = new Error(); + const dependencies = createStubDependencies({ + convertComments: jest.fn().mockResolvedValueOnce(convertCommentsError), + }); + + // Act + const result = await convertConfig(dependencies, stubSettings); + + // Assert + expect(result).toEqual({ + errors: [convertCommentsError], + status: ResultStatus.Failed, + }); + }); + + it("returns a successful result when all steps succeed", async () => { // Arrange const dependencies = createStubDependencies(); diff --git a/src/conversion/convertConfig.ts b/src/conversion/convertConfig.ts index eb3c706fa..a2fe7e766 100644 --- a/src/conversion/convertConfig.ts +++ b/src/conversion/convertConfig.ts @@ -1,18 +1,18 @@ import { SansDependencies } from "../binding"; +import { convertComments } from "../comments/convertComments"; import { simplifyPackageRules } from "../creation/simplification/simplifyPackageRules"; import { writeConversionResults } from "../creation/writeConversionResults"; import { findOriginalConfigurations } from "../input/findOriginalConfigurations"; import { reportConversionResults } from "../reporting/reportConversionResults"; import { convertRules } from "../rules/convertRules"; import { ResultStatus, ResultWithStatus, TSLintToESLintSettings } from "../types"; -import { convertComments } from "../rules/convertComments"; export type ConvertConfigDependencies = { + convertComments: SansDependencies; convertRules: SansDependencies; findOriginalConfigurations: SansDependencies; reportConversionResults: SansDependencies; simplifyPackageRules: SansDependencies; - convertComments: SansDependencies; writeConversionResults: SansDependencies; }; @@ -58,19 +58,11 @@ export const convertConfig = async ( }; } - // 4.B Convert comments. - const fileCommentsError = await dependencies.convertComments(); - if (fileCommentsError !== undefined) { - return { - errors: [fileCommentsError], - status: ResultStatus.Failed, - }; - } + // 5. Files to transform comments in have source text rewritten using the same rule conversion logic + const commentsResult = await dependencies.convertComments(settings.comments); - // 5. A summary of the results is printed to the user's console + // 6. A summary of the results is printed to the user's console dependencies.reportConversionResults(simplifiedConfiguration); - return { - status: ResultStatus.Succeeded, - }; + return commentsResult; }; diff --git a/src/rules/convertComments.ts b/src/rules/convertComments.ts deleted file mode 100644 index fbd4b7744..000000000 --- a/src/rules/convertComments.ts +++ /dev/null @@ -1,212 +0,0 @@ -import { FileSystem } from "../adapters/fileSystem"; -import { isError } from "../utils"; -import * as utils from "tsutils"; -import ts from "typescript"; -import { rulesConverters } from "./rulesConverters"; -import { formatRawTslintRule } from "./formatRawTslintRule"; -import { ConversionError } from "../errors/conversionError"; - -// Inspirated by: -// - https://github.com/Microsoft/TypeScript/issues/21049 -// - https://github.com/palantir/tslint/blob/master/src/enableDisableRules.ts -export type ConvertCommentsResultsDependencies = { - fileSystem: Pick; -}; - -const tslintRegex = new RegExp(/\s*tslint:(enable|disable)(?:-(line|next-line))?(:|\s|$)/g); - -export const convertComments = async (dependencies: ConvertCommentsResultsDependencies) => { - // TODO: Remove console logs - const filenames = await dependencies.fileSystem.readDir("./", { withFileTypes: true }); - if (!isError(filenames)) { - const filteredFilenames: string[] = filenames - .filter((fileEnt) => fileEnt.isFile()) - .map((fileEnt) => fileEnt.name); - // TODO: Remove console logs - for (const filename of filteredFilenames) { - // todo: cli should pass glob/wildcard - if (!filename.includes("index")) continue; - - const fileContent: string | Error = await dependencies.fileSystem.readFile(filename); - if (!isError(fileContent)) { - const writeFileRes = await replaceComments(dependencies, filename, fileContent); - if (isError(writeFileRes)) { - return Error("Failed to convert file comments"); - } - } - } - return undefined; - } - return Error("Failed to convert file comments"); -}; - -type CommentKind = ts.SyntaxKind.MultiLineCommentTrivia | ts.SyntaxKind.SingleLineCommentTrivia; - -type Modifier = "line" | "next-line" | undefined; // todo: split "line" into "block-start" and "block-end"? -function parseComment( - commentText: string, -): { rulesList: string[] | "all"; isEnabled: boolean; modifier: Modifier } | undefined { - const match = tslintRegex.exec(commentText); - if (match === null) { - return undefined; - } - - // remove everything matched by the previous regex to get only the specified rules - // split at whitespaces - // filter empty items coming from whitespaces at start, at end or empty list - let rulesList: string[] | "all" = splitOnSpaces(commentText.substr(match[0].length)); - if (rulesList.length === 0 && match[3] === ":") { - // nothing to do here: an explicit separator was specified but no rules to switch - return undefined; - } - if (rulesList.length === 0 || rulesList.includes("all")) { - // if list is empty we default to all enabled rules - // if `all` is specified we ignore the other rules and take all enabled rules - rulesList = "all"; - } - - return { rulesList, isEnabled: match[1] === "enable", modifier: match[2] as Modifier }; -} - -function splitOnSpaces(str: string): string[] { - return str.split(/\s+/).filter((s) => s !== ""); -} - -type IReplacement = { - start: number; - end: number; - replacementText: string; -}; - -const replaceComments = async ( - _dependencies: ConvertCommentsResultsDependencies, - fileName: string, - fileContent: string, -) => { - const sourceFile: ts.SourceFile = ts.createSourceFile( - fileName, - fileContent, - ts.ScriptTarget.ES2015, - /*setParentNodes */ true, - ); - // I assume that the token enum is `SingleLineCommentTrivia` to catch /* and // tokens. - // If not, it is `MultiLineCommentTrivia` as this method does check if it is a comment... - // or that's what I think it does. - utils.forEachComment(sourceFile, (fullText, comment) => { - const replacements: IReplacement[] = []; - const commentText = (comment.kind === ts.SyntaxKind.SingleLineCommentTrivia - ? fullText.substring(comment.pos + 2, comment.end) - : fullText.substring(comment.pos + 2, comment.end - 2) - ).trim(); - - const parsed = parseComment(commentText); - if (parsed !== undefined) { - const { rulesList, modifier } = parsed; - const switchRange = getSwitchRange(modifier, comment, sourceFile); - console.log({ modifier, comment }); - if (switchRange !== undefined) { - // Extra log to check what is going on - const rulesToSwitch = - rulesList === "all" - ? Array.from(rulesConverters.keys()) - : rulesList.filter((ruleKey) => rulesConverters.has(ruleKey)); - console.log({ rulesToSwitch }); - for (const ruleToSwitch of rulesToSwitch) { - const transformedRules = switchRule(ruleToSwitch); - if (transformedRules) { - replacements.push({ - start: comment.pos, - end: comment.end, - replacementText: createReplacementText( - comment.kind, - modifier, - transformedRules, - ), - }); - } - } - } - } - // Reverse the replacement list - replacements.reverse(); - - const newText = getNewText(fileContent, replacements); - // Check the output before writing to file. - // Write the file with the changes. - // At the moment, - _dependencies.fileSystem.writeFileSync(fileName, newText); - }); - return true; -}; - -function createReplacementText( - commentKind: CommentKind, - _modifier: Modifier, - transformedRules: string[], -) { - const directive = "eslint-disable-next-line"; // todo: map from modifier - const [left, right] = - commentKind === ts.SyntaxKind.SingleLineCommentTrivia ? ["// ", ""] : ["/* ", " */"]; - - // todo: no transformed rules? - return [left, directive, " ", transformedRules.join(" "), right].join(""); -} - -function getNewText(sourceText: string, replacementsInReverse: IReplacement[]) { - console.log({ replacementsInReverse }); - for (const { start, end, replacementText } of replacementsInReverse) { - sourceText = sourceText.slice(0, start) + replacementText + sourceText.slice(end); - } - - return sourceText; -} - -function switchRule(ruleName: string): string[] | null { - const tslintRuleConverter = rulesConverters.get(ruleName); - if (tslintRuleConverter) { - const tslintRule = formatRawTslintRule(ruleName, { ruleName }); - const conversion = tslintRuleConverter(tslintRule); - if (!(conversion instanceof ConversionError)) { - const eslintRules = conversion.rules.map((r) => r.ruleName); - return eslintRules; - } - } - return null; -} - -function getSwitchRange( - modifier: Modifier, - range: ts.TextRange, - sourceFile: ts.SourceFile, -): ts.TextRange | undefined { - const lineStarts = sourceFile.getLineStarts(); - - switch (modifier) { - case "line": - return { - // start at the beginning of the line where comment starts - pos: getStartOfLinePosition(range.pos), - // end at the beginning of the line following the comment - end: getStartOfLinePosition(range.end, 1), - }; - case "next-line": - // start at the beginning of the line following the comment - const pos = getStartOfLinePosition(range.end, 1); - if (pos === -1) { - // no need to switch anything, there is no next line - return undefined; - } - // end at the beginning of the line following the next line - return { pos, end: getStartOfLinePosition(range.end, 2) }; - default: - // switch rule for the rest of the file - // start at the current position, but skip end position - return { pos: range.pos, end: -1 }; - } - - /** Returns -1 for last line. */ - function getStartOfLinePosition(position: number, lineOffset = 0): number { - const line = ts.getLineAndCharacterOfPosition(sourceFile, position).line + lineOffset; - return line >= lineStarts.length ? -1 : lineStarts[line]; - } -} diff --git a/src/types.ts b/src/types.ts index cf081cf58..dec7d9285 100644 --- a/src/types.ts +++ b/src/types.ts @@ -4,6 +4,11 @@ export type TSLintToESLintSettings = { */ config: string; + /** + * File globs to convert `tslint:disable` comments within to `eslint-disable`. + */ + comments?: string | string[]; + /** * Original ESLint configuration file path, such as `.eslintrc.js`. */ @@ -28,11 +33,6 @@ export type TSLintToESLintSettings = { * Original Editor configuration file path, such as `.vscode/settings.json`. */ editor?: string; - - /** - * Opt-in flag to convert `tslint:disable` comments to `eslint-disable`. - */ - convertComments?: boolean; }; export type TSLintToESLintResult = ResultWithStatus; diff --git a/src/utils.ts b/src/utils.ts index c69d6f9df..40520cc84 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -2,8 +2,19 @@ export const isDefined = (item: Item | undefined): item is Item => !!item; export const isError = (item: Item | Error): item is Error => item instanceof Error; -export type RemoveErrors = { - [P in keyof Items]: Exclude; +export const separateErrors = (mixed: (Error | Item)[]): [Error[], Item[]] => { + const errors: Error[] = []; + const items: Item[] = []; + + for (const item of mixed) { + if (item instanceof Error) { + errors.push(item); + } else { + items.push(item); + } + } + + return [errors, items]; }; export type PromiseValue = T extends Promise ? R : never; From 63a3876f681d991cee882bafeb62913a9823408f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 03:59:17 -0400 Subject: [PATCH 10/17] Revert .prettierrc testing change --- .prettierrc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.prettierrc b/.prettierrc index 4d63829ac..11a639ca7 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,6 +1,5 @@ { - "arrowParens": "avoid", "printWidth": 100, "tabWidth": 4, "trailingComma": "all" -} +} \ No newline at end of file From 045631405601301d8201da11bf6d9ffe348b7d84 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 04:08:01 -0400 Subject: [PATCH 11/17] Love that nullish operator --- src/adapters/globAsync.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/adapters/globAsync.ts b/src/adapters/globAsync.ts index fb53eb1f2..d9d91bbb4 100644 --- a/src/adapters/globAsync.ts +++ b/src/adapters/globAsync.ts @@ -1,9 +1,9 @@ import glob from "glob"; export const globAsync = async (pattern: string) => { - return new Promise(resolve => { + return new Promise((resolve) => { glob(pattern, (error, matches) => { - resolve(error || matches); + resolve(error ?? matches); }); }); }; From e61a4341630d7b8e8dcd0fe418eb82a06c2cdbfd Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 11:08:51 -0400 Subject: [PATCH 12/17] Comments WIP --- src/adapters/fileSystem.stub.ts | 6 -- src/comments/convertComments.test.ts | 72 ++++++++++++++++++++++++ src/comments/convertComments.ts | 7 ++- src/comments/convertFileComments.test.ts | 46 +++++++++++++++ src/comments/parseFileComments.test.ts | 72 ++++++++++++++++++++++++ src/comments/parseFileComments.ts | 2 +- src/utils.test.ts | 4 ++ 7 files changed, 199 insertions(+), 10 deletions(-) create mode 100644 src/comments/convertComments.test.ts create mode 100644 src/comments/convertFileComments.test.ts create mode 100644 src/comments/parseFileComments.test.ts diff --git a/src/adapters/fileSystem.stub.ts b/src/adapters/fileSystem.stub.ts index eb1fdcc00..b962f53e6 100644 --- a/src/adapters/fileSystem.stub.ts +++ b/src/adapters/fileSystem.stub.ts @@ -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))), -}); diff --git a/src/comments/convertComments.test.ts b/src/comments/convertComments.test.ts new file mode 100644 index 000000000..2054b6909 --- /dev/null +++ b/src/comments/convertComments.test.ts @@ -0,0 +1,72 @@ +import { ResultStatus } from "../types"; +import { convertComments, ConvertCommentsDependencies } from "./convertComments"; + +const createStubDependencies = ( + overrides: Partial = {}, +): ConvertCommentsDependencies => ({ + convertFileComments: jest.fn(), + globAsync: jest.fn().mockResolvedValue(["src/index.ts"]), + ...overrides, +}); + +describe("convertComments", () => { + it("returns a successful result when there are no file path globs", async () => { + // Arrange + const dependencies = createStubDependencies(); + + // Act + const result = await convertComments(dependencies, []); + + // Assert + expect(result).toEqual({ + status: ResultStatus.Succeeded, + }); + }); + + 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({ + status: ResultStatus.Succeeded, + }); + }); +}); diff --git a/src/comments/convertComments.ts b/src/comments/convertComments.ts index d498f39cc..e0c9ed4d4 100644 --- a/src/comments/convertComments.ts +++ b/src/comments/convertComments.ts @@ -13,14 +13,15 @@ export const convertComments = async ( dependencies: ConvertCommentsDependencies, filePathGlobs: string | string[] | undefined, ): Promise => { - if (filePathGlobs === undefined) { + const uniqueFilePathGlobs = uniqueFromSources(filePathGlobs); + if (uniqueFilePathGlobs.length === 0) { return { status: ResultStatus.Succeeded, }; } const [fileGlobErrors, globbedFilePaths] = separateErrors( - await Promise.all(uniqueFromSources(filePathGlobs).map(dependencies.globAsync)), + await Promise.all(uniqueFilePathGlobs.map(dependencies.globAsync)), ); if (fileGlobErrors.length !== 0) { return { @@ -32,7 +33,7 @@ export const convertComments = async ( const ruleConversionCache = new Map(); const fileFailures = ( await Promise.all( - uniqueFromSources(...globbedFilePaths).map(async filePath => + uniqueFromSources(...globbedFilePaths).map(async (filePath) => dependencies.convertFileComments(filePath, ruleConversionCache), ), ) diff --git a/src/comments/convertFileComments.test.ts b/src/comments/convertFileComments.test.ts new file mode 100644 index 000000000..a17aa8e08 --- /dev/null +++ b/src/comments/convertFileComments.test.ts @@ -0,0 +1,46 @@ +import { createStubFileSystem } from "../adapters/fileSystem.stub"; +import { convertFileComments, ConvertFileCommentsDependencies } from "./convertFileComments"; + +const createStubDependencies = ( + overrides: Partial = {}, +): ConvertFileCommentsDependencies => ({ + fileSystem: createStubFileSystem(), + replaceFileComments: jest.fn(), + ...overrides, +}); + +describe("convertFileComments", () => { + it("returns the failure result when reading the file fails", async () => { + // Arrange + const readFileError = new Error(); + const dependencies = createStubDependencies({ + fileSystem: { + readFile: jest.fn().mockResolvedValueOnce(readFileError), + writeFile: jest.fn(), + }, + }); + + // Act + const result = await convertFileComments(dependencies, "src/index.ts", new Map()); + + // Assert + expect(result).toBe(readFileError); + }); + + it("returns the result from writing new file contents when reading the file succeeds", async () => { + // Arrange + const writeFileError = new Error(); + const dependencies = createStubDependencies({ + fileSystem: { + readFile: jest.fn().mockResolvedValue(""), + writeFile: jest.fn().mockResolvedValueOnce(writeFileError), + }, + }); + + // Act + const result = await convertFileComments(dependencies, "src/index.ts", new Map()); + + // Assert + expect(result).toBe(writeFileError); + }); +}); diff --git a/src/comments/parseFileComments.test.ts b/src/comments/parseFileComments.test.ts new file mode 100644 index 000000000..d4016c4a4 --- /dev/null +++ b/src/comments/parseFileComments.test.ts @@ -0,0 +1,72 @@ +import { parseFileComments } from "./parseFileComments"; + +const stubFilePath = "src/index.ts"; + +describe("parseFileComments", () => { + it("ignores a comment when it doesn't contain a directive", () => { + // Arrange + const content = ` + // Hello, world! + `; + + // Act + const comments = parseFileComments(stubFilePath, content); + + // Assert + expect(comments).toEqual([]); + }); + + it("parses TSLint directives to their matching ESLint directives", () => { + // Arrange + const content = ` + // tslint:disable + export const a = true; + + // tslint:disable-next-line + export const b = true; + + // tslint:enable + export const c = true; + + // tslint:enable-next-line + export const d = true; + `; + + // Act + const comments = parseFileComments(stubFilePath, content); + + // Assert + expect(comments).toMatchInlineSnapshot(` + Array [ + Object { + "commentKind": 2, + "directive": "tslint:disable", + "end": 30, + "pos": 13, + "ruleNames": Array [], + }, + Object { + "commentKind": 2, + "directive": "tslint:disable-next-line", + "end": 118, + "pos": 91, + "ruleNames": Array [], + }, + Object { + "commentKind": 2, + "directive": "tslint:enable", + "end": 195, + "pos": 179, + "ruleNames": Array [], + }, + Object { + "commentKind": 2, + "directive": "tslint:enable-next-line", + "end": 282, + "pos": 256, + "ruleNames": Array [], + }, + ] + `); + }); +}); diff --git a/src/comments/parseFileComments.ts b/src/comments/parseFileComments.ts index 35b00ab81..5ed6a82a0 100644 --- a/src/comments/parseFileComments.ts +++ b/src/comments/parseFileComments.ts @@ -45,7 +45,7 @@ const parseFileComment = (fullText: string, comment: ts.CommentRange): FileComme ? fullText.substring(comment.pos + 2, comment.end) : fullText.substring(comment.pos + 2, comment.end - 2) ).trim(); - const match = tslintRegex.exec(commentText); + const match = commentText.match(tslintRegex); if (match === null) { return undefined; } diff --git a/src/utils.test.ts b/src/utils.test.ts index f0180a1ff..c89ed4574 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -48,6 +48,10 @@ describe("isError", () => { }); }); +describe("separateErrors", () => { + // ... +}); + describe("uniqueFromSources", () => { it("returns unique items when multiple are given", () => { // Arange From 52484478ce1bdb8c091de00b83eee8e133d353aa Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 13:18:45 -0400 Subject: [PATCH 13/17] Seems to be all working and tested now --- src/cli/main.ts | 10 +- src/comments/convertFileComments.test.ts | 179 ++++++++++++++++++++--- src/comments/convertFileComments.ts | 23 ++- src/comments/parseFileComments.test.ts | 72 --------- src/comments/parseFileComments.ts | 10 +- src/comments/replaceFileComments.ts | 26 ++-- src/conversion/convertConfig.test.ts | 23 +-- src/rules/converter.stubs.ts | 11 ++ 8 files changed, 217 insertions(+), 137 deletions(-) delete mode 100644 src/comments/parseFileComments.test.ts create mode 100644 src/rules/converter.stubs.ts diff --git a/src/cli/main.ts b/src/cli/main.ts index 6e7a2efd7..1626f1037 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -12,10 +12,6 @@ import { convertFileComments, } from "../comments/convertFileComments"; import { convertConfig, ConvertConfigDependencies } from "../conversion/convertConfig"; -import { - replaceFileComments, - ReplaceFileCommentsDependencies, -} from "../comments/replaceFileComments"; import { convertEditorConfig, ConvertEditorConfigDependencies, @@ -61,13 +57,9 @@ import { mergers } from "../rules/mergers"; import { rulesConverters } from "../rules/rulesConverters"; import { runCli, RunCliDependencies } from "./runCli"; -const replaceFileCommentsDependencies: ReplaceFileCommentsDependencies = { - converters: rulesConverters, -}; - const convertFileCommentsDependencies: ConvertFileCommentsDependencies = { + converters: rulesConverters, fileSystem: fsFileSystem, - replaceFileComments: bind(replaceFileComments, replaceFileCommentsDependencies), }; const convertCommentsDependencies: ConvertCommentsDependencies = { diff --git a/src/comments/convertFileComments.test.ts b/src/comments/convertFileComments.test.ts index a17aa8e08..244eb87ff 100644 --- a/src/comments/convertFileComments.test.ts +++ b/src/comments/convertFileComments.test.ts @@ -1,46 +1,183 @@ import { createStubFileSystem } from "../adapters/fileSystem.stub"; +import { ConversionError } from "../errors/conversionError"; +import { createStubConverter } from "../rules/converter.stubs"; import { convertFileComments, ConvertFileCommentsDependencies } from "./convertFileComments"; const createStubDependencies = ( - overrides: Partial = {}, + readFileResult: string | Error, ): ConvertFileCommentsDependencies => ({ - fileSystem: createStubFileSystem(), - replaceFileComments: jest.fn(), - ...overrides, + converters: new Map([ + ["ts-a", createStubConverter(["es-a"])], + ["ts-b", createStubConverter(["es-b1", "es-b2"])], + ["ts-error", createStubConverter(ConversionError.forMerger("unknown"))], + ]), + fileSystem: { + ...createStubFileSystem(), + readFile: jest.fn().mockResolvedValueOnce(readFileResult), + }, }); +const stubFileName = "src/index.ts"; + describe("convertFileComments", () => { it("returns the failure result when reading the file fails", async () => { // Arrange const readFileError = new Error(); - const dependencies = createStubDependencies({ - fileSystem: { - readFile: jest.fn().mockResolvedValueOnce(readFileError), - writeFile: jest.fn(), - }, - }); + const dependencies = createStubDependencies(readFileError); // Act - const result = await convertFileComments(dependencies, "src/index.ts", new Map()); + const result = await convertFileComments(dependencies, stubFileName, new Map()); // Assert expect(result).toBe(readFileError); }); - it("returns the result from writing new file contents when reading the file succeeds", async () => { + it("doesn't overwrite a file when there are no matching comment directives", async () => { + // Arrange + const dependencies = createStubDependencies(` +// Hello, world! +`); + + // Act + await convertFileComments(dependencies, stubFileName, new Map()); + + // Assert + expect(dependencies.fileSystem.writeFile).not.toHaveBeenCalled(); + }); + + it("parses TSLint directives to their matching ESLint directives", async () => { + // Arrange + const dependencies = createStubDependencies(` +// tslint:disable +export const a = true; + +// tslint:disable-next-line +export const b = true; + +// tslint:enable +export const c = true; + +/* tslint:disable */ +export const d = true; + +/* tslint:disable-next-line */ +export const e = true; + +/* tslint:enable */ +export const f = true; +`); + + // Act + await convertFileComments(dependencies, stubFileName, new Map()); + + // Assert + expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( + stubFileName, + ` +// eslint-disable +export const a = true; + +// eslint-disable-line +export const b = true; + +// eslint-enable +export const c = true; + +/* eslint-disable */ +export const d = true; + +/* eslint-disable-line */ +export const e = true; + +/* eslint-enable */ +export const f = true; +`, + ); + }); + + it("parses rule names when they exist", async () => { + // Arrange + const dependencies = createStubDependencies(` +// tslint:disable:ts-a +export const a = true; + +// tslint:disable-next-line: ts-a ts-b +export const b = true; +`); + + // Act + await convertFileComments(dependencies, stubFileName, new Map()); + + // Assert + expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( + stubFileName, + ` +// eslint-disable es-a +export const a = true; + +// eslint-disable-line es-a, es-b1, es-b2 +export const b = true; +`, + ); + }); + + it("re-uses a rule conversion from 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"]])); + + // Assert + expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( + stubFileName, + ` +// eslint-disable es-cached +export const a = true; +`, + ); + }); + + it("ignores comment text when there is no matching converter", async () => { + // Arrange + const dependencies = createStubDependencies(` +// tslint:disable:ts-z +export const a = true; +`); + + // Act + await convertFileComments(dependencies, stubFileName, new Map()); + + // Assert + expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( + stubFileName, + ` +// eslint-disable +export const a = true; +`, + ); + }); + + it("ignores comment text when its matching converter results in an error", async () => { // Arrange - const writeFileError = new Error(); - const dependencies = createStubDependencies({ - fileSystem: { - readFile: jest.fn().mockResolvedValue(""), - writeFile: jest.fn().mockResolvedValueOnce(writeFileError), - }, - }); + const dependencies = createStubDependencies(` +// tslint:disable:ts-error +export const a = true; +`); // Act - const result = await convertFileComments(dependencies, "src/index.ts", new Map()); + await convertFileComments(dependencies, stubFileName, new Map()); // Assert - expect(result).toBe(writeFileError); + expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( + stubFileName, + ` +// eslint-disable +export const a = true; +`, + ); }); }); diff --git a/src/comments/convertFileComments.ts b/src/comments/convertFileComments.ts index 12e5866e2..de129ebd5 100644 --- a/src/comments/convertFileComments.ts +++ b/src/comments/convertFileComments.ts @@ -1,11 +1,11 @@ import { FileSystem } from "../adapters/fileSystem"; -import { SansDependencies } from "../binding"; import { parseFileComments } from "./parseFileComments"; import { replaceFileComments } from "./replaceFileComments"; +import { RuleConverter } from "../rules/converter"; export type ConvertFileCommentsDependencies = { + converters: Map; fileSystem: Pick; - replaceFileComments: SansDependencies; }; export const convertFileComments = async ( @@ -13,13 +13,20 @@ export const convertFileComments = async ( filePath: string, ruleConversionCache: Map, ) => { - const content = await dependencies.fileSystem.readFile(filePath); - if (content instanceof Error) { - return content; + const fileContent = await dependencies.fileSystem.readFile(filePath); + if (fileContent instanceof Error) { + return fileContent; } - const comments = parseFileComments(filePath, content); - const newFileContent = dependencies.replaceFileComments(content, comments, ruleConversionCache); + const comments = parseFileComments(filePath, fileContent); + const newFileContent = replaceFileComments( + fileContent, + comments, + dependencies.converters, + ruleConversionCache, + ); - return await dependencies.fileSystem.writeFile(filePath, newFileContent); + return fileContent === newFileContent + ? undefined + : await dependencies.fileSystem.writeFile(filePath, newFileContent); }; diff --git a/src/comments/parseFileComments.test.ts b/src/comments/parseFileComments.test.ts deleted file mode 100644 index d4016c4a4..000000000 --- a/src/comments/parseFileComments.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { parseFileComments } from "./parseFileComments"; - -const stubFilePath = "src/index.ts"; - -describe("parseFileComments", () => { - it("ignores a comment when it doesn't contain a directive", () => { - // Arrange - const content = ` - // Hello, world! - `; - - // Act - const comments = parseFileComments(stubFilePath, content); - - // Assert - expect(comments).toEqual([]); - }); - - it("parses TSLint directives to their matching ESLint directives", () => { - // Arrange - const content = ` - // tslint:disable - export const a = true; - - // tslint:disable-next-line - export const b = true; - - // tslint:enable - export const c = true; - - // tslint:enable-next-line - export const d = true; - `; - - // Act - const comments = parseFileComments(stubFilePath, content); - - // Assert - expect(comments).toMatchInlineSnapshot(` - Array [ - Object { - "commentKind": 2, - "directive": "tslint:disable", - "end": 30, - "pos": 13, - "ruleNames": Array [], - }, - Object { - "commentKind": 2, - "directive": "tslint:disable-next-line", - "end": 118, - "pos": 91, - "ruleNames": Array [], - }, - Object { - "commentKind": 2, - "directive": "tslint:enable", - "end": 195, - "pos": 179, - "ruleNames": Array [], - }, - Object { - "commentKind": 2, - "directive": "tslint:enable-next-line", - "end": 282, - "pos": 256, - "ruleNames": Array [], - }, - ] - `); - }); -}); diff --git a/src/comments/parseFileComments.ts b/src/comments/parseFileComments.ts index 5ed6a82a0..a93490d6a 100644 --- a/src/comments/parseFileComments.ts +++ b/src/comments/parseFileComments.ts @@ -9,14 +9,12 @@ export type FileComment = { ruleNames: string[]; }; -export type TSLintDirective = - | "tslint:disable" - | "tslint:disable-next-line" - | "tslint:enable" - | "tslint:enable-next-line"; +export type TSLintDirective = "tslint:disable" | "tslint:disable-next-line" | "tslint:enable"; +/** + * @see https://github.com/Microsoft/TypeScript/issues/21049 + */ export const parseFileComments = (filePath: string, content: string) => { - // See https://github.com/Microsoft/TypeScript/issues/21049 const sourceFile = ts.createSourceFile( filePath, content, diff --git a/src/comments/replaceFileComments.ts b/src/comments/replaceFileComments.ts index 813ae470e..81835b16b 100644 --- a/src/comments/replaceFileComments.ts +++ b/src/comments/replaceFileComments.ts @@ -1,18 +1,14 @@ import * as ts from "typescript"; import { RuleConverter } from "../rules/converter"; -import { FileComment } from "./parseFileComments"; import { uniqueFromSources } from "../utils"; import { ConversionError } from "../errors/conversionError"; - -export type ReplaceFileCommentsDependencies = { - converters: Map; -}; +import { FileComment } from "./parseFileComments"; export const replaceFileComments = ( - dependencies: ReplaceFileCommentsDependencies, content: string, comments: FileComment[], + converters: Map, ruleConversionCache: Map, ) => { const getNewRuleLists = (ruleName: string) => { @@ -20,7 +16,7 @@ export const replaceFileComments = ( return ruleConversionCache.get(ruleName); } - const converter = dependencies.converters.get(ruleName); + const converter = converters.get(ruleName); if (converter === undefined) { ruleConversionCache.set(ruleName, undefined); return undefined; @@ -29,11 +25,13 @@ export const replaceFileComments = ( const converted = converter({ ruleArguments: [] }); return converted instanceof ConversionError ? undefined - : converted.rules.map(conversion => conversion.ruleName).join(", "); + : converted.rules.map((conversion) => conversion.ruleName).join(", "); }; for (const comment of [...comments].reverse()) { - const directive = comment.directive.replace("tslint:", "eslint-"); + const directive = comment.directive + .replace("tslint:", "eslint-") + .replace("next-line", "line"); const ruleLists = uniqueFromSources(comment.ruleNames.map(getNewRuleLists)).filter(Boolean); const [left, right] = comment.commentKind === ts.SyntaxKind.SingleLineCommentTrivia @@ -42,9 +40,15 @@ export const replaceFileComments = ( content = [ content.slice(0, comment.pos), - `${left}${directive} ${ruleLists.join(", ")}${right}`, + left, + directive, + ruleLists.length !== 0 && " ", + ruleLists.join(", "), + right, content.slice(comment.end), - ].join(""); + ] + .filter(Boolean) + .join(""); } return content; diff --git a/src/conversion/convertConfig.test.ts b/src/conversion/convertConfig.test.ts index 747f5ac87..f55f8b540 100644 --- a/src/conversion/convertConfig.test.ts +++ b/src/conversion/convertConfig.test.ts @@ -71,31 +71,34 @@ describe("convertConfig", () => { it("returns the failure result when converting comments fails", async () => { // Arrange - const convertCommentsError = new Error(); + const convertCommentsResult = { + errors: [new Error()], + status: ResultStatus.Failed, + }; const dependencies = createStubDependencies({ - convertComments: jest.fn().mockResolvedValueOnce(convertCommentsError), + convertComments: jest.fn().mockResolvedValueOnce(convertCommentsResult), }); // Act const result = await convertConfig(dependencies, stubSettings); // Assert - expect(result).toEqual({ - errors: [convertCommentsError], - status: ResultStatus.Failed, - }); + expect(result).toEqual(convertCommentsResult); }); it("returns a successful result when all steps succeed", async () => { // Arrange - const dependencies = createStubDependencies(); + const convertCommentsResult = { + status: ResultStatus.Succeeded, + }; + const dependencies = createStubDependencies({ + convertComments: jest.fn().mockResolvedValueOnce(convertCommentsResult), + }); // Act const result = await convertConfig(dependencies, stubSettings); // Assert - expect(result).toEqual({ - status: ResultStatus.Succeeded, - }); + expect(result).toEqual(convertCommentsResult); }); }); diff --git a/src/rules/converter.stubs.ts b/src/rules/converter.stubs.ts new file mode 100644 index 000000000..587c42d20 --- /dev/null +++ b/src/rules/converter.stubs.ts @@ -0,0 +1,11 @@ +import { ConversionError } from "../errors/conversionError"; + +export const createStubConverter = (result: ConversionError | string[]) => { + return () => { + return result instanceof ConversionError + ? result + : { + rules: result.map((ruleName) => ({ ruleName })), + }; + }; +}; From 425280a323aa202866b71f193dd054fe8a816971 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 14:15:09 -0400 Subject: [PATCH 14/17] Feature add: logged directive counts as a summary --- src/cli/main.ts | 13 ++-- src/comments/convertComments.test.ts | 2 + src/comments/convertComments.ts | 14 ++-- src/conversion/convertConfig.test.ts | 1 + src/conversion/convertConfig.ts | 3 + src/input/findOriginalConfigurations.ts | 9 ++- src/reporting/dependencies.ts | 2 +- src/reporting/reportCommentResults.test.ts | 69 +++++++++++++++++++ src/reporting/reportCommentResults.ts | 42 +++++++++++ src/reporting/reportConversionResults.ts | 4 +- .../reportEditorSettingConversionResults.ts | 4 +- src/types.ts | 5 +- 12 files changed, 141 insertions(+), 27 deletions(-) create mode 100644 src/reporting/reportCommentResults.test.ts create mode 100644 src/reporting/reportCommentResults.ts diff --git a/src/cli/main.ts b/src/cli/main.ts index 1626f1037..2fd08a918 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -49,13 +49,14 @@ import { findTSLintConfiguration } from "../input/findTSLintConfiguration"; import { findTypeScriptConfiguration } from "../input/findTypeScriptConfiguration"; import { importer, ImporterDependencies } from "../input/importer"; import { mergeLintConfigurations } from "../input/mergeLintConfigurations"; -import { ReportConversionResultsDependencies } from "../reporting/dependencies"; +import { ReportDependencies } from "../reporting/dependencies"; import { reportConversionResults } from "../reporting/reportConversionResults"; import { reportEditorSettingConversionResults } from "../reporting/reportEditorSettingConversionResults"; import { convertRules, ConvertRulesDependencies } from "../rules/convertRules"; import { mergers } from "../rules/mergers"; import { rulesConverters } from "../rules/rulesConverters"; import { runCli, RunCliDependencies } from "./runCli"; +import { reportCommentResults } from "../reporting/reportCommentResults"; const convertFileCommentsDependencies: ConvertFileCommentsDependencies = { converters: rulesConverters, @@ -102,7 +103,7 @@ const findOriginalConfigurationsDependencies: FindOriginalConfigurationsDependen mergeLintConfigurations, }; -const reportConversionResultsDependencies: ReportConversionResultsDependencies = { +const reportDependencies: ReportDependencies = { logger: processLogger, }; @@ -122,10 +123,7 @@ const writeConversionResultsDependencies: WriteConversionResultsDependencies = { const convertEditorConfigDependencies: ConvertEditorConfigDependencies = { findEditorConfiguration: bind(findEditorConfiguration, findEditorConfigurationDependencies), convertEditorSettings: bind(convertEditorSettings, convertEditorSettingsDependencies), - reportConversionResults: bind( - reportEditorSettingConversionResults, - reportConversionResultsDependencies, - ), + reportConversionResults: bind(reportEditorSettingConversionResults, reportDependencies), writeConversionResults: bind( writeEditorConfigConversionResults, writeConversionResultsDependencies, @@ -139,7 +137,8 @@ const convertConfigDependencies: ConvertConfigDependencies = { findOriginalConfigurations, findOriginalConfigurationsDependencies, ), - reportConversionResults: bind(reportConversionResults, reportConversionResultsDependencies), + reportCommentResults: bind(reportCommentResults, reportDependencies), + reportConversionResults: bind(reportConversionResults, reportDependencies), simplifyPackageRules: bind(simplifyPackageRules, simplifyPackageRulesDependencies), writeConversionResults: bind(writeConversionResults, writeConversionResultsDependencies), }; diff --git a/src/comments/convertComments.test.ts b/src/comments/convertComments.test.ts index 2054b6909..b41abe307 100644 --- a/src/comments/convertComments.test.ts +++ b/src/comments/convertComments.test.ts @@ -19,6 +19,7 @@ describe("convertComments", () => { // Assert expect(result).toEqual({ + data: [], status: ResultStatus.Succeeded, }); }); @@ -66,6 +67,7 @@ describe("convertComments", () => { // Assert expect(result).toEqual({ + data: ["src/index.ts"], status: ResultStatus.Succeeded, }); }); diff --git a/src/comments/convertComments.ts b/src/comments/convertComments.ts index e0c9ed4d4..8d7f6f2bc 100644 --- a/src/comments/convertComments.ts +++ b/src/comments/convertComments.ts @@ -1,6 +1,6 @@ import { GlobAsync } from "../adapters/globAsync"; import { SansDependencies } from "../binding"; -import { ResultStatus, ResultWithStatus } from "../types"; +import { ResultStatus, ResultWithDataStatus } from "../types"; import { separateErrors, uniqueFromSources, isError } from "../utils"; import { convertFileComments } from "./convertFileComments"; @@ -12,14 +12,8 @@ export type ConvertCommentsDependencies = { export const convertComments = async ( dependencies: ConvertCommentsDependencies, filePathGlobs: string | string[] | undefined, -): Promise => { +): Promise> => { const uniqueFilePathGlobs = uniqueFromSources(filePathGlobs); - if (uniqueFilePathGlobs.length === 0) { - return { - status: ResultStatus.Succeeded, - }; - } - const [fileGlobErrors, globbedFilePaths] = separateErrors( await Promise.all(uniqueFilePathGlobs.map(dependencies.globAsync)), ); @@ -31,9 +25,10 @@ export const convertComments = async ( } const ruleConversionCache = new Map(); + const uniqueGlobbedFilePaths = uniqueFromSources(...globbedFilePaths); const fileFailures = ( await Promise.all( - uniqueFromSources(...globbedFilePaths).map(async (filePath) => + uniqueGlobbedFilePaths.map(async (filePath) => dependencies.convertFileComments(filePath, ruleConversionCache), ), ) @@ -46,6 +41,7 @@ export const convertComments = async ( } return { + data: uniqueGlobbedFilePaths, status: ResultStatus.Succeeded, }; }; diff --git a/src/conversion/convertConfig.test.ts b/src/conversion/convertConfig.test.ts index f55f8b540..60c3df660 100644 --- a/src/conversion/convertConfig.test.ts +++ b/src/conversion/convertConfig.test.ts @@ -14,6 +14,7 @@ const createStubDependencies = ( data: createStubOriginalConfigurationsData(), status: ResultStatus.Succeeded, }), + reportCommentResults: jest.fn(), reportConversionResults: jest.fn(), simplifyPackageRules: async (_configurations, data) => ({ ...data, diff --git a/src/conversion/convertConfig.ts b/src/conversion/convertConfig.ts index a2fe7e766..ba55bc402 100644 --- a/src/conversion/convertConfig.ts +++ b/src/conversion/convertConfig.ts @@ -4,6 +4,7 @@ import { simplifyPackageRules } from "../creation/simplification/simplifyPackage import { writeConversionResults } from "../creation/writeConversionResults"; import { findOriginalConfigurations } from "../input/findOriginalConfigurations"; import { reportConversionResults } from "../reporting/reportConversionResults"; +import { reportCommentResults } from "../reporting/reportCommentResults"; import { convertRules } from "../rules/convertRules"; import { ResultStatus, ResultWithStatus, TSLintToESLintSettings } from "../types"; @@ -11,6 +12,7 @@ export type ConvertConfigDependencies = { convertComments: SansDependencies; convertRules: SansDependencies; findOriginalConfigurations: SansDependencies; + reportCommentResults: SansDependencies; reportConversionResults: SansDependencies; simplifyPackageRules: SansDependencies; writeConversionResults: SansDependencies; @@ -63,6 +65,7 @@ export const convertConfig = async ( // 6. A summary of the results is printed to the user's console dependencies.reportConversionResults(simplifiedConfiguration); + dependencies.reportCommentResults(settings.comments, commentsResult); return commentsResult; }; diff --git a/src/input/findOriginalConfigurations.ts b/src/input/findOriginalConfigurations.ts index b112aae1b..34c1b2833 100644 --- a/src/input/findOriginalConfigurations.ts +++ b/src/input/findOriginalConfigurations.ts @@ -1,5 +1,10 @@ import { SansDependencies } from "../binding"; -import { ResultStatus, TSLintToESLintSettings, ResultWithDataStatus } from "../types"; +import { + ConfigurationErrorResult, + ResultStatus, + ResultWithDataStatus, + TSLintToESLintSettings, +} from "../types"; import { findESLintConfiguration, ESLintConfiguration } from "./findESLintConfiguration"; import { PackagesConfiguration, findPackagesConfiguration } from "./findPackagesConfiguration"; import { @@ -42,7 +47,7 @@ export type AllOriginalConfigurations = { export const findOriginalConfigurations = async ( dependencies: FindOriginalConfigurationsDependencies, rawSettings: TSLintToESLintSettings, -): Promise> => { +): Promise> => { // Simultaneously search for all required configuration types const [eslint, packages, tslint, typescript] = await Promise.all([ dependencies.findESLintConfiguration(rawSettings), diff --git a/src/reporting/dependencies.ts b/src/reporting/dependencies.ts index 77b3b6f1d..a85169019 100644 --- a/src/reporting/dependencies.ts +++ b/src/reporting/dependencies.ts @@ -1,5 +1,5 @@ import { Logger } from "../adapters/logger"; -export type ReportConversionResultsDependencies = { +export type ReportDependencies = { logger: Logger; }; diff --git a/src/reporting/reportCommentResults.test.ts b/src/reporting/reportCommentResults.test.ts new file mode 100644 index 000000000..5d5af57f5 --- /dev/null +++ b/src/reporting/reportCommentResults.test.ts @@ -0,0 +1,69 @@ +import { createStubLogger, expectEqualWrites } from "../adapters/logger.stubs"; +import { ResultStatus } from "../types"; +import { reportCommentResults } from "./reportCommentResults"; + +describe("reportCommentResults", () => { + it("logs a suggestion when no comment globs are provided", () => { + // Arrange + const logger = createStubLogger(); + + // Act + reportCommentResults({ logger }, undefined, { data: [], status: ResultStatus.Succeeded }); + + // Assert + expectEqualWrites( + logger.stdout.write, + `♻ Consider using --comment to replace TSLint comment directives in your source files. ♻`, + ); + }); + + it("logs a complaint when comment conversions failed", () => { + // Arrange + const logger = createStubLogger(); + const errors = [new Error()]; + + // Act + reportCommentResults({ logger }, ["src/index.ts"], { errors, status: ResultStatus.Failed }); + + // Assert + expectEqualWrites( + logger.stderr.write, + `❌ Failed to fully replace TSLint comment directives in --comment files. ❌`, + ` Check ${logger.debugFileName} for details.`, + ); + }); + + it("logs a singular success message when comment conversions succeeded on one file", () => { + // Arrange + const logger = createStubLogger(); + + // Act + reportCommentResults({ logger }, ["src/*.ts"], { + data: ["src/index.ts"], + status: ResultStatus.Succeeded, + }); + + // Assert + expectEqualWrites( + logger.stdout.write, + `♻ 1 file of TSLint comment directives converted to ESLint. ♻ `, + ); + }); + + it("logs a plural success message when comment conversions succeeded on two files", () => { + // Arrange + const logger = createStubLogger(); + + // Act + reportCommentResults({ logger }, ["src/*.ts"], { + data: ["src/index.ts", "src/data.ts"], + status: ResultStatus.Succeeded, + }); + + // Assert + expectEqualWrites( + logger.stdout.write, + `♻ 2 files of TSLint comment directives converted to ESLint. ♻ `, + ); + }); +}); diff --git a/src/reporting/reportCommentResults.ts b/src/reporting/reportCommentResults.ts new file mode 100644 index 000000000..2a03eccc8 --- /dev/null +++ b/src/reporting/reportCommentResults.ts @@ -0,0 +1,42 @@ +import chalk from "chalk"; +import { EOL } from "os"; + +import { ResultWithDataStatus, ResultStatus } from "../types"; +import { ReportDependencies } from "./dependencies"; + +export const reportCommentResults = ( + dependencies: ReportDependencies, + commentGlobs: string | string[] | undefined, + commentsResult: ResultWithDataStatus, +) => { + if (commentGlobs === undefined) { + dependencies.logger.stdout.write( + chalk.magentaBright( + `♻ Consider using --comment to replace TSLint comment directives in your source files. ♻${EOL}`, + ), + ); + return; + } + + if (commentsResult.status === ResultStatus.Failed) { + dependencies.logger.info.write(commentsResult.errors.join(EOL.repeat(2)) + EOL.repeat(2)); + dependencies.logger.stderr.write( + chalk.magentaBright( + `❌ Failed to fully replace TSLint comment directives in --comment files. ❌${EOL}`, + ), + ); + dependencies.logger.stderr.write( + chalk.magenta(` Check ${dependencies.logger.debugFileName} for details.${EOL}`), + ); + return; + } + + dependencies.logger.stdout.write(chalk.magentaBright(`♻ ${commentsResult.data.length}`)); + dependencies.logger.stdout.write( + chalk.magenta(` file${commentsResult.data.length === 1 ? "" : "s"}`), + ); + dependencies.logger.stdout.write( + chalk.magenta(` of TSLint comment directives converted to ESLint.`), + ); + dependencies.logger.stdout.write(chalk.magentaBright(` ♻${EOL}${EOL}`)); +}; diff --git a/src/reporting/reportConversionResults.ts b/src/reporting/reportConversionResults.ts index 438370f93..0b822e84f 100644 --- a/src/reporting/reportConversionResults.ts +++ b/src/reporting/reportConversionResults.ts @@ -4,7 +4,7 @@ import { EOL } from "os"; import { Logger } from "../adapters/logger"; import { RuleConversionResults } from "../rules/convertRules"; import { ESLintRuleOptions, TSLintRuleOptions } from "../rules/types"; -import { ReportConversionResultsDependencies } from "./dependencies"; +import { ReportDependencies } from "./dependencies"; import { logFailedConversions, logMissingConversionTarget, @@ -13,7 +13,7 @@ import { } from "./reportOutputs"; export const reportConversionResults = ( - dependencies: ReportConversionResultsDependencies, + dependencies: ReportDependencies, ruleConversionResults: RuleConversionResults, ) => { if (ruleConversionResults.converted.size !== 0) { diff --git a/src/reporting/reportEditorSettingConversionResults.ts b/src/reporting/reportEditorSettingConversionResults.ts index 938988abd..34e396a9c 100644 --- a/src/reporting/reportEditorSettingConversionResults.ts +++ b/src/reporting/reportEditorSettingConversionResults.ts @@ -2,7 +2,7 @@ import { EOL } from "os"; import { EditorSettingConversionResults } from "../editorSettings/convertEditorSettings"; import { EditorSetting } from "../editorSettings/types"; -import { ReportConversionResultsDependencies } from "./dependencies"; +import { ReportDependencies } from "./dependencies"; import { logFailedConversions, logMissingConversionTarget, @@ -10,7 +10,7 @@ import { } from "./reportOutputs"; export const reportEditorSettingConversionResults = ( - dependencies: ReportConversionResultsDependencies, + dependencies: ReportDependencies, editorSettingConversionResults: EditorSettingConversionResults, ) => { if (editorSettingConversionResults.converted.size !== 0) { diff --git a/src/types.ts b/src/types.ts index dec7d9285..5e78c2163 100644 --- a/src/types.ts +++ b/src/types.ts @@ -45,10 +45,7 @@ export enum ResultStatus { export type ResultWithStatus = ConfigurationErrorResult | FailedResult | SucceededResult; -export type ResultWithDataStatus = - | ConfigurationErrorResult - | FailedResult - | SucceededDataResult; +export type ResultWithDataStatus = FailedResult | SucceededDataResult; export type ConfigurationErrorResult = { readonly complaints: string[]; From 87e89535c3b10c318ae14fb351acf422ee9a91ec Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 15:11:38 -0400 Subject: [PATCH 15/17] Docs in CLI and README.md --- README.md | 12 ++++++++++++ src/cli/runCli.ts | 5 ++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a50d7d035..6d9dcfc39 100644 --- a/README.md +++ b/README.md @@ -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. @@ -49,6 +50,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 diff --git a/src/cli/runCli.ts b/src/cli/runCli.ts index d4c079daa..6598e270f 100644 --- a/src/cli/runCli.ts +++ b/src/cli/runCli.ts @@ -19,7 +19,10 @@ export const runCli = async ( ): Promise => { const command = new Command() .usage("[options] --language [language]") - .option("--comments [files]", "convert tslint:disable comments in matching files") + .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") From fa7fcda7f96c78322b38eac2ed524c7f725f5c55 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 21:58:52 -0400 Subject: [PATCH 16/17] Filled in basic test for separateErrors --- src/utils.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/utils.test.ts b/src/utils.test.ts index c89ed4574..e93823af2 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,4 +1,4 @@ -import { isDefined, isError, uniqueFromSources } from "./utils"; +import { isDefined, isError, uniqueFromSources, separateErrors } from "./utils"; describe("isDefined", () => { it("returns true when the item is defined", () => { @@ -49,7 +49,16 @@ describe("isError", () => { }); describe("separateErrors", () => { - // ... + it("splits the input array into errors and items", () => { + // Arrange + const mixed = ["value", new Error()]; + + // Act + const result = separateErrors(mixed); + + // Assert + expect(result).toEqual([[mixed[1]], [mixed[0]]]); + }); }); describe("uniqueFromSources", () => { From 2ec5b67fb1cacebee8ba369794f66573e6425f6f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Apr 2020 22:10:27 -0400 Subject: [PATCH 17/17] Normalized output per new norms --- src/reporting/reportCommentResults.test.ts | 33 ++++++++++++++++++++-- src/reporting/reportCommentResults.ts | 23 +++++++++------ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/reporting/reportCommentResults.test.ts b/src/reporting/reportCommentResults.test.ts index 5d5af57f5..114222be9 100644 --- a/src/reporting/reportCommentResults.test.ts +++ b/src/reporting/reportCommentResults.test.ts @@ -17,10 +17,10 @@ describe("reportCommentResults", () => { ); }); - it("logs a complaint when comment conversions failed", () => { + it("logs a singular complaint when one comment conversion fails", () => { // Arrange const logger = createStubLogger(); - const errors = [new Error()]; + const errors = [new Error("Hello")]; // Act reportCommentResults({ logger }, ["src/index.ts"], { errors, status: ResultStatus.Failed }); @@ -28,9 +28,36 @@ describe("reportCommentResults", () => { // Assert expectEqualWrites( logger.stderr.write, - `❌ Failed to fully replace TSLint comment directives in --comment files. ❌`, + `❌ 1 error converting TSLint comment directives in --comment files. ❌`, ` Check ${logger.debugFileName} for details.`, ); + expectEqualWrites( + logger.info.write, + `1 error converting TSLint comment directives in --comment files:`, + ` * Hello`, + ); + }); + + it("logs a plural complaint when multiple comment conversions fail", () => { + // Arrange + const logger = createStubLogger(); + const errors = [new Error("Hello"), new Error("World")]; + + // Act + reportCommentResults({ logger }, ["src/index.ts"], { errors, status: ResultStatus.Failed }); + + // Assert + expectEqualWrites( + logger.stderr.write, + `❌ 2 errors converting TSLint comment directives in --comment files. ❌`, + ` Check ${logger.debugFileName} for details.`, + ); + expectEqualWrites( + logger.info.write, + `2 errors converting TSLint comment directives in --comment files:`, + ` * Hello`, + ` * World`, + ); }); it("logs a singular success message when comment conversions succeeded on one file", () => { diff --git a/src/reporting/reportCommentResults.ts b/src/reporting/reportCommentResults.ts index e71ac7926..d2d4f1141 100644 --- a/src/reporting/reportCommentResults.ts +++ b/src/reporting/reportCommentResults.ts @@ -16,31 +16,36 @@ export const reportCommentResults = ( if (commentGlobs === undefined) { dependencies.logger.stdout.write( chalk.magentaBright( - `♻ Consider using --comment to replace TSLint comment directives in your source files. ♻${EOL}`, + `${EOL}♻ Consider using --comment to replace TSLint comment directives in your source files. ♻${EOL}`, ), ); return; } if (commentsResult.status === ResultStatus.Failed) { - dependencies.logger.info.write(commentsResult.errors.join(EOL.repeat(2)) + EOL.repeat(2)); - dependencies.logger.stderr.write( - chalk.magentaBright( - `❌ Failed to fully replace TSLint comment directives in --comment files. ❌${EOL}`, - ), - ); + const headline = `${commentsResult.errors.length} error${ + commentsResult.errors.length === 1 ? "" : "s" + } converting TSLint comment directives in --comment files`; + + dependencies.logger.stderr.write(chalk.magentaBright(`${EOL}❌ ${headline}. ❌${EOL}`)); dependencies.logger.stderr.write( chalk.magenta(` Check ${dependencies.logger.debugFileName} for details.${EOL}`), ); + + dependencies.logger.info.write(`${headline}:${EOL}`); + dependencies.logger.info.write( + commentsResult.errors.map((error) => ` * ${error.message}${EOL}`).join(""), + ); + dependencies.logger.info.write(EOL); return; } - dependencies.logger.stdout.write(chalk.magentaBright(`♻ ${commentsResult.data.length}`)); + dependencies.logger.stdout.write(chalk.magentaBright(`${EOL}♻ ${commentsResult.data.length}`)); dependencies.logger.stdout.write( chalk.magenta(` file${commentsResult.data.length === 1 ? "" : "s"}`), ); dependencies.logger.stdout.write( chalk.magenta(` of TSLint comment directives converted to ESLint.`), ); - dependencies.logger.stdout.write(chalk.magentaBright(` ♻${EOL}${EOL}`)); + dependencies.logger.stdout.write(chalk.magentaBright(` ♻${EOL}`)); };